events: remove reaches into _events internals

Refactor lib & src code to eliminate all deep reaches into the internal _events dictionary object, instead use available APIs and add an extra method to EventEmitter: rawListeners. PR-URL: #17440 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Timothy Gu <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: James M Snell <[email protected]>
nodejs · Jan 8, 2018 · 7008719 · 7008719
1 parent 97eaaf9
commit 7008719
Show file tree

Hide file tree

Showing 7 changed files with 69 additions and 22 deletions.
diff --git a/doc/api/events.md b/doc/api/events.md
@@ -574,6 +574,39 @@ to indicate an unlimited number of listeners.
 
 Returns a reference to the `EventEmitter`, so that calls can be chained.
 
+### emitter.rawListeners(eventName)
+<!-- YAML
+added: REPLACEME
+-->
+- `eventName` {any}
+
+Returns a copy of the array of listeners for the event named `eventName`,
+including any wrappers (such as those created by `.once`).
+
+```js
+const emitter = new EventEmitter();
+emitter.once('log', () => console.log('log once'));
+
+// Returns a new Array with a function `onceWrapper` which has a property
+// `listener` which contains the original listener bound above
+const listeners = emitter.rawListeners('log');
+const logFnWrapper = listeners[0];
+
+// logs "log once" to the console and does not unbind the `once` event
+logFnWrapper.listener();
+
+// logs "log once" to the console and removes the listener
+logFnWrapper();
+
+emitter.on('log', () => console.log('log persistently'));
+// will return a new Array with a single function bound by `on` above
+const newListeners = emitter.rawListeners('log');
+
+// logs "log persistently" twice
+newListeners[0]();
+emitter.emit('log');
+```
+
 [`--trace-warnings`]: cli.html#cli_trace_warnings
 [`EventEmitter.defaultMaxListeners`]: #events_eventemitter_defaultmaxlisteners
 [`domain`]: domain.html

diff --git a/lib/events.js b/lib/events.js
@@ -36,6 +36,7 @@ EventEmitter.usingDomains = false;
 
 EventEmitter.prototype.domain = undefined;
 EventEmitter.prototype._events = undefined;
+EventEmitter.prototype._eventsCount = 0;
 EventEmitter.prototype._maxListeners = undefined;
 
 // By default EventEmitters will print a warning if more than 10 listeners are
@@ -393,8 +394,8 @@ EventEmitter.prototype.removeAllListeners =
       return this;
     };
 
