node:events: mutate listener arrays in place; copy only when emit() has iterated them

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];
 }

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;

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[] = [];