Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit a3c8739

Browse filesBrowse files
geeksilva97ruyadorno
authored andcommitted
lib: clean up persisted signals when they are settled
PR-URL: #56001 Refs: #55328 Fixes: #55328 Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: Jason Zhang <xzha4350@gmail.com> Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
1 parent 2aa77c8 commit a3c8739
Copy full SHA for a3c8739

File tree

Expand file treeCollapse file tree

2 files changed

+77
-0
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

2 files changed

+77
-0
lines changed
Open diff view settings
Collapse file

‎lib/internal/abort_controller.js‎

Copy file name to clipboardExpand all lines: lib/internal/abort_controller.js
+22Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,8 +97,21 @@ const dependantSignalsCleanupRegistry = new SafeFinalizationRegistry((signalWeak
9797
}
9898
});
9999
});
100+
100101
const gcPersistentSignals = new SafeSet();
101102

103+
const sourceSignalsCleanupRegistry = new SafeFinalizationRegistry(({ sourceSignalRef, composedSignalRef }) => {
104+
const composedSignal = composedSignalRef.deref();
105+
if (composedSignal !== undefined) {
106+
composedSignal[kSourceSignals].delete(sourceSignalRef);
107+
108+
if (composedSignal[kSourceSignals].size === 0) {
109+
// This signal will no longer abort. There's no need to keep it in the gcPersistentSignals set.
110+
gcPersistentSignals.delete(composedSignal);
111+
}
112+
}
113+
});
114+
102115
const kAborted = Symbol('kAborted');
103116
const kReason = Symbol('kReason');
104117
const kCloneData = Symbol('kCloneData');
@@ -261,6 +274,10 @@ class AbortSignal extends EventTarget {
261274
resultSignal[kSourceSignals].add(signalWeakRef);
262275
signal[kDependantSignals].add(resultSignalWeakRef);
263276
dependantSignalsCleanupRegistry.register(resultSignal, signalWeakRef);
277+
sourceSignalsCleanupRegistry.register(signal, {
278+
sourceSignalRef: signalWeakRef,
279+
composedSignalRef: resultSignalWeakRef,
280+
});
264281
} else if (!signal[kSourceSignals]) {
265282
continue;
266283
} else {
@@ -278,6 +295,10 @@ class AbortSignal extends EventTarget {
278295
resultSignal[kSourceSignals].add(sourceSignalWeakRef);
279296
sourceSignal[kDependantSignals].add(resultSignalWeakRef);
280297
dependantSignalsCleanupRegistry.register(resultSignal, sourceSignalWeakRef);
298+
sourceSignalsCleanupRegistry.register(signal, {
299+
sourceSignalRef: sourceSignalWeakRef,
300+
composedSignalRef: resultSignalWeakRef,
301+
});
281302
}
282303
}
283304
}
@@ -434,6 +455,7 @@ class AbortController {
434455
*/
435456
get signal() {
436457
this.#signal ??= new AbortSignal(kDontThrowSymbol);
458+
437459
return this.#signal;
438460
}
439461

Collapse file

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

Copy file name to clipboardExpand all lines: test/parallel/test-abortsignal-drop-settled-signals.mjs
+55Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,41 @@ function runShortLivedSourceSignal(limit, done) {
6464
run(1);
6565
};
6666

67+
function runWithOrphanListeners(limit, done) {
68+
let composedSignalRef;
69+
const composedSignalRefs = [];
70+
const handler = () => { };
71+
72+
function run(iteration) {
73+
const ac = new AbortController();
74+
if (iteration > limit) {
75+
setImmediate(() => {
76+
global.gc();
77+
setImmediate(() => {
78+
global.gc();
79+
80+
done(composedSignalRefs);
81+
});
82+
});
83+
return;
84+
}
85+
86+
composedSignalRef = new WeakRef(AbortSignal.any([ac.signal]));
87+
composedSignalRef.deref().addEventListener('abort', handler);
88+
89+
const otherComposedSignalRef = new WeakRef(AbortSignal.any([composedSignalRef.deref()]));
90+
otherComposedSignalRef.deref().addEventListener('abort', handler);
91+
92+
composedSignalRefs.push(composedSignalRef, otherComposedSignalRef);
93+
94+
setImmediate(() => {
95+
run(iteration + 1);
96+
});
97+
}
98+
99+
run(1);
100+
}
101+
67102
const limit = 10_000;
68103

69104
describe('when there is a long-lived signal', () => {
@@ -120,3 +155,23 @@ it('drops settled dependant signals when signal is composite', (t, done) => {
120155
});
121156
});
122157
});
158+
159+
it('drops settled signals even when there are listeners', (t, done) => {
160+
runWithOrphanListeners(limit, (signalRefs) => {
161+
setImmediate(() => {
162+
global.gc();
163+
setImmediate(() => {
164+
global.gc(); // One more call needed to clean up the deeper composed signals
165+
setImmediate(() => {
166+
global.gc(); // One more call needed to clean up the deeper composed signals
167+
168+
const unGCedSignals = [...signalRefs].filter((ref) => ref.deref());
169+
170+
t.assert.strictEqual(unGCedSignals.length, 0);
171+
172+
done();
173+
});
174+
});
175+
});
176+
});
177+
});

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.