Skip to content

Commit 430c88e

Browse files
committed
events: remove the abort listener on iterator completion
Given an AbortSignal when passed into the events.on AsyncIterator API the attached abort listener should always be removed when the iterator completes. The abortHandler function is declared within the scope of the events.on function so cannot be removed by the caller which can lead to a memory leak. Adding the abort listener using the addEventListener helper returned by the listenersController helper adds the abort listener to the array of listeners that will be cleaned up by removeAll in the closeHandler for the AsyncIterator. Fixes: #51010
1 parent 8b690a1 commit 430c88e

File tree

2 files changed

+25
-1
lines changed

2 files changed

+25
-1
lines changed

lib/events.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -1169,7 +1169,7 @@ function on(emitter, event, options = kEmptyObject) {
11691169
}
11701170
if (signal) {
11711171
kResistStopPropagation ??= require('internal/event_target').kResistStopPropagation;
1172-
eventTargetAgnosticAddListener(
1172+
addEventListener(
11731173
signal,
11741174
'abort',
11751175
abortListener,

test/parallel/test-events-on-async-iterator.js

+24
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ const assert = require('assert');
66
const { on, EventEmitter } = require('events');
77
const {
88
NodeEventTarget,
9+
kEvents
910
} = require('internal/event_target');
1011

1112
async function basic() {
@@ -372,6 +373,28 @@ async function abortableOnAfterDone() {
372373
});
373374
}
374375

376+
async function abortListenerRemovedAfterComplete() {
377+
const ee = new EventEmitter();
378+
const ac = new AbortController();
379+
380+
const i = setInterval(() => ee.emit('foo', 'foo'), 1);
381+
try {
382+
// Return case
383+
const endedIterator = on(ee, 'foo', { signal: ac.signal });
384+
assert.ok(ac.signal[kEvents].size > 0);
385+
endedIterator.return();
386+
assert.strictEqual(ac.signal[kEvents].size, 0);
387+
388+
// Throw case
389+
const throwIterator = on(ee, 'foo', { signal: ac.signal });
390+
assert.ok(ac.signal[kEvents].size > 0);
391+
throwIterator.throw(new Error());
392+
assert.strictEqual(ac.signal[kEvents].size, 0);
393+
} finally {
394+
clearInterval(i);
395+
}
396+
}
397+
375398
async function run() {
376399
const funcs = [
377400
basic,
@@ -391,6 +414,7 @@ async function run() {
391414
eventTargetAbortableOnAfter,
392415
eventTargetAbortableOnAfter2,
393416
abortableOnAfterDone,
417+
abortListenerRemovedAfterComplete,
394418
];
395419

396420
for (const fn of funcs) {

0 commit comments

Comments
 (0)