Skip to content

Commit 447c36c

Browse files
DariusHQrichardlau
authored andcommitted
lib: fix AbortSignal.any() nested composite leak
AbortSignal.any() copies the source WeakRefs of a composite input into the new composite, so nested composites share them. Those WeakRefs were also used as the unregister tokens for the registrations that prune a composite's sources once they are collected. When an inner composite was collected first, its cleanup therefore also unregistered the outer composite from the same sources. An observed outer composite then stayed in gcPersistentSignals, together with its listeners, even after all of its sources were gone and nothing could abort it anymore. Use each composite's own WeakRef as the unregister token instead, so collecting one composite only removes its own registrations. Fixes: #66612 Refs: #62367 Assisted-by: Claude Code Signed-off-by: journaltraces <[email protected]> PR-URL: #66617 Reviewed-By: Chemi Atlow <[email protected]> Reviewed-By: Gürgün Dayıoğlu <[email protected]>
1 parent bd567f8 commit 447c36c

2 files changed

Lines changed: 31 additions & 4 deletions

File tree

‎lib/internal/abort_controller.js‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -86,8 +86,8 @@ function lazyMessageChannel() {
8686

8787
const clearTimeoutRegistry = new SafeFinalizationRegistry(clearTimeout);
8888
const dependantSignalsCleanupRegistry = new SafeFinalizationRegistry(
89-
({ sourceSignalRef, dependantSignalRef, sourceSignalsCleanupToken }) => {
90-
sourceSignalsCleanupRegistry.unregister(sourceSignalsCleanupToken);
89+
({ sourceSignalRef, dependantSignalRef }) => {
90+
sourceSignalsCleanupRegistry.unregister(dependantSignalRef);
9191

9292
const sourceSignal = sourceSignalRef.deref();
9393
if (sourceSignal === undefined) {
@@ -180,12 +180,14 @@ function followCompositeSignal(signal) {
180180
dependantSignalsCleanupRegistry.register(signal, {
181181
sourceSignalRef: sourceSignalWeakRef,
182182
dependantSignalRef: resultSignalWeakRef,
183-
sourceSignalsCleanupToken: sourceSignalWeakRef,
184183
});
184+
// Nested composites share their sources' WeakRefs, so use this signal's
185+
// own WeakRef as the unregister token. Otherwise collecting an inner
186+
// composite would also unregister the outer composite from its sources.
185187
sourceSignalsCleanupRegistry.register(sourceSignal, {
186188
sourceSignalRef: sourceSignalWeakRef,
187189
composedSignalRef: resultSignalWeakRef,
188-
}, sourceSignalWeakRef);
190+
}, resultSignalWeakRef);
189191
}
190192

191193
signal[kFollowing] = true;

‎test/parallel/test-abortsignal-drop-settled-signals.mjs‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -246,6 +246,31 @@ it('drops settled dependent signals when signal is composite', (t, done) => {
246246
}));
247247
});
248248

249+
it('drops an observed composite of a composite once its sources are collected', async (t) => {
250+
let controllers = [new AbortController(), new AbortController()];
251+
252+
// Only the outer composite is observed, so the inner one is unreachable and
253+
// is collected while the sources are still alive.
254+
const outerRef = (() => {
255+
const outer = AbortSignal.any([
256+
controllers[0].signal,
257+
AbortSignal.any([controllers[1].signal]),
258+
]);
259+
outer.addEventListener('abort', () => {});
260+
return new WeakRef(outer);
261+
})();
262+
263+
const kDependantSignals = Object.getOwnPropertySymbols(controllers[1].signal).find(
264+
(s) => s.toString() === 'Symbol(kDependantSignals)'
265+
);
266+
t.assert.strictEqual(controllers[1].signal[kDependantSignals].size, 2);
267+
268+
await gcUntil('inner composite is collected', () => controllers[1].signal[kDependantSignals].size === 1);
269+
270+
controllers = null;
271+
await gcUntil('outer composite is collected', () => outerRef.deref() === undefined);
272+
});
273+
249274
it('drops settled signals even when there are listeners', (t, done) => {
250275
runWithOrphanListeners(limit, async (signalRefs) => {
251276
await gcUntil('all signals are GCed', () => {

0 commit comments

Comments
 (0)