-EventEmitter.prototype.listeners = function listeners(type) {
-  const events = this._events;
+function _listeners(target, type, unwrap) {
+  const events = target._events;
 
   if (events === undefined)
     return [];
@@ -404,9 +405,18 @@ EventEmitter.prototype.listeners = function listeners(type) {
     return [];
 
   if (typeof evlistener === 'function')
-    return [evlistener.listener || evlistener];
+    return unwrap ? [evlistener.listener || evlistener] : [evlistener];
+
+  return unwrap ?
+    unwrapListeners(evlistener) : arrayClone(evlistener, evlistener.length);
+}
+
+EventEmitter.prototype.listeners = function listeners(type) {
+  return _listeners(this, type, true);
+};
 
-  return unwrapListeners(evlistener);
+EventEmitter.prototype.rawListeners = function rawListeners(type) {
+  return _listeners(this, type, false);
 };
 
 EventEmitter.listenerCount = function(emitter, type) {

diff --git a/lib/internal/bootstrap_node.js b/lib/internal/bootstrap_node.js
@@ -13,7 +13,6 @@
 
   function startup() {
     const EventEmitter = NativeModule.require('events');
-    process._eventsCount = 0;
 
     const origProcProto = Object.getPrototypeOf(process);
     Object.setPrototypeOf(origProcProto, EventEmitter.prototype);

diff --git a/lib/vm.js b/lib/vm.js
@@ -57,15 +57,15 @@ const realRunInThisContext = Script.prototype.runInThisContext;
 const realRunInContext = Script.prototype.runInContext;
 
 Script.prototype.runInThisContext = function(options) {
-  if (options && options.breakOnSigint && process._events.SIGINT) {
+  if (options && options.breakOnSigint && process.listenerCount('SIGINT') > 0) {
     return sigintHandlersWrap(realRunInThisContext, this, [options]);
   } else {
     return realRunInThisContext.call(this, options);
   }
 };
 
 Script.prototype.runInContext = function(contextifiedSandbox, options) {
-  if (options && options.breakOnSigint && process._events.SIGINT) {
+  if (options && options.breakOnSigint && process.listenerCount('SIGINT') > 0) {
     return sigintHandlersWrap(realRunInContext, this,
                               [contextifiedSandbox, options]);
   } else {
@@ -96,14 +96,7 @@ function createScript(code, options) {
 // Remove all SIGINT listeners and re-attach them after the wrapped function
 // has executed, so that caught SIGINT are handled by the listeners again.
 function sigintHandlersWrap(fn, thisArg, argsArray) {
-  // Using the internal list here to make sure `.once()` wrappers are used,
-  // not the original ones.
-  let sigintListeners = process._events.SIGINT;
-
-  if (Array.isArray(sigintListeners))
-    sigintListeners = sigintListeners.slice();
-  else
-    sigintListeners = [sigintListeners];
+  const sigintListeners = process.rawListeners('SIGINT');
 
   process.removeAllListeners('SIGINT');
 

diff --git a/src/env.h b/src/env.h
@@ -145,7 +145,6 @@ class ModuleWrap;
   V(env_pairs_string, "envPairs")                                             \
   V(errno_string, "errno")                                                    \
   V(error_string, "error")                                                    \
-  V(events_string, "_events")                                                 \
   V(exiting_string, "_exiting")                                               \
   V(exit_code_string, "exitCode")                                             \
   V(exit_string, "exit")                                                      \

diff --git a/src/node.cc b/src/node.cc
@@ -3684,12 +3684,6 @@ void SetupProcessObject(Environment* env,
   env->SetMethod(process, "_setupNextTick", SetupNextTick);
   env->SetMethod(process, "_setupPromises", SetupPromises);
   env->SetMethod(process, "_setupDomainUse", SetupDomainUse);
-
-  // pre-set _events object for faster emit checks
-  Local<Object> events_obj = Object::New(env->isolate());
-  CHECK(events_obj->SetPrototype(env->context(),
-                                 Null(env->isolate())).FromJust());
-  process->Set(env->events_string(), events_obj);
 }
 
 

diff --git a/test/parallel/test-event-emitter-listeners.js b/test/parallel/test-event-emitter-listeners.js
@@ -82,3 +82,22 @@ function listener2() {}
   const s = new TestStream();
   assert.deepStrictEqual(s.listeners('foo'), []);
 }
+
+{
+  const ee = new events.EventEmitter();
+  ee.on('foo', listener);
+  const wrappedListener = ee.rawListeners('foo');
+  assert.strictEqual(wrappedListener.length, 1);
+  assert.strictEqual(wrappedListener[0], listener);
+  assert.notStrictEqual(wrappedListener, ee.rawListeners('foo'));
+  ee.once('foo', listener);
+  const wrappedListeners = ee.rawListeners('foo');
+  assert.strictEqual(wrappedListeners.length, 2);
+  assert.strictEqual(wrappedListeners[0], listener);
+  assert.notStrictEqual(wrappedListeners[1], listener);
+  assert.strictEqual(wrappedListeners[1].listener, listener);
+  assert.notStrictEqual(wrappedListeners, ee.rawListeners('foo'));
+  ee.emit('foo');
+  assert.strictEqual(wrappedListeners.length, 2);
+  assert.strictEqual(wrappedListeners[1].listener, listener);
+}