Fix regression in signal unlisten and document current EE behaviour

doc/api/events.markdown

@@ -143,14 +143,18 @@ Return the number of listeners for a given event.
 * `event` {String} The event name
 * `listener` {Function} The event handler function
 
-This event is emitted any time a listener is added. When this event is triggered,
-the listener may not yet have been added to the array of listeners for the `event`.
+This event is emitted *before* a listener is added. When this event is
+triggered, the listener has not been added to the array of listeners for the
+`event`. Any listeners added to the event `name` in the newListener event
+callback will be added *before* the listener that is in the process of being
+added.
 
 
 ### Event: 'removeListener'
 
 * `event` {String} The event name
 * `listener` {Function} The event handler function
 
-This event is emitted any time someone removes a listener.  When this event is triggered,
-the listener may not yet have been removed from the array of listeners for the `event`.
+This event is emitted *after* a listener is removed.  When this event is
+triggered, the listener has been removed from the array of listeners for the
+`event`.

src/node.js

@@ -630,16 +630,14 @@
     // Load events module in order to access prototype elements on process like
     // process.addListener.
     var signalWraps = {};
-    var addListener = process.addListener;
-    var removeListener = process.removeListener;
 
     function isSignal(event) {
       return event.slice(0, 3) === 'SIG' &&
              startup.lazyConstants().hasOwnProperty(event);
     }
 
-    // Wrap addListener for the special signal types
-    process.on = process.addListener = function(type, listener) {
+    // Detect presence of a listener for the special signal types
+    process.on('newListener', function(type, listener) {
       if (isSignal(type) &&
           !signalWraps.hasOwnProperty(type)) {
         var Signal = process.binding('signal_wrap').Signal;
@@ -659,23 +657,15 @@
 
         signalWraps[type] = wrap;
       }
+    });
 
-      return addListener.apply(this, arguments);
-    };
-
-    process.removeListener = function(type, listener) {
-      var ret = removeListener.apply(this, arguments);
-      if (isSignal(type)) {
-        assert(signalWraps.hasOwnProperty(type));
-
-        if (NativeModule.require('events').listenerCount(this, type) === 0) {
-          signalWraps[type].close();
-          delete signalWraps[type];
-        }
+    process.on('removeListener', function(type, listener) {
+      if (signalWraps.hasOwnProperty(type) &&
+          NativeModule.require('events').listenerCount(this, type) === 0) {
+        signalWraps[type].close();
+        delete signalWraps[type];
       }
-
-      return ret;
-    };
+    });
   };
 
 

test/parallel/test-event-emitter-add-listeners.js

