Skip to content

Commit d56a9ad

Browse files
authored
fix: memleak in timeout w/ abort signal (#32)
1 parent 11b9622 commit d56a9ad

File tree

2 files changed

+64
-10
lines changed

2 files changed

+64
-10
lines changed
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
import * as events from 'node:events';
2+
import { timeout } from '../time';
3+
4+
describe('Helper tests', () => {
5+
test('timeout function should not cause memory leak by accumulating abort listeners on abort', async () => {
6+
const controller = new AbortController();
7+
const { signal } = controller;
8+
9+
const countListeners = () => events.getEventListeners(signal, 'abort').length;
10+
11+
// Ensure the initial listener count is zero
12+
expect(countListeners()).toBe(0);
13+
14+
// Run enough iterations to detect a pattern
15+
for (let i = 0; i < 100; i++) {
16+
try {
17+
const sleepPromise = timeout(1000, signal);
18+
controller.abort(); // Abort immediately
19+
await sleepPromise;
20+
} catch (err: any) {
21+
expect(err.toString()).toMatch(/aborted/i);
22+
}
23+
24+
// Assert that listener count does not increase
25+
expect(countListeners()).toBeLessThanOrEqual(1); // 1 listener may temporarily be added and removed
26+
}
27+
28+
// Final check to confirm listeners are cleaned up
29+
expect(countListeners()).toBe(0);
30+
});
31+
32+
test('timeout function should not cause memory leak by accumulating abort listeners on successful completion', async () => {
33+
const controller = new AbortController();
34+
const { signal } = controller;
35+
36+
const countListeners = () => events.getEventListeners(signal, 'abort').length;
37+
38+
// Ensure the initial listener count is zero
39+
expect(countListeners()).toBe(0);
40+
41+
// Run enough iterations to detect a pattern
42+
for (let i = 0; i < 100; i++) {
43+
await timeout(2, signal); // Complete sleep without abort
44+
45+
// Assert that listener count does not increase
46+
expect(countListeners()).toBe(0); // No listeners should remain after successful sleep completion
47+
}
48+
49+
// Final check to confirm listeners are cleaned up
50+
expect(countListeners()).toBe(0);
51+
});
52+
});

src/helpers/time.ts

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,24 @@
1+
import { addAbortListener } from 'node:events';
2+
13
/**
24
* Wait a set amount of milliseconds or until the timer is aborted.
35
* @param ms - Number of milliseconds to wait
4-
* @param abortController - Abort controller
6+
* @param abort - Abort controller
57
* @returns Promise
68
*/
7-
export function timeout(ms: number, abortController?: AbortController): Promise<void> {
9+
export function timeout(ms: number, abort?: AbortController | AbortSignal): Promise<void> {
810
return new Promise((resolve, reject) => {
11+
const signal = abort && 'signal' in abort ? abort.signal : abort;
12+
if (signal?.aborted) return reject(signal.reason);
13+
const disposable = signal ? addAbortListener(signal, onAbort) : undefined;
914
const timeout = setTimeout(() => {
15+
disposable?.[Symbol.dispose ?? (Symbol.for('nodejs.dispose') as typeof Symbol.dispose)]();
1016
resolve();
1117
}, ms);
12-
abortController?.signal.addEventListener(
13-
'abort',
14-
() => {
15-
clearTimeout(timeout);
16-
reject(new Error(`Timeout aborted`));
17-
},
18-
{ once: true }
19-
);
18+
function onAbort() {
19+
clearTimeout(timeout);
20+
reject(signal?.reason);
21+
}
2022
});
2123
}
2224

0 commit comments

Comments
 (0)