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: wrappedListeners.
nodejs · Nov 29, 2017 · 9534a22 · 9534a22
1 parent 9cf8c68
commit 9534a22
Show file tree

Hide file tree

Showing 7 changed files with 42 additions and 22 deletions.
diff --git a/doc/api/events.md b/doc/api/events.md
@@ -574,6 +574,15 @@ to indicate an unlimited number of listeners.
 
 Returns a reference to the `EventEmitter`, so that calls can be chained.
 
+### emitter.wrappedListeners(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`.
+
 [`--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
@@ -400,8 +400,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 [];
@@ -411,9 +411,17 @@ 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);
+}
+
+EventEmitter.prototype.listeners = function listeners(type) {
+  return _listeners(this, type, true);
+};
 
-  return unwrapListeners(evlistener);
+EventEmitter.prototype.wrappedListeners = function wrappedListeners(type) {
+  return _listeners(this, type, false);
 };
 
 EventEmitter.listenerCount = function(emitter, type) {
@@ -471,6 +479,13 @@ EventEmitter.prototype.eventNames = function eventNames() {
   return actualEventNames;
 };
 
+function arrayClone(arr) {
+  const copy = new Array(arr.length);
+  for (var i = 0; i < arr.length; ++i)
+    copy[i] = arr[i];
+  return copy;
+}
+
 function arrayCloneWithElement(arr, element, prepend) {
   const len = arr.length;
   const copy = new Array(len + 1);

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
@@ -43,15 +43,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')) {
     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')) {
     return sigintHandlersWrap(realRunInContext, this,
                               [contextifiedSandbox, options]);
   } else {
@@ -82,14 +82,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.wrappedListeners('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
@@ -3292,12 +3292,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,14 @@ function listener2() {}
   const s = new TestStream();
   assert.deepStrictEqual(s.listeners('foo'), []);
 }
+
+{
+  const ee = new events.EventEmitter();
+  ee.on('foo', listener);
+  ee.once('foo', listener);
+  const wrappedListeners = ee.wrappedListeners('foo');
+  assert.strictEqual(wrappedListeners.length, 2);
+  assert.strictEqual(wrappedListeners[0], listener);
+  assert.notStrictEqual(wrappedListeners[1], listener);
+  assert.strictEqual(wrappedListeners[1].listener, listener);
+}