Skip to content

Commit 5edcb28

Browse files
gurgundayRafaelGSS
authored andcommitted
lib: fix AbortSignal.any() with timeout signals
PR-URL: #57867 Reviewed-By: Edy Silva <edigleyssonsilva@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
1 parent 594269f commit 5edcb28

2 files changed

Lines changed: 67 additions & 0 deletions

File tree

‎lib/internal/abort_controller.js‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -259,14 +259,30 @@ class AbortSignal extends EventTarget {
259259
if(!signalsArray.length){
260260
returnresultSignal;
261261
}
262+
262263
constresultSignalWeakRef=newSafeWeakRef(resultSignal);
263264
resultSignal[kSourceSignals]=newSafeSet();
265+
266+
// Track if we have any timeout signals
267+
lethasTimeoutSignals=false;
268+
264269
for(leti=0;i<signalsArray.length;i++){
265270
constsignal=signalsArray[i];
271+
272+
// Check if this is a timeout signal
273+
if(signal[kTimeout]){
274+
hasTimeoutSignals=true;
275+
276+
// Add the timeout signal to gcPersistentSignals to keep it alive
277+
// This is what the kNewListener method would do when adding abort listeners
278+
gcPersistentSignals.add(signal);
279+
}
280+
266281
if(signal.aborted){
267282
abortSignal(resultSignal,signal.reason);
268283
returnresultSignal;
269284
}
285+
270286
signal[kDependantSignals]??=newSafeSet();
271287
if(!signal[kComposite]){
272288
constsignalWeakRef=newSafeWeakRef(signal);
@@ -301,6 +317,12 @@ class AbortSignal extends EventTarget {
301317
}
302318
}
303319
}
320+
321+
// If we have any timeout signals, add the composite signal to gcPersistentSignals
322+
if(hasTimeoutSignals&&resultSignal[kSourceSignals].size>0){
323+
gcPersistentSignals.add(resultSignal);
324+
}
325+
304326
returnresultSignal;
305327
}
306328

@@ -416,8 +438,10 @@ function abortSignal(signal, reason) {
416438
// otherwise to a new "AbortError" DOMException.
417439
signal[kAborted]=true;
418440
signal[kReason]=reason;
441+
419442
// 3. Let dependentSignalsToAbort be a new list.
420443
constdependentSignalsToAbort=ObjectSetPrototypeOf([],null);
444+
421445
// 4. For each dependentSignal of signal's dependent signals:
422446
signal[kDependantSignals]?.forEach((s)=>{
423447
constdependentSignal=s.deref();
@@ -433,12 +457,27 @@ function abortSignal(signal, reason) {
433457

434458
// 5. Run the abort steps for signal
435459
runAbort(signal);
460+
436461
// 6. For each dependentSignal of dependentSignalsToAbort,
437462
// run the abort steps for dependentSignal.
438463
for(leti=0;i<dependentSignalsToAbort.length;i++){
439464
constdependentSignal=dependentSignalsToAbort[i];
440465
runAbort(dependentSignal);
441466
}
467+
468+
// Clean up the signal from gcPersistentSignals
469+
gcPersistentSignals.delete(signal);
470+
471+
// If this is a composite signal, also remove all of its source signals from gcPersistentSignals
472+
// when they get dereferenced from the signal's kSourceSignals set
473+
if(signal[kComposite]&&signal[kSourceSignals]){
474+
signal[kSourceSignals].forEach((sourceWeakRef)=>{
475+
constsourceSignal=sourceWeakRef.deref();
476+
if(sourceSignal){
477+
gcPersistentSignals.delete(sourceSignal);
478+
}
479+
});
480+
}
442481
}
443482

444483
// To run the abort steps for an AbortSignal signal
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
'use strict';
2+
3+
require('../common');
4+
constassert=require('assert');
5+
const{ once }=require('node:events');
6+
const{ describe, it }=require('node:test');
7+
8+
describe('AbortSignal.any() with timeout signals',()=>{
9+
it('should abort when the first timeout signal fires',async()=>{
10+
constsignal=AbortSignal.any([AbortSignal.timeout(9000),AbortSignal.timeout(110000)]);
11+
12+
constabortPromise=Promise.race([
13+
once(signal,'abort').then(()=>{
14+
throwsignal.reason;
15+
}),
16+
newPromise((resolve)=>setTimeout(resolve,10000)),
17+
]);
18+
19+
// The promise should be aborted by the 9000ms timeout
20+
awaitassert.rejects(
21+
()=>abortPromise,
22+
{
23+
name: 'TimeoutError',
24+
message: 'The operation was aborted due to timeout'
25+
}
26+
);
27+
});
28+
});

0 commit comments

Comments
 (0)