From ffd3ed9b8ad34471a0ba30727b7321f86c0a30b5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= <biwanczuk@gmail.com>
Date: Mon, 22 May 2023 14:09:31 +0200
Subject: [PATCH] fix(ext/web): improve timers resolution for 0ms timeouts
 (#19212)

This commit changes the implementation of `ext/web` timers, by using
"op_void_async_deferred" for timeouts of 0ms.

0ms timeout is meant to be run at the end of the event loop tick and
currently Tokio timers that we use to back timeouts have at least 1ms
resolution. That means that 0ms timeout actually take >1ms. This
commit changes that and runs 0ms timeout at the end of the event
loop tick.

One consequence is that "unrefing" a 0ms timer will actually keep
the event loop alive (which I believe actually makes sense, the test
we had only worked because the timeout took more than 1ms).

Ref https://github.com/denoland/deno/issues/19034
---
 cli/js/40_testing.js          |  2 +-
 cli/tests/unit/timers_test.ts |  2 +-
 ext/web/02_timers.js          | 19 ++++++++++++++++---
 3 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/cli/js/40_testing.js b/cli/js/40_testing.js
index e269b9c9f2..ec83ce370f 100644
--- a/cli/js/40_testing.js
+++ b/cli/js/40_testing.js
@@ -50,7 +50,7 @@ function opSanitizerDelay() {
   return new Promise((resolve) => {
     setTimeout(() => {
       ArrayPrototypePush(opSanitizerDelayResolveQueue, resolve);
-    }, 0);
+    }, 1);
   });
 }
 
diff --git a/cli/tests/unit/timers_test.ts b/cli/tests/unit/timers_test.ts
index 8de7565169..c50cb779c6 100644
--- a/cli/tests/unit/timers_test.ts
+++ b/cli/tests/unit/timers_test.ts
@@ -557,7 +557,7 @@ Deno.test({
   permissions: { run: true, read: true },
   fn: async () => {
     const [statusCode, output] = await execCode(`
-      const timer = setTimeout(() => console.log("1"));
+      const timer = setTimeout(() => console.log("1"), 1);
       Deno.unrefTimer(timer);
     `);
     assertEquals(statusCode, 0);
diff --git a/ext/web/02_timers.js b/ext/web/02_timers.js
index cfd85a0553..ed9f1c6fb1 100644
--- a/ext/web/02_timers.js
+++ b/ext/web/02_timers.js
@@ -27,7 +27,10 @@ const {
 import * as webidl from "ext:deno_webidl/00_webidl.js";
 import { reportException } from "ext:deno_web/02_event.js";
 import { assert } from "ext:deno_web/00_infra.js";
-const { op_sleep } = core.generateAsyncOpHandler("op_sleep");
+const { op_sleep, op_void_async_deferred } = core.generateAsyncOpHandler(
+  "op_sleep",
+  "op_void_async_deferred",
+);
 
 const hrU8 = new Uint8Array(8);
 const hr = new Uint32Array(TypedArrayPrototypeGetBuffer(hrU8));
@@ -218,7 +221,16 @@ const scheduledTimers = { head: null, tail: null };
  */
 function runAfterTimeout(cb, millis, timerInfo) {
   const cancelRid = timerInfo.cancelRid;
-  const sleepPromise = op_sleep(millis, cancelRid);
+  let sleepPromise;
+  // If this timeout is scheduled for 0ms it means we want it to run at the
+  // end of the event loop turn. There's no point in setting up a Tokio timer,
+  // since its lowest resolution is 1ms. Firing of a "void async" op is better
+  // in this case, because the timer will take closer to 0ms instead of >1ms.
+  if (millis === 0) {
+    sleepPromise = op_void_async_deferred();
+  } else {
+    sleepPromise = op_sleep(millis, cancelRid);
+  }
   timerInfo.promiseId = sleepPromise[SymbolFor("Deno.core.internalPromiseId")];
   if (!timerInfo.isRef) {
     core.unrefOp(timerInfo.promiseId);
@@ -246,7 +258,8 @@ function runAfterTimeout(cb, millis, timerInfo) {
   PromisePrototypeThen(
     sleepPromise,
     (cancelled) => {
-      if (!cancelled) {
+      // "op_void_async_deferred" returns null
+      if (cancelled !== null && !cancelled) {
         // The timer was cancelled.
         removeFromScheduledTimers(timerObject);
         return;