From e5cbe73a82cd57d115f9bb29f127c686835c22b3 Mon Sep 17 00:00:00 2001
From: Carson McDonald <carson@ioncannon.net>
Date: Thu, 18 Mar 2010 11:50:15 -0400
Subject: [PATCH] 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.
---
 src/node.js                                   |  2 +-
 src/node_events.cc                            |  3 +--
 .../test-event-emitter-modify-in-emit.js      | 25 ++++++++++++++++---
 3 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/src/node.js b/src/node.js
index 127e1aa79a9..0b529665598 100644
--- 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 d88229fe8ad..4efe5c10e71 100644
--- 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 1dd94ba7127..95de54755a0 100644
--- 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)