Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 088167d

Browse filesBrowse files
gurgundayaduh95
authored andcommitted
events: avoid cloning listeners array on every emit
PR-URL: #62261 Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
1 parent d490b17 commit 088167d
Copy full SHA for 088167d

2 files changed

+63-18Lines changed: 63 additions & 18 deletions

File tree

Expand file treeCollapse file tree
Open diff view settings
Filter options
Expand file treeCollapse file tree
Open diff view settings
Collapse file

‎lib/events.js‎

Copy file name to clipboardExpand all lines: lib/events.js
+45-18Lines 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) {
Collapse file

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

Copy file name to clipboardExpand all lines: test/parallel/test-event-emitter-modify-in-emit.js
+18Lines 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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.