From 7033537a17ff22c18a94a3057464143123fba12f Mon Sep 17 00:00:00 2001 From: Felipe Coelho Date: Thu, 19 Mar 2026 09:46:51 -0300 Subject: [PATCH 1/2] fix: skip JS callback for settled Promise.race losers 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. Fixes: https://github.com/nodejs/node/issues/51452 Refs: https://github.com/nodejs/node/pull/60184 Refs: https://github.com/nodejs/node/pull/61960 --- lib/internal/process/promises.js | 13 +++------- src/node_task_queue.cc | 9 +++++-- .../parallel/test-promise-race-memory-leak.js | 24 +++++++++++++++++++ 3 files changed, 34 insertions(+), 12 deletions(-) create mode 100644 test/parallel/test-promise-race-memory-leak.js diff --git a/lib/internal/process/promises.js b/lib/internal/process/promises.js index 0ccea4d05a87f5..62f8e621e29ba7 100644 --- a/lib/internal/process/promises.js +++ b/lib/internal/process/promises.js @@ -14,8 +14,6 @@ const { promiseRejectEvents: { kPromiseRejectWithNoHandler, kPromiseHandlerAddedAfterReject, - kPromiseRejectAfterResolved, - kPromiseResolveAfterResolved, }, setPromiseRejectCallback, } = internalBinding('task_queue'); @@ -168,14 +166,9 @@ function promiseRejectHandler(type, promise, reason) { case kPromiseHandlerAddedAfterReject: // 1 handledRejection(promise); break; - case kPromiseRejectAfterResolved: // 2 - // Do nothing in this case. Previous we would emit a multipleResolves - // event but that was deprecated then later removed. - break; - case kPromiseResolveAfterResolved: // 3 - // Do nothing in this case. Previous we would emit a multipleResolves - // event but that was deprecated then later removed. - break; + // kPromiseRejectAfterResolved and kPromiseResolveAfterResolved are + // handled in C++ (src/node_task_queue.cc) by returning early, so they + // never reach this JS callback. No case branches needed. } } diff --git a/src/node_task_queue.cc b/src/node_task_queue.cc index f1c53c44f201b2..02cf26f5be6c67 100644 --- a/src/node_task_queue.cc +++ b/src/node_task_queue.cc @@ -78,9 +78,14 @@ void PromiseRejectCallback(PromiseRejectMessage message) { "unhandled", unhandledRejections, "handledAfter", rejectionsHandledAfter); } else if (event == kPromiseResolveAfterResolved) { - value = message.GetValue(); + // Intentional no-op. The multipleResolves event was EOL'd in v25 + // (PR #58707), and the JS handler does nothing for these events. + // Skipping the JS callback avoids accumulating C++-to-JS boundary + // references in tight Promise.race()/Promise.any() loops, which + // previously caused OOM (see #51452). + return; } else if (event == kPromiseRejectAfterResolved) { - value = message.GetValue(); + return; } else { return; } diff --git a/test/parallel/test-promise-race-memory-leak.js b/test/parallel/test-promise-race-memory-leak.js new file mode 100644 index 00000000000000..a68c0aae96c0d5 --- /dev/null +++ b/test/parallel/test-promise-race-memory-leak.js @@ -0,0 +1,24 @@ +// Flags: --max-old-space-size=20 +'use strict'; + +// Regression test for https://github.com/nodejs/node/issues/51452 +// When Promise.race() settles, V8 fires kPromiseResolveAfterResolved / +// kPromiseRejectAfterResolved for each "losing" promise. Before this fix, +// the C++ PromiseRejectCallback crossed into JS for these no-op events, +// accumulating references and causing OOM in tight async loops. +// With --max-old-space-size=20, this test would crash before completing +// if the leak is present. + +const common = require('../common'); + +async function main() { + for (let i = 0; i < 100_000; i++) { + await Promise.race([ + Promise.resolve(1), + Promise.resolve(2), + Promise.resolve(3), + ]); + } +} + +main().then(common.mustCall()); From ec229934e5237269f13689872cc96d4165381d3e Mon Sep 17 00:00:00 2001 From: Felipe Coelho Date: Thu, 19 Mar 2026 12:29:38 -0300 Subject: [PATCH 2/2] fix: improve Promise.race OOM fix placement and cleanup 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. --- lib/internal/process/promises.js | 5 ++--- src/node_task_queue.cc | 18 +++++++----------- test/parallel/test-promise-race-memory-leak.js | 4 ++-- 3 files changed, 11 insertions(+), 16 deletions(-) diff --git a/lib/internal/process/promises.js b/lib/internal/process/promises.js index 62f8e621e29ba7..8bb2062b3a9f0f 100644 --- a/lib/internal/process/promises.js +++ b/lib/internal/process/promises.js @@ -159,6 +159,8 @@ function promiseRejectHandler(type, promise, reason) { if (unhandledRejectionsMode === undefined) { unhandledRejectionsMode = getUnhandledRejectionsMode(); } + // kPromiseRejectAfterResolved and kPromiseResolveAfterResolved are + // filtered out in C++ (src/node_task_queue.cc) and never reach JS. switch (type) { case kPromiseRejectWithNoHandler: // 0 unhandledRejection(promise, reason); @@ -166,9 +168,6 @@ function promiseRejectHandler(type, promise, reason) { case kPromiseHandlerAddedAfterReject: // 1 handledRejection(promise); break; - // kPromiseRejectAfterResolved and kPromiseResolveAfterResolved are - // handled in C++ (src/node_task_queue.cc) by returning early, so they - // never reach this JS callback. No case branches needed. } } diff --git a/src/node_task_queue.cc b/src/node_task_queue.cc index 02cf26f5be6c67..11ed3b247f0ac9 100644 --- a/src/node_task_queue.cc +++ b/src/node_task_queue.cc @@ -55,6 +55,13 @@ void PromiseRejectCallback(PromiseRejectMessage message) { if (env == nullptr || !env->can_call_into_js()) return; + // multipleResolves was removed in v25 (PR #58707). Skip all work for + // these events to avoid OOM in tight Promise.race() loops (#51452). + if (event == kPromiseResolveAfterResolved || + event == kPromiseRejectAfterResolved) { + return; + } + Local callback = env->promise_reject_callback(); // The promise is rejected before JS land calls SetPromiseRejectCallback // to initializes the promise reject callback during bootstrap. @@ -77,15 +84,6 @@ void PromiseRejectCallback(PromiseRejectMessage message) { "rejections", "unhandled", unhandledRejections, "handledAfter", rejectionsHandledAfter); - } else if (event == kPromiseResolveAfterResolved) { - // Intentional no-op. The multipleResolves event was EOL'd in v25 - // (PR #58707), and the JS handler does nothing for these events. - // Skipping the JS callback avoids accumulating C++-to-JS boundary - // references in tight Promise.race()/Promise.any() loops, which - // previously caused OOM (see #51452). - return; - } else if (event == kPromiseRejectAfterResolved) { - return; } else { return; } @@ -178,8 +176,6 @@ static void Initialize(Local target, Local events = Object::New(isolate); NODE_DEFINE_CONSTANT(events, kPromiseRejectWithNoHandler); NODE_DEFINE_CONSTANT(events, kPromiseHandlerAddedAfterReject); - NODE_DEFINE_CONSTANT(events, kPromiseResolveAfterResolved); - NODE_DEFINE_CONSTANT(events, kPromiseRejectAfterResolved); target->Set(env->context(), FIXED_ONE_BYTE_STRING(isolate, "promiseRejectEvents"), diff --git a/test/parallel/test-promise-race-memory-leak.js b/test/parallel/test-promise-race-memory-leak.js index a68c0aae96c0d5..db00af4e4ef5f9 100644 --- a/test/parallel/test-promise-race-memory-leak.js +++ b/test/parallel/test-promise-race-memory-leak.js @@ -1,4 +1,4 @@ -// Flags: --max-old-space-size=20 +// Flags: --max-old-space-size=64 'use strict'; // Regression test for https://github.com/nodejs/node/issues/51452 @@ -6,7 +6,7 @@ // kPromiseRejectAfterResolved for each "losing" promise. Before this fix, // the C++ PromiseRejectCallback crossed into JS for these no-op events, // accumulating references and causing OOM in tight async loops. -// With --max-old-space-size=20, this test would crash before completing +// With --max-old-space-size=64, this test would crash before completing // if the leak is present. const common = require('../common');