fix(ext/node): ignore cancelled timer when node timer refresh (#19637)

For timers that have already executed clearTimeout, there is no need to recreate a new timer when refresh is executed again.

cli/tests/unit_node/timers_test.ts

@@ -1,6 +1,6 @@
 // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license.
 
-import { assert } from "../../../test_util/std/testing/asserts.ts";
+import { assert, fail } from "../../../test_util/std/testing/asserts.ts";
 import * as timers from "node:timers";
 import * as timersPromises from "node:timers/promises";
 
@@ -50,3 +50,13 @@ Deno.test("[node/timers/promises setTimeout]", () => {
   assert(p instanceof Promise);
   return p;
 });
+
+// Regression test for https://github.com/denoland/deno/issues/17981
+Deno.test("[node/timers refresh cancelled timer]", () => {
+  const { setTimeout, clearTimeout } = timers;
+  const p = setTimeout(() => {
+    fail();
+  }, 1);
+  clearTimeout(p);
+  p.refresh();
+});

ext/node/polyfills/internal/stream_base_commons.ts

@@ -35,7 +35,7 @@ import {
 import { isUint8Array } from "ext:deno_node/internal/util/types.ts";
 import { errnoException } from "ext:deno_node/internal/errors.ts";
 import { getTimerDuration, kTimeout } from "ext:deno_node/internal/timers.mjs";
-import { setUnrefTimeout } from "node:timers";
+import { clearTimeout, setUnrefTimeout } from "node:timers";
 import { validateFunction } from "ext:deno_node/internal/validators.mjs";
 import { codeMap } from "ext:deno_node/internal_binding/uv.ts";
 import { Buffer } from "node:buffer";

ext/node/polyfills/internal/timers.mjs

@@ -4,6 +4,13 @@
 // TODO(petamoriken): enable prefer-primordials for node polyfills
 // deno-lint-ignore-file prefer-primordials
 
+const primordials = globalThis.__bootstrap.primordials;
+const {
+  MapPrototypeDelete,
+  MapPrototypeSet,
+  SafeMap,
+} = primordials;
+
 import { inspect } from "ext:deno_node/internal/util/inspect.mjs";
 import { validateFunction, validateNumber } from "ext:deno_node/internal/validators.mjs";
 import { ERR_OUT_OF_RANGE } from "ext:deno_node/internal/errors.ts";
@@ -22,6 +29,14 @@ export const kTimeout = Symbol("timeout");
 const kRefed = Symbol("refed");
 const createTimer = Symbol("createTimer");
 
+/**
+ * The keys in this map correspond to the key ID's in the spec's map of active
+ * timers. The values are the timeout's status.
+ *
+ * @type {Map<number, Timeout>}
+ */
+export const activeTimers = new SafeMap();
+
 // Timer constructor function.
 export function Timeout(callback, after, args, isRepeat, isRefed) {
   if (typeof after === "number" && after > TIMEOUT_MAX) {
@@ -31,19 +46,26 @@ export function Timeout(callback, after, args, isRepeat, isRefed) {
   this._onTimeout = callback;
   this._timerArgs = args;
   this._isRepeat = isRepeat;
+  this._destroyed = false;
   this[kRefed] = isRefed;
   this[kTimerId] = this[createTimer]();
 }
 
 Timeout.prototype[createTimer] = function () {
   const callback = this._onTimeout;
-  const cb = (...args) => callback.bind(this)(...args);
+  const cb = (...args) => {
+    if (!this._isRepeat) {
+      MapPrototypeDelete(activeTimers, this[kTimerId])
+    }
+    return callback.bind(this)(...args);
+  }
   const id = this._isRepeat
     ? setInterval_(cb, this._idleTimeout, ...this._timerArgs)
     : setTimeout_(cb, this._idleTimeout, ...this._timerArgs);
   if (!this[kRefed]) {
     Deno.unrefTimer(id);
   }
+  MapPrototypeSet(activeTimers, id, this);
   return id;
 };
 
@@ -59,8 +81,10 @@ Timeout.prototype[inspect.custom] = function (_, options) {
 };
 
 Timeout.prototype.refresh = function () {
-  clearTimeout_(this[kTimerId]);
-  this[kTimerId] = this[createTimer]();
+  if (!this._destroyed) {
+    clearTimeout_(this[kTimerId]);
+    this[kTimerId] = this[createTimer]();
+  }
   return this;
 };
 

ext/node/polyfills/timers.ts

@@ -3,7 +3,17 @@
 // TODO(petamoriken): enable prefer-primordials for node polyfills
 // deno-lint-ignore-file prefer-primordials
 
-import { setUnrefTimeout, Timeout } from "ext:deno_node/internal/timers.mjs";
+const primordials = globalThis.__bootstrap.primordials;
+const {
+  MapPrototypeGet,
+  MapPrototypeDelete,
+} = primordials;
+
+import {
+  activeTimers,
+  setUnrefTimeout,
+  Timeout,
+} from "ext:deno_node/internal/timers.mjs";
 import { validateFunction } from "ext:deno_node/internal/validators.mjs";
 import { promisify } from "ext:deno_node/internal/util.mjs";
 export { setUnrefTimeout } from "ext:deno_node/internal/timers.mjs";
@@ -32,7 +42,13 @@ export function clearTimeout(timeout?: Timeout | number) {
   if (timeout == null) {
     return;
   }
-  clearTimeout_(+timeout);
+  const id = +timeout;
+  const timer = MapPrototypeGet(activeTimers, id);
+  if (timer) {
+    timeout._destroyed = true;
+    MapPrototypeDelete(activeTimers, id);
+  }
+  clearTimeout_(id);
 }
 export function setInterval(
   callback: (...args: unknown[]) => void,
@@ -46,7 +62,13 @@ export function clearInterval(timeout?: Timeout | number | string) {
   if (timeout == null) {
     return;
   }
-  clearInterval_(+timeout);
+  const id = +timeout;
+  const timer = MapPrototypeGet(activeTimers, id);
+  if (timer) {
+    timeout._destroyed = true;
+    MapPrototypeDelete(activeTimers, id);
+  }
+  clearInterval_(id);
 }
 // TODO(bartlomieju): implement the 'NodeJS.Immediate' versions of the timers.
 // https://github.com/DefinitelyTyped/DefinitelyTyped/blob/1163ead296d84e7a3c80d71e7c81ecbd1a130e9a/types/node/v12/globals.d.ts#L1120-L1131