lib: add weak event handlers

PR-URL: #36607
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>

Author: benjamingr

lib/events.js

@@ -697,7 +697,9 @@ function getEventListeners(emitterOrTarget, type) {
     const listeners = [];
     let handler = root?.next;
     while (handler?.listener !== undefined) {
-      ArrayPrototypePush(listeners, handler.listener);
+      const listener = handler.listener?.deref ?
+        handler.listener.deref() : handler.listener;
+      ArrayPrototypePush(listeners, listener);
       handler = handler.next;
     }
     return listeners;

lib/internal/event_target.js

@@ -15,11 +15,12 @@ const {
   ReflectApply,
   SafeArrayIterator,
   SafeMap,
+  SafeWeakMap,
+  SafeWeakSet,
   String,
   Symbol,
   SymbolFor,
   SymbolToStringTag,
-  SafeWeakSet,
 } = primordials;
 
 const {
@@ -47,6 +48,7 @@ const kEvents = Symbol('kEvents');
 const kStop = Symbol('kStop');
 const kTarget = Symbol('kTarget');
 const kHandlers = Symbol('khandlers');
+const kWeakHandler = Symbol('kWeak');
 
 const kHybridDispatch = SymbolFor('nodejs.internal.kHybridDispatch');
 const kCreateEvent = Symbol('kCreateEvent');
@@ -190,6 +192,21 @@ class NodeCustomEvent extends Event {
     }
   }
 }
+
+// Weak listener cleanup
+// This has to be lazy for snapshots to work
+let weakListenersState = null;
+// The resource needs to retain the callback so that it doesn't
+// get garbage collected now that it's weak.
+let objectToWeakListenerMap = null;
+function weakListeners() {
+  weakListenersState ??= new globalThis.FinalizationRegistry(
+    (listener) => listener.remove()
+  );
+  objectToWeakListenerMap ??= new SafeWeakMap();
+  return { registry: weakListenersState, map: objectToWeakListenerMap };
+}
+
 // The listeners for an EventTarget are maintained as a linked list.
 // Unfortunately, the way EventTarget is defined, listeners are accounted
 // using the tuple [handler,capture], and even if we don't actually make
