Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions src/node_task_queue.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
102 changes: 102 additions & 0 deletions test/parallel/test-promise-race-memory-leak.js
Original file line number Diff line number Diff line change
@@ -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");
}));
}