Repository: nodejs/node
Pull request: lib: fix AbortSignal.any() with timeout signals

Fixes #57736

This PR fixes an issue where AbortSignal.any([AbortSignal.timeout(ms)]) would sometimes fail. The problem occurred because timeout signals inside a composite signal (created by AbortSignal.any()) were being garbage collected before they could fire.

- Modified AbortSignal.any() to identify timeout signals and add them to the gcPersistentSignals set
- Added cleanup for source signals when a composite signal aborts

I don't know how to make a simpler test for this honestly. Open to suggestions.

I used the following test to check for memory leaks. I assume that it works because when I initially made a mistake trying to fix this, it did cause a memory leak and it was reflected in this test:

After:

```
gurgunday@DELL-G15:~/work/oss/node$ ./out/Release/node --expose-gc leak-test.js
Starting memory leak test for AbortSignal.any()
Initial memory usage: 3.62 MB
Batch 1/100, Memory: 3.74 MB
Batch 11/100, Memory: 3.83 MB
Batch 21/100, Memory: 3.84 MB
Batch 31/100, Memory: 3.84 MB
Batch 41/100, Memory: 3.86 MB
Batch 51/100, Memory: 3.87 MB
Batch 61/100, Memory: 3.86 MB
Batch 71/100, Memory: 3.90 MB
Batch 81/100, Memory: 3.92 MB
Batch 91/100, Memory: 3.92 MB
Batch 100/100, Memory: 3.92 MB

Test completed:
- Initial memory: 3.62 MB
- Final memory: 3.93 MB
- Difference: 0.30 MB
```

Before:

```
gurgunday@DELL-G15:~/work/oss/node$ node --expose-gc leak-test.js
Starting memory leak test for AbortSignal.any()
Initial memory usage: 3.65 MB
Batch 1/100, Memory: 3.76 MB
Batch 11/100, Memory: 3.78 MB
Batch 21/100, Memory: 3.82 MB
Batch 31/100, Memory: 3.87 MB
Batch 41/100, Memory: 3.87 MB
Batch 51/100, Memory: 3.88 MB
Batch 61/100, Memory: 3.88 MB
Batch 71/100, Memory: 3.93 MB
Batch 81/100, Memory: 3.94 MB
Batch 91/100, Memory: 3.94 MB
Batch 100/100, Memory: 3.95 MB

Test completed:
- Initial memory: 3.65 MB
- Final memory: 3.96 MB
- Difference: 0.30 MB
```

```js
'use strict';

// This test creates many composite signals with timeouts 
// and checks if memory usage grows unbounded

// We use --expose-gc to manually trigger garbage collection
// Run with: node --expose-gc leak-test.js

const ITERATIONS = 10000;
const BATCH_SIZE = 100;
const TIMEOUT = 100; // Small timeout so it resolves quickly

// Record initial memory
const getMemoryUsage = () => process.memoryUsage().heapUsed / 1024 / 1024;
let initialMemory = 0;
let finalMemory = 0;
let signals = [];

async function runTest() {
  console.log('Starting memory leak test for AbortSignal.any()');
  
  // Force garbage collection before we start
  if (global.gc) {
    global.gc();
    await new Promise(resolve => setTimeout(resolve, 100));
    global.gc();
  } else {
    console.warn('Start node with --expose-gc to get more accurate results');
  }
  
  // Record initial memory after GC
  initialMemory = getMemoryUsage();
  console.log(`Initial memory usage: ${initialMemory.toFixed(2)} MB`);
  
  // Run multiple batches and force GC between them
  for (let batch = 0; batch < ITERATIONS / BATCH_SIZE; batch++) {
    signals = [];
    for (let i = 0; i < BATCH_SIZE; i++) {
      const timeoutSignal = AbortSignal.timeout(TIMEOUT);
      const compositeSignal = AbortSignal.any([timeoutSignal]);
      
      // Hold a reference to the composite signal
      signals.push(compositeSignal);
    }
    
    await new Promise(resolve => setTimeout(resolve, TIMEOUT * 2));
    
    // Clear references and force GC
    signals = [];
    if (global.gc) {
      global.gc();
      await new Promise(resolve => setTimeout(resolve, 100));
      global.gc();
    }
    
    // Log memory usage
    if (batch % 10 === 0 || batch === (ITERATIONS / BATCH_SIZE) - 1) {
      const currentMemory = getMemoryUsage();
      console.log(`Batch ${batch + 1}/${ITERATIONS / BATCH_SIZE}, Memory: ${currentMemory.toFixed(2)} MB`);
    }
  }
  
  finalMemory = getMemoryUsage();
  
  console.log(`\nTest completed:`);
  console.log(`- Initial memory: ${initialMemory.toFixed(2)} MB`);
  console.log(`- Final memory: ${finalMemory.toFixed(2)} MB`);
  console.log(`- Difference: ${(finalMemory - initialMemory).toFixed(2)} MB`);
}

runTest().catch(console.error);
```

