mirror of
https://github.com/nodejs/node.git
synced 2026-10-28 10:15:07 +08:00
Aryan
2c07eda791
The weakListeners() retention map in EventTarget stored a single listener per retainer key. Multiple addEventListener() calls sharing the same retainer (e.g. two listeners on one target using the same AbortSignal) silently evicted each other's strong reference, allowing the evicted listener to be garbage collected before its signal aborted, so only the most recently registered listener was ever actually removed on abort. Changed the retention map to store a Set of listeners per retainer key instead of a single listener, and added matching cleanup in remove() so entries are released once their retainer's set is empty. This also fixes the same class of bug in events.aborted() and the streams kWeakHandler usage, since both go through the same EventTarget.prototype.addEventListener() code path. Fixes: https://github.com/nodejs/node/issues/63954 Signed-off-by: aryan7905 <aryansrivastava354@gmail.com> PR-URL: https://github.com/nodejs/node/pull/64024 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Aviv Keller <me@aviv.sh> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
52 lines
1.4 KiB
JavaScript
52 lines
1.4 KiB
JavaScript
// Flags: --no-warnings --expose-gc --expose-internals
|
|
'use strict';
|
|
require('../common');
|
|
|
|
const assert = require('assert');
|
|
|
|
const {
|
|
test,
|
|
} = require('node:test');
|
|
|
|
const {
|
|
kWeakHandler,
|
|
} = require('internal/event_target');
|
|
|
|
const { setTimeout: sleep } = require('timers/promises');
|
|
|
|
// The tests in this file depend on Node.js internal APIs. These are not necessarily
|
|
// portable to other runtimes
|
|
|
|
test('A weak event listener should not prevent gc', async () => {
|
|
// If the event listener is weak, however, it should not prevent gc
|
|
let ref;
|
|
function handler() {}
|
|
{
|
|
ref = new globalThis.WeakRef(AbortSignal.timeout(1_200_000));
|
|
ref.deref().addEventListener('abort', handler, { [kWeakHandler]: {} });
|
|
}
|
|
|
|
await sleep(10);
|
|
globalThis.gc();
|
|
assert.strictEqual(ref.deref(), undefined);
|
|
});
|
|
|
|
test('two weak listeners with the same retainer should both run on abort', async () => {
|
|
// Regression test for https://github.com/nodejs/node/issues/63954
|
|
const ac = new AbortController();
|
|
let aRan = false;
|
|
let bRan = false;
|
|
|
|
ac.signal.addEventListener('a', () => { aRan = true; }, { [kWeakHandler]: ac });
|
|
ac.signal.addEventListener('b', () => { bRan = true; }, { [kWeakHandler]: ac });
|
|
|
|
await sleep(10);
|
|
globalThis.gc();
|
|
|
|
ac.signal.dispatchEvent(new Event('a'));
|
|
ac.signal.dispatchEvent(new Event('b'));
|
|
|
|
assert.strictEqual(aRan, true);
|
|
assert.strictEqual(bRan, true);
|
|
});
|