@@ -198,7 +215,8 @@ class NodeCustomEvent extends Event {
 // the linked list makes dispatching faster, even if adding/removing is
 // slower.
 class Listener {
-  constructor(previous, listener, once, capture, passive, isNodeStyleListener) {
+  constructor(previous, listener, once, capture, passive,
+              isNodeStyleListener, weak) {
     this.next = undefined;
     if (previous !== undefined)
       previous.next = this;
@@ -210,15 +228,26 @@ class Listener {
     this.passive = passive;
     this.isNodeStyleListener = isNodeStyleListener;
     this.removed = false;
-
-    this.callback =
-      typeof listener === 'function' ?
-        listener :
-        FunctionPrototypeBind(listener.handleEvent, listener);
+    this.weak = Boolean(weak); // Don't retain the object
+
+    if (this.weak) {
+      this.callback = new globalThis.WeakRef(listener);
+      weakListeners().registry.register(listener, this, this);
+      // Make the retainer retain the listener in a WeakMap
+      weakListeners().map.set(weak, listener);
+      this.listener = this.callback;
+    } else if (typeof listener === 'function') {
+      this.callback = listener;
+      this.listener = listener;
+    } else {
+      this.callback = FunctionPrototypeBind(listener.handleEvent, listener);
+      this.listener = listener;
+    }
   }
 
   same(listener, capture) {
-    return this.listener === listener && this.capture === capture;
+    const myListener = this.weak ? this.listener.deref() : this.listener;
+    return myListener === listener && this.capture === capture;
   }
 
   remove() {
@@ -227,6 +256,8 @@ class Listener {
     if (this.next !== undefined)
       this.next.previous = this.previous;
     this.removed = true;
+    if (this.weak)
+      weakListeners().registry.unregister(this);
   }
 }
 
@@ -277,7 +308,8 @@ class EventTarget {
       capture,
       passive,
       signal,
-      isNodeStyleListener
+      isNodeStyleListener,
+      weak,
     } = validateEventListenerOptions(options);
 
     if (!shouldAddListener(listener)) {
@@ -302,15 +334,16 @@ class EventTarget {
       // not prevent the event target from GC.
       signal.addEventListener('abort', () => {
         this.removeEventListener(type, listener, options);
-      }, { once: true });
+      }, { once: true, [kWeakHandler]: this });
     }
 
     let root = this[kEvents].get(type);
 
     if (root === undefined) {
       root = { size: 1, next: undefined };
       // This is the first handler in our linked list.
-      new Listener(root, listener, once, capture, passive, isNodeStyleListener);
+      new Listener(root, listener, once, capture, passive,
+                   isNodeStyleListener, weak);
       this[kNewListener](root.size, type, listener, once, capture, passive);
       this[kEvents].set(type, root);
       return;
@@ -330,7 +363,7 @@ class EventTarget {
     }
 
     new Listener(previous, listener, once, capture, passive,
-                 isNodeStyleListener);
+                 isNodeStyleListener, weak);
     root.size++;
     this[kNewListener](root.size, type, listener, once, capture, passive);
   }
@@ -418,7 +451,12 @@ class EventTarget {
         } else {
           arg = createEvent();
         }
-        const result = FunctionPrototypeCall(handler.callback, this, arg);
+        const callback = handler.weak ?
+          handler.callback.deref() : handler.callback;
+        let result;
+        if (callback) {
+          result = FunctionPrototypeCall(callback, this, arg);
+        }
         if (result !== undefined && result !== null)
           addCatch(this, result, createEvent());
       } catch (err) {
@@ -569,6 +607,7 @@ function validateEventListenerOptions(options) {
     capture: Boolean(options.capture),
     passive: Boolean(options.passive),
     signal: options.signal,
+    weak: options[kWeakHandler],
     isNodeStyleListener: Boolean(options[kIsNodeStyleListener])
   };
 }
@@ -671,5 +710,6 @@ module.exports = {
   kTrustEvent,
   kRemoveListener,
   kEvents,
+  kWeakHandler,
   isEventTarget,
 };

test/parallel/test-events-static-geteventlisteners.js

@@ -1,6 +1,7 @@
 'use strict';
-
+// Flags: --expose-internals --no-warnings
 const common = require('../common');
+const { kWeakHandler } = require('internal/event_target');
 
 const {
   deepStrictEqual,
@@ -41,3 +42,11 @@ const { getEventListeners, EventEmitter } = require('events');
     getEventListeners('INVALID_EMITTER');
   }, /ERR_INVALID_ARG_TYPE/);
 }
+{
+  // Test weak listeners
+  const target = new EventTarget();
+  const fn = common.mustNotCall();
+  target.addEventListener('foo', fn, { [kWeakHandler]: {} });
+  const listeners = getEventListeners(target, 'foo');
+  deepStrictEqual(listeners, [fn]);
+}

test/parallel/test-eventtarget.js

@@ -1,8 +1,11 @@
-// Flags: --expose-internals --no-warnings
+// Flags: --expose-internals --no-warnings --expose-gc
 'use strict';
 
 const common = require('../common');
-const { defineEventHandler } = require('internal/event_target');
+const {
+  defineEventHandler,
+  kWeakHandler,
+} = require('internal/event_target');
 
 const {
   ok,
@@ -570,3 +573,27 @@ let asyncTest = Promise.resolve();
   const et = new EventTarget();
   strictEqual(et.constructor.name, 'EventTarget');
 }
+{
+  // Weak event handlers work
+  const et = new EventTarget();
+  const listener = common.mustCall();
+  et.addEventListener('foo', listener, { [kWeakHandler]: et });
+  et.dispatchEvent(new Event('foo'));
+}
+{
+  // Weak event handlers can be removed and weakness is not part of the key
+  const et = new EventTarget();
+  const listener = common.mustNotCall();
+  et.addEventListener('foo', listener, { [kWeakHandler]: et });
+  et.removeEventListener('foo', listener);
+  et.dispatchEvent(new Event('foo'));
+}
+{
+  // Test listeners are held weakly
+  const et = new EventTarget();
+  et.addEventListener('foo', common.mustNotCall(), { [kWeakHandler]: {} });
+  setImmediate(() => {
+    global.gc();
+    et.dispatchEvent(new Event('foo'));
+  });
+}