From 8dbe22a01a90e59ee92e467c339814af68aa2b92 Mon Sep 17 00:00:00 2001
From: Karl Seguin <k@openmymind.io>
Date: Mon, 23 Mar 2026 17:55:36 +0800
Subject: [PATCH] Use double-queue to better support recursive navigation

Enqueuing while processing the navigation queue is rare, but apparently can
happen. The most likely culprit is the microqueue task being processed which
enqueues a new navigation (e.g. when a promise resolves).

This was never well handled, with the possibility of a use-after-free or of
skipping the new navigation. This commit introduces a double queue, which is
swapped at the start of processing, so that we always have 1 list for queueing
new navigation requests, and one list that we're currently processing.
---
 src/browser/Session.zig              | 40 +++++++++++++++++++---------
 src/browser/tests/frames/frames.html |  6 ++---
 2 files changed, 30 insertions(+), 16 deletions(-)

diff --git a/src/browser/Session.zig b/src/browser/Session.zig
index 63a7a322..5801038b 100644
--- a/src/browser/Session.zig
+++ b/src/browser/Session.zig
@@ -77,7 +77,15 @@ arena_pool: *ArenaPool,
 
 page: ?Page,
 
-queued_navigation: std.ArrayList(*Page),
+// Double buffer so that, as we process one list of queued navigations, new entries
+// are added to the separate buffer. This ensures that we don't end up with
+// endless navigation loops AND that we don't invalidate the list while iterating
+// if a new entry gets appended
+queued_navigation_1: std.ArrayList(*Page),
+queued_navigation_2: std.ArrayList(*Page),
+// pointer to either queued_navigation_1 or queued_navigation_2
+queued_navigation: *std.ArrayList(*Page),
+
 // Temporary buffer for about:blank navigations during processing.
 // We process async navigations first (safe from re-entrance), then sync
 // about:blank navigations (which may add to queued_navigation).
@@ -107,11 +115,14 @@ pub fn init(self: *Session, browser: *Browser, notification: *Notification) !voi
         .navigation = .{ ._proto = undefined },
         .storage_shed = .{},
         .browser = browser,
-        .queued_navigation = .{},
+        .queued_navigation = undefined,
+        .queued_navigation_1 = .{},
+        .queued_navigation_2 = .{},
         .queued_queued_navigation = .{},
         .notification = notification,
         .cookie_jar = storage.Cookie.Jar.init(allocator),
     };
+    self.queued_navigation = &self.queued_navigation_1;
 }
 
 pub fn deinit(self: *Session) void {
@@ -284,7 +295,7 @@ pub fn runner(self: *Session, opts: Runner.Opts) !Runner {
 }
 
 pub fn scheduleNavigation(self: *Session, page: *Page) !void {
-    const list = &self.queued_navigation;
+    const list = self.queued_navigation;
 
     // Check if page is already queued
     for (list.items) |existing| {
@@ -298,7 +309,12 @@ pub fn scheduleNavigation(self: *Session, page: *Page) !void {
 }
 
 pub fn processQueuedNavigation(self: *Session) !void {
-    const navigations = &self.queued_navigation;
+    const navigations = self.queued_navigation;
+    if (self.queued_navigation == &self.queued_navigation_1) {
+        self.queued_navigation = &self.queued_navigation_2;
+    } else {
+        self.queued_navigation = &self.queued_navigation_1;
+    }
 
     if (self.page.?._queued_navigation != null) {
         // This is both an optimization and a simplification of sorts. If the
@@ -314,7 +330,6 @@ pub fn processQueuedNavigation(self: *Session) !void {
     defer about_blank_queue.clearRetainingCapacity();
 
     // First pass: process async navigations (non-about:blank)
-    // These cannot cause re-entrant navigation scheduling
     for (navigations.items) |page| {
         const qn = page._queued_navigation.?;
 
@@ -329,7 +344,6 @@ pub fn processQueuedNavigation(self: *Session) !void {
         };
     }
 
-    // Clear the queue after first pass
     navigations.clearRetainingCapacity();
 
     // Second pass: process synchronous navigations (about:blank)
@@ -339,15 +353,17 @@ pub fn processQueuedNavigation(self: *Session) !void {
         try self.processFrameNavigation(page, qn);
     }
 
-    // Safety: Remove any about:blank navigations that were queued during the
-    // second pass to prevent infinite loops
+    // Safety: Remove any about:blank navigations that were queued during
+    // processing to prevent infinite loops. New navigations have been queued
+    // in the other buffer.
+    const new_navigations = self.queued_navigation;
     var i: usize = 0;
-    while (i < navigations.items.len) {
-        const page = navigations.items[i];
+    while (i < new_navigations.items.len) {
+        const page = new_navigations.items[i];
         if (page._queued_navigation) |qn| {
             if (qn.is_about_blank) {
-                log.warn(.page, "recursive about    blank", .{});
-                _ = navigations.swapRemove(i);
+                log.warn(.page, "recursive about blank", .{});
+                _ = self.queued_navigation.swapRemove(i);
                 continue;
             }
         }
diff --git a/src/browser/tests/frames/frames.html b/src/browser/tests/frames/frames.html
index 75eb8631..7f9b636c 100644
--- a/src/browser/tests/frames/frames.html
+++ b/src/browser/tests/frames/frames.html
@@ -118,8 +118,6 @@
   }
 </script>
 
-<!--
-This test is flaky. Temporarily disabled until Runner can handle more advanced cases.
 <script id=link_click>
   testing.async(async (restore) => {
     await new Promise((resolve) => {
@@ -139,9 +137,9 @@ This test is flaky. Temporarily disabled until Runner can handle more advanced c
     restore();
     testing.expectEqual("<html><head></head><body>It was clicked!\n</body></html>", f6.contentDocument.documentElement.outerHTML);
   });
-</script> -->
+</script>
 
-<script id=count>
+<script id=about_blank_nav>
   {
     let i = document.createElement('iframe');
     document.documentElement.appendChild(i);