Skip to content
This repository has been archived by the owner on Jan 22, 2025. It is now read-only.

Commit

Permalink
An implementation of Promise.race that prevents memory leaks (#3071)
Browse files Browse the repository at this point in the history
# Summary

If a member of `Promise.race` never settles, every promise in the race will be retained indefinitely. This causes memory leaks. See nodejs/node#17469 for much more information.

In this PR we fork the excellent work of @brainkim, @danfuzz, and @szakharchenko; a ‘safe’ version of `race` that ensures that each member of the race settles when the winner settles.

# Test Plan

See next PR in this stack

Addresses #3069.
  • Loading branch information
steveluscher authored Aug 7, 2024
1 parent f2bb4e8 commit b4bf318
Show file tree
Hide file tree
Showing 6 changed files with 200 additions and 1 deletion.
5 changes: 5 additions & 0 deletions .changeset/silent-cooks-marry.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@solana/promises': patch
---

Created a helper that you can use to race two or more promises without having to worry about them leaking memory
6 changes: 6 additions & 0 deletions packages/promises/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,9 @@ const result = await getAbortablePromise(
AbortSignal.timeout(5000),
);
```

### `safeRace(...promises)`

An implementation of `Promise.race` that causes all of the losing promises to settle. This allows them to be released and garbage collected, preventing memory leaks.

Read more here: https://github.com/nodejs/node/issues/17469
73 changes: 73 additions & 0 deletions packages/promises/src/__tests__/race-test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
/**
* Forked from https://github.com/digitalloggers/race-as-promised/tree/master
*
* Authored by Brian Kim:
* https://github.com/nodejs/node/issues/17469#issuecomment-685216777
*
* Adapted to module structure.
*
* Adjusted to run for a finite time and perform explicit leak checks.
*/

import { safeRace } from '../race';

async function randomString(length: number) {
await new Promise(resolve => setTimeout(resolve, 1));
let result = '';
const characters = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789';
for (let i = 0; i < length; i++) {
result += characters.charAt(Math.floor(Math.random() * characters.length));
}
return result;
}

const iterationCount = 1000;
const stringSize = 10000;

function usageMeaningfullyIncreasing(usages: readonly NodeJS.MemoryUsage[], key: 'heapUsed' | 'rss') {
return (
usages[2][key] - usages[0][key] > 2 * iterationCount * stringSize &&
usages[2][key] - usages[1][key] > (usages[1][key] - usages[0][key]) / 2
);
}

function detectLeak(usages: readonly NodeJS.MemoryUsage[]) {
return usageMeaningfullyIncreasing(usages, 'rss') || usageMeaningfullyIncreasing(usages, 'heapUsed');
}

async function run(race: typeof Promise.race) {
const pending = new Promise(() => {});
for (let i = 0; i < iterationCount; i++) {
// We use random strings to prevent string interning.
// Pass a different length string to see effects on memory usage.
await race([pending, randomString(stringSize)]);
}
}

describe('Promise.race', () => {
let nativeRace: typeof Promise.race;
beforeEach(() => {
nativeRace = Promise.race.bind(Promise);
});
it('leaks memory', async () => {
const usages = [];
usages.push(process.memoryUsage());
await run(nativeRace);
usages.push(process.memoryUsage());
await run(nativeRace);
usages.push(process.memoryUsage());
expect(detectLeak(usages)).toBe(true);
}, 30_000 /* timeout */);
});

describe('safeRace', () => {
it('does not leak memory', async () => {
const usages = [];
usages.push(process.memoryUsage());
await run(safeRace);
usages.push(process.memoryUsage());
await run(safeRace);
usages.push(process.memoryUsage());
expect(detectLeak(usages)).toBe(false);
}, 30_000 /* timeout */);
});
4 changes: 3 additions & 1 deletion packages/promises/src/abortable.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import { safeRace } from './race';

export function getAbortablePromise<T>(promise: Promise<T>, abortSignal?: AbortSignal): Promise<T> {
if (!abortSignal) {
return promise;
} else {
return Promise.race([
return safeRace([
// This promise only ever rejects if the signal is aborted. Otherwise it idles forever.
// It's important that this come before the input promise; in the event of an abort, we
// want to throw even if the input promise's result is ready
Expand Down
1 change: 1 addition & 0 deletions packages/promises/src/index.ts
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
export * from './abortable';
export * from './race';
112 changes: 112 additions & 0 deletions packages/promises/src/race.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
/**
* Forked from https://github.com/digitalloggers/race-as-promised/tree/master
*
* Authored by Brian Kim:
* https://github.com/nodejs/node/issues/17469#issuecomment-685216777
*
* Adapted to module structure.
*
* This is free and unencumbered software released into the public domain.
*
* Anyone is free to copy, modify, publish, use, compile, sell, or
* distribute this software, either in source code form or as a compiled
* binary, for any purpose, commercial or non-commercial, and by any
* means.
*
* In jurisdictions that recognize copyright laws, the author or authors
* of this software dedicate any and all copyright interest in the
* software to the public domain. We make this dedication for the benefit
* of the public at large and to the detriment of our heirs and
* successors. We intend this dedication to be an overt act of
* relinquishment in perpetuity of all present and future rights to this
* software under copyright law.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
* EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
* MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.
* IN NO EVENT SHALL THE AUTHORS BE LIABLE FOR ANY CLAIM, DAMAGES OR
* OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
* ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
* OTHER DEALINGS IN THE SOFTWARE.
*
* For more information, please refer to <http://unlicense.org/>
*/

type Deferred = Readonly<{
reject: (reason?: unknown) => void;
resolve: (value: unknown) => void;
}>;

function isObject(value: unknown): value is object {
return value !== null && (typeof value === 'object' || typeof value === 'function');
}

function addRaceContender(contender: object) {
const deferreds = new Set<Deferred>();
const record = { deferreds, settled: false };

// This call to `then` happens once for the lifetime of the value.
Promise.resolve(contender).then(
value => {
for (const { resolve } of deferreds) {
resolve(value);
}

deferreds.clear();
record.settled = true;
},
err => {
for (const { reject } of deferreds) {
reject(err);
}

deferreds.clear();
record.settled = true;
},
);
return record;
}

// Keys are the values passed to race, values are a record of data containing a
// set of deferreds and whether the value has settled.
const wm = new WeakMap<object, { deferreds: Set<Deferred>; settled: boolean }>();
export async function safeRace<T extends readonly unknown[] | []>(contenders: T): Promise<Awaited<T[number]>> {
let deferred: Deferred;
const result = new Promise((resolve, reject) => {
deferred = { reject, resolve };
for (const contender of contenders) {
if (!isObject(contender)) {
// If the contender is a primitive, attempting to use it as a key in the
// weakmap would throw an error. Luckily, it is safe to call
// `Promise.resolve(contender).then` on a primitive value multiple times
// because the promise fulfills immediately.
Promise.resolve(contender).then(resolve, reject);
continue;
}

let record = wm.get(contender);
if (record === undefined) {
record = addRaceContender(contender);
record.deferreds.add(deferred);
wm.set(contender, record);
} else if (record.settled) {
// If the value has settled, it is safe to call
// `Promise.resolve(contender).then` on it.
Promise.resolve(contender).then(resolve, reject);
} else {
record.deferreds.add(deferred);
}
}
});

// The finally callback executes when any value settles, preventing any of
// the unresolved values from retaining a reference to the resolved value.
return await (result.finally(() => {
for (const contender of contenders) {
if (isObject(contender)) {
const record = wm.get(contender)!;
record.deferreds.delete(deferred);
}
}
}) as Promise<Awaited<T[number]>>);
}

0 comments on commit b4bf318

Please sign in to comment.