events: make memory leak warning more programatically accessible#8298
Uh oh!
There was an error while loading. Please reload this page.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
This makes the famous `EventEmitter memory leak` warnings occurring when the listener count for a given event exceeds a specified number more programatically accessible, by giving them properties referring to the event emitter instance and the event itself. This can be useful for debugging the origins of such a warning when the stack itself doesn’t reveal enough information about the event emitter instance itself, e.g. when manual inspection of the already-registered listeners is expected to be useful.
addaleax added events 
Contributor
cjihrig commented Aug 27, 2016
LGTM |
| constw=newError('Possible EventEmitter memory leak detected. '+ | ||
| `${existing.length}${type} listeners added. `+ | ||
| 'Use emitter.setMaxListeners() to increase limit'); | ||
| w.name='Warning'; |
Member
There was a problem hiding this comment.
Assigning a specific warning name would make differentiating a bit easier as well.
MemberAuthor
There was a problem hiding this comment.
+1 … but that would be a semver-major change?
Member
There was a problem hiding this comment.
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
.
Member
jasnell commented Aug 27, 2016
Couple of suggestions but otherwise LGTM |
Contributor
Fishrock123 commented Aug 27, 2016
LGTM if CI is green |
MemberAuthor
addaleax commented Aug 30, 2016
Landed in 932c824 |
addaleax added a commit that referenced this pull request Aug 30, 2016
This makes the famous `EventEmitter memory leak` warnings occurring when the listener count for a given event exceeds a specified number more programatically accessible, by giving them properties referring to the event emitter instance and the event itself. This can be useful for debugging the origins of such a warning when the stack itself doesn’t reveal enough information about the event emitter instance itself, e.g. when manual inspection of the already-registered listeners is expected to be useful. PR-URL: #8298 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Jeremiah Senkpiel <[email protected]>
addaleax added a commit to addaleax/node that referenced this pull request Aug 30, 2016
Switch from a generic `Warning` to the more specific `MaxListenersExceededWarning`. Ref: nodejs#8298
4 tasks
addaleax added a commit that referenced this pull request Sep 4, 2016
Switch from a generic `Warning` to the more specific `MaxListenersExceededWarning`. Ref: #8298 PR-URL: #8341 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Evan Lucas <[email protected]>
Closed
Fishrock123 pushed a commit to Fishrock123/node that referenced this pull request Sep 8, 2016
This makes the famous `EventEmitter memory leak` warnings occurring when the listener count for a given event exceeds a specified number more programatically accessible, by giving them properties referring to the event emitter instance and the event itself. This can be useful for debugging the origins of such a warning when the stack itself doesn’t reveal enough information about the event emitter instance itself, e.g. when manual inspection of the already-registered listeners is expected to be useful. PR-URL: nodejs#8298 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Jeremiah Senkpiel <[email protected]>
Fishrock123 pushed a commit that referenced this pull request Sep 9, 2016
This makes the famous `EventEmitter memory leak` warnings occurring when the listener count for a given event exceeds a specified number more programatically accessible, by giving them properties referring to the event emitter instance and the event itself. This can be useful for debugging the origins of such a warning when the stack itself doesn’t reveal enough information about the event emitter instance itself, e.g. when manual inspection of the already-registered listeners is expected to be useful. PR-URL: #8298 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Jeremiah Senkpiel <[email protected]>
Merged
Fishrock123 added a commit that referenced this pull request Sep 14, 2016
Notable changes: * crypto: Added `crypto.timingSafeEqual()`. (not-an-aardvark) #8304 * events: Made the "max event listeners" memory leak warning more accessible. (Anna Henningsen) #8298 * promises: Unhandled rejections now emit a process warning after the first tick. (Benjamin Gruenbaum) #8223 * repl: Added auto alignment for `.editor` mode. (Prince J Wesley) #8241 * util: Some functionality has been added to `util.inspect()`: - Returning `this` from a custom inspect function now works. (Anna Henningsen) #8174 - Added support for Symbol-based custom inspection methods. (Anna Henningsen) #8174 Refs: #8428 Refs: #8457 PR-URL: #8466
Fishrock123 added a commit to Fishrock123/node that referenced this pull request Sep 15, 2016
Notable changes: * crypto: Added `crypto.timingSafeEqual()`. (not-an-aardvark) nodejs#8304 * events: Made the "max event listeners" memory leak warning more accessible. (Anna Henningsen) nodejs#8298 * promises: Unhandled rejections now emit a process warning after the first tick. (Benjamin Gruenbaum) nodejs#8223 * repl: Added auto alignment for `.editor` mode. (Prince J Wesley) nodejs#8241 * util: Some functionality has been added to `util.inspect()`: - Returning `this` from a custom inspect function now works. (Anna Henningsen) nodejs#8174 - Added support for Symbol-based custom inspection methods. (Anna Henningsen) nodejs#8174 Refs: nodejs#8428 Refs: nodejs#8457 PR-URL: nodejs#8466
imyller added a commit to imyller/meta-nodejs that referenced this pull request Sep 15, 2016
Notable changes: * crypto: Added `crypto.timingSafeEqual()`. (not-an-aardvark) nodejs/node#8304 * events: Made the "max event listeners" memory leak warning more accessible. (Anna Henningsen) nodejs/node#8298 * promises: Unhandled rejections now emit a process warning after the first tick. (Benjamin Gruenbaum) nodejs/node#8223 * repl: Added auto alignment for `.editor` mode. (Prince J Wesley) nodejs/node#8241 * util: Some functionality has been added to `util.inspect()`: - Returning `this` from a custom inspect function now works. (Anna Henningsen) nodejs/node#8174 - Added support for Symbol-based custom inspection methods. (Anna Henningsen) nodejs/node#8174 Signed-off-by: Ilkka Myller <[email protected]>
imyller added a commit to imyller/meta-nodejs that referenced this pull request Sep 15, 2016
Notable changes: * crypto: Added `crypto.timingSafeEqual()`. (not-an-aardvark) nodejs/node#8304 * events: Made the "max event listeners" memory leak warning more accessible. (Anna Henningsen) nodejs/node#8298 * promises: Unhandled rejections now emit a process warning after the first tick. (Benjamin Gruenbaum) nodejs/node#8223 * repl: Added auto alignment for `.editor` mode. (Prince J Wesley) nodejs/node#8241 * util: Some functionality has been added to `util.inspect()`: - Returning `this` from a custom inspect function now works. (Anna Henningsen) nodejs/node#8174 - Added support for Symbol-based custom inspection methods. (Anna Henningsen) nodejs/node#8174 Signed-off-by: Ilkka Myller <[email protected]>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Checklist
make -j4 test(UNIX), orvcbuild test nosign(Windows) passesAffected core subsystem(s)
events
Description of change
This makes the famous
EventEmitter memory leakwarnings occurring when the listener count for a given event exceeds a specified number more programatically accessible, by giving them properties referring to the event emitter instance and the event itself.This can be useful for debugging the origins of such a warning when the stack itself doesn’t reveal enough information about the event emitter instance itself, e.g. when manual inspection of the
already-registered listeners is expected to be useful.
(It seems CI on this has to wait until Jenkins is running smoothly again…)CI: https://ci.nodejs.org/job/node-test-commit/4794/