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: make memory leak warning more programatically accessible #8298

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
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
6 changes: 6 additions & 0 deletions doc/api/events.md
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
Member

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
Member Author

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
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
Member

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
Member Author

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
Member

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 notifications@github.com
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
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() {});