diff --git a/lib/internal/abort_controller.js b/lib/internal/abort_controller.js index 51461f9cacdb76..754c57cc19d6e2 100644 --- a/lib/internal/abort_controller.js +++ b/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; } } diff --git a/test/parallel/test-abortcontroller.js b/test/parallel/test-abortcontroller.js index 1ed0afd6215964..7f27ba85ff7d6d 100644 --- a/test/parallel/test-abortcontroller.js +++ b/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); +}