From 9534a22411540eb9c3fcdc4ce717df095b261cc1 Mon Sep 17 00:00:00 2001 From: Anatoli Papirovski Date: Sat, 25 Nov 2017 13:26:28 -0500 Subject: [PATCH] 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. --- doc/api/events.md | 9 ++++++++ lib/events.js | 23 +++++++++++++++---- lib/internal/bootstrap_node.js | 1 - lib/vm.js | 13 +++-------- src/env.h | 1 - src/node.cc | 6 ----- test/parallel/test-event-emitter-listeners.js | 11 +++++++++ 7 files changed, 42 insertions(+), 22 deletions(-) diff --git a/doc/api/events.md b/doc/api/events.md index 443137f1705e26..bffd31e059598b 100644 --- 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) + +- `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 index 1607071a1c7ae6..ebed3f85d24de6 100644 --- 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 index c75578f7692068..3f43c23e8d7c0b 100644 --- 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 index b34f10dbee5ff8..3e62a924dc4684 100644 --- a/lib/vm.js +++ b/lib/vm.js @@ -43,7 +43,7 @@ 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); @@ -51,7 +51,7 @@ Script.prototype.runInThisContext = function(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 index 083c09a8134efb..dcb9f57646182e 100644 --- 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 index c35fd612b48149..c06335f8bf7cdc 100644 --- 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 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 index 0736e3103e9e9e..97dcdd80f54322 100644 --- 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); +}