Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 9ce9b01

Browse filesBrowse files
jasnellcodebytere
jasnell
authored and
codebytere
committed
events: add max listener warning for EventTarget
Signed-off-by: James M Snell <jasnell@gmail.com> PR-URL: #36001 Fixes: #35990 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
1 parent eb9295b commit 9ce9b01
Copy full SHA for 9ce9b01

File tree

Expand file treeCollapse file tree

4 files changed

+178
-30
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

4 files changed

+178
-30
lines changed
Open diff view settings
Collapse file

‎doc/api/events.md‎

Copy file name to clipboardExpand all lines: doc/api/events.md
+23Lines changed: 23 additions & 0 deletions
  • Display the source diff
  • Display the rich diff
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
Collapse file

‎lib/events.js‎

Copy file name to clipboardExpand all lines: lib/events.js
+47Lines changed: 47 additions & 0 deletions
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 ||
Collapse file

‎lib/internal/event_target.js‎

Copy file name to clipboardExpand all lines: lib/internal/event_target.js
+29-30Lines changed: 29 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,19 @@ const {
2626
ERR_INVALID_THIS,
2727
}
2828
} = require('internal/errors');
29-
const { validateInteger, validateObject } = require('internal/validators');
29+
const { validateObject } = require('internal/validators');
3030

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

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

36+
const EventEmitter = require('events');
37+
const {
38+
kMaxEventTargetListeners,
39+
kMaxEventTargetListenersWarned,
40+
} = EventEmitter;
41+
3642
const kEvents = Symbol('kEvents');
3743
const kStop = Symbol('kStop');
3844
const kTarget = Symbol('kTarget');
@@ -43,8 +49,6 @@ const kCreateEvent = Symbol('kCreateEvent');
4349
const kNewListener = Symbol('kNewListener');
4450
const kRemoveListener = Symbol('kRemoveListener');
4551
const kIsNodeStyleListener = Symbol('kIsNodeStyleListener');
46-
const kMaxListeners = Symbol('kMaxListeners');
47-
const kMaxListenersWarned = Symbol('kMaxListenersWarned');
4852
const kTrustEvent = Symbol('kTrustEvent');
4953

5054
// Lazy load perf_hooks to avoid the additional overhead on startup
@@ -221,6 +225,8 @@ class Listener {
221225

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

226232
class EventTarget {
@@ -233,7 +239,24 @@ class EventTarget {
233239
initEventTarget(this);
234240
}
235241

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

239262
addEventListener(type, listener, options = {}) {
@@ -417,9 +440,6 @@ ObjectDefineProperty(EventTarget.prototype, SymbolToStringTag, {
417440

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

425445
class NodeEventTarget extends EventTarget {
@@ -430,33 +450,12 @@ class NodeEventTarget extends EventTarget {
430450
initNodeEventTarget(this);
431451
}
432452

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

458457
getMaxListeners() {
459-
return this[kMaxListeners];
458+
return this[kMaxEventTargetListeners];
460459
}
461460

462461
eventNames() {
Collapse file

‎test/parallel/test-eventtarget-memoryleakwarning.js‎

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

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.