Better EventEmitter modify-in-emit

Changed ReallyEmit so that it clones the Array of listeners before
processing the emit. Added better tests to make sure that modifying
listeners inside event handlers doesn't cause later listeners to be skipped
or added.

diff --git a/src/node.js b/src/node.js
index 127e1aa79a90b25731d90c98d444688ee607365a..0b5296655981cecd9d25bfcc07cf3861eb3d8003 100644 (file)
--- a/src/node.js
+++ b/src/node.js
@@ -182,7 +182,7 @@ process.mixin = function() {
 var eventsModule = createInternalModule('events', function (exports) {
   exports.EventEmitter = process.EventEmitter;
 
-  // process.EventEmitter is defined in src/events.cc
+  // process.EventEmitter is defined in src/node_events.cc
   // process.EventEmitter.prototype.emit() is also defined there.
   process.EventEmitter.prototype.addListener = function (type, listener) {
     if (!(listener instanceof Function)) {

diff --git a/src/node_events.cc b/src/node_events.cc
index d88229fe8adb0b1686020433d8c605061cc4dbca..4efe5c10e71de00f07394a711fa58372d7a08b17 100644 (file)
--- a/src/node_events.cc
+++ b/src/node_events.cc
@@ -50,7 +50,6 @@ static bool ReallyEmit(Handle<Object> self,
   Local<Object> events = events_v->ToObject();
 
   Local<Value> listeners_v = events->Get(event);
-  Local<Function> listener;
 
   TryCatch try_catch;
 
@@ -66,7 +65,7 @@ static bool ReallyEmit(Handle<Object> self,
     }
 
   } else if (listeners_v->IsArray()) {
-    Local<Array> listeners = Local<Array>::Cast(listeners_v);
+    Local<Array> listeners = Local<Array>::Cast(listeners_v->ToObject()->Clone());
 
     for (uint32_t i = 0; i < listeners->Length(); i++) {
       Local<Value> listener_v = listeners->Get(i);

diff --git a/test/simple/test-event-emitter-modify-in-emit.js b/test/simple/test-event-emitter-modify-in-emit.js
index 1dd94ba7127ddf3b190b679080cbfb7ba9497778..95de54755a066507b88a088a7b0237f977d0852f 100644 (file)
--- a/test/simple/test-event-emitter-modify-in-emit.js
+++ b/test/simple/test-event-emitter-modify-in-emit.js
@@ -8,6 +8,7 @@ var e = new events.EventEmitter();
 function callback1() {
     callbacks_called.push("callback1");
     e.addListener("foo", callback2);
+    e.addListener("foo", callback3);
     e.removeListener("foo", callback1);
 }
 
@@ -16,23 +17,39 @@ function callback2() {
     e.removeListener("foo", callback2);
 }
 
+function callback3() {
+    callbacks_called.push("callback3");
+    e.removeListener("foo", callback3);
+}
+
 e.addListener("foo", callback1);
 assert.equal(1, e.listeners("foo").length);
 
 e.emit("foo");
-assert.equal(1, e.listeners("foo").length);
+assert.equal(2, e.listeners("foo").length);
 assert.deepEqual(["callback1"], callbacks_called);
 
 e.emit("foo");
 assert.equal(0, e.listeners("foo").length);
-assert.deepEqual(["callback1", "callback2"], callbacks_called);
+assert.deepEqual(["callback1", "callback2", "callback3"], callbacks_called);
 
 e.emit("foo");
 assert.equal(0, e.listeners("foo").length);
-assert.deepEqual(["callback1", "callback2"], callbacks_called);
+assert.deepEqual(["callback1", "callback2", "callback3"], callbacks_called);
 
 e.addListener("foo", callback1);
 e.addListener("foo", callback2);
 assert.equal(2, e.listeners("foo").length)
 e.removeAllListeners("foo")
-assert.equal(0, e.listeners("foo").length)
\ No newline at end of file
+assert.equal(0, e.listeners("foo").length)
+
+// Verify that removing callbacks while in emit allows emits to propagate to 
+// all listeners
+callbacks_called = [ ];
+
+e.addListener("foo", callback2);
+e.addListener("foo", callback3);
+assert.equal(2, e.listeners("foo").length)
+e.emit("foo");
+assert.deepEqual(["callback2", "callback3"], callbacks_called);
+assert.equal(0, e.listeners("foo").length)