Skip to content

Commit 9d6626a

Browse files
authored
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 d8e9e05 commit 9d6626a

File tree

2 files changed

+67
-0
lines changed

2 files changed

+67
-0
lines changed

lib/internal/abort_controller.js

+39
Original file line numberDiff line numberDiff line change
@@ -259,14 +259,30 @@ class AbortSignal extends EventTarget {
259259
if (!signalsArray.length) {
260260
return resultSignal;
261261
}
262+
262263
const resultSignalWeakRef = new SafeWeakRef(resultSignal);
263264
resultSignal[kSourceSignals] = new SafeSet();
265+
266+
// Track if we have any timeout signals
267+
let hasTimeoutSignals = false;
268+
264269
for (let i = 0; i < signalsArray.length; i++) {
265270
const signal = 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
return resultSignal;
269284
}
285+
270286
signal[kDependantSignals] ??= new SafeSet();
271287
if (!signal[kComposite]) {
272288
const signalWeakRef = new SafeWeakRef(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
return resultSignal;
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
const dependentSignalsToAbort = ObjectSetPrototypeOf([], null);
444+
421445
// 4. For each dependentSignal of signal's dependent signals:
422446
signal[kDependantSignals]?.forEach((s) => {
423447
const dependentSignal = 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 (let i = 0; i < dependentSignalsToAbort.length; i++) {
439464
const dependentSignal = 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+
const sourceSignal = sourceWeakRef.deref();
476+
if (sourceSignal) {
477+
gcPersistentSignals.delete(sourceSignal);
478+
}
479+
});
480+
}
442481
}
443482

444483
// To run the abort steps for an AbortSignal signal
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)