From 7008719fb6d9ec880ecf5e124f1aa618436e5430 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: rawListeners. PR-URL: https://github.com/nodejs/node/pull/17440 Reviewed-By: Anna Henningsen Reviewed-By: Timothy Gu Reviewed-By: Ruben Bridgewater Reviewed-By: Matteo Collina Reviewed-By: James M Snell --- doc/api/events.md | 33 +++++++++++++++++++ lib/events.js | 18 +++++++--- 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 | 19 +++++++++++ 7 files changed, 69 insertions(+), 22 deletions(-) diff --git a/doc/api/events.md b/doc/api/events.md index 8eaa8cae102b75..c935b7de0f78b1 100644 --- 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) + +- `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 index 2a83ab7dc64c7b..d8c542a01264ef 100644 --- 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 index 96e49f82629c3d..72101cb6408cc9 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 669fa23396735f..3c59e32805c297 100644 --- a/lib/vm.js +++ b/lib/vm.js @@ -57,7 +57,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') > 0) { return sigintHandlersWrap(realRunInThisContext, this, [options]); } else { return realRunInThisContext.call(this, options); @@ -65,7 +65,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') > 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 index 1dc012594c140d..b9928a85ae6228 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 5ab186b6a3b70b..40800a5e7047df 100644 --- 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 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..9d2892d2e891ae 100644 --- 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); +}