Skip to content

Instantly share code, notes, and snippets.

Show Gist options
  • Save koichik/782778 to your computer and use it in GitHub Desktop.
Save koichik/782778 to your computer and use it in GitHub Desktop.
fix emitter.on()/addListener()'s check for listener leak
From 9b36eb9474fb56391f63d5d29b3c4fbebb665541 Mon Sep 17 00:00:00 2001
From: koichik <koichik@improvement.jp>
Date: Mon, 17 Jan 2011 19:30:43 +0900
Subject: [PATCH] fix emitter.on()/addListener()'s check for listener leak
Because the check is performed before a listener is added to an array,
the warning occurred with the 12th listener not the 11th listener.
example:
var emitter = new events.EventEmitter();
for (var i = 0; i < 10; i++) {
emitter.on('data', function() {});
}
emitter.on('data', function() {}); // 11th, no warning
emitter.on('data', function() {}); // 12th, warning occurred
---
lib/events.js | 5 +--
.../test-event-emitter-check-listener-leaks.js | 29 ++++++++++++++++++++
2 files changed, 31 insertions(+), 3 deletions(-)
create mode 100644 test/simple/test-event-emitter-check-listener-leaks.js
diff --git a/lib/events.js b/lib/events.js
index 6505606..c5856c7 100644
--- a/lib/events.js
+++ b/lib/events.js
@@ -82,6 +82,8 @@ EventEmitter.prototype.addListener = function(type, listener) {
// Optimize the case of one listener. Don't need the extra array object.
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) {
@@ -101,9 +103,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];
diff --git a/test/simple/test-event-emitter-check-listener-leaks.js b/test/simple/test-event-emitter-check-listener-leaks.js
new file mode 100644
index 0000000..66e257c
--- /dev/null
+++ b/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'));
+
--
1.7.1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment