Skip to content

Commit 3006302

Browse files
jasnelltargos
authored andcommitted
events: add max listener warning for EventTarget
Signed-off-by: James M Snell <jasnell@gmail.com> PR-URL: #36001 Backport-PR-URL: #38386 Fixes: #35990 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
1 parent cf4fa79 commit 3006302

File tree

4 files changed

+180
-30
lines changed

4 files changed

+180
-30
lines changed

Diff for: doc/api/events.md

+23
Original file line numberDiff line numberDiff line change
@@ -383,6 +383,29 @@ Installing a listener using this symbol does not change the behavior once an
383383
`'error'` event is emitted, therefore the process will still crash if no
384384
regular `'error'` listener is installed.
385385

386+
### `EventEmitter.setMaxListeners(n[, ...eventTargets])`
387+
<!-- YAML
388+
added: REPLACEME
389+
-->
390+
391+
* `n` {number} A non-negative number. The maximum number of listeners per
392+
`EventTarget` event.
393+
* `...eventsTargets` {EventTarget[]|EventEmitter[]} Zero or more {EventTarget}
394+
or {EventEmitter} instances. If none are specified, `n` is set as the default
395+
max for all newly created {EventTarget} and {EventEmitter} objects.
396+
397+
```js
398+
const {
399+
setMaxListeners,
400+
EventEmitter
401+
} = require('events');
402+
403+
const target = new EventTarget();
404+
const emitter = new EventEmitter();
405+
406+
setMaxListeners(5, target, emitter);
407+
```
408+
386409
### `emitter.addListener(eventName, listener)`
387410
<!-- YAML
388411
added: v0.1.26

Diff for: lib/events.js

