From 4062948e0ac3432be3fe5f3a6e67e7644b8d8fa6 Mon Sep 17 00:00:00 2001 From: kayossouza Date: Thu, 9 Oct 2025 19:33:06 -0300 Subject: [PATCH] src: fix Promise.race() memory leak with deprecated events MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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: https://github.com/nodejs/node/issues/51452 --- src/node_task_queue.cc | 10 ++- .../parallel/test-promise-race-memory-leak.js | 79 +++++++++++++++++++ 2 files changed, 87 insertions(+), 2 deletions(-) create mode 100644 test/parallel/test-promise-race-memory-leak.js diff --git a/src/node_task_queue.cc b/src/node_task_queue.cc index f1c53c44f201b2..d58d317a37fc16 100644 --- a/src/node_task_queue.cc +++ b/src/node_task_queue.cc @@ -78,9 +78,15 @@ void PromiseRejectCallback(PromiseRejectMessage message) { "unhandled", unhandledRejections, "handledAfter", rejectionsHandledAfter); } else if (event == kPromiseResolveAfterResolved) { - value = message.GetValue(); + // The multipleResolves event was deprecated in Node.js v17 and removed. + // No need to call into JavaScript for this event as it's a no-op. + // Fixes: https://github.com/nodejs/node/issues/51452 + return; } else if (event == kPromiseRejectAfterResolved) { - value = message.GetValue(); + // The multipleResolves event was deprecated in Node.js v17 and removed. + // No need to call into JavaScript for this event as it's a no-op. + // Fixes: https://github.com/nodejs/node/issues/51452 + 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..701ec67045f493 --- /dev/null +++ b/test/parallel/test-promise-race-memory-leak.js @@ -0,0 +1,79 @@ +'use strict'; + +// Test for memory leak when racing immediately-resolving Promises +// Refs: https://github.com/nodejs/node/issues/51452 + +const common = require('../common'); +const assert = require('assert'); + +// This test verifies that Promise.race() with immediately-resolving promises +// does not cause unbounded memory growth. +// +// Root cause: When Promise.race() settles, V8 attempts to resolve the other +// promises in the race, triggering 'multipleResolves' events. These events +// are queued in nextTick, but if the event loop never gets a chance to drain +// the queue (tight loop), memory grows unbounded. + +async function promiseValue(value) { + return value; +} + +async function testPromiseRace() { + const iterations = 100000; + const memBefore = process.memoryUsage().heapUsed; + + for (let i = 0; i < iterations; i++) { + await Promise.race([promiseValue('foo'), promiseValue('bar')]); + + // Allow event loop to drain nextTick queue periodically + if (i % 1000 === 0) { + await new Promise(setImmediate); + } + } + + const memAfter = process.memoryUsage().heapUsed; + const growth = memAfter - memBefore; + const growthMB = growth / 1024 / 1024; + + console.log(`Memory growth: ${growthMB.toFixed(2)} MB`); + + // Memory growth should be reasonable (< 50MB for 100k iterations) + // Without the fix, this would grow 100s of MBs + assert.ok(growthMB < 50, + `Excessive memory growth: ${growthMB.toFixed(2)} MB (expected < 50 MB)`); +} + +async function testPromiseAny() { + const iterations = 100000; + const memBefore = process.memoryUsage().heapUsed; + + for (let i = 0; i < iterations; i++) { + await Promise.any([promiseValue('foo'), promiseValue('bar')]); + + // Allow event loop to drain nextTick queue periodically + if (i % 1000 === 0) { + await new Promise(setImmediate); + } + } + + const memAfter = process.memoryUsage().heapUsed; + const growth = memAfter - memBefore; + const growthMB = growth / 1024 / 1024; + + console.log(`Memory growth (any): ${growthMB.toFixed(2)} MB`); + + // Memory growth should be reasonable + assert.ok(growthMB < 50, + `Excessive memory growth: ${growthMB.toFixed(2)} MB (expected < 50 MB)`); +} + +// Run tests +(async () => { + console.log('Testing Promise.race() memory leak...'); + await testPromiseRace(); + + console.log('Testing Promise.any() memory leak...'); + await testPromiseAny(); + + console.log('All tests passed!'); +})().catch(common.mustNotCall());