Skip to content

Commit 9d4466e

Browse files
committed
util: fix debounce/throttle abort listener leak
debounce() and throttle() added an abort listener to the signal when they were created and never removed it, so a long-lived signal kept every debounced or throttled function, and its data, in memory. Only listen for abort while calls are pending. Calls made after the signal is aborted still reject, since each call checks signal.aborted. Assisted-by: a closed-source coding agent Signed-off-by: vedchaudhari <vedc2853@gmail.com>
1 parent f88a1d2 commit 9d4466e

4 files changed

Lines changed: 123 additions & 8 deletions

File tree

‎lib/internal/util/debounce.js‎

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ const {
66
PromiseReject,
77
PromiseWithResolvers,
88
ReflectApply,
9+
SymbolDispose,
910
} = primordials;
1011

1112
const {
@@ -60,6 +61,7 @@ function debounce(fn, wait, options = kEmptyObject) {
6061

6162
let args;
6263
let abortError;
64+
let abortListener;
6365
let pendingCalls = [];
6466
let refed = true;
6567
let timeout;
@@ -76,6 +78,7 @@ function debounce(fn, wait, options = kEmptyObject) {
7678
if (timeout === undefined) return;
7779
clearTimeout(timeout);
7880
timeout = undefined;
81+
stopListeningForAbort();
7982
args = undefined;
8083
rejectPending(error);
8184
}
@@ -97,6 +100,19 @@ function debounce(fn, wait, options = kEmptyObject) {
97100
cancelWithError(abortError);
98101
}
99102

103+
function listenForAbort() {
104+
if (signal !== undefined && abortListener === undefined) {
105+
abortListener = addAbortListener(signal, abort);
106+
}
107+
}
108+
109+
function stopListeningForAbort() {
110+
if (abortListener !== undefined) {
111+
abortListener[SymbolDispose]();
112+
abortListener = undefined;
113+
}
114+
}
115+
100116
function ref() {
101117
refed = true;
102118
timeout?.ref();
@@ -143,6 +159,7 @@ function debounce(fn, wait, options = kEmptyObject) {
143159

144160
function onTimeout() {
145161
timeout = undefined;
162+
stopListeningForAbort();
146163
invokePending();
147164
}
148165

@@ -151,6 +168,7 @@ function debounce(fn, wait, options = kEmptyObject) {
151168

152169
clearTimeout(timeout);
153170
timeout = undefined;
171+
stopListeningForAbort();
154172
invokePending();
155173
}
156174

@@ -170,6 +188,7 @@ function debounce(fn, wait, options = kEmptyObject) {
170188
} else {
171189
timeout = setTimeout(onTimeout, wait);
172190
if (!refed) timeout.unref();
191+
listenForAbort();
173192
}
174193

175194
if (signal?.aborted) {
@@ -227,10 +246,6 @@ function debounce(fn, wait, options = kEmptyObject) {
227246
},
228247
});
229248

230-
if (signal !== undefined) {
231-
addAbortListener(signal, abort);
232-
}
233-
234249
return debounced;
235250
}
236251

‎lib/internal/util/throttle.js‎

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ const {
99
PromiseResolve,
1010
PromiseWithResolvers,
1111
ReflectApply,
12+
SymbolDispose,
1213
} = primordials;
1314

1415
const {
@@ -79,6 +80,7 @@ function throttle(fn, limit, interval, options = kEmptyObject) {
7980
}
8081

8182
let abortError;
83+
let abortListener;
8284
let activeCount = 0;
8385
let pendingCalls = [];
8486
let pendingIndex = 0;
@@ -100,6 +102,7 @@ function throttle(fn, limit, interval, options = kEmptyObject) {
100102
if (pendingIndex === pendingCalls.length) {
101103
pendingCalls = [];
102104
pendingIndex = 0;
105+
stopListeningForAbort();
103106
} else if (pendingIndex > 1024 && pendingIndex * 2 >= pendingCalls.length) {
104107
pendingCalls = ArrayPrototypeSlice(pendingCalls, pendingIndex);
105108
pendingIndex = 0;
@@ -113,6 +116,7 @@ function throttle(fn, limit, interval, options = kEmptyObject) {
113116
const start = pendingIndex;
114117
pendingCalls = [];
115118
pendingIndex = 0;
119+
stopListeningForAbort();
116120
for (let i = start; i < calls.length; i++) {
117121
calls[i].reject(error);
118122
}
@@ -150,6 +154,19 @@ function throttle(fn, limit, interval, options = kEmptyObject) {
150154
cancelWithError(abortError);
151155
}
152156

157+
function listenForAbort() {
158+
if (signal !== undefined && abortListener === undefined) {
159+
abortListener = addAbortListener(signal, abort);
160+
}
161+
}
162+
163+
function stopListeningForAbort() {
164+
if (abortListener !== undefined) {
165+
abortListener[SymbolDispose]();
166+
abortListener = undefined;
167+
}
168+
}
169+
153170
function ref() {
154171
refed = true;
155172
timeout?.ref();
@@ -306,6 +323,7 @@ function throttle(fn, limit, interval, options = kEmptyObject) {
306323
invoke(call);
307324
} else {
308325
ArrayPrototypePush(pendingCalls, call);
326+
listenForAbort();
309327
schedule();
310328
}
311329

@@ -360,10 +378,6 @@ function throttle(fn, limit, interval, options = kEmptyObject) {
360378
},
361379
});
362380

363-
if (signal !== undefined) {
364-
addAbortListener(signal, abort);
365-
}
366-
367381
return throttled;
368382
}
369383

‎test/parallel/test-util-debounce.js‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,15 @@
44
const common = require('../common');
55
const assert = require('node:assert');
66
const { createHook } = require('node:async_hooks');
7+
const { getEventListeners } = require('node:events');
78
const { setTimeout } = require('node:timers/promises');
89
const { debounce } = require('node:util');
910
const { TIMEOUT_MAX } = require('internal/timers');
1011

12+
function abortListenerCount(signal) {
13+
return getEventListeners(signal, 'abort').length;
14+
}
15+
1116
for (const value of [undefined, null, true, 0, 'fn', {}, [], Symbol()]) {
1217
assert.throws(() => debounce(value, 1), {
1318
code: 'ERR_INVALID_ARG_TYPE',
@@ -320,6 +325,41 @@ assert.throws(() => debounce(() => {}, 1, { signal: {} }), {
320325
assert.strictEqual(await result, 1);
321326
}
322327

328+
{
329+
const { signal } = new AbortController();
330+
const debounced = debounce(common.mustCall((value) => value, 2), 1, {
331+
signal,
332+
});
333+
assert.strictEqual(abortListenerCount(signal), 0);
334+
const first = debounced(1);
335+
assert.strictEqual(abortListenerCount(signal), 1);
336+
assert.strictEqual(await first, 1);
337+
assert.strictEqual(abortListenerCount(signal), 0);
338+
339+
const second = debounced(2);
340+
debounced.flush();
341+
assert.strictEqual(abortListenerCount(signal), 0);
342+
assert.strictEqual(await second, 2);
343+
344+
const third = debounced(3);
345+
debounced.cancel();
346+
assert.strictEqual(abortListenerCount(signal), 0);
347+
await assert.rejects(third, { code: 'ABORT_ERR' });
348+
}
349+
350+
{
351+
const { signal } = new AbortController();
352+
const debounced = debounce(common.mustCall((value) => value), 10, {
353+
leading: true,
354+
signal,
355+
});
356+
assert.strictEqual(await debounced(1), 1);
357+
// The leading call keeps the listener until the wait is over.
358+
assert.strictEqual(abortListenerCount(signal), 1);
359+
await setTimeout(20);
360+
assert.strictEqual(abortListenerCount(signal), 0);
361+
}
362+
323363
{
324364
const debounced = debounce(common.mustCall((value) => value), 1);
325365
assert.strictEqual(await debounced(42), 42);

‎test/parallel/test-util-throttle.js‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,15 @@
44
const common = require('../common');
55
const assert = require('node:assert');
66
const { createHook } = require('node:async_hooks');
7+
const { getEventListeners } = require('node:events');
78
const { setImmediate } = require('node:timers/promises');
89
const { throttle } = require('node:util');
910
const { TIMEOUT_MAX } = require('internal/timers');
1011

12+
function abortListenerCount(signal) {
13+
return getEventListeners(signal, 'abort').length;
14+
}
15+
1116
process.on('unhandledRejection', common.mustNotCall());
1217

1318
for (const value of [undefined, null, true, 0, 'fn', {}, [], Symbol()]) {
@@ -257,6 +262,47 @@ throttle(() => {}, 1, 1, {
257262
);
258263
}
259264

265+
{
266+
const { signal } = new AbortController();
267+
const throttled = throttle(common.mustCall((value) => value, 2), 1, 10, {
268+
signal,
269+
});
270+
assert.strictEqual(abortListenerCount(signal), 0);
271+
const first = throttled(1);
272+
assert.strictEqual(abortListenerCount(signal), 0);
273+
const second = throttled(2);
274+
assert.strictEqual(abortListenerCount(signal), 1);
275+
assert.deepStrictEqual(await Promise.all([first, second]), [1, 2]);
276+
assert.strictEqual(abortListenerCount(signal), 0);
277+
}
278+
279+
{
280+
const { signal } = new AbortController();
281+
const throttled = throttle(common.mustCall(), 1, TIMEOUT_MAX, { signal });
282+
throttled();
283+
const queued = throttled();
284+
assert.strictEqual(abortListenerCount(signal), 1);
285+
throttled.cancel();
286+
assert.strictEqual(abortListenerCount(signal), 0);
287+
await assert.rejects(queued, { code: 'ABORT_ERR' });
288+
}
289+
290+
{
291+
// Calls waiting on concurrency are queued too, so they keep the listener.
292+
const { signal } = new AbortController();
293+
const { promise, resolve } = Promise.withResolvers();
294+
const throttled = throttle(common.mustCall(() => promise, 2), 2, TIMEOUT_MAX, {
295+
concurrency: 1,
296+
signal,
297+
});
298+
const first = throttled();
299+
const second = throttled();
300+
assert.strictEqual(abortListenerCount(signal), 1);
301+
resolve();
302+
await Promise.all([first, second]);
303+
assert.strictEqual(abortListenerCount(signal), 0);
304+
}
305+
260306
{
261307
const expected = new Error('failure');
262308
const throttled = throttle(common.mustCall((value) => {

0 commit comments

Comments
 (0)