Skip to content

Commit f01263b

Browse files
gurgundayaduh95
authored andcommitted
lib: fix AbortSignal.any() with timeout signals
PR-URL: #57867 Reviewed-By: Edy Silva Reviewed-By: James M Snell Reviewed-By: Chemi Atlow
1 parent 4db8fe5 commit f01263b

File tree

2 files changed

+67
-0
lines changed

2 files changed

+67
-0
lines changed

lib/internal/abort_controller.js

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -260,14 +260,30 @@ class AbortSignal extends EventTarget {
260260
if (!signalsArray.length) {
261261
return resultSignal;
262262
}
263+
263264
const resultSignalWeakRef = new SafeWeakRef(resultSignal);
264265
resultSignal[kSourceSignals] = new SafeSet();
266+
267+
// Track if we have any timeout signals
268+
let hasTimeoutSignals = false;
269+
265270
for (let i = 0; i < signalsArray.length; i++) {
266271
const signal = signalsArray[i];
272+
273+
// Check if this is a timeout signal
274+
if (signal[kTimeout]) {
275+
hasTimeoutSignals = true;
276+
277+
// Add the timeout signal to gcPersistentSignals to keep it alive
278+
// This is what the kNewListener method would do when adding abort listeners
279+
gcPersistentSignals.add(signal);
280+
}
281+
267282
if (signal.aborted) {
268283
abortSignal(resultSignal, signal.reason);
269284
return resultSignal;
270285
}
286+
271287
signal[kDependantSignals] ??= new SafeSet();
272288
if (!signal[kComposite]) {
273289
const signalWeakRef = new SafeWeakRef(signal);
@@ -302,6 +318,12 @@ class AbortSignal extends EventTarget {
302318
}
303319
}
304320
}
321+
322+
// If we have any timeout signals, add the composite signal to gcPersistentSignals
323+
if (hasTimeoutSignals && resultSignal[kSourceSignals].size > 0) {
324+
gcPersistentSignals.add(resultSignal);
325+
}
326+
305327
return resultSignal;
306328
}
307329

@@ -414,8 +436,10 @@ function abortSignal(signal, reason) {
414436
// otherwise to a new "AbortError" DOMException.
415437
signal[kAborted] = true;
416438
signal[kReason] = reason;
439+
417440
// 3. Let dependentSignalsToAbort be a new list.
418441
const dependentSignalsToAbort = ObjectSetPrototypeOf([], null);
442+
419443
// 4. For each dependentSignal of signal's dependent signals:
420444
signal[kDependantSignals]?.forEach((s) => {
421445
const dependentSignal = s.deref();
@@ -431,12 +455,27 @@ function abortSignal(signal, reason) {
431455

432456
// 5. Run the abort steps for signal
433457
runAbort(signal);
458+
434459
// 6. For each dependentSignal of dependentSignalsToAbort,
435460
// run the abort steps for dependentSignal.
436461
for (let i = 0; i < dependentSignalsToAbort.length; i++) {
437462
const dependentSignal = dependentSignalsToAbort[i];
438463
runAbort(dependentSignal);
439464
}
465+
466+
// Clean up the signal from gcPersistentSignals
467+
gcPersistentSignals.delete(signal);
468+
469+
// If this is a composite signal, also remove all of its source signals from gcPersistentSignals
470+
// when they get dereferenced from the signal's kSourceSignals set
471+
if (signal[kComposite] && signal[kSourceSignals]) {
472+
signal[kSourceSignals].forEach((sourceWeakRef) => {
473+
const sourceSignal = sourceWeakRef.deref();
474+
if (sourceSignal) {
475+
gcPersistentSignals.delete(sourceSignal);
476+
}
477+
});
478+
}
440479
}
441480

442481
// 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+
const assert = 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+
const signal = AbortSignal.any([AbortSignal.timeout(9000), AbortSignal.timeout(110000)]);
11+
12+
const abortPromise = Promise.race([
13+
once(signal, 'abort').then(() => {
14+
throw signal.reason;
15+
}),
16+
new Promise((resolve) => setTimeout(resolve, 10000)),
17+
]);
18+
19+
// The promise should be aborted by the 9000ms timeout
20+
await assert.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)