Permalink
Browse files

events: fix checking max listeners with `1`

Fixes #2490.
  • Loading branch information...
1 parent 9a79bb6 commit 22d7fe12067be32ac456045f8f2014526e4aabb4 @tricknotes tricknotes committed with koichik Jan 8, 2012
Showing with 27 additions and 18 deletions.
  1. +20 −18 lib/events.js
  2. +7 −0 test/simple/test-event-emitter-check-listener-leaks.js
View
@@ -115,27 +115,29 @@ EventEmitter.prototype.addListener = function(type, listener) {
// If we've already got an array, just append.
this._events[type].push(listener);
- // Check for listener leak
- if (!this._events[type].warned) {
- var m;
- if (this._maxListeners !== undefined) {
- m = this._maxListeners;
- } else {
- m = defaultMaxListeners;
- }
-
- if (m && m > 0 && this._events[type].length > m) {
- this._events[type].warned = true;
- console.error('(node) warning: possible EventEmitter memory ' +
- 'leak detected. %d listeners added. ' +
- 'Use emitter.setMaxListeners() to increase limit.',
- this._events[type].length);
- console.trace();
- }
- }
} else {
// Adding the second element, need to change to array.
this._events[type] = [this._events[type], listener];
+
+ }
+
+ // Check for listener leak
+ if (isArray(this._events[type]) && !this._events[type].warned) {
+ var m;
+ if (this._maxListeners !== undefined) {
+ m = this._maxListeners;
+ } else {
+ m = defaultMaxListeners;
+ }
+
+ if (m && m > 0 && this._events[type].length > m) {
+ this._events[type].warned = true;
+ console.error('(node) warning: possible EventEmitter memory ' +
+ 'leak detected. %d listeners added. ' +
+ 'Use emitter.setMaxListeners() to increase limit.',
+ this._events[type].length);
+ console.trace();
+ }
}
return this;
@@ -41,6 +41,13 @@ assert.ok(!e._events['specific'].hasOwnProperty('warned'));
e.on('specific', function() {});
assert.ok(e._events['specific'].warned);
+// only one
+e.setMaxListeners(1);
+e.on('only one', function() {});
+assert.ok(!e._events['only one'].hasOwnProperty('warned'));
+e.on('only one', function() {});
+assert.ok(e._events['only one'].hasOwnProperty('warned'));
+
// unlimited
e.setMaxListeners(0);
for (var i = 0; i < 1000; i++) {

3 comments on commit 22d7fe1

Owner

bnoordhuis commented on 22d7fe1 Jan 9, 2012

This commit broke test/simple/test-event-emitter-check-listener-leaks.js

EDIT: Forgot to add 'in debug mode'.

$ python tools/test.py --mode=release,debug simple/test-event-emitter-check-listener-leaks
=== debug test-event-emitter-check-listener-leaks ===                      
Path: simple/test-event-emitter-check-listener-leaks
(node) warning: possible EventEmitter memory leak detected. 11 listeners added. Use emitter.setMaxListeners() to increase limit.
Trace: 
    at EventEmitter.<anonymous> (events.js:133:17)
    at Object.<anonymous> (/home/bnoordhuis/src/nodejs/v0.6/test/simple/test-event-emitter-check-listener-leaks.js:32:3)
    at Module._compile (module.js:432:26)
    at Object..js (module.js:450:10)
    at Module.load (module.js:351:31)
    at Function._load (module.js:310:12)
    at Array.0 (module.js:470:10)
    at EventEmitter._tickCallback (node.js:192:40)
(node) warning: possible EventEmitter memory leak detected. 6 listeners added. Use emitter.setMaxListeners() to increase limit.
Trace: 
    at EventEmitter.<anonymous> (events.js:133:17)
    at Object.<anonymous> (/home/bnoordhuis/src/nodejs/v0.6/test/simple/test-event-emitter-check-listener-leaks.js:41:3)
    at Module._compile (module.js:432:26)
    at Object..js (module.js:450:10)
    at Module.load (module.js:351:31)
    at Function._load (module.js:310:12)
    at Array.0 (module.js:470:10)
    at EventEmitter._tickCallback (node.js:192:40)

node.js:201
        throw e; // process.nextTick error, or 'error' event on first tick
              ^
AssertionError: false == true
    at Object.<anonymous> (/home/bnoordhuis/src/nodejs/v0.6/test/simple/test-event-emitter-check-listener-leaks.js:49:8)
    at Module._compile (module.js:432:26)
    at Object..js (module.js:450:10)
    at Module.load (module.js:351:31)
    at Function._load (module.js:310:12)
    at Array.0 (module.js:470:10)
    at EventEmitter._tickCallback (node.js:192:40)
Command: out/Debug/node /home/bnoordhuis/src/nodejs/v0.6/test/simple/test-event-emitter-check-listener-leaks.js
[00:00|% 100|+   1|-   1]: Done                                         

Hmm... I cannot reproduce it in my environment (Ubuntu 11.04 on VirtualBox on Windows):

$ ./node -v
v0.6.8-pre
$ ./node_g -v
v0.6.8-pre
$ python tools/test.py --mode=release,debug simple/test-event-emitter-check-listener-leaks
[00:00|% 100|+   2|-   0]: Done                                            

In your result, the following line (2 listeners added) is not printed:

(node) warning: possible EventEmitter memory leak detected. 2 listeners added. Use emitter.setMaxListeners() to increase limit.

It is printed out before assertion (by line 48):

$ ./node_g test/simple/test-event-emitter-check-listener-leaks
(node) warning: possible EventEmitter memory leak detected. 11 listeners added. Use emitter.setMaxListeners() to increase limit.
Trace: 
    at EventEmitter.<anonymous> (events.js:139:15)
    at Object.<anonymous> (/home/koichik/git/joyent/node-v0.6/test/simple/test-event-emitter-check-listener-leaks.js:32:3)
    at Module._compile (module.js:432:26)
    at Object..js (module.js:450:10)
    at Module.load (module.js:351:31)
    at Function._load (module.js:310:12)
    at Array.0 (module.js:470:10)
    at EventEmitter._tickCallback (node.js:192:40)
(node) warning: possible EventEmitter memory leak detected. 6 listeners added. Use emitter.setMaxListeners() to increase limit.
Trace: 
    at EventEmitter.<anonymous> (events.js:139:15)
    at Object.<anonymous> (/home/koichik/git/joyent/node-v0.6/test/simple/test-event-emitter-check-listener-leaks.js:41:3)
    at Module._compile (module.js:432:26)
    at Object..js (module.js:450:10)
    at Module.load (module.js:351:31)
    at Function._load (module.js:310:12)
    at Array.0 (module.js:470:10)
    at EventEmitter._tickCallback (node.js:192:40)
(node) warning: possible EventEmitter memory leak detected. 2 listeners added. Use emitter.setMaxListeners() to increase limit.
Trace: 
    at EventEmitter.<anonymous> (events.js:139:15)
    at Object.<anonymous> (/home/koichik/git/joyent/node-v0.6/test/simple/test-event-emitter-check-listener-leaks.js:48:3)
    at Module._compile (module.js:432:26)
    at Object..js (module.js:450:10)
    at Module.load (module.js:351:31)
    at Function._load (module.js:310:12)
    at Array.0 (module.js:470:10)
    at EventEmitter._tickCallback (node.js:192:40)
Owner

bnoordhuis replied Jan 10, 2012

Sorry, false alarm. It turns out that waf wasn't rebuilding out/Debug/node. After removing said file by hand and running make test-debug again, the test passes.

Please sign in to comment.