Skip to content

Commit

Permalink
events: remove reaches into _events internals
Browse files Browse the repository at this point in the history
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 <anna@addaleax.net>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
apapirovski authored and MylesBorins committed Jan 8, 2018
1 parent 97eaaf9 commit 7008719
Show file tree
Hide file tree
Showing 7 changed files with 69 additions and 22 deletions.
33 changes: 33 additions & 0 deletions doc/api/events.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
18 changes: 14 additions & 4 deletions lib/events.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 [];
Expand All @@ -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) {
Expand Down
1 change: 0 additions & 1 deletion lib/internal/bootstrap_node.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@

function startup() {
const EventEmitter = NativeModule.require('events');
process._eventsCount = 0;

const origProcProto = Object.getPrototypeOf(process);
Object.setPrototypeOf(origProcProto, EventEmitter.prototype);
Expand Down
13 changes: 3 additions & 10 deletions lib/vm.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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');

Expand Down
1 change: 0 additions & 1 deletion src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -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") \
Expand Down
6 changes: 0 additions & 6 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}


Expand Down
19 changes: 19 additions & 0 deletions test/parallel/test-event-emitter-listeners.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

0 comments on commit 7008719

Please sign in to comment.