@@ -12,6 +12,8 @@ var times_hello_emited = 0;
 assert.equal(e.addListener, e.on);
 
 e.on('newListener', function(event, listener) {
+  if (event === 'newListener')
+    return; // Don't track our adding of newListener listeners.
   console.log('newListener: ' + event);
   events_new_listener_emited.push(event);
   listeners_new_listener_emited.push(listener);
@@ -23,6 +25,11 @@ function hello(a, b) {
   assert.equal('a', a);
   assert.equal('b', b);
 }
+e.once('newListener', function(name, listener) {
+  assert.equal(name, 'hello');
+  assert.equal(listener, hello);
+  assert.deepEqual(this.listeners('hello'), []);
+});
 e.on('hello', hello);
 
 var foo = function() {};
@@ -44,4 +51,17 @@ process.on('exit', function() {
   assert.equal(1, times_hello_emited);
 });
 
-
+var listen1 = function listen1() {};
+var listen2 = function listen2() {};
+var e1 = new events.EventEmitter;
+e1.once('newListener', function() {
+  assert.deepEqual(e1.listeners('hello'), []);
+  e1.once('newListener', function() {
+    assert.deepEqual(e1.listeners('hello'), []);
+  });
+  e1.on('hello', listen2);
+});
+e1.on('hello', listen1);
+// The order of listeners on an event is not always the order in which the
+// listeners were added.
+assert.deepEqual(e1.listeners('hello'), [listen2, listen1]);

test/parallel/test-event-emitter-remove-all-listeners.js

@@ -57,3 +57,15 @@ e3.on('removeListener', listener);
 // there exists a removeListener listener, but there exists
 // no listeners for the provided event type
 assert.doesNotThrow(e3.removeAllListeners.bind(e3, 'foo'));
+
+var e4 = new events.EventEmitter();
+var expectLength = 2;
+e4.on('removeListener', function(name, listener) {
+  assert.equal(expectLength--, this.listeners('baz').length);
+});
+e4.on('baz', function(){});
+e4.on('baz', function(){});
+e4.on('baz', function(){});
+assert.equal(e4.listeners('baz').length, expectLength+1);
+e4.removeAllListeners('baz');
+assert.equal(e4.listeners('baz').length, 0);

test/parallel/test-event-emitter-remove-listeners.js

@@ -45,12 +45,20 @@ assert.deepEqual([listener1], e2.listeners('hello'));
 var e3 = new events.EventEmitter();
 e3.on('hello', listener1);
 e3.on('hello', listener2);
-e3.on('removeListener', common.mustCall(function(name, cb) {
+e3.once('removeListener', common.mustCall(function(name, cb) {
   assert.equal(name, 'hello');
   assert.equal(cb, listener1);
+  assert.deepEqual([listener2], e3.listeners('hello'));
 }));
 e3.removeListener('hello', listener1);
 assert.deepEqual([listener2], e3.listeners('hello'));
+e3.once('removeListener', common.mustCall(function(name, cb) {
+  assert.equal(name, 'hello');
+  assert.equal(cb, listener2);
+  assert.deepEqual([], e3.listeners('hello'));
+}));
+e3.removeListener('hello', listener2);
+assert.deepEqual([], e3.listeners('hello'));
 
 var e4 = new events.EventEmitter();
 e4.on('removeListener', common.mustCall(function(name, cb) {
@@ -61,3 +69,21 @@ e4.on('removeListener', common.mustCall(function(name, cb) {
 e4.on('quux', remove1);
 e4.on('quux', remove2);
 e4.removeListener('quux', remove1);
+
+var e5 = new events.EventEmitter();
+e5.on('hello', listener1);
+e5.on('hello', listener2);
+e5.once('removeListener', common.mustCall(function(name, cb) {
+  assert.equal(name, 'hello');
+  assert.equal(cb, listener1);
+  assert.deepEqual([listener2], e5.listeners('hello'));
+  e5.once('removeListener', common.mustCall(function(name, cb) {
+    assert.equal(name, 'hello');
+    assert.equal(cb, listener2);
+    assert.deepEqual([], e5.listeners('hello'));
+  }));
+  e5.removeListener('hello', listener2);
+  assert.deepEqual([], e5.listeners('hello'));
+}));
+e5.removeListener('hello', listener1);
+assert.deepEqual([], e5.listeners('hello'));

test/parallel/test-process-remove-all-signal-listeners.js

@@ -0,0 +1,40 @@
+if (process.platform === 'win32') {
+  // Win32 doesn't have signals, just a kindof emulation, insufficient
+  // for this test to apply.
+  return;
+}
+
+var assert = require('assert');
+var spawn = require('child_process').spawn;
+var ok;
+
+if (process.argv[2] !== '--do-test') {
+  // We are the master, fork a child so we can verify it exits with correct
+  // status.
+  process.env.DOTEST = 'y';
+  var child = spawn(process.execPath, [__filename, '--do-test']);
+
+  child.once('exit', function(code, signal) {
+    assert.equal(signal, 'SIGINT');
+    ok = true;
+  });
+
+  process.on('exit', function() {
+    assert(ok);
+  });
+
+  return;
+}
+
+process.on('SIGINT', function() {
+  // Remove all handlers and kill ourselves. We should terminate by SIGINT
+  // now that we have no handlers.
+  process.removeAllListeners('SIGINT');
+  process.kill(process.pid, 'SIGINT');
+});
+
+// Signal handlers aren't sufficient to keep node alive, so resume stdin
+process.stdin.resume();
+
+// Demonstrate that signals are being handled
+process.kill(process.pid, 'SIGINT');