Don't dispatch to listeners added during dispatching

Use the last current listener as a sentinel, so that any listener added during
dispatching can be skipped.

src/browser/EventManager.zig

@@ -40,7 +40,8 @@ arena: Allocator,
 listener_pool: std.heap.MemoryPool(Listener),
 list_pool: std.heap.MemoryPool(std.DoublyLinkedList),
 lookup: std.AutoHashMapUnmanaged(usize, *std.DoublyLinkedList),
-dispatch_depth: u32 = 0,
+dispatch_depth: usize,
+deferred_removals: std.ArrayList(struct { list: *std.DoublyLinkedList, listener: *Listener }),
 
 pub fn init(page: *Page) EventManager {
     return .{
@@ -50,6 +51,7 @@ pub fn init(page: *Page) EventManager {
         .list_pool = std.heap.MemoryPool(std.DoublyLinkedList).init(page.arena),
         .listener_pool = std.heap.MemoryPool(Listener).init(page.arena),
         .dispatch_depth = 0,
+        .deferred_removals = .{},
     };
 }
 
@@ -119,9 +121,6 @@ pub fn register(self: *EventManager, target: *EventTarget, typ: []const u8, call
 }
 
 pub fn remove(self: *EventManager, target: *EventTarget, typ: []const u8, callback: Callback, use_capture: bool) void {
-    if (comptime IS_DEBUG) {
-        log.debug(.event, "eventManager.remove", .{ .type = typ, .capture = use_capture, .target = target });
-    }
     const list = self.lookup.get(@intFromPtr(target)) orelse return;
     if (findListener(list, typ, callback, use_capture)) |listener| {
         self.removeListener(list, listener);
@@ -299,38 +298,53 @@ fn dispatchPhase(self: *EventManager, list: *std.DoublyLinkedList, current_targe
     const page = self.page;
     const typ = event._type_string;
 
-    // Track that we're dispatching to prevent immediate removal
+    // Track dispatch depth for deferred removal
     self.dispatch_depth += 1;
     defer {
-        self.dispatch_depth -= 1;
-        // Clean up any marked listeners in this target's list after this phase
-        // We do this regardless of depth to handle cross-target removals correctly
-        self.cleanupMarkedListeners(list);
+        const dispatch_depth = self.dispatch_depth;
+        // Only destroy deferred listeners when we exit the outermost dispatch
+        if (dispatch_depth == 1) {
+            for (self.deferred_removals.items) |removal| {
+                removal.list.remove(&removal.listener.node);
+                self.listener_pool.destroy(removal.listener);
+            }
+            self.deferred_removals.clearRetainingCapacity();
+        } else {
+            self.dispatch_depth = dispatch_depth - 1;
+        }
     }
 
+    // Use the last listener in the list as sentinel - listeners added during dispatch will be after it
+    const last_node = list.last orelse return;
+    const last_listener: *Listener = @alignCast(@fieldParentPtr("node", last_node));
+
+    // Iterate through the list, stopping after we've encountered the last_listener
     var node = list.first;
+    var is_done = false;
     while (node) |n| {
-        // do this now, in case we need to remove n (once: true or aborted signal)
-        node = n.next;
-
-        const listener: *Listener = @alignCast(@fieldParentPtr("node", n));
-
-        // Skip listeners that were marked for removal
-        if (listener.marked_for_removal) {
-            continue;
+        if (is_done) {
+            break;
         }
 
+        const listener: *Listener = @alignCast(@fieldParentPtr("node", n));
+        is_done = (listener == last_listener);
+        node = n.next;
+
+        // Skip non-matching listeners
         if (!listener.typ.eql(typ)) {
             continue;
         }
-
-        // Can be null when dispatching to the target itself
         if (comptime capture_only) |capture| {
             if (listener.capture != capture) {
                 continue;
             }
         }
 
+        // Skip removed listeners
+        if (listener.removed) {
+            continue;
+        }
+
         // If the listener has an aborted signal, remove it and skip
         if (listener.signal) |signal| {
             if (signal.getAborted()) {
@@ -339,6 +353,11 @@ fn dispatchPhase(self: *EventManager, list: *std.DoublyLinkedList, current_targe
             }
         }
 
+        // Remove "once" listeners BEFORE calling them so nested dispatches don't see them
+        if (listener.once) {
+            self.removeListener(list, listener);
+        }
+
         was_handled.* = true;
         event._current_target = current_target;
 
@@ -366,10 +385,6 @@ fn dispatchPhase(self: *EventManager, list: *std.DoublyLinkedList, current_targe
             event._target = original_target;
         }
 
-        if (listener.once) {
-            self.removeListener(list, listener);
-        }
-
         if (event._stop_immediate_propagation) {
             return;
         }
@@ -382,29 +397,17 @@ fn dispatchAll(self: *EventManager, list: *std.DoublyLinkedList, current_target:
 }
 
 fn removeListener(self: *EventManager, list: *std.DoublyLinkedList, listener: *Listener) void {
+    // If we're in a dispatch, defer removal to avoid invalidating iteration
     if (self.dispatch_depth > 0) {
-        // We're in the middle of dispatching, just mark for removal
-        // This prevents invalidating the linked list during iteration
-        listener.marked_for_removal = true;
+        listener.removed = true;
+        self.deferred_removals.append(self.arena, .{ .list = list, .listener = listener }) catch unreachable;
     } else {
-        // Safe to remove immediately
+        // Outside dispatch, remove immediately
         list.remove(&listener.node);
         self.listener_pool.destroy(listener);
     }
 }
 
-fn cleanupMarkedListeners(self: *EventManager, list: *std.DoublyLinkedList) void {
-    var node = list.first;
-    while (node) |n| {
-        node = n.next;
-        const listener: *Listener = @alignCast(@fieldParentPtr("node", n));
-        if (listener.marked_for_removal) {
-            list.remove(&listener.node);
-            self.listener_pool.destroy(listener);
-        }
-    }
-}
-
 fn findListener(list: *const std.DoublyLinkedList, typ: []const u8, callback: Callback, capture: bool) ?*Listener {
     var node = list.first;
     while (node) |n| {
@@ -436,7 +439,7 @@ const Listener = struct {
     function: Function,
     signal: ?*@import("webapi/AbortSignal.zig") = null,
     node: std.DoublyLinkedList.Node,
-    marked_for_removal: bool = false,
+    removed: bool = false,
 };
 
 const Function = union(enum) {

src/browser/tests/event/listener_removal.html

@@ -0,0 +1,53 @@
+<!DOCTYPE html>
+<script src="../testing.js"></script>
+<script id="removeListenerDuringDispatch">
+const target = document.createElement("div");
+
+let listener1Called = 0;
+let listener2Called = 0;
+let listener3Called = 0;
+
+function listener1() {
+    listener1Called++;
+    console.warn("listener1 called, removing listener2 and adding listener3");
+    target.removeEventListener("foo", listener2);
+    target.addEventListener("foo", listener3);
+}
+
+function listener2() {
+    listener2Called++;
+    console.warn("listener2 called (SHOULD NOT HAPPEN)");
+}
+
+function listener3() {
+    listener3Called++;
+    console.warn("listener3 called (SHOULD NOT HAPPEN IN FIRST DISPATCH)");
+}
+
+target.addEventListener("foo", listener1);
+target.addEventListener("foo", listener2);
+
+console.warn("Dispatching first event");
+target.dispatchEvent(new Event("foo"));
+
+console.warn("After first dispatch:");
+console.warn("  listener1Called:", listener1Called);
+console.warn("  listener2Called:", listener2Called);
+console.warn("  listener3Called:", listener3Called);
+
+testing.expectEqual(1, listener1Called);
+testing.expectEqual(0, listener2Called);
+testing.expectEqual(0, listener3Called);
+
+console.warn("Dispatching second event");
+target.dispatchEvent(new Event("foo"));
+
+console.warn("After second dispatch:");
+console.warn("  listener1Called:", listener1Called);
+console.warn("  listener2Called:", listener2Called);
+console.warn("  listener3Called:", listener3Called);
+
+testing.expectEqual(2, listener1Called);
+testing.expectEqual(0, listener2Called);
+testing.expectEqual(1, listener3Called);
+</script>