Skip to content

events: make memory leak warning more programatically accessible #8298

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.

Sign up for GitHub

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

Closed
addaleax wants to merge 2 commits into from
Closed

events: make memory leak warning more programatically accessible #8298

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
802e6be
events: make memory leak warning more accessible
addaleax Aug 27, 2016
473c38d
[squash] add note about `--trace-warnings` to docs
addaleax Aug 27, 2016
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
6 changes: 6 additions & 0 deletions doc/api/events.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,11 @@ emitter.once('event', () => {
});
```

The emitted warning can be inspected with [`process.on('warning')`][] and will
have the additional `emitter`, `type` and `count` properties, referring to
the event emitter instance, the event’s name and the number of attached
listeners, respectively.

Copy link
Copy Markdown
Member

@jasnell jasnell Aug 27, 2016

Choose a reason for hiding this comment

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

May be worth also reiterating that --trace-warnings will print out the stack trace detail.

Copy link
Copy Markdown
Member Author

@addaleax addaleax Aug 27, 2016

Choose a reason for hiding this comment

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

@jasnell good idea, done!

### emitter.addListener(eventName, listener)
<!-- YAML
added: v0.1.26
Expand Down Expand Up @@ -562,4 +567,5 @@ Returns a reference to the `EventEmitter`, so that calls can be chained.
[`emitter.listenerCount()`]: #events_emitter_listenercount_eventname
[`domain`]: domain.html
[`process` object's `uncaughtException` event]: process.html#process_event_uncaughtexception
[`process.on('warning')`]: process.html#process_event_warning
[stream]: stream.html
7 changes: 6 additions & 1 deletion lib/events.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,9 +256,14 @@ function _addListener(target, type, listener, prepend) {
m = $getMaxListeners(target);
if (m && m > 0 && existing.length > m) {
existing.warned = true;
process.emitWarning('Possible EventEmitter memory leak detected. ' +
const w = new Error('Possible EventEmitter memory leak detected. ' +
`${existing.length} ${type} listeners added. ` +
'Use emitter.setMaxListeners() to increase limit');
w.name = 'Warning';
Copy link
Copy Markdown
Member

@jasnell jasnell Aug 27, 2016

Choose a reason for hiding this comment

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

Assigning a specific warning name would make differentiating a bit easier as well.

Copy link
Copy Markdown
Member Author

@addaleax addaleax Aug 27, 2016

Choose a reason for hiding this comment

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

+1 … but that would be a semver-major change?

Copy link
Copy Markdown
Member

@jasnell jasnell Aug 27, 2016

Choose a reason for hiding this comment

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

Yeah, good point. Nevermind then

On Saturday, August 27, 2016, Anna Henningsen [email protected]
wrote:

In lib/events.js
#8298 (comment):

                         `${existing.length} ${type} listeners added. ` +
                         'Use emitter.setMaxListeners() to increase limit');

  •    w.name = 'Warning';

+1 … but that would be a semver-major change?

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/nodejs/node/pull/8298/files/802e6be4c7e2bfd08224fb6769fbe7b2d8ed0239#r76515949,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAa2eW6tZ7CDeSmG54J9RAysM9fS9TBZks5qkDgHgaJpZM4Jupwl
.

w.emitter = target;
w.type = type;
w.count = existing.length;
process.emitWarning(w);
}
}
}
Expand Down
21 changes: 21 additions & 0 deletions test/parallel/test-event-emitter-max-listeners-warning.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// Flags: --no-warnings
// The flag suppresses stderr output but the warning event will still emit
'use strict';

const common = require('../common');
const events = require('events');
const assert = require('assert');

const e = new events.EventEmitter();
e.setMaxListeners(1);

process.on('warning', common.mustCall((warning) => {
assert.ok(warning instanceof Error);
assert.strictEqual(warning.name, 'Warning');
assert.strictEqual(warning.emitter, e);
assert.strictEqual(warning.count, 2);
assert.strictEqual(warning.type, 'event-type');
}));

e.on('event-type', function() {});
e.on('event-type', function() {});