From dc79f3f37caf6f25b8efee4623bec31e2c20f595 Mon Sep 17 00:00:00 2001
From: James M Snell <jasnell@gmail.com>
Date: Fri, 6 Nov 2020 08:09:42 -0800
Subject: [PATCH] events: add max listener warning for EventTarget

Signed-off-by: James M Snell <jasnell@gmail.com>

PR-URL: https://github.com/nodejs/node/pull/36001
Fixes: https://github.com/nodejs/node/issues/35990
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
---
 doc/api/events.md                             | 23 ++++++
 lib/events.js                                 | 47 +++++++++++
 lib/internal/event_target.js                  | 59 +++++++-------
 .../test-eventtarget-memoryleakwarning.js     | 79 +++++++++++++++++++
 4 files changed, 178 insertions(+), 30 deletions(-)
 create mode 100644 test/parallel/test-eventtarget-memoryleakwarning.js

diff --git a/doc/api/events.md b/doc/api/events.md
index 5c6b058123c..1ce815ce92c 100644
--- a/doc/api/events.md
+++ b/doc/api/events.md
@@ -383,6 +383,29 @@ Installing a listener using this symbol does not change the behavior once an
 `'error'` event is emitted, therefore the process will still crash if no
 regular `'error'` listener is installed.
 
+### `EventEmitter.setMaxListeners(n[, ...eventTargets])`
+<!-- YAML
+added: REPLACEME
+-->
+
+* `n` {number} A non-negative number. The maximum number of listeners per
+  `EventTarget` event.
+* `...eventsTargets` {EventTarget[]|EventEmitter[]} Zero or more {EventTarget}
+  or {EventEmitter} instances. If none are specified, `n` is set as the default
+  max for all newly created {EventTarget} and {EventEmitter} objects.
+
+```js
+const {
+  setMaxListeners,
+  EventEmitter
+} = require('events');
+
+const target = new EventTarget();
+const emitter = new EventEmitter();
+
+setMaxListeners(5, target, emitter);
+```
+
 ### `emitter.addListener(eventName, listener)`
 <!-- YAML
 added: v0.1.26
