-
Notifications
You must be signed in to change notification settings - Fork 30k
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. +1 … but that would be a semver-major change? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
|
||
w.emitter = target; | ||
w.type = type; | ||
w.count = existing.length; | ||
process.emitWarning(w); | ||
} | ||
} | ||
} | ||
|
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() {}); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jasnell good idea, done!