Skip to content

Commit d335209

Browse files
committed
events: avoid cloning listeners array on every emit
1 parent 8ccbe8e commit d335209

File tree

2 files changed

+63
-18
lines changed

2 files changed

+63
-18
lines changed

lib/events.js

Lines changed: 45 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@ const { addAbortListener } = require('internal/events/abort_listener');
8787
const kCapture = Symbol('kCapture');
8888
const kErrorMonitor = Symbol('events.errorMonitor');
8989
const kShapeMode = Symbol('shapeMode');
90+
const kEmitting = Symbol('events.emitting');
9091
const kMaxEventTargetListeners = Symbol('events.maxEventTargetListeners');
9192
const kMaxEventTargetListenersWarned =
9293
Symbol('events.maxEventTargetListenersWarned');
@@ -514,19 +515,22 @@ EventEmitter.prototype.emit = function emit(type, ...args) {
514515
addCatch(this, result, type, args);
515516
}
516517
} else {
517-
const len = handler.length;
518-
const listeners = arrayClone(handler);
519-
for (let i = 0; i < len; ++i) {
520-
const result = ReflectApply(listeners[i], this, args);
521-
522-
// We check if result is undefined first because that
523-
// is the most common case so we do not pay any perf
524-
// penalty.
525-
// This code is duplicated because extracting it away
526-
// would make it non-inlineable.
527-
if (result !== undefined && result !== null) {
528-
addCatch(this, result, type, args);
518+
handler[kEmitting]++;
519+
try {
520+
for (let i = 0; i < handler.length; ++i) {
521+
const result = ReflectApply(handler[i], this, args);
522+
523+
// We check if result is undefined first because that
524+
// is the most common case so we do not pay any perf
525+
// penalty.
526+
// This code is duplicated because extracting it away
527+
// would make it non-inlineable.
528+
if (result !== undefined && result !== null) {
529+
addCatch(this, result, type, args);
530+
}
529531
}
532+
} finally {
533+
handler[kEmitting]--;
530534
}
531535
}
532536

@@ -565,13 +569,17 @@ function _addListener(target, type, listener, prepend) {
565569
} else {
566570
if (typeof existing === 'function') {
567571
// Adding the second element, need to change to array.
568-
existing = events[type] =
569-
prepend ? [listener, existing] : [existing, listener];
572+
existing = prepend ? [listener, existing] : [existing, listener];
573+
existing[kEmitting] = 0;
574+
events[type] = existing;
570575
// If we've already got an array, just append.
571-
} else if (prepend) {
572-
existing.unshift(listener);
573576
} else {
574-
existing.push(listener);
577+
existing = ensureMutableListenerArray(events, type, existing);
578+
if (prepend) {
579+
existing.unshift(listener);
580+
} else {
581+
existing.push(listener);
582+
}
575583
}
576584

577585
// Check for listener leak
@@ -674,7 +682,7 @@ EventEmitter.prototype.removeListener =
674682
if (events === undefined)
675683
return this;
676684

677-
const list = events[type];
685+
let list = events[type];
678686
if (list === undefined)
679687
return this;
680688

@@ -692,6 +700,7 @@ EventEmitter.prototype.removeListener =
692700
if (events.removeListener !== undefined)
693701
this.emit('removeListener', type, list.listener || listener);
694702
} else if (typeof list !== 'function') {
703+
list = ensureMutableListenerArray(events, type, list);
695704
let position = -1;
696705

697706
for (let i = list.length - 1; i >= 0; i--) {
@@ -875,6 +884,24 @@ function arrayClone(arr) {
875884
return ArrayPrototypeSlice(arr);
876885
}
877886

887+
function cloneEventListenerArray(arr) {
888+
const copy = arrayClone(arr);
889+
copy[kEmitting] = 0;
890+
if (arr.warned) {
891+
copy.warned = true;
892+
}
893+
return copy;
894+
}
895+
896+
function ensureMutableListenerArray(events, type, handler) {
897+
if (handler[kEmitting] > 0) {
898+
const copy = cloneEventListenerArray(handler);
899+
events[type] = copy;
900+
return copy;
901+
}
902+
return handler;
903+
}
904+
878905
function unwrapListeners(arr) {
879906
const ret = arrayClone(arr);
880907
for (let i = 0; i < ret.length; ++i) {

test/parallel/test-event-emitter-modify-in-emit.js

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,3 +78,21 @@ assert.strictEqual(e.listeners('foo').length, 2);
7878
e.emit('foo');
7979
assert.deepStrictEqual(['callback2', 'callback3'], callbacks_called);
8080
assert.strictEqual(e.listeners('foo').length, 0);
81+
82+
// Verify that removing all callbacks while in emit allows the current emit to
83+
// propagate to all listeners.
84+
callbacks_called = [];
85+
86+
function callback4() {
87+
callbacks_called.push('callback4');
88+
e.removeAllListeners('foo');
89+
}
90+
91+
e.on('foo', callback4);
92+
e.on('foo', callback2);
93+
e.on('foo', callback3);
94+
assert.strictEqual(e.listeners('foo').length, 3);
95+
e.emit('foo');
96+
assert.deepStrictEqual(['callback4', 'callback2', 'callback3'],
97+
callbacks_called);
98+
assert.strictEqual(e.listeners('foo').length, 0);

0 commit comments

Comments
 (0)