From 9d27708ee722b1d846c15e3f065b0f3c9bc7e63b Mon Sep 17 00:00:00 2001
From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com>
Date: Fri, 28 Jul 2023 17:54:03 +0300
Subject: [PATCH 01/19] events: remove weak listener for event target

Fixes: https://github.com/nodejs/node/issues/48951
---
 lib/internal/event_target.js | 39 +++++++++++++++++++++++++++++++-----
 1 file changed, 34 insertions(+), 5 deletions(-)

diff --git a/lib/internal/event_target.js b/lib/internal/event_target.js
index c6fbc81a27b4f2..ec8914431e3fac 100644
--- a/lib/internal/event_target.js
+++ b/lib/internal/event_target.js
@@ -406,7 +406,7 @@ let weakListenersState = null;
 let objectToWeakListenerMap = null;
 function weakListeners() {
   weakListenersState ??= new SafeFinalizationRegistry(
-    (listener) => listener.remove(),
+    ({ eventTarget, listener, eventType }) => eventTarget.removeInternalListener(eventType, listener),
   );
   objectToWeakListenerMap ??= new SafeWeakMap();
   return { registry: weakListenersState, map: objectToWeakListenerMap };
@@ -428,7 +428,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 +455,12 @@ class Listener {
 
     if (this.weak) {
       this.callback = new SafeWeakRef(listener);
-      weakListeners().registry.register(listener, this, this);
+      weakListeners().registry.register(listener, {
+        __proto__: null,
+        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 +626,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 +653,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 +696,30 @@ class EventTarget {
     }
   }
 
+  // TODO - rename this function
+  removeInternalListener(type, listener) {
+    type = webidl.converters.DOMString(type);
+
+    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);
+        this[kRemoveListener](root.size, type, listener.listener, capture);
+        break;
+      }
+      handler = handler.next;
+    }
+  }
+
   /**
    * @param {Event} event
    */

From c32cd85838171b44483b4be7a3c667114f7fb2b9 Mon Sep 17 00:00:00 2001
From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com>
Date: Fri, 28 Jul 2023 17:59:48 +0300
Subject: [PATCH 02/19] pass listener

---
 lib/internal/event_target.js | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/lib/internal/event_target.js b/lib/internal/event_target.js
index ec8914431e3fac..8a06bda1cc6213 100644
--- a/lib/internal/event_target.js
+++ b/lib/internal/event_target.js
@@ -697,23 +697,23 @@ class EventTarget {
   }
 
   // TODO - rename this function
-  removeInternalListener(type, listener) {
-    type = webidl.converters.DOMString(type);
-
+  removeInternalListener(type, listenerWrapper) {
     const root = this[kEvents].get(type);
     if (root === undefined || root.next === undefined)
       return;
 
-    const capture = listener.capture === true;
+    const capture = listenerWrapper.capture === true;
+
+    const listener = listenerWrapper.weak ? listenerWrapper.listener.deref() : listenerWrapper.listener
 
     let handler = root.next;
     while (handler !== undefined) {
-      if (handler === listener) {
+      if (handler === listenerWrapper) {
         handler.remove();
         root.size--;
         if (root.size === 0)
           this[kEvents].delete(type);
-        this[kRemoveListener](root.size, type, listener.listener, capture);
+        this[kRemoveListener](root.size, type, listener, capture);
         break;
       }
       handler = handler.next;

From 20a177797cd070dc7923286f00704c9dd0c2ff63 Mon Sep 17 00:00:00 2001
From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com>
Date: Fri, 28 Jul 2023 18:09:38 +0300
Subject: [PATCH 03/19] add test for not leaking memory

---
 test/parallel/test-abortcontroller.js | 39 +++++++++++++++++++++++++--
 1 file changed, 37 insertions(+), 2 deletions(-)

diff --git a/test/parallel/test-abortcontroller.js b/test/parallel/test-abortcontroller.js
index 4099a018df7daf..75a36dd5036c5d 100644
--- a/test/parallel/test-abortcontroller.js
+++ b/test/parallel/test-abortcontroller.js
@@ -2,7 +2,7 @@
 'use strict';
 
 const common = require('../common');
-const { inspect } = require('util');
+const { inspect, aborted} = require('util');
 
 const {
   ok,
@@ -15,7 +15,7 @@ const {
   kWeakHandler,
 } = require('internal/event_target');
 
-const { setTimeout: sleep } = require('timers/promises');
+const { setTimeout: sleep, setImmediate} = require('timers/promises');
 
 {
   // Tests that abort is fired with the correct event type on AbortControllers
@@ -231,6 +231,41 @@ const { setTimeout: sleep } = require('timers/promises');
   AbortSignal.timeout(1_200_000);
 }
 
+// Make sure AbortSignal.timeout not leaking memory
+{
+  function getMemoryAllocatedInMB() {
+    return Math.round(process.memoryUsage().rss / 1024 / 1024 * 100) / 100;
+  }
+  async function createALotOfAbortSignals() {
+    for (let i = 0; i < 10000; i++) {
+      function lis() {
+
+      }
+
+      const timeoutSignal = AbortSignal.timeout(1_000_000_000);
+      timeoutSignal.addEventListener('abort', lis);
+      aborted(timeoutSignal, {});
+      timeoutSignal.removeEventListener('abort', lis);
+    }
+
+    await sleep(10);
+    global.gc();
+  }
+
+  (async () => {
+    // Making sure we create some data so we won't catch something that is related to the infra
+    await createALotOfAbortSignals().then(common.mustCall());
+
+    const currentMemory = getMemoryAllocatedInMB();
+
+    await createALotOfAbortSignals().then(common.mustCall());
+
+    const newMemory = getMemoryAllocatedInMB();
+
+    strictEqual(newMemory - currentMemory < 100, true, new Error(`After consuming 10 items the memory increased by ${Math.floor(newMemory - currentMemory)}MB`));
+  })().then(common.mustCall());
+}
+
 {
   // Test AbortSignal.reason default
   const signal = AbortSignal.abort();

From 7375d07103ff7ec5359b2d451a0d783cd7e9dbb9 Mon Sep 17 00:00:00 2001
From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com>
Date: Fri, 28 Jul 2023 18:18:00 +0300
Subject: [PATCH 04/19] fix event target not GCed

---
 lib/internal/event_target.js | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/lib/internal/event_target.js b/lib/internal/event_target.js
index 8a06bda1cc6213..7edad083d311e2 100644
--- a/lib/internal/event_target.js
+++ b/lib/internal/event_target.js
@@ -406,7 +406,7 @@ let weakListenersState = null;
 let objectToWeakListenerMap = null;
 function weakListeners() {
   weakListenersState ??= new SafeFinalizationRegistry(
-    ({ eventTarget, listener, eventType }) => eventTarget.removeInternalListener(eventType, listener),
+    ({ eventTarget, listener, eventType }) => eventTarget.deref()?.removeInternalListener(eventType, listener),
   );
   objectToWeakListenerMap ??= new SafeWeakMap();
   return { registry: weakListenersState, map: objectToWeakListenerMap };
@@ -457,7 +457,8 @@ class Listener {
       this.callback = new SafeWeakRef(listener);
       weakListeners().registry.register(listener, {
         __proto__: null,
-        eventTarget,
+        // Weak ref so the listener won't hold the eventTarget alive
+        eventTarget: new SafeWeakRef(eventTarget),
         listener: this,
         eventType
       }, this);

From 3fdc3e3d72aeb0fa3b523a2d9e00319d9f576443 Mon Sep 17 00:00:00 2001
From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com>
Date: Fri, 28 Jul 2023 18:19:56 +0300
Subject: [PATCH 05/19] format

---
 lib/internal/event_target.js          | 4 ++--
 test/parallel/test-abortcontroller.js | 5 +++--
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/lib/internal/event_target.js b/lib/internal/event_target.js
index 7edad083d311e2..a9d98b250e14b6 100644
--- a/lib/internal/event_target.js
+++ b/lib/internal/event_target.js
@@ -460,7 +460,7 @@ class Listener {
         // Weak ref so the listener won't hold the eventTarget alive
         eventTarget: new SafeWeakRef(eventTarget),
         listener: this,
-        eventType
+        eventType,
       }, this);
       // Make the retainer retain the listener in a WeakMap
       weakListeners().map.set(weak, listener);
@@ -705,7 +705,7 @@ class EventTarget {
 
     const capture = listenerWrapper.capture === true;
 
-    const listener = listenerWrapper.weak ? listenerWrapper.listener.deref() : listenerWrapper.listener
+    const listener = listenerWrapper.weak ? listenerWrapper.listener.deref() : listenerWrapper.listener;
 
     let handler = root.next;
     while (handler !== undefined) {
diff --git a/test/parallel/test-abortcontroller.js b/test/parallel/test-abortcontroller.js
index 75a36dd5036c5d..48f3f1d7377fdb 100644
--- a/test/parallel/test-abortcontroller.js
+++ b/test/parallel/test-abortcontroller.js
@@ -2,7 +2,7 @@
 'use strict';
 
 const common = require('../common');
-const { inspect, aborted} = require('util');
+const { inspect, aborted } = require('util');
 
 const {
   ok,
@@ -15,7 +15,7 @@ const {
   kWeakHandler,
 } = require('internal/event_target');
 
-const { setTimeout: sleep, setImmediate} = require('timers/promises');
+const { setTimeout: sleep } = require('timers/promises');
 
 {
   // Tests that abort is fired with the correct event type on AbortControllers
@@ -236,6 +236,7 @@ const { setTimeout: sleep, setImmediate} = require('timers/promises');
   function getMemoryAllocatedInMB() {
     return Math.round(process.memoryUsage().rss / 1024 / 1024 * 100) / 100;
   }
+
   async function createALotOfAbortSignals() {
     for (let i = 0; i < 10000; i++) {
       function lis() {

From d7e1823b242f5d85f271072f01bf3758c04f1f7c Mon Sep 17 00:00:00 2001
From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com>
Date: Fri, 28 Jul 2023 18:26:29 +0300
Subject: [PATCH 06/19] clean

---
 test/parallel/test-abortcontroller.js | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/test/parallel/test-abortcontroller.js b/test/parallel/test-abortcontroller.js
index 48f3f1d7377fdb..1fe95b404802cb 100644
--- a/test/parallel/test-abortcontroller.js
+++ b/test/parallel/test-abortcontroller.js
@@ -250,16 +250,16 @@ const { setTimeout: sleep } = require('timers/promises');
     }
 
     await sleep(10);
-    global.gc();
+    globalThis.gc();
   }
 
   (async () => {
     // Making sure we create some data so we won't catch something that is related to the infra
-    await createALotOfAbortSignals().then(common.mustCall());
+    await createALotOfAbortSignals();
 
     const currentMemory = getMemoryAllocatedInMB();
 
-    await createALotOfAbortSignals().then(common.mustCall());
+    await createALotOfAbortSignals();
 
     const newMemory = getMemoryAllocatedInMB();
 

From 0471c77dc331887c0c404fceb1bd32cc27b6bdb4 Mon Sep 17 00:00:00 2001
From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com>
Date: Sat, 29 Jul 2023 21:03:54 +0300
Subject: [PATCH 07/19] events: rename to `kRemoveListenerHelper` and used
 symbol

---
 lib/internal/event_target.js | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/internal/event_target.js b/lib/internal/event_target.js
index a9d98b250e14b6..ed3666e7b50840 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 kRemoveListenerHelper = SymbolFor('nodejs.internal.removeListenerHelper');
 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(
-    ({ eventTarget, listener, eventType }) => eventTarget.deref()?.removeInternalListener(eventType, listener),
+    ({ eventTarget, listener, eventType }) => eventTarget.deref()?.[kRemoveListenerHelper](eventType, listener),
   );
   objectToWeakListenerMap ??= new SafeWeakMap();
   return { registry: weakListenersState, map: objectToWeakListenerMap };
@@ -697,8 +698,7 @@ class EventTarget {
     }
   }
 
-  // TODO - rename this function
-  removeInternalListener(type, listenerWrapper) {
+  [kRemoveListenerHelper](type, listenerWrapper) {
     const root = this[kEvents].get(type);
     if (root === undefined || root.next === undefined)
       return;

From efbce3e3acc1f40844cb96962350e9bb969520d5 Mon Sep 17 00:00:00 2001
From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com>
Date: Sat, 29 Jul 2023 21:30:28 +0300
Subject: [PATCH 08/19] move AbortSignal.timeout to different test file to be
 less flaky

---
 test/parallel/test-abortcontroller.js     | 113 +---------------------
 test/parallel/test-abortsignal-timeout.js | 113 ++++++++++++++++++++++
 2 files changed, 115 insertions(+), 111 deletions(-)
 create mode 100644 test/parallel/test-abortsignal-timeout.js

diff --git a/test/parallel/test-abortcontroller.js b/test/parallel/test-abortcontroller.js
index 1fe95b404802cb..d7460cd9cd337d 100644
--- a/test/parallel/test-abortcontroller.js
+++ b/test/parallel/test-abortcontroller.js
@@ -1,22 +1,15 @@
-// Flags: --no-warnings --expose-gc --expose-internals
+// Flags: --no-warnings
 'use strict';
 
 const common = require('../common');
-const { inspect, aborted } = require('util');
+const { inspect } = require('util');
 
 const {
   ok,
-  notStrictEqual,
   strictEqual,
   throws,
 } = require('assert');
 
-const {
-  kWeakHandler,
-} = require('internal/event_target');
-
-const { setTimeout: sleep } = require('timers/promises');
-
 {
   // Tests that abort is fired with the correct event type on AbortControllers
   const ac = new AbortController();
@@ -165,108 +158,6 @@ const { setTimeout: sleep } = require('timers/promises');
   strictEqual(signal.reason, 'reason');
 }
 
-{
-  // Test AbortSignal timeout
-  const signal = AbortSignal.timeout(10);
-  ok(!signal.aborted);
-  setTimeout(common.mustCall(() => {
-    ok(signal.aborted);
-    strictEqual(signal.reason.name, 'TimeoutError');
-    strictEqual(signal.reason.code, 23);
-  }), 20);
-}
-
-{
-  (async () => {
-    // Test AbortSignal timeout doesn't prevent the signal
-    // from being garbage collected.
-    let ref;
-    {
-      ref = new globalThis.WeakRef(AbortSignal.timeout(1_200_000));
-    }
-
-    await sleep(10);
-    globalThis.gc();
-    strictEqual(ref.deref(), undefined);
-  })().then(common.mustCall());
-
-  (async () => {
-    // Test that an AbortSignal with a timeout is not gc'd while
-    // there is an active listener on it.
-    let ref;
-    function handler() {}
-    {
-      ref = new globalThis.WeakRef(AbortSignal.timeout(1_200_000));
-      ref.deref().addEventListener('abort', handler);
-    }
-
-    await sleep(10);
-    globalThis.gc();
-    notStrictEqual(ref.deref(), undefined);
-    ok(ref.deref() instanceof AbortSignal);
-
-    ref.deref().removeEventListener('abort', handler);
-
-    await sleep(10);
-    globalThis.gc();
-    strictEqual(ref.deref(), undefined);
-  })().then(common.mustCall());
-
-  (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();
-    strictEqual(ref.deref(), undefined);
-  })().then(common.mustCall());
-
-  // Setting a long timeout (20 minutes here) should not
-  // keep the Node.js process open (the timer is unref'd)
-  AbortSignal.timeout(1_200_000);
-}
-
-// Make sure AbortSignal.timeout not leaking memory
-{
-  function getMemoryAllocatedInMB() {
-    return Math.round(process.memoryUsage().rss / 1024 / 1024 * 100) / 100;
-  }
-
-  async function createALotOfAbortSignals() {
-    for (let i = 0; i < 10000; i++) {
-      function lis() {
-
-      }
-
-      const timeoutSignal = AbortSignal.timeout(1_000_000_000);
-      timeoutSignal.addEventListener('abort', lis);
-      aborted(timeoutSignal, {});
-      timeoutSignal.removeEventListener('abort', lis);
-    }
-
-    await sleep(10);
-    globalThis.gc();
-  }
-
-  (async () => {
-    // Making sure we create some data so we won't catch something that is related to the infra
-    await createALotOfAbortSignals();
-
-    const currentMemory = getMemoryAllocatedInMB();
-
-    await createALotOfAbortSignals();
-
-    const newMemory = getMemoryAllocatedInMB();
-
-    strictEqual(newMemory - currentMemory < 100, true, new Error(`After consuming 10 items the memory increased by ${Math.floor(newMemory - currentMemory)}MB`));
-  })().then(common.mustCall());
-}
-
 {
   // Test AbortSignal.reason default
   const signal = AbortSignal.abort();
diff --git a/test/parallel/test-abortsignal-timeout.js b/test/parallel/test-abortsignal-timeout.js
new file mode 100644
index 00000000000000..0da13fbbea40fa
--- /dev/null
+++ b/test/parallel/test-abortsignal-timeout.js
@@ -0,0 +1,113 @@
+// Flags: --no-warnings --expose-gc --expose-internals
+'use strict';
+
+require('../common');
+const { aborted } = require('util');
+
+const {
+  ok,
+  notStrictEqual,
+  strictEqual,
+} = require('assert');
+
+const {
+  kWeakHandler,
+} = require('internal/event_target');
+
+const { setTimeout: sleep } = require('timers/promises');
+const { test } = require('node:test');
+
+test('Test AbortSignal timeout', async () => {
+  // Test AbortSignal timeout
+  const signal = AbortSignal.timeout(10);
+  ok(!signal.aborted);
+  await sleep(20);
+  ok(signal.aborted);
+  strictEqual(signal.reason.name, 'TimeoutError');
+  strictEqual(signal.reason.code, 23);
+});
+
+test("AbortSignal timeout doesn't prevent the signal from being garbage collected", async () => {
+  let ref;
+  {
+    ref = new globalThis.WeakRef(AbortSignal.timeout(1_200_000));
+  }
+
+  await sleep(10);
+  globalThis.gc();
+  strictEqual(ref.deref(), undefined);
+});
+
+test("AAbortSignal with a timeout is not gc'd while there is an active listener on it", async () => {
+  // Test that an AbortSignal with a timeout is not gc'd while
+  // there is an active listener on it.
+  let ref;
+  function handler() {}
+  {
+    ref = new globalThis.WeakRef(AbortSignal.timeout(1_200_000));
+    ref.deref().addEventListener('abort', handler);
+  }
+
+  await sleep(10);
+  globalThis.gc();
+  notStrictEqual(ref.deref(), undefined);
+  ok(ref.deref() instanceof AbortSignal);
+
+  ref.deref().removeEventListener('abort', handler);
+
+  await sleep(10);
+  globalThis.gc();
+  strictEqual(ref.deref(), undefined);
+});
+
+test('If the event listener is weak, however, it should not prevent gc', async () => {
+  let ref;
+  function handler() {}
+  {
+    ref = new globalThis.WeakRef(AbortSignal.timeout(1_200_000));
+    ref.deref().addEventListener('abort', handler, { [kWeakHandler]: {} });
+  }
+
+  await sleep(10);
+  globalThis.gc();
+  strictEqual(ref.deref(), undefined);
+});
+
+test('should not keep the process open', () => {
+  // Setting a long timeout (20 minutes here) should not
+  // keep the Node.js process open (the timer is unref'd)
+  AbortSignal.timeout(1_200_000);
+});
+
+test('AbortSignal.timeout should not leak memory when using weak and regular listeners', async () => {
+  function getMemoryAllocatedInMB() {
+    return Math.round(process.memoryUsage().rss / 1024 / 1024 * 100) / 100;
+  }
+
+  async function createALotOfAbortSignals() {
+    for (let i = 0; i < 10000; i++) {
+      function lis() {
+
+      }
+
+      const timeoutSignal = AbortSignal.timeout(1_000_000_000);
+      timeoutSignal.addEventListener('abort', lis);
+      aborted(timeoutSignal, {});
+      timeoutSignal.removeEventListener('abort', lis);
+    }
+
+    await sleep(10);
+    globalThis.gc();
+  }
+
+  // Making sure we create some data so we won't catch something that is related to the infra
+  await createALotOfAbortSignals();
+
+  const currentMemory = getMemoryAllocatedInMB();
+
+  await createALotOfAbortSignals();
+
+  const newMemory = getMemoryAllocatedInMB();
+
+  strictEqual(newMemory - currentMemory < 100, true, new Error(`After consuming 10 items the memory increased by ${Math.floor(newMemory - currentMemory)}MB`));
+});

From 86d02704db319bb1a1e5945f8eed7eb460229333 Mon Sep 17 00:00:00 2001
From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com>
Date: Sun, 30 Jul 2023 10:17:36 +0300
Subject: [PATCH 09/19] Update test/parallel/test-abortsignal-timeout.js

Co-authored-by: Chemi Atlow <chemi@atlow.co.il>
---
 test/parallel/test-abortsignal-timeout.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/parallel/test-abortsignal-timeout.js b/test/parallel/test-abortsignal-timeout.js
index 0da13fbbea40fa..3d7837ad6911ae 100644
--- a/test/parallel/test-abortsignal-timeout.js
+++ b/test/parallel/test-abortsignal-timeout.js
@@ -38,7 +38,7 @@ test("AbortSignal timeout doesn't prevent the signal from being garbage collecte
   strictEqual(ref.deref(), undefined);
 });
 
-test("AAbortSignal with a timeout is not gc'd while there is an active listener on it", async () => {
+test("AbortSignal with a timeout is not gc'd while there is an active listener on it", async () => {
   // Test that an AbortSignal with a timeout is not gc'd while
   // there is an active listener on it.
   let ref;

From a83c38b0c473fb4bd713b210136d0fc4a539677b Mon Sep 17 00:00:00 2001
From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com>
Date: Sun, 30 Jul 2023 10:25:36 +0300
Subject: [PATCH 10/19] rename test

---
 test/parallel/test-abortsignal-timeout.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/parallel/test-abortsignal-timeout.js b/test/parallel/test-abortsignal-timeout.js
index 3d7837ad6911ae..39dfc6d30f6ef2 100644
--- a/test/parallel/test-abortsignal-timeout.js
+++ b/test/parallel/test-abortsignal-timeout.js
@@ -60,7 +60,7 @@ test("AbortSignal with a timeout is not gc'd while there is an active listener o
   strictEqual(ref.deref(), undefined);
 });
 
-test('If the event listener is weak, however, it should not prevent gc', async () => {
+test("AbortSignal.timeout should be gc'd when there is only a weak listener active on it", async () => {
   let ref;
   function handler() {}
   {

From 6002db841255875fbead1618437bbe397d31cdd2 Mon Sep 17 00:00:00 2001
From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com>
Date: Sun, 30 Jul 2023 16:19:10 +0300
Subject: [PATCH 11/19] revert tests due to them not working

---
 test/parallel/test-abortcontroller.js     |  75 +++++++++++++-
 test/parallel/test-abortsignal-timeout.js | 113 ----------------------
 2 files changed, 74 insertions(+), 114 deletions(-)
 delete mode 100644 test/parallel/test-abortsignal-timeout.js

diff --git a/test/parallel/test-abortcontroller.js b/test/parallel/test-abortcontroller.js
index d7460cd9cd337d..4099a018df7daf 100644
--- a/test/parallel/test-abortcontroller.js
+++ b/test/parallel/test-abortcontroller.js
@@ -1,4 +1,4 @@
-// Flags: --no-warnings
+// Flags: --no-warnings --expose-gc --expose-internals
 'use strict';
 
 const common = require('../common');
@@ -6,10 +6,17 @@ const { inspect } = require('util');
 
 const {
   ok,
+  notStrictEqual,
   strictEqual,
   throws,
 } = require('assert');
 
+const {
+  kWeakHandler,
+} = require('internal/event_target');
+
+const { setTimeout: sleep } = require('timers/promises');
+
 {
   // Tests that abort is fired with the correct event type on AbortControllers
   const ac = new AbortController();
@@ -158,6 +165,72 @@ const {
   strictEqual(signal.reason, 'reason');
 }
 
+{
+  // Test AbortSignal timeout
+  const signal = AbortSignal.timeout(10);
+  ok(!signal.aborted);
+  setTimeout(common.mustCall(() => {
+    ok(signal.aborted);
+    strictEqual(signal.reason.name, 'TimeoutError');
+    strictEqual(signal.reason.code, 23);
+  }), 20);
+}
+
+{
+  (async () => {
+    // Test AbortSignal timeout doesn't prevent the signal
+    // from being garbage collected.
+    let ref;
+    {
+      ref = new globalThis.WeakRef(AbortSignal.timeout(1_200_000));
+    }
+
+    await sleep(10);
+    globalThis.gc();
+    strictEqual(ref.deref(), undefined);
+  })().then(common.mustCall());
+
+  (async () => {
+    // Test that an AbortSignal with a timeout is not gc'd while
+    // there is an active listener on it.
+    let ref;
+    function handler() {}
+    {
+      ref = new globalThis.WeakRef(AbortSignal.timeout(1_200_000));
+      ref.deref().addEventListener('abort', handler);
+    }
+
+    await sleep(10);
+    globalThis.gc();
+    notStrictEqual(ref.deref(), undefined);
+    ok(ref.deref() instanceof AbortSignal);
+
+    ref.deref().removeEventListener('abort', handler);
+
+    await sleep(10);
+    globalThis.gc();
+    strictEqual(ref.deref(), undefined);
+  })().then(common.mustCall());
+
+  (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();
+    strictEqual(ref.deref(), undefined);
+  })().then(common.mustCall());
+
+  // Setting a long timeout (20 minutes here) should not
+  // keep the Node.js process open (the timer is unref'd)
+  AbortSignal.timeout(1_200_000);
+}
+
 {
   // Test AbortSignal.reason default
   const signal = AbortSignal.abort();
diff --git a/test/parallel/test-abortsignal-timeout.js b/test/parallel/test-abortsignal-timeout.js
deleted file mode 100644
index 39dfc6d30f6ef2..00000000000000
--- a/test/parallel/test-abortsignal-timeout.js
+++ /dev/null
@@ -1,113 +0,0 @@
-// Flags: --no-warnings --expose-gc --expose-internals
-'use strict';
-
-require('../common');
-const { aborted } = require('util');
-
-const {
-  ok,
-  notStrictEqual,
-  strictEqual,
-} = require('assert');
-
-const {
-  kWeakHandler,
-} = require('internal/event_target');
-
-const { setTimeout: sleep } = require('timers/promises');
-const { test } = require('node:test');
-
-test('Test AbortSignal timeout', async () => {
-  // Test AbortSignal timeout
-  const signal = AbortSignal.timeout(10);
-  ok(!signal.aborted);
-  await sleep(20);
-  ok(signal.aborted);
-  strictEqual(signal.reason.name, 'TimeoutError');
-  strictEqual(signal.reason.code, 23);
-});
-
-test("AbortSignal timeout doesn't prevent the signal from being garbage collected", async () => {
-  let ref;
-  {
-    ref = new globalThis.WeakRef(AbortSignal.timeout(1_200_000));
-  }
-
-  await sleep(10);
-  globalThis.gc();
-  strictEqual(ref.deref(), undefined);
-});
-
-test("AbortSignal with a timeout is not gc'd while there is an active listener on it", async () => {
-  // Test that an AbortSignal with a timeout is not gc'd while
-  // there is an active listener on it.
-  let ref;
-  function handler() {}
-  {
-    ref = new globalThis.WeakRef(AbortSignal.timeout(1_200_000));
-    ref.deref().addEventListener('abort', handler);
-  }
-
-  await sleep(10);
-  globalThis.gc();
-  notStrictEqual(ref.deref(), undefined);
-  ok(ref.deref() instanceof AbortSignal);
-
-  ref.deref().removeEventListener('abort', handler);
-
-  await sleep(10);
-  globalThis.gc();
-  strictEqual(ref.deref(), undefined);
-});
-
-test("AbortSignal.timeout should be gc'd when there is only a weak listener active on it", async () => {
-  let ref;
-  function handler() {}
-  {
-    ref = new globalThis.WeakRef(AbortSignal.timeout(1_200_000));
-    ref.deref().addEventListener('abort', handler, { [kWeakHandler]: {} });
-  }
-
-  await sleep(10);
-  globalThis.gc();
-  strictEqual(ref.deref(), undefined);
-});
-
-test('should not keep the process open', () => {
-  // Setting a long timeout (20 minutes here) should not
-  // keep the Node.js process open (the timer is unref'd)
-  AbortSignal.timeout(1_200_000);
-});
-
-test('AbortSignal.timeout should not leak memory when using weak and regular listeners', async () => {
-  function getMemoryAllocatedInMB() {
-    return Math.round(process.memoryUsage().rss / 1024 / 1024 * 100) / 100;
-  }
-
-  async function createALotOfAbortSignals() {
-    for (let i = 0; i < 10000; i++) {
-      function lis() {
-
-      }
-
-      const timeoutSignal = AbortSignal.timeout(1_000_000_000);
-      timeoutSignal.addEventListener('abort', lis);
-      aborted(timeoutSignal, {});
-      timeoutSignal.removeEventListener('abort', lis);
-    }
-
-    await sleep(10);
-    globalThis.gc();
-  }
-
-  // Making sure we create some data so we won't catch something that is related to the infra
-  await createALotOfAbortSignals();
-
-  const currentMemory = getMemoryAllocatedInMB();
-
-  await createALotOfAbortSignals();
-
-  const newMemory = getMemoryAllocatedInMB();
-
-  strictEqual(newMemory - currentMemory < 100, true, new Error(`After consuming 10 items the memory increased by ${Math.floor(newMemory - currentMemory)}MB`));
-});

From 7134b93cd993b73c4a10b0d41da9e011712396db Mon Sep 17 00:00:00 2001
From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com>
Date: Sun, 30 Jul 2023 19:03:10 +0300
Subject: [PATCH 12/19] add test

---
 test/parallel/test-abortcontroller.js | 49 ++++++---------------------
 1 file changed, 10 insertions(+), 39 deletions(-)

diff --git a/test/parallel/test-abortcontroller.js b/test/parallel/test-abortcontroller.js
index 4099a018df7daf..ac0823e95aaec2 100644
--- a/test/parallel/test-abortcontroller.js
+++ b/test/parallel/test-abortcontroller.js
@@ -15,6 +15,8 @@ const {
   kWeakHandler,
 } = require('internal/event_target');
 
+const {aborted} = require('util');
+
 const { setTimeout: sleep } = require('timers/promises');
 
 {
@@ -178,57 +180,26 @@ const { setTimeout: sleep } = require('timers/promises');
 
 {
   (async () => {
-    // Test AbortSignal timeout doesn't prevent the signal
-    // from being garbage collected.
+    // Test AbortSignal timeout can be GCed when no listeners exist
     let ref;
     {
-      ref = new globalThis.WeakRef(AbortSignal.timeout(1_200_000));
-    }
+      function lis() {
 
-    await sleep(10);
-    globalThis.gc();
-    strictEqual(ref.deref(), undefined);
-  })().then(common.mustCall());
-
-  (async () => {
-    // Test that an AbortSignal with a timeout is not gc'd while
-    // there is an active listener on it.
-    let ref;
-    function handler() {}
-    {
+      }
       ref = new globalThis.WeakRef(AbortSignal.timeout(1_200_000));
-      ref.deref().addEventListener('abort', handler);
-    }
+      ref.deref().addEventListener('abort', lis);
+      aborted(ref.deref(), {});
 
-    await sleep(10);
-    globalThis.gc();
-    notStrictEqual(ref.deref(), undefined);
-    ok(ref.deref() instanceof AbortSignal);
-
-    ref.deref().removeEventListener('abort', handler);
-
-    await sleep(10);
-    globalThis.gc();
-    strictEqual(ref.deref(), undefined);
-  })().then(common.mustCall());
+      await sleep(10);
+      globalThis.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]: {} });
+      ref.deref().removeEventListener('abort', lis);
     }
 
     await sleep(10);
     globalThis.gc();
     strictEqual(ref.deref(), undefined);
   })().then(common.mustCall());
-
-  // Setting a long timeout (20 minutes here) should not
-  // keep the Node.js process open (the timer is unref'd)
-  AbortSignal.timeout(1_200_000);
 }
 
 {

From 7a0fde3f12285a73381908c110d9231bcbed277d Mon Sep 17 00:00:00 2001
From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com>
Date: Sun, 30 Jul 2023 19:04:32 +0300
Subject: [PATCH 13/19] format

---
 test/parallel/test-abortcontroller.js | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/test/parallel/test-abortcontroller.js b/test/parallel/test-abortcontroller.js
index ac0823e95aaec2..a828f43953e7ce 100644
--- a/test/parallel/test-abortcontroller.js
+++ b/test/parallel/test-abortcontroller.js
@@ -6,16 +6,11 @@ const { inspect } = require('util');
 
 const {
   ok,
-  notStrictEqual,
   strictEqual,
   throws,
 } = require('assert');
 
-const {
-  kWeakHandler,
-} = require('internal/event_target');
-
-const {aborted} = require('util');
+const { aborted } = require('util');
 
 const { setTimeout: sleep } = require('timers/promises');
 

From f7dd524acd27c9a167cc26457b5923bfe67de2ca Mon Sep 17 00:00:00 2001
From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com>
Date: Sun, 30 Jul 2023 19:45:20 +0300
Subject: [PATCH 14/19] add back the tests

---
 test/parallel/test-abortcontroller.js | 60 +++++++++++++++++++++++++++
 1 file changed, 60 insertions(+)

diff --git a/test/parallel/test-abortcontroller.js b/test/parallel/test-abortcontroller.js
index a828f43953e7ce..1c591d732d22fd 100644
--- a/test/parallel/test-abortcontroller.js
+++ b/test/parallel/test-abortcontroller.js
@@ -6,10 +6,14 @@ const { inspect } = require('util');
 
 const {
   ok,
+  notStrictEqual,
   strictEqual,
   throws,
 } = require('assert');
 
+const {
+  kWeakHandler,
+} = require('internal/event_target');
 const { aborted } = require('util');
 
 const { setTimeout: sleep } = require('timers/promises');
@@ -173,6 +177,62 @@ const { setTimeout: sleep } = require('timers/promises');
   }), 20);
 }
 
+
+{
+  (async () => {
+    // Test AbortSignal timeout doesn't prevent the signal
+    // from being garbage collected.
+    let ref;
+    {
+      ref = new globalThis.WeakRef(AbortSignal.timeout(1_200_000));
+    }
+
+    await sleep(10);
+    globalThis.gc();
+    strictEqual(ref.deref(), undefined);
+  })().then(common.mustCall());
+
+  (async () => {
+    // Test that an AbortSignal with a timeout is not gc'd while
+    // there is an active listener on it.
+    let ref;
+    function handler() {}
+    {
+      ref = new globalThis.WeakRef(AbortSignal.timeout(1_200_000));
+      ref.deref().addEventListener('abort', handler);
+    }
+
+    await sleep(10);
+    globalThis.gc();
+    notStrictEqual(ref.deref(), undefined);
+    ok(ref.deref() instanceof AbortSignal);
+
+    ref.deref().removeEventListener('abort', handler);
+
+    await sleep(10);
+    globalThis.gc();
+    strictEqual(ref.deref(), undefined);
+  })().then(common.mustCall());
+
+  (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();
+    strictEqual(ref.deref(), undefined);
+  })().then(common.mustCall());
+
+  // Setting a long timeout (20 minutes here) should not
+  // keep the Node.js process open (the timer is unref'd)
+  AbortSignal.timeout(1_200_000);
+}
+
 {
   (async () => {
     // Test AbortSignal timeout can be GCed when no listeners exist

From a85adafe26f0a9e36c7e8626b1afb4f8d6dec888 Mon Sep 17 00:00:00 2001
From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com>
Date: Tue, 1 Aug 2023 21:21:59 +0300
Subject: [PATCH 15/19] increase sleep in test

---
 test/parallel/test-abortcontroller.js | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/test/parallel/test-abortcontroller.js b/test/parallel/test-abortcontroller.js
index 1c591d732d22fd..9c0a04f52f0cf0 100644
--- a/test/parallel/test-abortcontroller.js
+++ b/test/parallel/test-abortcontroller.js
@@ -245,13 +245,13 @@ const { setTimeout: sleep } = require('timers/promises');
       ref.deref().addEventListener('abort', lis);
       aborted(ref.deref(), {});
 
-      await sleep(10);
+      await sleep(100);
       globalThis.gc();
 
       ref.deref().removeEventListener('abort', lis);
     }
 
-    await sleep(10);
+    await sleep(100);
     globalThis.gc();
     strictEqual(ref.deref(), undefined);
   })().then(common.mustCall());

From 9ab6de604f83254f193eb645ae9619f45abf345b Mon Sep 17 00:00:00 2001
From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com>
Date: Thu, 3 Aug 2023 01:40:59 +0300
Subject: [PATCH 16/19] test: test that memory leak warning not emitted for
 weak refs that got cleaned

---
 test/parallel/test-abortcontroller.js         | 26 -------------------
 .../test-eventtarget-memoryleakwarning.js     | 21 ++++++++++++++-
 test/parallel/test-eventtarget.js             |  4 +--
 3 files changed, 22 insertions(+), 29 deletions(-)

diff --git a/test/parallel/test-abortcontroller.js b/test/parallel/test-abortcontroller.js
index 9c0a04f52f0cf0..4099a018df7daf 100644
--- a/test/parallel/test-abortcontroller.js
+++ b/test/parallel/test-abortcontroller.js
@@ -14,7 +14,6 @@ const {
 const {
   kWeakHandler,
 } = require('internal/event_target');
-const { aborted } = require('util');
 
 const { setTimeout: sleep } = require('timers/promises');
 
@@ -177,7 +176,6 @@ const { setTimeout: sleep } = require('timers/promises');
   }), 20);
 }
 
-
 {
   (async () => {
     // Test AbortSignal timeout doesn't prevent the signal
@@ -233,30 +231,6 @@ const { setTimeout: sleep } = require('timers/promises');
   AbortSignal.timeout(1_200_000);
 }
 
-{
-  (async () => {
-    // Test AbortSignal timeout can be GCed when no listeners exist
-    let ref;
-    {
-      function lis() {
-
-      }
-      ref = new globalThis.WeakRef(AbortSignal.timeout(1_200_000));
-      ref.deref().addEventListener('abort', lis);
-      aborted(ref.deref(), {});
-
-      await sleep(100);
-      globalThis.gc();
-
-      ref.deref().removeEventListener('abort', lis);
-    }
-
-    await sleep(100);
-    globalThis.gc();
-    strictEqual(ref.deref(), undefined);
-  })().then(common.mustCall());
-}
-
 {
   // Test AbortSignal.reason default
   const signal = AbortSignal.abort();
diff --git a/test/parallel/test-eventtarget-memoryleakwarning.js b/test/parallel/test-eventtarget-memoryleakwarning.js
index bb4002e30c6e6d..48e323a0f54ad3 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();
+    }
+  })().catch(common.mustNotCall()).then(common.mustCall());
+}
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);

From 5f8e448191d913a44e4f01db91e3ce0462ef9fbd Mon Sep 17 00:00:00 2001
From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com>
Date: Thu, 3 Aug 2023 01:45:04 +0300
Subject: [PATCH 17/19] events: rename kRemoveListenerHelper to indicate it's
 only for weak listeners

---
 lib/internal/event_target.js | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/lib/internal/event_target.js b/lib/internal/event_target.js
index ed3666e7b50840..b126913c80090b 100644
--- a/lib/internal/event_target.js
+++ b/lib/internal/event_target.js
@@ -58,7 +58,7 @@ const kWeakHandler = Symbol('kWeak');
 const kResistStopPropagation = Symbol('kResistStopPropagation');
 
 const kHybridDispatch = SymbolFor('nodejs.internal.kHybridDispatch');
-const kRemoveListenerHelper = SymbolFor('nodejs.internal.removeListenerHelper');
+const kRemoveWeakListenerHelper = SymbolFor('nodejs.internal.removeWeakListenerHelper');
 const kCreateEvent = Symbol('kCreateEvent');
 const kNewListener = Symbol('kNewListener');
 const kRemoveListener = Symbol('kRemoveListener');
@@ -407,7 +407,7 @@ let weakListenersState = null;
 let objectToWeakListenerMap = null;
 function weakListeners() {
   weakListenersState ??= new SafeFinalizationRegistry(
-    ({ eventTarget, listener, eventType }) => eventTarget.deref()?.[kRemoveListenerHelper](eventType, listener),
+    ({ eventTarget, listener, eventType }) => eventTarget.deref()?.[kRemoveWeakListenerHelper](eventType, listener),
   );
   objectToWeakListenerMap ??= new SafeWeakMap();
   return { registry: weakListenersState, map: objectToWeakListenerMap };
@@ -698,23 +698,22 @@ class EventTarget {
     }
   }
 
-  [kRemoveListenerHelper](type, listenerWrapper) {
+  [kRemoveWeakListenerHelper](type, listener) {
     const root = this[kEvents].get(type);
     if (root === undefined || root.next === undefined)
       return;
 
-    const capture = listenerWrapper.capture === true;
-
-    const listener = listenerWrapper.weak ? listenerWrapper.listener.deref() : listenerWrapper.listener;
+    const capture = listener.capture === true;
 
     let handler = root.next;
     while (handler !== undefined) {
-      if (handler === listenerWrapper) {
+      if (handler === listener) {
         handler.remove();
         root.size--;
         if (root.size === 0)
           this[kEvents].delete(type);
-        this[kRemoveListener](root.size, type, listener, capture);
+        // undefined is passed as the listener as the listener was GCed
+        this[kRemoveListener](root.size, type, undefined, capture);
         break;
       }
       handler = handler.next;

From 3b1266e63aa0c68d064272bb99486ec44ea6fa47 Mon Sep 17 00:00:00 2001
From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com>
Date: Sun, 6 Aug 2023 01:01:51 +0300
Subject: [PATCH 18/19] Update
 test/parallel/test-eventtarget-memoryleakwarning.js

Co-authored-by: Chemi Atlow <chemi@atlow.co.il>
---
 test/parallel/test-eventtarget-memoryleakwarning.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/parallel/test-eventtarget-memoryleakwarning.js b/test/parallel/test-eventtarget-memoryleakwarning.js
index 48e323a0f54ad3..36fe068fb806d6 100644
--- a/test/parallel/test-eventtarget-memoryleakwarning.js
+++ b/test/parallel/test-eventtarget-memoryleakwarning.js
@@ -90,5 +90,5 @@ common.expectWarning({
       await setTimeout(0);
       global.gc();
     }
-  })().catch(common.mustNotCall()).then(common.mustCall());
+  })().then(common.mustCall(), common.mustNotCall());
 }

From ece725c1d369a153fa795f1feaf9ad09b8cc8e77 Mon Sep 17 00:00:00 2001
From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com>
Date: Sun, 6 Aug 2023 01:05:55 +0300
Subject: [PATCH 19/19] use symbol

---
 lib/internal/event_target.js | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/internal/event_target.js b/lib/internal/event_target.js
index b126913c80090b..5ab2a23f980549 100644
--- a/lib/internal/event_target.js
+++ b/lib/internal/event_target.js
@@ -58,7 +58,7 @@ const kWeakHandler = Symbol('kWeak');
 const kResistStopPropagation = Symbol('kResistStopPropagation');
 
 const kHybridDispatch = SymbolFor('nodejs.internal.kHybridDispatch');
-const kRemoveWeakListenerHelper = SymbolFor('nodejs.internal.removeWeakListenerHelper');
+const kRemoveWeakListenerHelper = Symbol('nodejs.internal.removeWeakListenerHelper');
 const kCreateEvent = Symbol('kCreateEvent');
 const kNewListener = Symbol('kNewListener');
 const kRemoveListener = Symbol('kRemoveListener');
@@ -712,7 +712,7 @@ class EventTarget {
         root.size--;
         if (root.size === 0)
           this[kEvents].delete(type);
-        // undefined is passed as the listener as the listener was GCed
+        // Undefined is passed as the listener as the listener was GCed
         this[kRemoveListener](root.size, type, undefined, capture);
         break;
       }