From 2b29ad264fc264f04918a6f1a8418218839c44a4 Mon Sep 17 00:00:00 2001 From: sangwook Date: Sun, 22 Mar 2026 02:09:11 +0900 Subject: [PATCH 1/5] lib: defer AbortSignal.any() following Avoid registering AbortSignal.any() composites as dependants until they are actually observed. This fixes the long-lived source retention pattern from #62363 while preserving abort semantics through lazy refresh and follow paths. Also unregister fired timeout signals from the timeout finalization registry so timeout churn releases memory more promptly. --- lib/internal/abort_controller.js | 113 +++++++++++++----- .../test-abortsignal-drop-settled-signals.mjs | 31 ++++- .../test-abortsignal-timeout-memory.js | 60 ++++++++++ 3 files changed, 176 insertions(+), 28 deletions(-) create mode 100644 test/parallel/test-abortsignal-timeout-memory.js diff --git a/lib/internal/abort_controller.js b/lib/internal/abort_controller.js index ef77e0af30e40a..a24b5b556e1a5e 100644 --- a/lib/internal/abort_controller.js +++ b/lib/internal/abort_controller.js @@ -85,17 +85,16 @@ function lazyMessageChannel() { } const clearTimeoutRegistry = new SafeFinalizationRegistry(clearTimeout); -const dependantSignalsCleanupRegistry = new SafeFinalizationRegistry((signalWeakRef) => { - const signal = signalWeakRef.deref(); - if (signal === undefined) { - return; - } - signal[kDependantSignals].forEach((ref) => { - if (ref.deref() === undefined) { - signal[kDependantSignals].delete(ref); +const dependantSignalsCleanupRegistry = new SafeFinalizationRegistry( + ({ sourceSignalRef, dependantSignalRef, sourceSignalsCleanupToken }) => { + sourceSignalsCleanupRegistry.unregister(sourceSignalsCleanupToken); + + const sourceSignal = sourceSignalRef.deref(); + if (sourceSignal === undefined) { + return; } + sourceSignal[kDependantSignals].delete(dependantSignalRef); }); -}); const gcPersistentSignals = new SafeSet(); @@ -117,6 +116,8 @@ const kCloneData = Symbol('kCloneData'); const kTimeout = Symbol('kTimeout'); const kMakeTransferable = Symbol('kMakeTransferable'); const kComposite = Symbol('kComposite'); +const kFollowing = Symbol('kFollowing'); +const kResultSignalWeakRef = Symbol('kResultSignalWeakRef'); const kSourceSignals = Symbol('kSourceSignals'); const kDependantSignals = Symbol('kDependantSignals'); @@ -136,6 +137,60 @@ function validateThisAbortSignal(obj) { throw new ERR_INVALID_THIS('AbortSignal'); } +function refreshCompositeSignal(signal) { + if (!signal[kComposite] || signal[kAborted] || !signal[kSourceSignals]?.size) { + return; + } + + for (const sourceSignalWeakRef of signal[kSourceSignals]) { + const sourceSignal = sourceSignalWeakRef.deref(); + if (sourceSignal === undefined) { + signal[kSourceSignals].delete(sourceSignalWeakRef); + continue; + } + + if (sourceSignal.aborted) { + abortSignal(signal, sourceSignal.reason); + return; + } + } +} + +function followCompositeSignal(signal) { + if (signal[kFollowing] || signal[kAborted] || !signal[kSourceSignals]?.size) { + return; + } + + const resultSignalWeakRef = signal[kResultSignalWeakRef] ??= new SafeWeakRef(signal); + + for (const sourceSignalWeakRef of signal[kSourceSignals]) { + const sourceSignal = sourceSignalWeakRef.deref(); + if (sourceSignal === undefined) { + signal[kSourceSignals].delete(sourceSignalWeakRef); + continue; + } + + if (sourceSignal.aborted) { + abortSignal(signal, sourceSignal.reason); + return; + } + + sourceSignal[kDependantSignals] ??= new SafeSet(); + sourceSignal[kDependantSignals].add(resultSignalWeakRef); + dependantSignalsCleanupRegistry.register(signal, { + sourceSignalRef: sourceSignalWeakRef, + dependantSignalRef: resultSignalWeakRef, + sourceSignalsCleanupToken: sourceSignalWeakRef, + }); + sourceSignalsCleanupRegistry.register(sourceSignal, { + sourceSignalRef: sourceSignalWeakRef, + composedSignalRef: resultSignalWeakRef, + }, sourceSignalWeakRef); + } + + signal[kFollowing] = true; +} + // 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 @@ -148,6 +203,7 @@ function setWeakAbortSignalTimeout(weakRef, delay) { const timeout = setTimeout(() => { const signal = weakRef.deref(); if (signal !== undefined) { + clearTimeoutRegistry.unregister(signal); gcPersistentSignals.delete(signal); abortSignal( signal, @@ -198,6 +254,7 @@ class AbortSignal extends EventTarget { */ get aborted() { validateThisAbortSignal(this); + refreshCompositeSignal(this); return !!this[kAborted]; } @@ -206,11 +263,13 @@ class AbortSignal extends EventTarget { */ get reason() { validateThisAbortSignal(this); + refreshCompositeSignal(this); return this[kReason]; } throwIfAborted() { validateThisAbortSignal(this); + refreshCompositeSignal(this); if (this[kAborted]) { throw this[kReason]; } @@ -241,7 +300,8 @@ class AbortSignal extends EventTarget { signal[kTimeout] = true; clearTimeoutRegistry.register( signal, - setWeakAbortSignalTimeout(new SafeWeakRef(signal), delay)); + setWeakAbortSignalTimeout(new SafeWeakRef(signal), delay), + signal); return signal; } @@ -260,7 +320,6 @@ class AbortSignal extends EventTarget { return resultSignal; } - const resultSignalWeakRef = new SafeWeakRef(resultSignal); resultSignal[kSourceSignals] = new SafeSet(); // Track if we have any timeout signals @@ -283,44 +342,39 @@ class AbortSignal extends EventTarget { return resultSignal; } - signal[kDependantSignals] ??= new SafeSet(); if (!signal[kComposite]) { const signalWeakRef = new SafeWeakRef(signal); resultSignal[kSourceSignals].add(signalWeakRef); - signal[kDependantSignals].add(resultSignalWeakRef); - dependantSignalsCleanupRegistry.register(resultSignal, signalWeakRef); - sourceSignalsCleanupRegistry.register(signal, { - sourceSignalRef: signalWeakRef, - composedSignalRef: resultSignalWeakRef, - }); } else if (!signal[kSourceSignals]) { continue; } else { + refreshCompositeSignal(signal); + if (signal.aborted) { + abortSignal(resultSignal, signal.reason); + return resultSignal; + } for (const sourceSignalWeakRef of signal[kSourceSignals]) { const sourceSignal = sourceSignalWeakRef.deref(); if (!sourceSignal) { continue; } - assert(!sourceSignal.aborted); assert(!sourceSignal[kComposite]); + if (sourceSignal.aborted) { + abortSignal(resultSignal, sourceSignal.reason); + return resultSignal; + } + if (resultSignal[kSourceSignals].has(sourceSignalWeakRef)) { continue; } resultSignal[kSourceSignals].add(sourceSignalWeakRef); - sourceSignal[kDependantSignals].add(resultSignalWeakRef); - dependantSignalsCleanupRegistry.register(resultSignal, sourceSignalWeakRef); - sourceSignalsCleanupRegistry.register(signal, { - sourceSignalRef: sourceSignalWeakRef, - composedSignalRef: resultSignalWeakRef, - }); } } } - // If we have any timeout signals, add the composite signal to gcPersistentSignals if (hasTimeoutSignals && resultSignal[kSourceSignals].size > 0) { - gcPersistentSignals.add(resultSignal); + resultSignal[kTimeout] = true; } return resultSignal; @@ -328,6 +382,11 @@ class AbortSignal extends EventTarget { [kNewListener](size, type, listener, once, capture, passive, weak) { super[kNewListener](size, type, listener, once, capture, passive, weak); + + if (this[kComposite] && type === 'abort' && !this.aborted && size === 1) { + followCompositeSignal(this); + } + const isTimeoutOrNonEmptyCompositeSignal = this[kTimeout] || (this[kComposite] && this[kSourceSignals]?.size); if (isTimeoutOrNonEmptyCompositeSignal && type === 'abort' && diff --git a/test/parallel/test-abortsignal-drop-settled-signals.mjs b/test/parallel/test-abortsignal-drop-settled-signals.mjs index 29a0dcf8b58638..c481db069d6f13 100644 --- a/test/parallel/test-abortsignal-drop-settled-signals.mjs +++ b/test/parallel/test-abortsignal-drop-settled-signals.mjs @@ -23,7 +23,9 @@ function makeSubsequentCalls(limit, done, holdReferences = false) { } if (holdReferences) { - retainedSignals.push(AbortSignal.any([ac.signal])); + const signal = AbortSignal.any([ac.signal]); + signal.addEventListener('abort', handler); + retainedSignals.push(signal); } else { // Using a WeakRef to avoid retaining information that will interfere with the test signalRef = new WeakRef(AbortSignal.any([ac.signal])); @@ -119,6 +121,27 @@ describe('when there is a long-lived signal', () => { done(); }, true); }); + + it('does not keep retained dependent signals without listeners', (t, done) => { + const ac = new AbortController(); + const retainedSignals = []; + const kDependantSignals = Object.getOwnPropertySymbols(ac.signal).find( + (s) => s.toString() === 'Symbol(kDependantSignals)' + ); + + function run(iteration) { + if (iteration > limit) { + t.assert.strictEqual(ac.signal[kDependantSignals]?.size ?? 0, 0); + done(); + return; + } + + retainedSignals.push(AbortSignal.any([ac.signal])); + setImmediate(() => run(iteration + 1)); + } + + run(1); + }); }); it('does not prevent source signal from being GCed if it is short-lived', (t, done) => { @@ -134,10 +157,13 @@ it('does not prevent source signal from being GCed if it is short-lived', (t, do it('drops settled dependent signals when signal is composite', (t, done) => { const controllers = Array.from({ length: 2 }, () => new AbortController()); + const handler = () => {}; // Using WeakRefs to avoid this test to retain information that will make the test fail const composedSignal1 = new WeakRef(AbortSignal.any([controllers[0].signal])); const composedSignalRef = new WeakRef(AbortSignal.any([composedSignal1.deref(), controllers[1].signal])); + composedSignal1.deref().addEventListener('abort', handler); + composedSignalRef.deref().addEventListener('abort', handler); const kDependantSignals = Object.getOwnPropertySymbols(controllers[0].signal).find( (s) => s.toString() === 'Symbol(kDependantSignals)' @@ -147,6 +173,9 @@ it('drops settled dependent signals when signal is composite', (t, done) => { t.assert.strictEqual(controllers[1].signal[kDependantSignals].size, 1); setImmediate(mustCall(() => { + composedSignal1.deref()?.removeEventListener('abort', handler); + composedSignalRef.deref()?.removeEventListener('abort', handler); + globalThis.gc({ execution: 'async' }).then(async () => { await gcUntil('all signals are GCed', () => { const totalDependantSignals = Math.max( diff --git a/test/parallel/test-abortsignal-timeout-memory.js b/test/parallel/test-abortsignal-timeout-memory.js new file mode 100644 index 00000000000000..922dc66bf14916 --- /dev/null +++ b/test/parallel/test-abortsignal-timeout-memory.js @@ -0,0 +1,60 @@ +// Flags: --expose-gc +'use strict'; + +require('../common'); +const assert = require('assert'); +const { spawnSync } = require('child_process'); + +function runMemoryChurn(script) { + const child = spawnSync(process.execPath, [ + '--expose-gc', + '--max-old-space-size=16', + '-e', + script, + ], { + encoding: 'utf8', + timeout: 60_000, + }); + + assert.strictEqual(child.status, 0, child.stderr || child.stdout); + assert.match(child.stdout, /ok/); +} + +const timeoutChurnScript = ` +let i = 0; +function run() { + AbortSignal.timeout(1); + if (++i === 100_000) { + global.gc(); + global.gc(); + setImmediate(() => { + global.gc(); + console.log('ok'); + }); + return; + } + setImmediate(run); +} +run(); +`; + +const anyTimeoutChurnScript = ` +let i = 0; +function run() { + AbortSignal.any([AbortSignal.timeout(1)]); + if (++i === 100_000) { + global.gc(); + global.gc(); + setImmediate(() => { + global.gc(); + console.log('ok'); + }); + return; + } + setImmediate(run); +} +run(); +`; + +runMemoryChurn(timeoutChurnScript); +runMemoryChurn(anyTimeoutChurnScript); From 303d57b745e56509160f212bfb15b3ec41b7d693 Mon Sep 17 00:00:00 2001 From: sangwook Date: Sun, 22 Mar 2026 19:25:44 +0900 Subject: [PATCH 2/5] test: remove flaky timeout churn regression The test depends on GC and heap behavior under a very small memory limit, which makes it too sensitive for CI. Remove the test and keep the cleanup change. --- .../test-abortsignal-timeout-memory.js | 60 ------------------- 1 file changed, 60 deletions(-) delete mode 100644 test/parallel/test-abortsignal-timeout-memory.js diff --git a/test/parallel/test-abortsignal-timeout-memory.js b/test/parallel/test-abortsignal-timeout-memory.js deleted file mode 100644 index 922dc66bf14916..00000000000000 --- a/test/parallel/test-abortsignal-timeout-memory.js +++ /dev/null @@ -1,60 +0,0 @@ -// Flags: --expose-gc -'use strict'; - -require('../common'); -const assert = require('assert'); -const { spawnSync } = require('child_process'); - -function runMemoryChurn(script) { - const child = spawnSync(process.execPath, [ - '--expose-gc', - '--max-old-space-size=16', - '-e', - script, - ], { - encoding: 'utf8', - timeout: 60_000, - }); - - assert.strictEqual(child.status, 0, child.stderr || child.stdout); - assert.match(child.stdout, /ok/); -} - -const timeoutChurnScript = ` -let i = 0; -function run() { - AbortSignal.timeout(1); - if (++i === 100_000) { - global.gc(); - global.gc(); - setImmediate(() => { - global.gc(); - console.log('ok'); - }); - return; - } - setImmediate(run); -} -run(); -`; - -const anyTimeoutChurnScript = ` -let i = 0; -function run() { - AbortSignal.any([AbortSignal.timeout(1)]); - if (++i === 100_000) { - global.gc(); - global.gc(); - setImmediate(() => { - global.gc(); - console.log('ok'); - }); - return; - } - setImmediate(run); -} -run(); -`; - -runMemoryChurn(timeoutChurnScript); -runMemoryChurn(anyTimeoutChurnScript); From d8c2e3bd18f16d48618a7091336dfabc6372f1d0 Mon Sep 17 00:00:00 2001 From: sangwook Date: Sun, 22 Mar 2026 20:24:20 +0900 Subject: [PATCH 3/5] build: trigger ci rerun From cde6bee00cfa74ff3a76dcd9f1b0001f08c00651 Mon Sep 17 00:00:00 2001 From: sangwook Date: Sun, 22 Mar 2026 21:50:46 +0900 Subject: [PATCH 4/5] build: trigger ci rerun From f7a442cffcbe5980137124688f0a499795ca9a63 Mon Sep 17 00:00:00 2001 From: sangwook Date: Sun, 22 Mar 2026 23:45:40 +0900 Subject: [PATCH 5/5] build: trigger ci rerun