From aaeeba5b13fcb3a208e318426082cb207278785b Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 25 Jul 2026 22:18:18 +0000 Subject: [PATCH 1/4] node:events: mutate listener arrays in place; copy only when emit() has iterated them Adding or removing a listener allocated a fresh N-element array per call so emit() could iterate without a defensive clone. That made N adds, or a tail-first drain of N listeners, O(N^2): 64k listeners took ~5.7s to drain and ~3.8s to add. Listener arrays are now mutated in place (push / unshift / spliceOne, as in Node). emit() marks the array it walks with a sticky symbol; a mutator that sees the mark installs a fresh copy instead, so the running loop still sees its original snapshot. The copy is unmarked, so the next mutation is in-place again. The mark is one property read (and a write only on the first emit of a given array), so the emit hot path is unchanged within noise. Fixes #3770, fixes #3734. --- src/js/internal/streams/legacy.ts | 3 +- src/js/node/events.ts | 77 ++++++++++++++++------- test/js/node/events/event-emitter.test.ts | 63 +++++++++++++++++++ 3 files changed, 120 insertions(+), 23 deletions(-) diff --git a/src/js/internal/streams/legacy.ts b/src/js/internal/streams/legacy.ts index 2ed60fff0b9c..ebb91f9449f4 100644 --- a/src/js/internal/streams/legacy.ts +++ b/src/js/internal/streams/legacy.ts @@ -112,7 +112,8 @@ function prependListener(emitter, event, fn) { let events, existing; if (!(events = emitter._events) || !(existing = events[event])) emitter.on(event, fn); // A fresh array, not unshift(): node:events iterates stored arrays without - // cloning, so a stored `_events` array must never be mutated in place. + // cloning and marks them so its own mutators copy-on-write, but this path + // bypasses that mark check, so only a new array is guaranteed safe here. else if (ArrayIsArray(existing)) events[event] = [fn, ...existing]; else events[event] = [fn, existing]; } diff --git a/src/js/node/events.ts b/src/js/node/events.ts index cca5fa632b62..db16de2d0b46 100644 --- a/src/js/node/events.ts +++ b/src/js/node/events.ts @@ -43,6 +43,15 @@ const ArrayPrototypeUnshift = Array.prototype.unshift; const ReflectOwnKeys = Reflect.ownKeys; const kCapture = Symbol("kCapture"); +// Set on a listener array the first time emit() iterates it, and never +// cleared. Mutators (on / prepend / removeListener) install a copy instead of +// editing in place when they see it, so any emit() loop still running on that +// array keeps a stable snapshot; the fresh copy has no mark, so subsequent +// mutations are in-place again. Sticky (not a counter) keeps emit()'s hot +// path to a single read-and-maybe-write instead of two writes, and sidesteps +// the "nested emit clears the outer mark" problem a clear-after-loop boolean +// would have. +const kIterated = Symbol("kIterated"); // Set when `_events` was preallocated (streams do this): removeListener then // writes `undefined` instead of `delete`, keeping one shared JSC Structure // so the (StructureID, name)-keyed megamorphic cache stays hot. @@ -142,13 +151,14 @@ function emitError(emitter, args) { } // A listener list is a bare function for a single listener, else an array -// (like node). Arrays are never mutated in place - mutators install a copy - -// so a stored list can be iterated with no defensive clone. +// (like node). Arrays are mutated in place by on/off; the kIterated mark makes +// any concurrent mutation install a copy instead, so this snapshot stays stable. function applyHandlers(handlers, emitter, args) { if (typeof handlers === "function") { handlers.$apply(emitter, args); return; } + if (!handlers[kIterated]) handlers[kIterated] = true; for (let i = 0, { length } = handlers; i < length; i++) { handlers[i].$apply(emitter, args); } @@ -208,9 +218,9 @@ const emitWithoutRejectionCapture = function emit(type, ...args) { } return true; } - // No defensive clone: stored arrays are never mutated in place (mutators - // install a copy), so this list stays stable for the whole loop even if a - // listener adds/removes listeners. + // No defensive clone: the kIterated mark makes any listener that adds or + // removes on this event install a fresh array, so `handler` stays stable. + if (!handler[kIterated]) handler[kIterated] = true; for (let i = 0, { length } = handler; i < length; i++) { const listener = handler[i]; switch (args.length) { @@ -268,9 +278,9 @@ const emitWithRejectionCapture = function emit(type, ...args) { } return true; } - // No defensive clone: stored arrays are never mutated in place (mutators - // install a copy), so this list stays stable for the whole loop even if a - // listener adds/removes listeners. + // No defensive clone: the kIterated mark makes any listener that adds or + // removes on this event install a fresh array, so `handler` stays stable. + if (!handler[kIterated]) handler[kIterated] = true; for (let i = 0, { length } = handler; i < length; i++) { const listener = handler[i]; let result; @@ -322,8 +332,15 @@ function _addListener(target, type, fn, prepend) { var handlers; if (typeof existing === "function") { handlers = events[type] = prepend ? [fn, existing] : [existing, fn]; - } else { + } else if (existing[kIterated]) { + // emit() has iterated (and may still be iterating) this exact array; + // install a copy so its loop stays stable. The copy has no mark, so the + // next mutation is in-place again. handlers = events[type] = copyWithInserted(existing, fn, prepend); + } else { + if (prepend) ArrayPrototypeUnshift.$call(existing, fn); + else $arrayPush(existing, fn); + handlers = existing; } var m = _getMaxListeners(target); if (m > 0 && handlers.length > m && !handlers.warned) { @@ -343,9 +360,9 @@ EventEmitterPrototype.prependListener = function prependListener(type, fn) { return this; }; -// Copy-on-write: emit iterates stored arrays with no clone, so new listeners -// land in a fresh array; `warned` carries over so the leak warning fires once. -// An inline loop beats concat/slice here ~10x (host-call boundary). +// Copy path for when `list` carries kIterated (emit() has walked it): a fresh +// array leaves the iterated one untouched. `warned` carries over so the leak +// warning fires once. An inline loop beats concat/slice ~10x (host-call cost). function copyWithInserted(list, fn, prepend) { const n = list.length; const copy = $newArrayWithSize(n + 1); @@ -447,21 +464,35 @@ EventEmitterPrototype.removeListener = function removeListener(type, listener) { } if (position < 0) return this; - // Copy-remove (arrays are never mutated in place), and store a lone - // survivor bare like node does, so `_events[type]` shape matches theirs. - const n = list.length; - const copy = $newArrayWithSize(n - 1); - for (let i = 0, j = 0; i < n; i++) { - if (i !== position) copy[j++] = list[i]; + if (list[kIterated]) { + // emit() has iterated (and may still be iterating) this exact array; + // install a copy so its loop stays stable. A lone survivor is stored bare + // so `_events[type]` matches node. + const n = list.length; + const copy = $newArrayWithSize(n - 1); + for (let i = 0, j = 0; i < n; i++) { + if (i !== position) copy[j++] = list[i]; + } + if (list.warned) copy.warned = true; + events[type] = copy.length === 1 ? copy[0] : copy; + } else if (list.length === 2) { + events[type] = list[1 - position]; + } else { + spliceOne(list, position); } - if (list.warned) copy.warned = true; - events[type] = copy.length === 1 ? copy[0] : copy; if (events.removeListener !== undefined) this.emit("removeListener", type, listener.listener ?? listener); return this; }; +// In-place single-element remove; node's internal/util spliceOne. Removing the +// last element is O(1), which makes a tail-first drain of N listeners linear. +function spliceOne(list, index) { + for (; index + 1 < list.length; index++) list[index] = list[index + 1]; + list.pop(); +} + EventEmitterPrototype.off = EventEmitterPrototype.removeListener; EventEmitterPrototype.removeAllListeners = function removeAllListeners(type) { @@ -498,8 +529,10 @@ EventEmitterPrototype.removeAllListeners = function removeAllListeners(type) { if (typeof listeners === "function") { this.removeListener(type, listeners); } else if (listeners !== undefined) { - // LIFO order. `listeners` is our own snapshot; each removeListener call - // installs a fresh array (or bare fn / nothing), so it stays intact here. + // LIFO order, same as node's loop. Unmarked arrays shrink from the tail as + // we walk indices high-to-low; a marked array is left intact (the first + // removeListener COWs it away), so either way `listeners[i]` is the right + // function at every step. for (let i = listeners.length - 1; i >= 0; i--) this.removeListener(type, listeners[i]); } return this; diff --git a/test/js/node/events/event-emitter.test.ts b/test/js/node/events/event-emitter.test.ts index f95a0464cdc1..1839bbe68f09 100644 --- a/test/js/node/events/event-emitter.test.ts +++ b/test/js/node/events/event-emitter.test.ts @@ -264,6 +264,69 @@ describe("EventEmitter", () => { expect(EventEmitter.prototype.removeListener).toBe(EventEmitter.prototype.off); }); + // Node mutates the stored listener array in place (push / spliceOne), so + // capturing `_events[type]` and then adding or removing keeps the same array + // reference. A copy-on-write scheme allocates a fresh array per call instead, + // turning N adds (or a tail-first drain of N listeners) into O(N^2) work. + // https://github.com/oven-sh/bun/issues/3770 + // https://github.com/oven-sh/bun/issues/3734 + test("on/removeListener mutate the stored listener array in place", () => { + const ee = new EventEmitter() as any; + ee.setMaxListeners(0); + const f1 = () => {}; + const f2 = () => {}; + const f3 = () => {}; + const f4 = () => {}; + ee.on("x", f1); + ee.on("x", f2); + const list = ee._events.x; + expect(Array.isArray(list)).toBe(true); + + ee.on("x", f3); + expect(ee._events.x).toBe(list); + expect(list).toEqual([f1, f2, f3]); + + ee.prependListener("x", f4); + expect(ee._events.x).toBe(list); + expect(list).toEqual([f4, f1, f2, f3]); + + ee.removeListener("x", f3); + expect(ee._events.x).toBe(list); + expect(list).toEqual([f4, f1, f2]); + + ee.removeListener("x", f4); + expect(ee._events.x).toBe(list); + expect(list).toEqual([f1, f2]); + }); + + // The in-place mutation above is only safe because emit() marks the array it + // is iterating: a listener that adds or removes on the same event gets a + // fresh copy, so the running loop still sees its original snapshot. This + // also has to hold across a nested emit() of the same event, which must not + // clear the outer emit()'s mark when it finishes. + test("nested emit + removeListener inside a listener preserves the outer emit snapshot", () => { + const ee = new EventEmitter(); + const calls: string[] = []; + let depth = 0; + const a = () => calls.push("a" + depth); + const b = () => { + calls.push("b" + depth); + if (depth === 0) { + depth = 1; + ee.emit("x"); + depth = 0; + ee.removeListener("x", c); + } + }; + const c = () => calls.push("c" + depth); + ee.on("x", a); + ee.on("x", b); + ee.on("x", c); + ee.emit("x"); + expect(calls).toEqual(["a0", "b0", "a1", "b1", "c1", "c0"]); + expect(ee.listeners("x")).toEqual([a, b]); + }); + test("prependListener", () => { const myEmitter = new EventEmitter(); const order: number[] = []; From fde54c26d856a4c06106fa3fe013124b4ee1b498 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 26 Jul 2026 02:14:53 +0000 Subject: [PATCH 2/4] ci: retrigger From ac05716c7cc937dda51ba25a567f2bb46134642e Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 26 Jul 2026 02:30:28 +0000 Subject: [PATCH 3/4] tighten kIterated comments: one invariant doc at the symbol, drop the per-site repeats --- src/js/internal/streams/legacy.ts | 5 ++-- src/js/node/events.ts | 40 +++++++++---------------------- 2 files changed, 13 insertions(+), 32 deletions(-) diff --git a/src/js/internal/streams/legacy.ts b/src/js/internal/streams/legacy.ts index ebb91f9449f4..0ea4221d1c34 100644 --- a/src/js/internal/streams/legacy.ts +++ b/src/js/internal/streams/legacy.ts @@ -111,9 +111,8 @@ function prependListener(emitter, event, fn) { // the prependListener() method. The goal is to eventually remove this hack. let events, existing; if (!(events = emitter._events) || !(existing = events[event])) emitter.on(event, fn); - // A fresh array, not unshift(): node:events iterates stored arrays without - // cloning and marks them so its own mutators copy-on-write, but this path - // bypasses that mark check, so only a new array is guaranteed safe here. + // A fresh array, not unshift(): emit() in node:events iterates without + // cloning, and this path can't see the kIterated mark that guards in-place edits. else if (ArrayIsArray(existing)) events[event] = [fn, ...existing]; else events[event] = [fn, existing]; } diff --git a/src/js/node/events.ts b/src/js/node/events.ts index db16de2d0b46..ad48e6dfee9a 100644 --- a/src/js/node/events.ts +++ b/src/js/node/events.ts @@ -43,14 +43,9 @@ const ArrayPrototypeUnshift = Array.prototype.unshift; const ReflectOwnKeys = Reflect.ownKeys; const kCapture = Symbol("kCapture"); -// Set on a listener array the first time emit() iterates it, and never -// cleared. Mutators (on / prepend / removeListener) install a copy instead of -// editing in place when they see it, so any emit() loop still running on that -// array keeps a stable snapshot; the fresh copy has no mark, so subsequent -// mutations are in-place again. Sticky (not a counter) keeps emit()'s hot -// path to a single read-and-maybe-write instead of two writes, and sidesteps -// the "nested emit clears the outer mark" problem a clear-after-loop boolean -// would have. +// Sticky mark emit() puts on a listener array before iterating it. on/prepend/ +// removeListener mutate in place unless they see this mark, in which case they +// install a fresh (unmarked) copy so the in-flight emit() keeps a stable view. const kIterated = Symbol("kIterated"); // Set when `_events` was preallocated (streams do this): removeListener then // writes `undefined` instead of `delete`, keeping one shared JSC Structure @@ -150,9 +145,8 @@ function emitError(emitter, args) { throw err; // Unhandled 'error' event } -// A listener list is a bare function for a single listener, else an array -// (like node). Arrays are mutated in place by on/off; the kIterated mark makes -// any concurrent mutation install a copy instead, so this snapshot stays stable. +// A listener list is a bare function for a single listener, else an array (as +// in node). kIterated on the array keeps it stable through the loop; see above. function applyHandlers(handlers, emitter, args) { if (typeof handlers === "function") { handlers.$apply(emitter, args); @@ -218,8 +212,7 @@ const emitWithoutRejectionCapture = function emit(type, ...args) { } return true; } - // No defensive clone: the kIterated mark makes any listener that adds or - // removes on this event install a fresh array, so `handler` stays stable. + // kIterated diverts reentrant on/off to a copy, so `handler` stays stable. if (!handler[kIterated]) handler[kIterated] = true; for (let i = 0, { length } = handler; i < length; i++) { const listener = handler[i]; @@ -278,8 +271,7 @@ const emitWithRejectionCapture = function emit(type, ...args) { } return true; } - // No defensive clone: the kIterated mark makes any listener that adds or - // removes on this event install a fresh array, so `handler` stays stable. + // kIterated diverts reentrant on/off to a copy, so `handler` stays stable. if (!handler[kIterated]) handler[kIterated] = true; for (let i = 0, { length } = handler; i < length; i++) { const listener = handler[i]; @@ -333,9 +325,6 @@ function _addListener(target, type, fn, prepend) { if (typeof existing === "function") { handlers = events[type] = prepend ? [fn, existing] : [existing, fn]; } else if (existing[kIterated]) { - // emit() has iterated (and may still be iterating) this exact array; - // install a copy so its loop stays stable. The copy has no mark, so the - // next mutation is in-place again. handlers = events[type] = copyWithInserted(existing, fn, prepend); } else { if (prepend) ArrayPrototypeUnshift.$call(existing, fn); @@ -360,8 +349,7 @@ EventEmitterPrototype.prependListener = function prependListener(type, fn) { return this; }; -// Copy path for when `list` carries kIterated (emit() has walked it): a fresh -// array leaves the iterated one untouched. `warned` carries over so the leak +// Fresh-array insert for a kIterated `list`. Propagates `warned` so the leak // warning fires once. An inline loop beats concat/slice ~10x (host-call cost). function copyWithInserted(list, fn, prepend) { const n = list.length; @@ -465,9 +453,6 @@ EventEmitterPrototype.removeListener = function removeListener(type, listener) { if (position < 0) return this; if (list[kIterated]) { - // emit() has iterated (and may still be iterating) this exact array; - // install a copy so its loop stays stable. A lone survivor is stored bare - // so `_events[type]` matches node. const n = list.length; const copy = $newArrayWithSize(n - 1); for (let i = 0, j = 0; i < n; i++) { @@ -486,8 +471,7 @@ EventEmitterPrototype.removeListener = function removeListener(type, listener) { return this; }; -// In-place single-element remove; node's internal/util spliceOne. Removing the -// last element is O(1), which makes a tail-first drain of N listeners linear. +// Node's internal/util spliceOne: O(1) at the tail, so a tail-first drain is linear. function spliceOne(list, index) { for (; index + 1 < list.length; index++) list[index] = list[index + 1]; list.pop(); @@ -529,10 +513,8 @@ EventEmitterPrototype.removeAllListeners = function removeAllListeners(type) { if (typeof listeners === "function") { this.removeListener(type, listeners); } else if (listeners !== undefined) { - // LIFO order, same as node's loop. Unmarked arrays shrink from the tail as - // we walk indices high-to-low; a marked array is left intact (the first - // removeListener COWs it away), so either way `listeners[i]` is the right - // function at every step. + // LIFO, as in node. Unmarked `listeners` shrinks from the tail under us; + // a kIterated one is COW'd away on the first call and stays intact. for (let i = listeners.length - 1; i >= 0; i--) this.removeListener(type, listeners[i]); } return this; From 019e53d3f4c0094087adbf7bf82f98f422252172 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 26 Jul 2026 02:35:46 +0000 Subject: [PATCH 4/4] events: one-line the added comments; use captured ArrayPrototypePop in spliceOne --- src/js/internal/streams/legacy.ts | 3 +-- src/js/node/events.ts | 16 ++++++---------- 2 files changed, 7 insertions(+), 12 deletions(-) diff --git a/src/js/internal/streams/legacy.ts b/src/js/internal/streams/legacy.ts index 0ea4221d1c34..eefd50006b48 100644 --- a/src/js/internal/streams/legacy.ts +++ b/src/js/internal/streams/legacy.ts @@ -111,8 +111,7 @@ function prependListener(emitter, event, fn) { // the prependListener() method. The goal is to eventually remove this hack. let events, existing; if (!(events = emitter._events) || !(existing = events[event])) emitter.on(event, fn); - // A fresh array, not unshift(): emit() in node:events iterates without - // cloning, and this path can't see the kIterated mark that guards in-place edits. + // A fresh array, not unshift(): this path can't see node:events' kIterated mark, so in-place isn't safe here. else if (ArrayIsArray(existing)) events[event] = [fn, ...existing]; else events[event] = [fn, existing]; } diff --git a/src/js/node/events.ts b/src/js/node/events.ts index ad48e6dfee9a..054803fd39ae 100644 --- a/src/js/node/events.ts +++ b/src/js/node/events.ts @@ -39,13 +39,12 @@ const types = require("node:util/types"); let inspect: typeof import("node:util").inspect | undefined; const SymbolFor = Symbol.for; +const ArrayPrototypePop = Array.prototype.pop; const ArrayPrototypeUnshift = Array.prototype.unshift; const ReflectOwnKeys = Reflect.ownKeys; const kCapture = Symbol("kCapture"); -// Sticky mark emit() puts on a listener array before iterating it. on/prepend/ -// removeListener mutate in place unless they see this mark, in which case they -// install a fresh (unmarked) copy so the in-flight emit() keeps a stable view. +// Sticky mark emit() sets before iterating a listener array; on/off mutate in place unless they see it, then they install a fresh copy so the running emit() stays stable. const kIterated = Symbol("kIterated"); // Set when `_events` was preallocated (streams do this): removeListener then // writes `undefined` instead of `delete`, keeping one shared JSC Structure @@ -145,8 +144,7 @@ function emitError(emitter, args) { throw err; // Unhandled 'error' event } -// A listener list is a bare function for a single listener, else an array (as -// in node). kIterated on the array keeps it stable through the loop; see above. +// A listener list is a bare function for a single listener, else an array (as in node). function applyHandlers(handlers, emitter, args) { if (typeof handlers === "function") { handlers.$apply(emitter, args); @@ -349,8 +347,7 @@ EventEmitterPrototype.prependListener = function prependListener(type, fn) { return this; }; -// Fresh-array insert for a kIterated `list`. Propagates `warned` so the leak -// warning fires once. An inline loop beats concat/slice ~10x (host-call cost). +// Fresh-array insert for a kIterated `list`; propagates `warned`, inline loop beats concat/slice ~10x. function copyWithInserted(list, fn, prepend) { const n = list.length; const copy = $newArrayWithSize(n + 1); @@ -474,7 +471,7 @@ EventEmitterPrototype.removeListener = function removeListener(type, listener) { // Node's internal/util spliceOne: O(1) at the tail, so a tail-first drain is linear. function spliceOne(list, index) { for (; index + 1 < list.length; index++) list[index] = list[index + 1]; - list.pop(); + ArrayPrototypePop.$call(list); } EventEmitterPrototype.off = EventEmitterPrototype.removeListener; @@ -513,8 +510,7 @@ EventEmitterPrototype.removeAllListeners = function removeAllListeners(type) { if (typeof listeners === "function") { this.removeListener(type, listeners); } else if (listeners !== undefined) { - // LIFO, as in node. Unmarked `listeners` shrinks from the tail under us; - // a kIterated one is COW'd away on the first call and stays intact. + // LIFO, as in node; `listeners` either shrinks from the tail in step with i, or (if kIterated) is COW'd away and stays intact. for (let i = listeners.length - 1; i >= 0; i--) this.removeListener(type, listeners[i]); } return this;