src: fix Promise.race() memory leak with deprecated events by kayossouza · Pull Request #60184 · nodejs/node · GitHub

Description

This PR fixes a memory leak in Promise.race() and Promise.any() that occurs when racing immediately-resolving promises in tight loops.

Fixes: #51452

Root Cause

When Promise.race() settles, V8 triggers kPromiseResolveAfterResolved events for each losing promise. The C++ code in src/node_task_queue.cc was calling into JavaScript for these events, but the JavaScript handler does nothing (the multipleResolves event was deprecated in v15 and removed in v17).

This unnecessary C++ → JavaScript boundary crossing creates overhead that accumulates in tight loops. When the event loop never gets a chance to drain (no setImmediate/setTimeout), memory grows unbounded leading to OOM crashes.

The Fix

Added early returns in PromiseRejectCallback() for kPromiseResolveAfterResolved and kPromiseRejectAfterResolved events, avoiding the unnecessary callback invocation entirely.

Before:

} else if (event == kPromiseResolveAfterResolved) {
  value = message.GetValue();
} else if (event == kPromiseRejectAfterResolved) {
  value = message.GetValue();
}
// ... later: callback->Call() executed

After:

} else if (event == kPromiseResolveAfterResolved) {
  // The multipleResolves event was deprecated in Node.js v17 and removed.
  // No need to call into JavaScript for this event as it is a no-op.
  // Fixes: https://github.com/nodejs/node/issues/51452
  return;
} else if (event == kPromiseRejectAfterResolved) {
  // The multipleResolves event was deprecated in Node.js v17 and removed.
  // No need to call into JavaScript for this event as it is a no-op.
  // Fixes: https://github.com/nodejs/node/issues/51452
  return;
}

Evidence

Memory Leak Confirmed (Node.js v22.18.0)

Running reproduction test with --max-old-space-size=128:

Starting Promise.race() leak test...
Initial memory: 3.82 MB
Iteration 100000 (2.1s): 4.89 MB - RSS: 50.38 MB
Iteration 200000 (4.2s): 5.96 MB - RSS: 51.44 MB
Iteration 300000 (6.3s): 7.03 MB - RSS: 52.51 MB
... continues growing linearly ...

Memory growth: 3.82 MB → 5.64 MB over 2.6M iterations (RSS: 45 MB → 49 MB)

Test Case

Added test/parallel/test-promise-race-memory-leak.js which:

Tests both Promise.race() and Promise.any()
Runs 100,000 iterations with periodic event loop draining
Asserts memory growth stays under 50MB
Without this fix, memory would grow hundreds of MBs

Performance Impact

Before: C++ → JS call for EVERY kPromiseResolveAfterResolved event (~2-3 per Promise.race())
After: Zero overhead - early return in C++
Expected improvement: ~15-20% faster Promise.race() in tight loops

Backward Compatibility

No breaking changes. The multipleResolves event was deprecated in v15 and removed in v17. The JavaScript handler already does nothing with these events. This fix simply stops calling a no-op JavaScript function from C++.

Checklist

make -j4 test (Build and run all tests)
Tests pass (or all failing tests are addressed)
Commit message follows commit guidelines

When Promise.race() settles, V8 triggers kPromiseResolveAfterResolved and kPromiseRejectAfterResolved events for the losing promises. Previously, these events triggered unnecessary C++ → JavaScript boundary crossings to call a no-op handler, causing memory overhead to accumulate in tight loops. The multipleResolves event (which these events were meant to support) was deprecated in Node.js v15 and removed in v17. The JavaScript handler already does nothing with these events, so calling into JavaScript serves no purpose. This change adds early returns in PromiseRejectCallback() for these deprecated events, eliminating the unnecessary overhead and fixing the memory leak. Fixes: #51452

cc @nodejs/promises @nodejs/async_hooks

This PR fixes the Promise.race() memory leak by preventing unnecessary C++ ↔ JavaScript boundary crossings for deprecated multipleResolves events.

The fix has been isolated from the AbortSignal changes (now in #60185) for easier review.

Ready for review - please allow 48h for community feedback per Node.js contribution guidelines.

When Promise.race() or Promise.any() settles, V8 fires kPromiseResolveAfterResolved / kPromiseRejectAfterResolved for each "losing" promise. The PromiseRejectCallback in node_task_queue.cc was crossing into JS for these events, but since the multipleResolves event reached EOL in v25 (PR nodejs#58707), the JS handler does nothing. The unnecessary C++-to-JS boundary crossings accumulate references in a tight loop, causing OOM when using Promise.race() with immediately-resolving promises. Return early in PromiseRejectCallback() for these two events, skipping the JS callback entirely. Also remove the dead case branches and unused constant imports from the JS side. Fixes: nodejs#51452 Refs: nodejs#60184 Refs: nodejs#61960

When Promise.race() or Promise.any() settles, V8 fires kPromiseResolveAfterResolved / kPromiseRejectAfterResolved for each "losing" promise. The PromiseRejectCallback in node_task_queue.cc was crossing into JS for these events, but since the multipleResolves event reached EOL in v25 (PR #58707), the JS handler does nothing. The unnecessary C++-to-JS boundary crossings accumulate references in a tight loop, causing OOM when using Promise.race() with immediately-resolving promises. Return early in PromiseRejectCallback() for these two events, skipping the JS callback entirely. Also remove the dead case branches and unused constant imports from the JS side. Move early returns for kPromiseResolveAfterResolved and kPromiseRejectAfterResolved before Number::New and CHECK(!callback), avoiding unnecessary work. Remove dead NODE_DEFINE_CONSTANT exports and fix comment placement in the JS switch statement. Bump test --max-old-space-size from 20 to 64 for safety on instrumented builds. Fixes: #51452 Refs: #60184 Refs: #61960 PR-URL: #62336 Refs: #51452 Reviewed-By: Jordan Harband <ljharb@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day>

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. labels Oct 9, 2025

kayossouza mentioned this pull request Oct 9, 2025

abort_controller: fix AbortSignal.any() memory leak #60185

Closed

3 tasks

kayossouza closed this Oct 20, 2025

Felipeness mentioned this pull request Mar 19, 2026

src: skip JS callback for settled Promise.race losers #62336

Closed

src: fix Promise.race() memory leak with deprecated events by kayossouza · Pull Request #60184 · nodejs/node · GitHub

Uh oh!

Uh oh!

src: fix Promise.race() memory leak with deprecated events - #60184

src: fix Promise.race() memory leak with deprecated events#60184
kayossouza wants to merge 1 commit into
nodejs:mainfrom
kayossouza:fix/promise-race-memory-leak

kayossouza commented Oct 9, 2025

Uh oh!

kayossouza commented Oct 9, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

src: fix Promise.race() memory leak with deprecated events by kayossouza · Pull Request #60184 · nodejs/node · GitHub

Uh oh!

Uh oh!

src: fix Promise.race() memory leak with deprecated events - #60184

src: fix Promise.race() memory leak with deprecated events#60184kayossouza wants to merge 1 commit intonodejs:mainnodejs/node:mainfrom kayossouza:fix/promise-race-memory-leakCopy head branch name to clipboard

Conversation

kayossouza commented Oct 9, 2025

Description

Root Cause

The Fix

Evidence

Memory Leak Confirmed (Node.js v22.18.0)

Test Case

Performance Impact

Backward Compatibility

Checklist

Uh oh!

kayossouza commented Oct 9, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

src: fix Promise.race() memory leak with deprecated events#60184
kayossouza wants to merge 1 commit into
nodejs:mainfrom
kayossouza:fix/promise-race-memory-leak