Skip to content

Commit a9473bb

Browse files
geeksilva97aduh95
authored andcommitted
lib: remove settled dependant signals when they are GCed
PR-URL: #55354 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
1 parent d9f0407 commit a9473bb

2 files changed

Lines changed: 145 additions & 9 deletions

File tree

‎lib/internal/abort_controller.js‎

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,17 @@ function lazyMessageChannel() {
8484
}
8585

8686
constclearTimeoutRegistry=newSafeFinalizationRegistry(clearTimeout);
87+
constdependantSignalsCleanupRegistry=newSafeFinalizationRegistry((signalWeakRef)=>{
88+
constsignal=signalWeakRef.deref();
89+
if(signal===undefined){
90+
return;
91+
}
92+
signal[kDependantSignals].forEach((ref)=>{
93+
if(ref.deref()===undefined){
94+
signal[kDependantSignals].delete(ref);
95+
}
96+
});
97+
});
8798
constgcPersistentSignals=newSafeSet();
8899

89100
constkAborted=Symbol('kAborted');
@@ -243,24 +254,27 @@ class AbortSignal extends EventTarget {
243254
}
244255
signal[kDependantSignals]??=newSafeSet();
245256
if(!signal[kComposite]){
246-
resultSignal[kSourceSignals].add(newSafeWeakRef(signal));
257+
constsignalWeakRef=newSafeWeakRef(signal);
258+
resultSignal[kSourceSignals].add(signalWeakRef);
247259
signal[kDependantSignals].add(resultSignalWeakRef);
260+
dependantSignalsCleanupRegistry.register(resultSignal,signalWeakRef);
248261
}elseif(!signal[kSourceSignals]){
249262
continue;
250263
}else{
251-
for(constsourceSignalofsignal[kSourceSignals]){
252-
constsourceSignalRef=sourceSignal.deref();
253-
if(!sourceSignalRef){
264+
for(constsourceSignalWeakRefofsignal[kSourceSignals]){
265+
constsourceSignal=sourceSignalWeakRef.deref();
266+
if(!sourceSignal){
254267
continue;
255268
}
256-
assert(!sourceSignalRef.aborted);
257-
assert(!sourceSignalRef[kComposite]);
269+
assert(!sourceSignal.aborted);
270+
assert(!sourceSignal[kComposite]);
258271

259-
if(resultSignal[kSourceSignals].has(sourceSignal)){
272+
if(resultSignal[kSourceSignals].has(sourceSignalWeakRef)){
260273
continue;
261274
}
262-
resultSignal[kSourceSignals].add(sourceSignal);
263-
sourceSignalRef[kDependantSignals].add(resultSignalWeakRef);
275+
resultSignal[kSourceSignals].add(sourceSignalWeakRef);
276+
sourceSignal[kDependantSignals].add(resultSignalWeakRef);
277+
dependantSignalsCleanupRegistry.register(resultSignal,sourceSignalWeakRef);
264278
}
265279
}
266280
}
Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
// Flags: --expose_gc
2+
//
3+
import'../common/index.mjs';
4+
import{describe,it}from'node:test';
5+
6+
functionmakeSubsequentCalls(limit,done,holdReferences=false){
7+
letdependantSymbol;
8+
letsignalRef;
9+
constac=newAbortController();
10+
constretainedSignals=[];
11+
consthandler=()=>{};
12+
13+
functionrun(iteration){
14+
if(iteration>limit){
15+
// This setImmediate is necessary to ensure that in the last iteration the remaining signal is GCed (if not
16+
// retained)
17+
setImmediate(()=>{
18+
global.gc();
19+
done(ac.signal,dependantSymbol);
20+
});
21+
return;
22+
}
23+
24+
if(holdReferences){
25+
retainedSignals.push(AbortSignal.any([ac.signal]));
26+
}else{
27+
// Using a WeakRef to avoid retaining information that will interfere with the test
28+
signalRef=newWeakRef(AbortSignal.any([ac.signal]));
29+
signalRef.deref().addEventListener('abort',handler);
30+
}
31+
32+
dependantSymbol??=Object.getOwnPropertySymbols(ac.signal).find(
33+
(s)=>s.toString()==='Symbol(kDependantSignals)'
34+
);
35+
36+
setImmediate(()=>{
37+
// Removing the event listener at some moment in the future
38+
// Which will then allow the signal to be GCed
39+
signalRef?.deref()?.removeEventListener('abort',handler);
40+
run(iteration+1);
41+
});
42+
}
43+
44+
run(1);
45+
};
46+
47+
functionrunShortLivedSourceSignal(limit,done){
48+
constsignalRefs=newSet();
49+
50+
functionrun(iteration){
51+
if(iteration>limit){
52+
global.gc();
53+
done(signalRefs);
54+
return;
55+
}
56+
57+
constac=newAbortController();
58+
signalRefs.add(newWeakRef(ac.signal));
59+
AbortSignal.any([ac.signal]);
60+
61+
setImmediate(()=>run(iteration+1));
62+
}
63+
64+
run(1);
65+
};
66+
67+
constlimit=10_000;
68+
69+
describe('when there is a long-lived signal',()=>{
70+
it('drops settled dependant signals',(t,done)=>{
71+
makeSubsequentCalls(limit,(signal,depandantSignalsKey)=>{
72+
setImmediate(()=>{
73+
t.assert.strictEqual(signal[depandantSignalsKey].size,0);
74+
done();
75+
});
76+
});
77+
});
78+
79+
it('keeps all active dependant signals',(t,done)=>{
80+
makeSubsequentCalls(limit,(signal,depandantSignalsKey)=>{
81+
t.assert.strictEqual(signal[depandantSignalsKey].size,limit);
82+
83+
done();
84+
},true);
85+
});
86+
});
87+
88+
it('does not prevent source signal from being GCed if it is short-lived',(t,done)=>{
89+
runShortLivedSourceSignal(limit,(signalRefs)=>{
90+
setImmediate(()=>{
91+
constunGCedSignals=[...signalRefs].filter((ref)=>ref.deref());
92+
93+
t.assert.strictEqual(unGCedSignals.length,0);
94+
done();
95+
});
96+
});
97+
});
98+
99+
it('drops settled dependant signals when signal is composite',(t,done)=>{
100+
constcontrollers=Array.from({length: 2},()=>newAbortController());
101+
constcomposedSignal1=AbortSignal.any([controllers[0].signal]);
102+
constcomposedSignalRef=newWeakRef(AbortSignal.any([composedSignal1,controllers[1].signal]));
103+
104+
constkDependantSignals=Object.getOwnPropertySymbols(controllers[0].signal).find(
105+
(s)=>s.toString()==='Symbol(kDependantSignals)'
106+
);
107+
108+
setImmediate(()=>{
109+
global.gc();
110+
111+
t.assert.strictEqual(composedSignalRef.deref(),undefined);
112+
t.assert.strictEqual(controllers[0].signal[kDependantSignals].size,2);
113+
t.assert.strictEqual(controllers[1].signal[kDependantSignals].size,1);
114+
115+
setImmediate(()=>{
116+
t.assert.strictEqual(controllers[0].signal[kDependantSignals].size,0);
117+
t.assert.strictEqual(controllers[1].signal[kDependantSignals].size,0);
118+
119+
done();
120+
});
121+
});
122+
});

0 commit comments

Comments
 (0)