Skip to content

Commit

Permalink
events: add max listener warning for EventTarget
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
jasnell authored and nodejs-github-bot committed Nov 16, 2020
1 parent cd31340 commit dc79f3f
Show file tree
Hide file tree
Showing 4 changed files with 178 additions and 30 deletions.
23 changes: 23 additions & 0 deletions doc/api/events.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
47 changes: 47 additions & 0 deletions lib/events.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ const {
NumberIsNaN,
ObjectCreate,
ObjectDefineProperty,
ObjectDefineProperties,
ObjectGetPrototypeOf,
ObjectSetPrototypeOf,
Promise,
Expand Down Expand Up @@ -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) => {
Expand Down Expand Up @@ -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') {
Expand All @@ -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 ||
Expand Down
59 changes: 29 additions & 30 deletions lib/internal/event_target.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand All @@ -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
Expand Down Expand Up @@ -221,6 +225,8 @@ class Listener {

function initEventTarget(self) {
self[kEvents] = new SafeMap();
self[kMaxEventTargetListeners] = EventEmitter.defaultMaxListeners;
self[kMaxEventTargetListenersWarned] = false;
}

class EventTarget {
Expand All @@ -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 = {}) {
Expand Down Expand Up @@ -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 {
Expand All @@ -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() {
Expand Down
79 changes: 79 additions & 0 deletions test/parallel/test-eventtarget-memoryleakwarning.js
Original file line number Diff line number Diff line change
@@ -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);
}

0 comments on commit dc79f3f

Please sign in to comment.