Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

events: add max listener warning for EventTarget #36001

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
jasnell marked this conversation as resolved.
Show resolved Hide resolved
`EventTarget` event.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
`EventTarget` event.
`EventTarget` or `EventEmitter`.

* `...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.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
max for all newly created {EventTarget} and {EventEmitter} objects.
maximum 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
jasnell marked this conversation as resolved.
Show resolved Hide resolved
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');
jasnell marked this conversation as resolved.
Show resolved Hide resolved
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);
}