diff --git a/lib/events.js b/lib/events.js
index cf1922f1475..dc08042578b 100644
--- a/lib/events.js
+++ b/lib/events.js
@@ -30,6 +30,7 @@ const {
   NumberIsNaN,
   ObjectCreate,
   ObjectDefineProperty,
+  ObjectDefineProperties,
   ObjectGetPrototypeOf,
   ObjectSetPrototypeOf,
   Promise,
@@ -67,6 +68,9 @@ const {
 
 const kCapture = Symbol('kCapture');
 const kErrorMonitor = Symbol('events.errorMonitor');
+const kMaxEventTargetListeners = Symbol('events.maxEventTargetListeners');
+const kMaxEventTargetListenersWarned =
+  Symbol('events.maxEventTargetListenersWarned');
 
 let DOMException;
 const lazyDOMException = hideStackFrames((message, name) => {
@@ -120,6 +124,7 @@ EventEmitter.prototype._maxListeners = undefined;
 // By default EventEmitters will print a warning if more than 10 listeners are
 // added to it. This is a useful default which helps finding memory leaks.
 let defaultMaxListeners = 10;
+let isEventTarget;
 
 function checkListener(listener) {
   if (typeof listener !== 'function') {
@@ -142,6 +147,48 @@ ObjectDefineProperty(EventEmitter, 'defaultMaxListeners', {
   }
 });
 
+ObjectDefineProperties(EventEmitter, {
+  kMaxEventTargetListeners: {
+    value: kMaxEventTargetListeners,
+    enumerable: false,
+    configurable: false,
+    writable: false,
+  },
+  kMaxEventTargetListenersWarned: {
+    value: kMaxEventTargetListenersWarned,
+    enumerable: false,
+    configurable: false,
+    writable: false,
+  }
+});
+
+EventEmitter.setMaxListeners =
+  function(n = defaultMaxListeners, ...eventTargets) {
+    if (typeof n !== 'number' || n < 0 || NumberIsNaN(n))
+      throw new ERR_OUT_OF_RANGE('n', 'a non-negative number', n);
+    if (eventTargets.length === 0) {
+      defaultMaxListeners = n;
+    } else {
+      if (isEventTarget === undefined)
+        isEventTarget = require('internal/event_target').isEventTarget;
+
+      // Performance for forEach is now comparable with regular for-loop
+      eventTargets.forEach((target) => {
+        if (isEventTarget(target)) {
+          target[kMaxEventTargetListeners] = n;
+          target[kMaxEventTargetListenersWarned] = false;
+        } else if (typeof target.setMaxListeners === 'function') {
+          target.setMaxListeners(n);
+        } else {
+          throw new ERR_INVALID_ARG_TYPE(
+            'eventTargets',
+            ['EventEmitter', 'EventTarget'],
+            target);
+        }
+      });
+    }
+  };
+
 EventEmitter.init = function(opts) {
 
   if (this._events === undefined ||
diff --git a/lib/internal/event_target.js b/lib/internal/event_target.js
index 9e237af45f7..1f6ae1c17c8 100644
--- a/lib/internal/event_target.js
+++ b/lib/internal/event_target.js
@@ -26,13 +26,19 @@ const {
     ERR_INVALID_THIS,
   }
 } = require('internal/errors');
-const { validateInteger, validateObject } = require('internal/validators');
+const { validateObject } = require('internal/validators');
 
 const { customInspectSymbol } = require('internal/util');
 const { inspect } = require('util');
 
 const kIsEventTarget = SymbolFor('nodejs.event_target');
 
+const EventEmitter = require('events');
+const {
+  kMaxEventTargetListeners,
+  kMaxEventTargetListenersWarned,
+} = EventEmitter;
+
 const kEvents = Symbol('kEvents');
 const kStop = Symbol('kStop');
 const kTarget = Symbol('kTarget');
@@ -43,8 +49,6 @@ const kCreateEvent = Symbol('kCreateEvent');
 const kNewListener = Symbol('kNewListener');
 const kRemoveListener = Symbol('kRemoveListener');
 const kIsNodeStyleListener = Symbol('kIsNodeStyleListener');
-const kMaxListeners = Symbol('kMaxListeners');
-const kMaxListenersWarned = Symbol('kMaxListenersWarned');
 const kTrustEvent = Symbol('kTrustEvent');
 
 // Lazy load perf_hooks to avoid the additional overhead on startup
@@ -221,6 +225,8 @@ class Listener {
 
 function initEventTarget(self) {
   self[kEvents] = new SafeMap();
+  self[kMaxEventTargetListeners] = EventEmitter.defaultMaxListeners;
+  self[kMaxEventTargetListenersWarned] = false;
 }
 
 class EventTarget {
@@ -233,7 +239,24 @@ class EventTarget {
     initEventTarget(this);
   }
 
-  [kNewListener](size, type, listener, once, capture, passive) {}
+  [kNewListener](size, type, listener, once, capture, passive) {
+    if (this[kMaxEventTargetListeners] > 0 &&
+        size > this[kMaxEventTargetListeners] &&
+        !this[kMaxEventTargetListenersWarned]) {
+      this[kMaxEventTargetListenersWarned] = true;
+      // No error code for this since it is a Warning
+      // eslint-disable-next-line no-restricted-syntax
+      const w = new Error('Possible EventTarget memory leak detected. ' +
+                          `${size} ${type} listeners ` +
+                          `added to ${inspect(this, { depth: -1 })}. Use ` +
+                          'events.setMaxListeners() to increase limit');
+      w.name = 'MaxListenersExceededWarning';
+      w.target = this;
+      w.type = type;
+      w.count = size;
+      process.emitWarning(w);
+    }
+  }
   [kRemoveListener](size, type, listener, capture) {}
 
   addEventListener(type, listener, options = {}) {
@@ -417,9 +440,6 @@ ObjectDefineProperty(EventTarget.prototype, SymbolToStringTag, {
 
 function initNodeEventTarget(self) {
   initEventTarget(self);
-  // eslint-disable-next-line no-use-before-define
-  self[kMaxListeners] = NodeEventTarget.defaultMaxListeners;
-  self[kMaxListenersWarned] = false;
 }
 
 class NodeEventTarget extends EventTarget {
@@ -430,33 +450,12 @@ class NodeEventTarget extends EventTarget {
     initNodeEventTarget(this);
   }
 
-  [kNewListener](size, type, listener, once, capture, passive) {
-    if (this[kMaxListeners] > 0 &&
-        size > this[kMaxListeners] &&
-        !this[kMaxListenersWarned]) {
-      this[kMaxListenersWarned] = true;
-      // No error code for this since it is a Warning
-      // eslint-disable-next-line no-restricted-syntax
-      const w = new Error('Possible EventTarget memory leak detected. ' +
-                          `${size} ${type} listeners ` +
-                          `added to ${inspect(this, { depth: -1 })}. Use ` +
-                          'setMaxListeners() to increase limit');
-      w.name = 'MaxListenersExceededWarning';
-      w.target = this;
-      w.type = type;
-      w.count = size;
-      process.emitWarning(w);
-    }
-  }
-
   setMaxListeners(n) {
-    validateInteger(n, 'n', 0);
-    this[kMaxListeners] = n;
-    return this;
+    EventEmitter.setMaxListeners(n, this);
   }
 
   getMaxListeners() {
-    return this[kMaxListeners];
+    return this[kMaxEventTargetListeners];
   }
 
   eventNames() {
diff --git a/test/parallel/test-eventtarget-memoryleakwarning.js b/test/parallel/test-eventtarget-memoryleakwarning.js
new file mode 100644
index 00000000000..bcd355c11b8
--- /dev/null
+++ b/test/parallel/test-eventtarget-memoryleakwarning.js
@@ -0,0 +1,79 @@
+// Flags: --no-warnings
+'use strict';
+const common = require('../common');
+const {
+  setMaxListeners,
+  EventEmitter
+} = require('events');
+const assert = require('assert');
+
+common.expectWarning({
+  MaxListenersExceededWarning: [
+    ['Possible EventTarget memory leak detected. 3 foo listeners added to ' +
+     'EventTarget. Use events.setMaxListeners() ' +
+     'to increase limit'],
+    ['Possible EventTarget memory leak detected. 3 foo listeners added to ' +
+     '[MessagePort [EventTarget]]. ' +
+     'Use events.setMaxListeners() to increase ' +
+     'limit'],
+    ['Possible EventTarget memory leak detected. 3 foo listeners added to ' +
+     '[MessagePort [EventTarget]]. ' +
+     'Use events.setMaxListeners() to increase ' +
+     'limit'],
+    ['Possible EventTarget memory leak detected. 3 foo listeners added to ' +
+     '[AbortSignal [EventTarget]]. ' +
+     'Use events.setMaxListeners() to increase ' +
+     'limit'],
+  ],
+  ExperimentalWarning: [[
+    'AbortController is an experimental feature. This feature could change ' +
+    'at any time'
+  ]]
+});
+
+
+{
+  const et = new EventTarget();
+  setMaxListeners(2, et);
+  et.addEventListener('foo', () => {});
+  et.addEventListener('foo', () => {});
+  et.addEventListener('foo', () => {});
+}
+
+{
+  // No warning emitted because prior limit was only for that
+  // one EventTarget.
+  const et = new EventTarget();
+  et.addEventListener('foo', () => {});
+  et.addEventListener('foo', () => {});
+  et.addEventListener('foo', () => {});
+}
+
+{
+  const mc = new MessageChannel();
+  setMaxListeners(2, mc.port1);
+  mc.port1.addEventListener('foo', () => {});
+  mc.port1.addEventListener('foo', () => {});
+  mc.port1.addEventListener('foo', () => {});
+}
+
+{
+  // Set the default for newly created EventTargets
+  setMaxListeners(2);
+  const mc = new MessageChannel();
+  mc.port1.addEventListener('foo', () => {});
+  mc.port1.addEventListener('foo', () => {});
+  mc.port1.addEventListener('foo', () => {});
+
+  const ac = new AbortController();
+  ac.signal.addEventListener('foo', () => {});
+  ac.signal.addEventListener('foo', () => {});
+  ac.signal.addEventListener('foo', () => {});
+}
+
+{
+  // It works for EventEmitters also
+  const ee = new EventEmitter();
+  setMaxListeners(2, ee);
+  assert.strictEqual(ee.getMaxListeners(), 2);
+}