lib: fix AbortSignal.any() with timeout signals by gurgunday · Pull Request #57867 · nodejs/node · GitHub

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

'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);

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.24%. Comparing base (964e41c) to head (77f6539).
Report is 31 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff            @@
##             main   #57867    +/-   ##
========================================
  Coverage   90.23%   90.24%            
========================================
  Files         630      630            
  Lines      185518   185726   +208     
  Branches    36369    36410    +41     
========================================
+ Hits       167401   167604   +203     
+ Misses      11005    10999     -6     
- Partials     7112     7123    +11

Files with missing lines	Coverage Δ
lib/internal/abort_controller.js	98.26% <100.00%> (+0.12%)	⬆️

... and 36 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

LGTM

CI: https://ci.nodejs.org/job/node-test-pull-request/66266/

Do we really need this?
Can't we validate the reason is a timeout signal?

Updated, does it look better?

CI: https://ci.nodejs.org/job/node-test-pull-request/66317/

CI: https://ci.nodejs.org/job/node-test-pull-request/66323/

Landed in 9d6626a

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>

The relevant bug was fixed upstream: nodejs/node#57867

`AbortSignal.any([caller, AbortSignal.timeout(ms)])` has a history of dropping the timeout: the composed signal can be collected while the request is in flight, so the deadline never fires and the request hangs — the exact failure the deadline exists to prevent (nodejs/node#57736, fixed by nodejs/node#57867). Raising the package's Node floor would fix it, but that is a breaking constraint on every consumer to work around an implementation detail. Wiring the controller by hand behaves identically on every supported version, and lets the timer be cleared once the request settles rather than leaving one pending per request for its full timeout. The body read moved inside the deadline too — headers can arrive promptly while the body stalls. Also from review: - Docs: `--session` is cloud-only, so the flag table says so. - Docs: the isolation wording implied `storageMode: 'readonly'` removed project-file interaction. It stops *writes*; the session still reads the same shared files. Split into what is and is not isolated. - Docs: the sessions example called `getNotebook` without importing it, and passed the optional `sessionNotebookId` where a string is required — neither would have compiled if copied. - Tests: the pending-fetch helper now rejects immediately for an already-aborted signal, as real fetch does. Added cases for timer cleanup on both the success and failure paths, and for an already-aborted caller signal. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

gurgunday added 2 commits April 13, 2025 23:23

lib: fix AbortSignal.any() with timeout signals

cb31cff

add test

4bba365

nodejs-github-bot added the needs-ci PRs that need a full CI run. label Apr 13, 2025

geeksilva97 requested a review from atlowChemi April 14, 2025 02:47

geeksilva97 approved these changes Apr 14, 2025

View reviewed changes

gurgunday added the abortcontroller Issues and PRs related to the AbortController API label Apr 14, 2025

jasnell approved these changes Apr 14, 2025

View reviewed changes

geeksilva97 added author ready PRs that have at least one approval, no outstanding review comments, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Apr 14, 2025

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 14, 2025

atlowChemi reviewed Apr 15, 2025

View reviewed changes

improve test

7fd4140

gurgunday requested a review from atlowChemi April 15, 2025 16:34

lint

77f6539

atlowChemi approved these changes Apr 16, 2025

View reviewed changes

geeksilva97 added request-ci Add this label to start a Jenkins CI on a PR. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels Apr 16, 2025

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 16, 2025

geeksilva97 added the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 17, 2025

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 17, 2025

nodejs-github-bot merged commit 9d6626a into nodejs:main Apr 17, 2025

RafaelGSS pushed a commit that referenced this pull request May 1, 2025

lib: fix AbortSignal.any() with timeout signals

92bba29

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>

RafaelGSS mentioned this pull request May 1, 2025

2025-05-06, Version 24.0.0 (Current) #57609

Merged

RafaelGSS pushed a commit that referenced this pull request May 2, 2025

lib: fix AbortSignal.any() with timeout signals

5edcb28

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>

aduh95 pushed a commit that referenced this pull request May 6, 2025

lib: fix AbortSignal.any() with timeout signals

520b546

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>

aduh95 pushed a commit that referenced this pull request May 6, 2025

lib: fix AbortSignal.any() with timeout signals

ce8eb19

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>

RafaelGSS pushed a commit that referenced this pull request May 14, 2025

lib: fix AbortSignal.any() with timeout signals

9c60375

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>

aduh95 pushed a commit that referenced this pull request May 16, 2025

lib: fix AbortSignal.any() with timeout signals

f01263b

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>

aduh95 pushed a commit that referenced this pull request May 17, 2025

lib: fix AbortSignal.any() with timeout signals

6951d92

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>

aduh95 pushed a commit that referenced this pull request May 18, 2025

lib: fix AbortSignal.any() with timeout signals

93c2be9

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>

github-actions Bot mentioned this pull request May 19, 2025

2025-05-21, Version 22.16.0 'Jod' (LTS) #58388

Merged

aduh95 pushed a commit that referenced this pull request May 19, 2025

lib: fix AbortSignal.any() with timeout signals

d4fc282

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>

ghost mentioned this pull request Jun 8, 2025

### Notable Changes autofix-ci/action#28

Closed

sholladay added a commit to sholladay/sky that referenced this pull request Aug 7, 2025

Remove AbortSignal.any() polyfill

5253e5e

The relevant bug was fixed upstream: nodejs/node#57867

coderabbitai Bot mentioned this pull request Jun 25, 2026

feat(oauth): add xAI/Grok and Cursor subscription OAuth login darkspadez/plexus#66

Open

coderabbitai Bot mentioned this pull request Jul 14, 2026

Add Leader Election module to noobaa core noobaa/noobaa-core#9765

Closed

2 tasks

This was referenced Jul 30, 2026

feat: Add Isolate kit Lamatic/AgentKit#291

Merged

feat(ui,llm): settings accordion · timeline fold · context budget nehcuh/cmspark#131

Merged

coderabbitai Bot mentioned this pull request Aug 8, 2026

feat: migrate Vite SPA to Next.js with Supabase realtime PSergio984/task-buddy-frontend#1

Merged

coderabbitai Bot mentioned this pull request Aug 25, 2026

feat: Implement @singleton-sd/post-kit-client package singleton-sd/post-kit#34

Open

3 tasks

+                  const duration = Date.now() - start;
+                  // Verify the timeout happened at approximately the right time (with some margin)
+                  assert.ok(duration >= 8900, `Timeout happened too early: ${duration}ms`);
+                  assert.ok(duration < 11000, `Timeout happened too late: ${duration}ms`);

lib: fix AbortSignal.any() with timeout signals by gurgunday · Pull Request #57867 · nodejs/node · GitHub

Uh oh!

Uh oh!

lib: fix AbortSignal.any() with timeout signals - #57867

lib: fix AbortSignal.any() with timeout signals#57867
nodejs-github-bot merged 4 commits into
nodejs:mainfrom
gurgunday:fix/abortsignal-any

gurgunday commented Apr 13, 2025 •
edited
Loading

Uh oh!

Uh oh!

codecov Bot commented Apr 13, 2025 •
edited
Loading

Uh oh!

Uh oh!

geeksilva97 left a comment

Uh oh!

nodejs-github-bot commented Apr 14, 2025

Uh oh!

Uh oh!

atlowChemi Apr 15, 2025

Uh oh!

gurgunday Apr 15, 2025

Uh oh!

nodejs-github-bot commented Apr 16, 2025

Uh oh!

nodejs-github-bot commented Apr 16, 2025

Uh oh!

nodejs-github-bot commented Apr 17, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

lib: fix AbortSignal.any() with timeout signals by gurgunday · Pull Request #57867 · nodejs/node · GitHub

Uh oh!

Uh oh!

lib: fix AbortSignal.any() with timeout signals - #57867

lib: fix AbortSignal.any() with timeout signals#57867nodejs-github-bot merged 4 commits intonodejs:mainnodejs/node:mainfrom gurgunday:fix/abortsignal-anygurgunday/node:fix/abortsignal-anyCopy head branch name to clipboard

Conversation

gurgunday commented Apr 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Apr 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

geeksilva97 left a comment

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented Apr 14, 2025

Uh oh!

Uh oh!

atlowChemi Apr 15, 2025

Choose a reason for hiding this comment

Uh oh!

gurgunday Apr 15, 2025

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented Apr 16, 2025

Uh oh!

nodejs-github-bot commented Apr 16, 2025

Uh oh!

nodejs-github-bot commented Apr 17, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

lib: fix AbortSignal.any() with timeout signals#57867
nodejs-github-bot merged 4 commits into
nodejs:mainfrom
gurgunday:fix/abortsignal-any

gurgunday commented Apr 13, 2025 •
edited
Loading

codecov Bot commented Apr 13, 2025 •
edited
Loading