Skip to content

Commit d065e99

Browse files
Han5991aduh95
authored andcommitted
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. PR-URL: #62367Fixes: #62363 Refs: #54614 Reviewed-By: Edy Silva <edigleyssonsilva@gmail.com> Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
1 parent 899d780 commit d065e99

2 files changed

Lines changed: 116 additions & 28 deletions

File tree

‎lib/internal/abort_controller.js‎

Lines changed: 86 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -85,17 +85,16 @@ function lazyMessageChannel() {
8585
}
8686

8787
constclearTimeoutRegistry=newSafeFinalizationRegistry(clearTimeout);
88-
constdependantSignalsCleanupRegistry=newSafeFinalizationRegistry((signalWeakRef)=>{
89-
constsignal=signalWeakRef.deref();
90-
if(signal===undefined){
91-
return;
92-
}
93-
signal[kDependantSignals].forEach((ref)=>{
94-
if(ref.deref()===undefined){
95-
signal[kDependantSignals].delete(ref);
88+
constdependantSignalsCleanupRegistry=newSafeFinalizationRegistry(
89+
({ sourceSignalRef, dependantSignalRef, sourceSignalsCleanupToken })=>{
90+
sourceSignalsCleanupRegistry.unregister(sourceSignalsCleanupToken);
91+
92+
constsourceSignal=sourceSignalRef.deref();
93+
if(sourceSignal===undefined){
94+
return;
9695
}
96+
sourceSignal[kDependantSignals].delete(dependantSignalRef);
9797
});
98-
});
9998

10099
constgcPersistentSignals=newSafeSet();
101100

@@ -117,6 +116,8 @@ const kCloneData = Symbol('kCloneData');
117116
constkTimeout=Symbol('kTimeout');
118117
constkMakeTransferable=Symbol('kMakeTransferable');
119118
constkComposite=Symbol('kComposite');
119+
constkFollowing=Symbol('kFollowing');
120+
constkResultSignalWeakRef=Symbol('kResultSignalWeakRef');
120121
constkSourceSignals=Symbol('kSourceSignals');
121122
constkDependantSignals=Symbol('kDependantSignals');
122123

@@ -136,6 +137,60 @@ function validateThisAbortSignal(obj) {
136137
thrownewERR_INVALID_THIS('AbortSignal');
137138
}
138139

140+
functionrefreshCompositeSignal(signal){
141+
if(!signal[kComposite]||signal[kAborted]||!signal[kSourceSignals]?.size){
142+
return;
143+
}
144+
145+
for(constsourceSignalWeakRefofsignal[kSourceSignals]){
146+
constsourceSignal=sourceSignalWeakRef.deref();
147+
if(sourceSignal===undefined){
148+
signal[kSourceSignals].delete(sourceSignalWeakRef);
149+
continue;
150+
}
151+
152+
if(sourceSignal.aborted){
153+
abortSignal(signal,sourceSignal.reason);
154+
return;
155+
}
156+
}
157+
}
158+
159+
functionfollowCompositeSignal(signal){
160+
if(signal[kFollowing]||signal[kAborted]||!signal[kSourceSignals]?.size){
161+
return;
162+
}
163+
164+
constresultSignalWeakRef=signal[kResultSignalWeakRef]??=newSafeWeakRef(signal);
165+
166+
for(constsourceSignalWeakRefofsignal[kSourceSignals]){
167+
constsourceSignal=sourceSignalWeakRef.deref();
168+
if(sourceSignal===undefined){
169+
signal[kSourceSignals].delete(sourceSignalWeakRef);
170+
continue;
171+
}
172+
173+
if(sourceSignal.aborted){
174+
abortSignal(signal,sourceSignal.reason);
175+
return;
176+
}
177+
178+
sourceSignal[kDependantSignals]??=newSafeSet();
179+
sourceSignal[kDependantSignals].add(resultSignalWeakRef);
180+
dependantSignalsCleanupRegistry.register(signal,{
181+
sourceSignalRef: sourceSignalWeakRef,
182+
dependantSignalRef: resultSignalWeakRef,
183+
sourceSignalsCleanupToken: sourceSignalWeakRef,
184+
});
185+
sourceSignalsCleanupRegistry.register(sourceSignal,{
186+
sourceSignalRef: sourceSignalWeakRef,
187+
composedSignalRef: resultSignalWeakRef,
188+
},sourceSignalWeakRef);
189+
}
190+
191+
signal[kFollowing]=true;
192+
}
193+
139194
// Because the AbortSignal timeout cannot be canceled, we don't want the
140195
// presence of the timer alone to keep the AbortSignal from being garbage
141196
// collected if it otherwise no longer accessible. We also don't want the
@@ -148,6 +203,7 @@ function setWeakAbortSignalTimeout(weakRef, delay) {
148203
consttimeout=setTimeout(()=>{
149204
constsignal=weakRef.deref();
150205
if(signal!==undefined){
206+
clearTimeoutRegistry.unregister(signal);
151207
gcPersistentSignals.delete(signal);
152208
abortSignal(
153209
signal,
@@ -198,6 +254,7 @@ class AbortSignal extends EventTarget {
198254
*/
199255
getaborted(){
200256
validateThisAbortSignal(this);
257+
refreshCompositeSignal(this);
201258
return!!this[kAborted];
202259
}
203260

@@ -206,11 +263,13 @@ class AbortSignal extends EventTarget {
206263
*/
207264
getreason(){
208265
validateThisAbortSignal(this);
266+
refreshCompositeSignal(this);
209267
returnthis[kReason];
210268
}
211269

212270
throwIfAborted(){
213271
validateThisAbortSignal(this);
272+
refreshCompositeSignal(this);
214273
if(this[kAborted]){
215274
throwthis[kReason];
216275
}
@@ -241,7 +300,8 @@ class AbortSignal extends EventTarget {
241300
signal[kTimeout]=true;
242301
clearTimeoutRegistry.register(
243302
signal,
244-
setWeakAbortSignalTimeout(newSafeWeakRef(signal),delay));
303+
setWeakAbortSignalTimeout(newSafeWeakRef(signal),delay),
304+
signal);
245305
returnsignal;
246306
}
247307

@@ -260,7 +320,6 @@ class AbortSignal extends EventTarget {
260320
returnresultSignal;
261321
}
262322

263-
constresultSignalWeakRef=newSafeWeakRef(resultSignal);
264323
resultSignal[kSourceSignals]=newSafeSet();
265324

266325
// Track if we have any timeout signals
@@ -283,51 +342,51 @@ class AbortSignal extends EventTarget {
283342
returnresultSignal;
284343
}
285344

286-
signal[kDependantSignals]??=newSafeSet();
287345
if(!signal[kComposite]){
288346
constsignalWeakRef=newSafeWeakRef(signal);
289347
resultSignal[kSourceSignals].add(signalWeakRef);
290-
signal[kDependantSignals].add(resultSignalWeakRef);
291-
dependantSignalsCleanupRegistry.register(resultSignal,signalWeakRef);
292-
sourceSignalsCleanupRegistry.register(signal,{
293-
sourceSignalRef: signalWeakRef,
294-
composedSignalRef: resultSignalWeakRef,
295-
});
296348
}elseif(!signal[kSourceSignals]){
297349
continue;
298350
}else{
351+
refreshCompositeSignal(signal);
352+
if(signal.aborted){
353+
abortSignal(resultSignal,signal.reason);
354+
returnresultSignal;
355+
}
299356
for(constsourceSignalWeakRefofsignal[kSourceSignals]){
300357
constsourceSignal=sourceSignalWeakRef.deref();
301358
if(!sourceSignal){
302359
continue;
303360
}
304-
assert(!sourceSignal.aborted);
305361
assert(!sourceSignal[kComposite]);
306362

363+
if(sourceSignal.aborted){
364+
abortSignal(resultSignal,sourceSignal.reason);
365+
returnresultSignal;
366+
}
367+
307368
if(resultSignal[kSourceSignals].has(sourceSignalWeakRef)){
308369
continue;
309370
}
310371
resultSignal[kSourceSignals].add(sourceSignalWeakRef);
311-
sourceSignal[kDependantSignals].add(resultSignalWeakRef);
312-
dependantSignalsCleanupRegistry.register(resultSignal,sourceSignalWeakRef);
313-
sourceSignalsCleanupRegistry.register(signal,{
314-
sourceSignalRef: sourceSignalWeakRef,
315-
composedSignalRef: resultSignalWeakRef,
316-
});
317372
}
318373
}
319374
}
320375

321-
// If we have any timeout signals, add the composite signal to gcPersistentSignals
322376
if(hasTimeoutSignals&&resultSignal[kSourceSignals].size>0){
323-
gcPersistentSignals.add(resultSignal);
377+
resultSignal[kTimeout]=true;
324378
}
325379

326380
returnresultSignal;
327381
}
328382

329383
[kNewListener](size,type,listener,once,capture,passive,weak){
330384
super[kNewListener](size,type,listener,once,capture,passive,weak);
385+
386+
if(this[kComposite]&&type==='abort'&&!this.aborted&&size===1){
387+
followCompositeSignal(this);
388+
}
389+
331390
constisTimeoutOrNonEmptyCompositeSignal=this[kTimeout]||(this[kComposite]&&this[kSourceSignals]?.size);
332391
if(isTimeoutOrNonEmptyCompositeSignal&&
333392
type==='abort'&&

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

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,9 @@ function makeSubsequentCalls(limit, done, holdReferences = false) {
2323
}
2424

2525
if(holdReferences){
26-
retainedSignals.push(AbortSignal.any([ac.signal]));
26+
constsignal=AbortSignal.any([ac.signal]);
27+
signal.addEventListener('abort',handler);
28+
retainedSignals.push(signal);
2729
}else{
2830
// Using a WeakRef to avoid retaining information that will interfere with the test
2931
signalRef=newWeakRef(AbortSignal.any([ac.signal]));
@@ -119,6 +121,27 @@ describe('when there is a long-lived signal', () => {
119121
done();
120122
},true);
121123
});
124+
125+
it('does not keep retained dependent signals without listeners',(t,done)=>{
126+
constac=newAbortController();
127+
constretainedSignals=[];
128+
constkDependantSignals=Object.getOwnPropertySymbols(ac.signal).find(
129+
(s)=>s.toString()==='Symbol(kDependantSignals)'
130+
);
131+
132+
functionrun(iteration){
133+
if(iteration>limit){
134+
t.assert.strictEqual(ac.signal[kDependantSignals]?.size??0,0);
135+
done();
136+
return;
137+
}
138+
139+
retainedSignals.push(AbortSignal.any([ac.signal]));
140+
setImmediate(()=>run(iteration+1));
141+
}
142+
143+
run(1);
144+
});
122145
});
123146

124147
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
134157

135158
it('drops settled dependent signals when signal is composite',(t,done)=>{
136159
constcontrollers=Array.from({length: 2},()=>newAbortController());
160+
consthandler=()=>{};
137161

138162
// Using WeakRefs to avoid this test to retain information that will make the test fail
139163
constcomposedSignal1=newWeakRef(AbortSignal.any([controllers[0].signal]));
140164
constcomposedSignalRef=newWeakRef(AbortSignal.any([composedSignal1.deref(),controllers[1].signal]));
165+
composedSignal1.deref().addEventListener('abort',handler);
166+
composedSignalRef.deref().addEventListener('abort',handler);
141167

142168
constkDependantSignals=Object.getOwnPropertySymbols(controllers[0].signal).find(
143169
(s)=>s.toString()==='Symbol(kDependantSignals)'
@@ -147,6 +173,9 @@ it('drops settled dependent signals when signal is composite', (t, done) => {
147173
t.assert.strictEqual(controllers[1].signal[kDependantSignals].size,1);
148174

149175
setImmediate(mustCall(()=>{
176+
composedSignal1.deref()?.removeEventListener('abort',handler);
177+
composedSignalRef.deref()?.removeEventListener('abort',handler);
178+
150179
globalThis.gc({execution: 'async'}).then(async()=>{
151180
awaitgcUntil('all signals are GCed',()=>{
152181
consttotalDependantSignals=Math.max(

0 commit comments

Comments
 (0)