diff --git a/lib/internal/event_target.js b/lib/internal/event_target.js index c6fbc81a27b4f2..5ab2a23f980549 100644 --- a/lib/internal/event_target.js +++ b/lib/internal/event_target.js @@ -58,6 +58,7 @@ const kWeakHandler = Symbol('kWeak'); const kResistStopPropagation = Symbol('kResistStopPropagation'); const kHybridDispatch = SymbolFor('nodejs.internal.kHybridDispatch'); +const kRemoveWeakListenerHelper = Symbol('nodejs.internal.removeWeakListenerHelper'); const kCreateEvent = Symbol('kCreateEvent'); const kNewListener = Symbol('kNewListener'); const kRemoveListener = Symbol('kRemoveListener'); @@ -406,7 +407,7 @@ let weakListenersState = null; let objectToWeakListenerMap = null; function weakListeners() { weakListenersState ??= new SafeFinalizationRegistry( - (listener) => listener.remove(), + ({ eventTarget, listener, eventType }) => eventTarget.deref()?.[kRemoveWeakListenerHelper](eventType, listener), ); objectToWeakListenerMap ??= new SafeWeakMap(); return { registry: weakListenersState, map: objectToWeakListenerMap }; @@ -428,7 +429,7 @@ const kFlagResistStopPropagation = 1 << 6; // the linked list makes dispatching faster, even if adding/removing is // slower. class Listener { - constructor(previous, listener, once, capture, passive, + constructor(eventTarget, eventType, previous, listener, once, capture, passive, isNodeStyleListener, weak, resistStopPropagation) { this.next = undefined; if (previous !== undefined) @@ -455,7 +456,13 @@ class Listener { if (this.weak) { this.callback = new SafeWeakRef(listener); - weakListeners().registry.register(listener, this, this); + weakListeners().registry.register(listener, { + __proto__: null, + // Weak ref so the listener won't hold the eventTarget alive + eventTarget: new SafeWeakRef(eventTarget), + listener: this, + eventType, + }, this); // Make the retainer retain the listener in a WeakMap weakListeners().map.set(weak, listener); this.listener = this.callback; @@ -621,7 +628,7 @@ class EventTarget { if (root === undefined) { root = { size: 1, next: undefined, resistStopPropagation: Boolean(resistStopPropagation) }; // This is the first handler in our linked list. - new Listener(root, listener, once, capture, passive, + new Listener(this, type, root, listener, once, capture, passive, isNodeStyleListener, weak, resistStopPropagation); this[kNewListener]( root.size, @@ -648,7 +655,7 @@ class EventTarget { return; } - new Listener(previous, listener, once, capture, passive, + new Listener(this, type, previous, listener, once, capture, passive, isNodeStyleListener, weak, resistStopPropagation); root.size++; root.resistStopPropagation ||= Boolean(resistStopPropagation); @@ -691,6 +698,28 @@ class EventTarget { } } + [kRemoveWeakListenerHelper](type, listener) { + const root = this[kEvents].get(type); + if (root === undefined || root.next === undefined) + return; + + const capture = listener.capture === true; + + let handler = root.next; + while (handler !== undefined) { + if (handler === listener) { + handler.remove(); + root.size--; + if (root.size === 0) + this[kEvents].delete(type); + // Undefined is passed as the listener as the listener was GCed + this[kRemoveListener](root.size, type, undefined, capture); + break; + } + handler = handler.next; + } + } + /** * @param {Event} event */ diff --git a/test/parallel/test-eventtarget-memoryleakwarning.js b/test/parallel/test-eventtarget-memoryleakwarning.js index bb4002e30c6e6d..36fe068fb806d6 100644 --- a/test/parallel/test-eventtarget-memoryleakwarning.js +++ b/test/parallel/test-eventtarget-memoryleakwarning.js @@ -1,4 +1,4 @@ -// Flags: --no-warnings +// Flags: --expose-internals --no-warnings --expose-gc 'use strict'; const common = require('../common'); const { @@ -6,6 +6,8 @@ const { EventEmitter, } = require('events'); const assert = require('assert'); +const { kWeakHandler } = require('internal/event_target'); +const { setTimeout } = require('timers/promises'); common.expectWarning({ MaxListenersExceededWarning: [ @@ -73,3 +75,20 @@ common.expectWarning({ setMaxListeners(2, ee); assert.strictEqual(ee.getMaxListeners(), 2); } + +{ + (async () => { + // Test that EventTarget listener don't emit MaxListenersExceededWarning for weak listeners that GCed + const et = new EventTarget(); + setMaxListeners(2, et); + + for (let i = 0; i <= 3; i++) { + et.addEventListener('foo', () => {}, { + [kWeakHandler]: {}, + }); + + await setTimeout(0); + global.gc(); + } + })().then(common.mustCall(), common.mustNotCall()); +} diff --git a/test/parallel/test-eventtarget.js b/test/parallel/test-eventtarget.js index 016d357c9d7c91..193c47893b5a9e 100644 --- a/test/parallel/test-eventtarget.js +++ b/test/parallel/test-eventtarget.js @@ -16,8 +16,8 @@ const { const { once } = require('events'); -const { promisify, inspect } = require('util'); -const delay = promisify(setTimeout); +const { inspect } = require('util'); +const { setTimeout: delay } = require('timers/promises'); // The globals are defined. ok(Event);