Skip to content

Commit b95e5f9

Browse files
bitpshraduh95
authored andcommitted
lib: fix AbortSignal.any() observed-composite leak
An observed composite signal (one with an `abort` listener) is added to gcPersistentSignals so it stays alive long enough to fire. When one of its sources aborted, the composite was marked aborted but never removed from that set, so it was retained forever and its WeakRef was never pruned from a long-lived source's kDependantSignals. That set therefore grew without bound. Drop each transitively-aborted dependent from gcPersistentSignals, the same way the directly-aborted signal is already removed, so it can be collected and its dependant entries pruned. Fixes: #64476 Signed-off-by: Paul Bouchon <mail@bitpshr.net> PR-URL: #64481 Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: Edy Silva <edigleyssonsilva@gmail.com>
1 parent 4cb72ee commit b95e5f9

2 files changed

Lines changed: 38 additions & 0 deletions

File tree

lib/internal/abort_controller.js

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -522,6 +522,13 @@ function abortSignal(signal, reason) {
522522
for (let i = 0; i < dependentSignalsToAbort.length; i++) {
523523
const dependentSignal = dependentSignalsToAbort[i];
524524
runAbort(dependentSignal);
525+
// A transitively-aborted dependent signal can never abort again, so there
526+
// is no longer any reason to keep it alive. Dropping it from
527+
// gcPersistentSignals lets it be collected, which in turn prunes its
528+
// WeakRef from its sources' kDependantSignals sets. Otherwise, an observed
529+
// composite that follows a long-lived source stays retained forever and
530+
// its entry accumulates in that source's kDependantSignals.
531+
gcPersistentSignals.delete(dependentSignal);
525532
}
526533

527534
// Clean up the signal from gcPersistentSignals

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

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,37 @@ describe('when there is a long-lived signal', () => {
142142

143143
run(1);
144144
});
145+
146+
it('drops observed dependent signals once they are transitively aborted', async () => {
147+
const longLived = new AbortController();
148+
const handler = () => {};
149+
const size = () => {
150+
const sym = Object.getOwnPropertySymbols(longLived.signal).find(
151+
(s) => s.toString() === 'Symbol(kDependantSignals)'
152+
);
153+
return sym ? longLived.signal[sym].size : 0;
154+
};
155+
156+
// Each composite observes the long-lived source and a per-request source,
157+
// then is aborted through the per-request source without ever removing its
158+
// listener. The aborted composites can never fire again, so the long-lived
159+
// source's dependant set must not accumulate them. Using a helper keeps the
160+
// last iteration's signals from lingering on the stack for the assertion.
161+
const createObservedAbortedComposite = () => {
162+
const perReq = new AbortController();
163+
const composite = AbortSignal.any([perReq.signal, longLived.signal]);
164+
composite.addEventListener('abort', handler);
165+
perReq.abort();
166+
};
167+
for (let i = 0; i < limit; i++) {
168+
createObservedAbortedComposite();
169+
}
170+
171+
await gcUntil(
172+
'observed dependents are dropped after transitive abort',
173+
() => size() === 0,
174+
);
175+
});
145176
});
146177

147178
it('does not prevent source signal from being GCed if it is short-lived', (t, done) => {

0 commit comments

Comments
 (0)