From 4f6611a4d53a591271b2d131f093ed169c36e692 Mon Sep 17 00:00:00 2001 From: Hermes Agent Date: Sat, 11 Jul 2026 01:09:25 +0000 Subject: [PATCH] abort_controller: retain AbortSignal with registered listeners during gc Per the DOM spec's "AbortSignal garbage collection" section, an AbortSignal with registered event listeners must not be garbage collected while those listeners are attached. Previously only timeout and non-empty composite signals were retained in gcPersistentSignals, so a signal that merely had a listener (e.g. the signal exposed by a fetch Request) could be collected while still in use, causing its listeners to silently stop firing after the owning object was gc'd. Fixes: https://github.com/nodejs/node/issues/55428 --- lib/internal/abort_controller.js | 17 ++++--- .../test-abortsignal-gc-with-listener.js | 45 +++++++++++++++++++ 2 files changed, 53 insertions(+), 9 deletions(-) create mode 100644 test/parallel/test-abortsignal-gc-with-listener.js diff --git a/lib/internal/abort_controller.js b/lib/internal/abort_controller.js index a24b5b556e1a5e..717c31f4f7a624 100644 --- a/lib/internal/abort_controller.js +++ b/lib/internal/abort_controller.js @@ -387,24 +387,23 @@ class AbortSignal extends EventTarget { followCompositeSignal(this); } - const isTimeoutOrNonEmptyCompositeSignal = this[kTimeout] || (this[kComposite] && this[kSourceSignals]?.size); - if (isTimeoutOrNonEmptyCompositeSignal && - type === 'abort' && + if (type === 'abort' && !this.aborted && !weak && size === 1) { - // If this is a timeout signal, or a non-empty composite signal, and we're adding a non-weak abort - // listener, then we don't want it to be gc'd while the listener - // is attached and the timer still hasn't fired. So, we retain a - // strong ref that is held for as long as the listener is registered. + // Per the DOM spec's "AbortSignal garbage collection" section, an + // AbortSignal with registered event listeners must not be garbage + // collected while those listeners are attached. Retain a strong ref + // for as long as at least one non-weak `abort` listener is registered. + // This also covers timeout and non-empty composite signals, which were + // previously the only signals retained here. gcPersistentSignals.add(this); } } [kRemoveListener](size, type, listener, capture) { super[kRemoveListener](size, type, listener, capture); - const isTimeoutOrNonEmptyCompositeSignal = this[kTimeout] || (this[kComposite] && this[kSourceSignals]?.size); - if (isTimeoutOrNonEmptyCompositeSignal && type === 'abort' && size === 0) { + if (type === 'abort' && size === 0) { gcPersistentSignals.delete(this); } } diff --git a/test/parallel/test-abortsignal-gc-with-listener.js b/test/parallel/test-abortsignal-gc-with-listener.js new file mode 100644 index 00000000000000..70f44914f09c7a --- /dev/null +++ b/test/parallel/test-abortsignal-gc-with-listener.js @@ -0,0 +1,45 @@ +// Flags: --no-warnings --expose-gc --expose-internals +'use strict'; +require('../common'); + +const assert = require('assert'); + +const { + test, +} = require('node:test'); + +const { setTimeout: sleep } = require('timers/promises'); + +// Regression test for https://github.com/nodejs/node/issues/55428 +// When a signal "follows" another signal (a listener on the source signal +// aborts a dependent signal), the dependent signal must remain alive while the +// relationship exists, per the DOM spec's "AbortSignal garbage collection" +// section. This mirrors what `fetch`/`Request` does: the request's signal +// follows the user-provided signal, and it must keep firing its own listeners +// even after the `Request` object has been garbage collected. + +test('a following AbortSignal must survive gc', async () => { + const parent = new AbortController(); + let fired = false; + + { + const dependent = new AbortController(); + // The dependent signal "follows" parent: when parent aborts, dependent + // aborts too. This is the same pattern used by `Request`. + parent.signal.addEventListener('abort', () => dependent.abort(), { once: true }); + dependent.signal.addEventListener('abort', () => { fired = true; }); + // `dependent` goes out of scope here; only `parent` is reachable. + } + + await sleep(10); + globalThis.gc(); + + // Aborting the parent must still propagate to, and fire listeners on, the + // dependent signal even though the dependent controller is no longer + // referenced from JS. + parent.abort(); + + await sleep(10); + assert.strictEqual(fired, true, + 'listener on the following signal should fire after gc of its controller'); +});