+47
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ const {
3030
NumberIsNaN,
3131
ObjectCreate,
3232
ObjectDefineProperty,
33+
ObjectDefineProperties,
3334
ObjectGetPrototypeOf,
3435
ObjectSetPrototypeOf,
3536
Promise,
@@ -67,6 +68,9 @@ const {
6768

6869
const kCapture = Symbol('kCapture');
6970
const kErrorMonitor = Symbol('events.errorMonitor');
71+
const kMaxEventTargetListeners = Symbol('events.maxEventTargetListeners');
72+
const kMaxEventTargetListenersWarned =
73+
Symbol('events.maxEventTargetListenersWarned');
7074

7175
let DOMException;
7276
const lazyDOMException = hideStackFrames((message, name) => {
@@ -120,6 +124,7 @@ EventEmitter.prototype._maxListeners = undefined;
120124
// By default EventEmitters will print a warning if more than 10 listeners are
121125
// added to it. This is a useful default which helps finding memory leaks.
122126
let defaultMaxListeners = 10;
127+
let isEventTarget;
123128

124129
function checkListener(listener) {
125130
if (typeof listener !== 'function') {
@@ -142,6 +147,48 @@ ObjectDefineProperty(EventEmitter, 'defaultMaxListeners', {
142147
}
143148
});
144149

150+
ObjectDefineProperties(EventEmitter, {
151+
kMaxEventTargetListeners: {
152+
value: kMaxEventTargetListeners,
153+
enumerable: false,
154+
configurable: false,
155+
writable: false,
156+
},
157+
kMaxEventTargetListenersWarned: {
158+
value: kMaxEventTargetListenersWarned,
159+
enumerable: false,
160+
configurable: false,
161+
writable: false,
162+
}
163+
});
164+
165+
EventEmitter.setMaxListeners =
166+
function(n = defaultMaxListeners, ...eventTargets) {
167+
if (typeof n !== 'number' || n < 0 || NumberIsNaN(n))
168+
throw new ERR_OUT_OF_RANGE('n', 'a non-negative number', n);
169+
if (eventTargets.length === 0) {
170+
defaultMaxListeners = n;
171+
} else {
172+
if (isEventTarget === undefined)
173+
isEventTarget = require('internal/event_target').isEventTarget;
174+
175+
// Performance for forEach is now comparable with regular for-loop
176+
eventTargets.forEach((target) => {
177+
if (isEventTarget(target)) {
178+
target[kMaxEventTargetListeners] = n;
179+
target[kMaxEventTargetListenersWarned] = false;
180+
} else if (typeof target.setMaxListeners === 'function') {
181+
target.setMaxListeners(n);
182+
} else {
183+
throw new ERR_INVALID_ARG_TYPE(
184+
'eventTargets',
185+
['EventEmitter', 'EventTarget'],
186+
target);
187+
}
188+
});
189+
}
190+
};
191+
145192
EventEmitter.init = function(opts) {
146193

147194
if (this._events === undefined ||

Diff for: lib/internal/event_target.js

+29-30
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,19 @@ const {
2525
ERR_INVALID_THIS,
2626
}
2727
} = require('internal/errors');
28-
const { validateInteger, validateObject } = require('internal/validators');
28+
const { validateObject } = require('internal/validators');
2929

3030
const { customInspectSymbol } = require('internal/util');
3131
const { inspect } = require('util');
3232

3333
const kIsEventTarget = SymbolFor('nodejs.event_target');
3434

35+
const EventEmitter = require('events');
36+
const {
37+
kMaxEventTargetListeners,
38+
kMaxEventTargetListenersWarned,
39+
} = EventEmitter;
40+
3541
const kEvents = Symbol('kEvents');
3642
const kStop = Symbol('kStop');
3743
const kTarget = Symbol('kTarget');
@@ -42,8 +48,6 @@ const kCreateEvent = Symbol('kCreateEvent');
4248
const kNewListener = Symbol('kNewListener');
4349
const kRemoveListener = Symbol('kRemoveListener');
4450
const kIsNodeStyleListener = Symbol('kIsNodeStyleListener');
45-
const kMaxListeners = Symbol('kMaxListeners');
46-
const kMaxListenersWarned = Symbol('kMaxListenersWarned');
4751
const kTrustEvent = Symbol('kTrustEvent');
4852

4953
// Lazy load perf_hooks to avoid the additional overhead on startup
@@ -220,6 +224,8 @@ class Listener {
220224

221225
function initEventTarget(self) {
222226
self[kEvents] = new SafeMap();
227+
self[kMaxEventTargetListeners] = EventEmitter.defaultMaxListeners;
228+
self[kMaxEventTargetListenersWarned] = false;
223229
}
224230

225231
class EventTarget {
@@ -232,7 +238,24 @@ class EventTarget {
232238
initEventTarget(this);
233239
}
234240

235-
[kNewListener](size, type, listener, once, capture, passive) {}
241+
[kNewListener](size, type, listener, once, capture, passive) {
242+
if (this[kMaxEventTargetListeners] > 0 &&
243+
size > this[kMaxEventTargetListeners] &&
244+
!this[kMaxEventTargetListenersWarned]) {
245+
this[kMaxEventTargetListenersWarned] = true;
246+
// No error code for this since it is a Warning
247+
// eslint-disable-next-line no-restricted-syntax
248+
const w = new Error('Possible EventTarget memory leak detected. ' +
249+
`${size} ${type} listeners ` +
250+
`added to ${inspect(this, { depth: -1 })}. Use ` +
251+
'events.setMaxListeners() to increase limit');
252+
w.name = 'MaxListenersExceededWarning';
253+
w.target = this;
254+
w.type = type;
255+
w.count = size;
256+
process.emitWarning(w);
257+
}
258+
}
236259
[kRemoveListener](size, type, listener, capture) {}
237260

238261
addEventListener(type, listener, options = {}) {
@@ -416,9 +439,6 @@ Object.defineProperty(EventTarget.prototype, SymbolToStringTag, {
416439

417440
function initNodeEventTarget(self) {
418441
initEventTarget(self);
419-
// eslint-disable-next-line no-use-before-define
420-
self[kMaxListeners] = NodeEventTarget.defaultMaxListeners;
421-
self[kMaxListenersWarned] = false;
422442
}
423443

424444
class NodeEventTarget extends EventTarget {
@@ -429,33 +449,12 @@ class NodeEventTarget extends EventTarget {
429449
initNodeEventTarget(this);
430450
}
431451

432-
[kNewListener](size, type, listener, once, capture, passive) {
433-
if (this[kMaxListeners] > 0 &&
434-
size > this[kMaxListeners] &&
435-
!this[kMaxListenersWarned]) {
436-
this[kMaxListenersWarned] = true;
437-
// No error code for this since it is a Warning
438-
// eslint-disable-next-line no-restricted-syntax
439-
const w = new Error('Possible EventTarget memory leak detected. ' +
440-
`${size} ${type} listeners ` +
441-
`added to ${inspect(this, { depth: -1 })}. Use ` +
442-
'setMaxListeners() to increase limit');
443-
w.name = 'MaxListenersExceededWarning';
444-
w.target = this;
445-
w.type = type;
446-
w.count = size;
447-
process.emitWarning(w);
448-
}
449-
}
450-
451452
setMaxListeners(n) {
452-
validateInteger(n, 'n', 0);
453-
this[kMaxListeners] = n;
454-
return this;
453+
EventEmitter.setMaxListeners(n, this);
455454
}
456455

457456
getMaxListeners() {
458-
return this[kMaxListeners];
457+
return this[kMaxEventTargetListeners];
459458
}
460459

461460
eventNames() {

Diff for: test/parallel/test-eventtarget-memoryleakwarning.js

+81
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
// Flags: --no-warnings --expose-internals --experimental-abortcontroller
2+
'use strict';
3+
const common = require('../common');
4+
const {
5+
setMaxListeners,
6+
EventEmitter
7+
} = require('events');
8+
const assert = require('assert');
9+
const { MessageChannel } = require('worker_threads');
10+
const { EventTarget } = require('internal/event_target');
11+
12+
common.expectWarning({
13+
MaxListenersExceededWarning: [
14+
['Possible EventTarget memory leak detected. 3 foo listeners added to ' +
15+
'EventTarget. Use events.setMaxListeners() ' +
16+
'to increase limit'],
17+
['Possible EventTarget memory leak detected. 3 foo listeners added to ' +
18+
'[MessagePort [EventTarget]]. ' +
19+
'Use events.setMaxListeners() to increase ' +
20+
'limit'],
21+
['Possible EventTarget memory leak detected. 3 foo listeners added to ' +
22+
'[MessagePort [EventTarget]]. ' +
23+
'Use events.setMaxListeners() to increase ' +
24+
'limit'],
25+
['Possible EventTarget memory leak detected. 3 foo listeners added to ' +
26+
'[AbortSignal [EventTarget]]. ' +
27+
'Use events.setMaxListeners() to increase ' +
28+
'limit'],
29+
],
30+
ExperimentalWarning: [[
31+
'AbortController is an experimental feature. This feature could change ' +
32+
'at any time'
33+
]]
34+
});
35+
36+
37+
{
38+
const et = new EventTarget();
39+
setMaxListeners(2, et);
40+
et.addEventListener('foo', () => {});
41+
et.addEventListener('foo', () => {});
42+
et.addEventListener('foo', () => {});
43+
}
44+
45+
{
46+
// No warning emitted because prior limit was only for that
47+
// one EventTarget.
48+
const et = new EventTarget();
49+
et.addEventListener('foo', () => {});
50+
et.addEventListener('foo', () => {});
51+
et.addEventListener('foo', () => {});
52+
}
53+
54+
{
55+
const mc = new MessageChannel();
56+
setMaxListeners(2, mc.port1);
57+
mc.port1.addEventListener('foo', () => {});
58+
mc.port1.addEventListener('foo', () => {});
59+
mc.port1.addEventListener('foo', () => {});
60+
}
61+
62+
{
63+
// Set the default for newly created EventTargets
64+
setMaxListeners(2);
65+
const mc = new MessageChannel();
66+
mc.port1.addEventListener('foo', () => {});
67+
mc.port1.addEventListener('foo', () => {});
68+
mc.port1.addEventListener('foo', () => {});
69+
70+
const ac = new AbortController();
71+
ac.signal.addEventListener('foo', () => {});
72+
ac.signal.addEventListener('foo', () => {});
73+
ac.signal.addEventListener('foo', () => {});
74+
}
75+
76+
{
77+
// It works for EventEmitters also
78+
const ee = new EventEmitter();
79+
setMaxListeners(2, ee);
80+
assert.strictEqual(ee.getMaxListeners(), 2);
81+
}

0 commit comments

Comments
 (0)