Changed files:
filename: lib/internal/abort_controller.js
patch: @@ -259,14 +259,30 @@ class AbortSignal extends EventTarget {
     if (!signalsArray.length) {
       return resultSignal;
     }
+
     const resultSignalWeakRef = new SafeWeakRef(resultSignal);
     resultSignal[kSourceSignals] = new SafeSet();
+
+    // Track if we have any timeout signals
+    let hasTimeoutSignals = false;
+
     for (let i = 0; i < signalsArray.length; i++) {
       const signal = signalsArray[i];
+
+      // Check if this is a timeout signal
+      if (signal[kTimeout]) {
+        hasTimeoutSignals = true;
+
+        // Add the timeout signal to gcPersistentSignals to keep it alive
+        // This is what the kNewListener method would do when adding abort listeners
+        gcPersistentSignals.add(signal);
+      }
+
       if (signal.aborted) {
         abortSignal(resultSignal, signal.reason);
         return resultSignal;
       }
+
       signal[kDependantSignals] ??= new SafeSet();
       if (!signal[kComposite]) {
         const signalWeakRef = new SafeWeakRef(signal);
@@ -301,6 +317,12 @@ class AbortSignal extends EventTarget {
         }
       }
     }
+
+    // If we have any timeout signals, add the composite signal to gcPersistentSignals
+    if (hasTimeoutSignals && resultSignal[kSourceSignals].size > 0) {
+      gcPersistentSignals.add(resultSignal);
+    }
+
     return resultSignal;
   }
 
@@ -416,8 +438,10 @@ function abortSignal(signal, reason) {
   //    otherwise to a new "AbortError" DOMException.
   signal[kAborted] = true;
   signal[kReason] = reason;
+
   // 3. Let dependentSignalsToAbort be a new list.
   const dependentSignalsToAbort = ObjectSetPrototypeOf([], null);
+
   // 4. For each dependentSignal of signal's dependent signals:
   signal[kDependantSignals]?.forEach((s) => {
     const dependentSignal = s.deref();
@@ -433,12 +457,27 @@ function abortSignal(signal, reason) {
 
   // 5. Run the abort steps for signal
   runAbort(signal);
+
   // 6. For each dependentSignal of dependentSignalsToAbort,
   //    run the abort steps for dependentSignal.
   for (let i = 0; i < dependentSignalsToAbort.length; i++) {
     const dependentSignal = dependentSignalsToAbort[i];
     runAbort(dependentSignal);
   }
+
+  // Clean up the signal from gcPersistentSignals
+  gcPersistentSignals.delete(signal);
+
+  // If this is a composite signal, also remove all of its source signals from gcPersistentSignals
+  // when they get dereferenced from the signal's kSourceSignals set
+  if (signal[kComposite] && signal[kSourceSignals]) {
+    signal[kSourceSignals].forEach((sourceWeakRef) => {
+      const sourceSignal = sourceWeakRef.deref();
+      if (sourceSignal) {
+        gcPersistentSignals.delete(sourceSignal);
+      }
+    });
+  }
 }
 
 // To run the abort steps for an AbortSignal signal

filename: test/parallel/test-abort-controller-any-timeout.js
patch: @@ -0,0 +1,28 @@
+'use strict';
+
+require('../common');
+const assert = require('assert');
+const { once } = require('node:events');
+const { describe, it } = require('node:test');
+
+describe('AbortSignal.any() with timeout signals', () => {
+  it('should abort when the first timeout signal fires', async () => {
+    const signal = AbortSignal.any([AbortSignal.timeout(9000), AbortSignal.timeout(110000)]);
+
+    const abortPromise = Promise.race([
+      once(signal, 'abort').then(() => {
+        throw signal.reason;
+      }),
+      new Promise((resolve) => setTimeout(resolve, 10000)),
+    ]);
+
+    // The promise should be aborted by the 9000ms timeout
+    await assert.rejects(
+      () => abortPromise,
+      {
+        name: 'TimeoutError',
+        message: 'The operation was aborted due to timeout'
+      }
+    );
+  });
+});