fixup! lib: add AbortSignal.timeout

lib/internal/abort_controller.js

@@ -10,6 +10,7 @@ const {
   ObjectDefineProperty,
   Symbol,
   SymbolToStringTag,
+  WeakRef,
 } = primordials;
 
 const {
@@ -41,7 +42,6 @@ const { setTimeout } = require('timers');
 
 const kAborted = Symbol('kAborted');
 const kReason = Symbol('kReason');
-const kTimeout = Symbol('kTimeout');
 
 function customInspect(self, obj, depth, options) {
   if (depth < 0)
@@ -59,6 +59,27 @@ function validateAbortSignal(obj) {
     throw new ERR_INVALID_THIS('AbortSignal');
 }
 
+// Because the AbortSignal timeout cannot be canceled, we don't want the
+// presence of the timer alone to keep the AbortSignal from being garbage
+// collected if it otherwise no longer accessible. We also don't want the
+// timer to keep the Node.js process open on it's own. Therefore, we wrap
+// the AbortSignal in a WeakRef and have the setTimeout callback close
+// over the WeakRef rather than directly over the AbortSignal, and we unref
+// the created timer object.
+function setWeakAbortSignalTimeout(weakRef, delay) {
+  const timeout = setTimeout(() => {
+    const signal = weakRef.deref();
+    if (signal !== undefined) {
+      abortSignal(
+        signal,
+        new DOMException(
+          'The operation was aborted due to timeout',
+          'TimeoutError'));
+    }
+  }, delay);
+  timeout.unref();
+}
+
 class AbortSignal extends EventTarget {
   constructor() {
     throw new ERR_ILLEGAL_CONSTRUCTOR();
@@ -101,13 +122,7 @@ class AbortSignal extends EventTarget {
   static timeout(delay) {
     validateUint32(delay, 'delay', true);
     const signal = createAbortSignal();
-    signal[kTimeout] = setTimeout(() => {
-      abortSignal(
-        signal,
-        new DOMException(
-          'The operation was aborted due to timeout',
-          'TimeoutError'));
-    }, delay);
+    setWeakAbortSignalTimeout(new WeakRef(signal), delay);
     return signal;
   }
 }

test/parallel/test-abortcontroller.js

@@ -1,10 +1,11 @@
-// Flags: --no-warnings
+// Flags: --no-warnings --expose-gc
 'use strict';
 
 const common = require('../common');
 const { inspect } = require('util');
 
 const { ok, strictEqual, throws } = require('assert');
+const { setTimeout: sleep } = require('timers/promises');
 
 {
   // Tests that abort is fired with the correct event type on AbortControllers
@@ -164,3 +165,22 @@ const { ok, strictEqual, throws } = require('assert');
     strictEqual(signal.reason.code, 23);
   }), 20);
 }
+
+{
+  (async () => {
+    // Test AbortSignal timeout doesn't prevent the signal
+    // from being garbage collected.
+    let ref;
+    {
+      ref = new globalThis.WeakRef(AbortSignal.timeout(1_200_000));
+    }
+
+    await sleep(10);
+    globalThis.gc();
+    strictEqual(ref.deref(), undefined);
+  })().then(common.mustCall());
+
+  // Setting a long timeout (20 minutes here) should not
+  // keep the Node.js process open (the timer is unref'd)
+  AbortSignal.timeout(1_200_000);
+}