events: remove weak listener for event target

Fixes: #48951 PR-URL: #48952 Reviewed-By: Chemi Atlow <[email protected]>

Author: rluvaton

lib/internal/event_target.js

@@ -62,6 +62,7 @@ const kWeakHandler = Symbol('kWeak');
 constkResistStopPropagation=Symbol('kResistStopPropagation');
 
 constkHybridDispatch=SymbolFor('nodejs.internal.kHybridDispatch');
+constkRemoveWeakListenerHelper=Symbol('nodejs.internal.removeWeakListenerHelper');
 constkCreateEvent=Symbol('kCreateEvent');
 constkNewListener=Symbol('kNewListener');
 constkRemoveListener=Symbol('kRemoveListener');
@@ -392,7 +393,7 @@ let weakListenersState = null;
 letobjectToWeakListenerMap=null;
 functionweakListeners(){
 weakListenersState??=newSafeFinalizationRegistry(
-(listener)=>listener.remove(),
+({ eventTarget,listener, eventType })=>eventTarget.deref()?.[kRemoveWeakListenerHelper](eventType,listener),
 );
 objectToWeakListenerMap??=newSafeWeakMap();
 return{registry: weakListenersState,map: objectToWeakListenerMap};
@@ -414,7 +415,7 @@ const kFlagResistStopPropagation = 1 << 6;
 // the linked list makes dispatching faster, even if adding/removing is
 // slower.
 classListener{
-constructor(previous,listener,once,capture,passive,
+constructor(eventTarget,eventType,previous,listener,once,capture,passive,
 isNodeStyleListener,weak,resistStopPropagation){
 this.next=undefined;
 if(previous!==undefined)
@@ -441,7 +442,13 @@ class Listener{
 
 if(this.weak){
 this.callback=newSafeWeakRef(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: newSafeWeakRef(eventTarget),
+listener: this,
+ eventType,
+},this);
 // Make the retainer retain the listener in a WeakMap
 weakListeners().map.set(weak,listener);
 this.listener=this.callback;
@@ -604,7 +611,7 @@ class EventTarget{
 if(root===undefined){
 root={size: 1,next: undefined,resistStopPropagation: Boolean(resistStopPropagation)};
 // This is the first handler in our linked list.
-newListener(root,listener,once,capture,passive,
+newListener(this,type,root,listener,once,capture,passive,
 isNodeStyleListener,weak,resistStopPropagation);
 this[kNewListener](
 root.size,
@@ -631,7 +638,7 @@ class EventTarget{
 return;
 }
 
-newListener(previous,listener,once,capture,passive,
+newListener(this,type,previous,listener,once,capture,passive,
 isNodeStyleListener,weak,resistStopPropagation);
 root.size++;
 root.resistStopPropagation||=Boolean(resistStopPropagation);
@@ -674,6 +681,28 @@ class EventTarget{
 }
 }
 
+[kRemoveWeakListenerHelper](type,listener){
+constroot=this[kEvents].get(type);
+if(root===undefined||root.next===undefined)
+return;
+
+constcapture=listener.capture===true;
+
+lethandler=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
  */

test/parallel/test-eventtarget-memoryleakwarning.js

@@ -1,11 +1,13 @@
-// Flags: --no-warnings
+// Flags: --expose-internals --no-warnings --expose-gc
 'use strict';
 constcommon=require('../common');
 const{
  setMaxListeners,
  EventEmitter,
 }=require('events');
 constassert=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
+constet=newEventTarget();
+setMaxListeners(2,et);
+
+for(leti=0;i<=3;i++){
+et.addEventListener('foo',()=>{},{
+[kWeakHandler]: {},
+});
+
+awaitsetTimeout(0);
+global.gc();
+}
+})().then(common.mustCall(),common.mustNotCall());
+}

test/parallel/test-eventtarget.js

@@ -16,8 +16,8 @@ const{
 
 const{ once }=require('events');
 
-const{promisify,inspect }=require('util');
-constdelay=promisify(setTimeout);
+const{ inspect }=require('util');
+const{setTimeout: delay}=require('timers/promises');
 
 // The globals are defined.
 ok(Event);