Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Fix event listener leak check timing #1041

Closed
wants to merge 1 commit into from

2 participants

@koichik
Owner

Because check for listener leak is performed before the listener was added to an array, the warning occurred the 12th listener not the 11th listener.

Reproduce:

$ cat leak.js
var events = require('events');
var emitter = new events.EventEmitter();
for (var i = 0; i < 10; ++i) {
  emitter.on('data', function() {});
}
console.log('added 10 listeners');
console.log('adding 11th listener');
emitter.on('data', function() {});
console.log('adding 12th listener');
emitter.on('data', function() {});

$ node leak.js
added 10 listeners
adding 11th listener
adding 12th listener
(node) warning: possible EventEmitter memory leak detected. 11 listeners added. Use emitter.setMaxListeners() to increase limit.
Trace: 
    at EventEmitter.<anonymous> (events.js:123:17)
    at Object.<anonymous> (/tmp/leak.js:10:9)
    at Module._compile (module.js:404:26)
    at Object..js (module.js:410:10)
    at Module.load (module.js:336:31)
    at Function._load (module.js:297:12)
    at Array.0 (module.js:423:10)
    at EventEmitter._tickCallback (node.js:126:26)
@ry ry closed this pull request from a commit
@koichik koichik Fix event listener leak check timing
Fixes #1041.
80c2fe9
@ry ry closed this in 80c2fe9
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on May 12, 2011
  1. @koichik
This page is out of date. Refresh to see the latest.
View
6 lib/events.js
@@ -105,6 +105,9 @@ EventEmitter.prototype.addListener = function(type, listener) {
this._events[type] = listener;
} else if (isArray(this._events[type])) {
+ // 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;
@@ -123,9 +126,6 @@ EventEmitter.prototype.addListener = function(type, listener) {
console.trace();
}
}
-
- // If we've already got an array, just append.
- this._events[type].push(listener);
} else {
// Adding the second element, need to change to array.
this._events[type] = [this._events[type], listener];
View
29 test/simple/test-event-emitter-check-listener-leaks.js
@@ -0,0 +1,29 @@
+var assert = require('assert');
+var events = require('events');
+
+var e = new events.EventEmitter();
+
+// default
+for (var i = 0; i < 10; i++) {
+ e.on('default', function() {});
+}
+assert.ok(!e._events['default'].hasOwnProperty('warned'));
+e.on('default', function() {});
+assert.ok(e._events['default'].warned);
+
+// specific
+e.setMaxListeners(5);
+for (var i = 0; i < 5; i++) {
+ e.on('specific', function() {});
+}
+assert.ok(!e._events['specific'].hasOwnProperty('warned'));
+e.on('specific', function() {});
+assert.ok(e._events['specific'].warned);
+
+// unlimited
+e.setMaxListeners(0);
+for (var i = 0; i < 1000; i++) {
+ e.on('unlimited', function() {});
+}
+assert.ok(!e._events['unlimited'].hasOwnProperty('warned'));
+
Something went wrong with that request. Please try again.