diff --git a/src/node_task_queue.cc b/src/node_task_queue.cc index f1c53c44f201b2..a6e42d258a129e 100644 --- a/src/node_task_queue.cc +++ b/src/node_task_queue.cc @@ -77,10 +77,14 @@ void PromiseRejectCallback(PromiseRejectMessage message) { "rejections", "unhandled", unhandledRejections, "handledAfter", rejectionsHandledAfter); - } else if (event == kPromiseResolveAfterResolved) { - value = message.GetValue(); - } else if (event == kPromiseRejectAfterResolved) { - value = message.GetValue(); + } else if (event == kPromiseResolveAfterResolved || + event == kPromiseRejectAfterResolved) { + // Do not notify JS land about these events. These events are triggered + // when a promise is resolved/rejected after already being resolved. + // Notifying JS land would cause memory leaks in tight loops with + // immediately-resolving promises (e.g., Promise.race). + // See: 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..6725724c0ee497 --- /dev/null +++ b/test/parallel/test-promise-race-memory-leak.js @@ -0,0 +1,102 @@ +// Copyright Node.js contributors. All rights reserved. +'use strict'; + +// Tests for memory leak in Promise.race with immediately-resolving promises. +// See: https://github.com/nodejs/node/issues/51452 + +const common = require('../common'); +const assert = require('node:assert'); + +// Test 1: Promise.race with immediately-resolving promises should not leak +{ + async function promiseValue(value) { + return value; + } + + async function run() { + let count = 0; + const maxIterations = 1000; + + for (let i = 0; i < maxIterations; i++) { + await Promise.race([promiseValue("foo"), promiseValue("bar")]); + count++; + } + + assert.strictEqual(count, maxIterations); + } + + run().then(common.mustCall()); +} + +// Test 2: Promise.any with immediately-resolving promises should not leak +{ + async function promiseValue(value) { + return value; + } + + async function run() { + let count = 0; + const maxIterations = 1000; + + for (let i = 0; i < maxIterations; i++) { + await Promise.any([promiseValue("foo"), promiseValue("bar")]); + count++; + } + + assert.strictEqual(count, maxIterations); + } + + run().then(common.mustCall()); +} + +// Test 3: Mixed immediately-resolving and delayed promises should work correctly +{ + async function immediateValue(value) { + return value; + } + + function delayedValue(value, delay) { + return new Promise((resolve) => { + setTimeout(() => resolve(value), delay); + }); + } + + async function run() { + let count = 0; + const maxIterations = 100; + + for (let i = 0; i < maxIterations; i++) { + await Promise.race([ + immediateValue("immediate"), + delayedValue("delayed", 10) + ]); + count++; + } + + assert.strictEqual(count, maxIterations); + } + + run().then(common.mustCall()); +} + +// Test 4: Verify that Promise.race still resolves correctly with immediate values +{ + async function promiseValue(value) { + return value; + } + + Promise.race([promiseValue("first"), promiseValue("second")]).then(common.mustCall((result) => { + assert.strictEqual(result, "first"); + })); +} + +// Test 5: Verify that Promise.any still resolves correctly with immediate values +{ + async function promiseValue(value) { + return value; + } + + Promise.any([promiseValue("first"), promiseValue("second")]).then(common.mustCall((result) => { + assert.strictEqual(result, "first"); + })); +}