From 0bfd0222f2cb5d5885127a0c57ba33022b9ec2af Mon Sep 17 00:00:00 2001
From: Scott Taylor <scott.c.taylor@mac.com>
Date: Thu, 4 Jun 2026 23:17:24 -0400
Subject: [PATCH] fix(http): abort blocking-script sync wait when a teardown
 command is queued
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

A blocking <script src> loads via HttpClient.syncRequest, which spins on
tick(200, .sync_wait). That drain mode only dispatches Fetch-interception
methods (it can't free Page/Frame state mid-parse), so Target.closeTarget /
Target.disposeBrowserContext / Page.close sit undispatched until the blocking
fetch completes — i.e. up to the per-request timeout, per blocking script.

For storefronts with slow/never-completing third-party subresources this
stalls page.close()/context dispose for tens of seconds and collapses CDP
throughput under concurrency (the process stays up; other connections are
unaffected).

Fix: while waiting, peek the inbox; if a teardown/close command (or a
client close/disconnect) is queued, abort the in-flight blocking transfer.
syncRequest then returns the same way it already does on a fetch failure,
the parser unwinds to the next safe .all drain, and the queued command
runs there. No command is dispatched mid-parse, so no UAF.

Repro: 8 workers looping newContext->goto(commit)->page.close against a page
with 15 never-answered subresources went from 0.1 ctx/s (page.close stalling
~30-60s) to ~25 ctx/s; a no-hanging-subresource baseline is unchanged.

Fixes #2646
---
 src/Inbox.zig              | 15 ++++++++
 src/browser/HttpClient.zig | 74 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 89 insertions(+)

diff --git a/src/Inbox.zig b/src/Inbox.zig
index 1828f0f6..0064d2aa 100644
--- a/src/Inbox.zig
+++ b/src/Inbox.zig
@@ -73,6 +73,21 @@ pub fn pop(self: *Inbox) ?*Message {
     return @fieldParentPtr("node", node);
 }
 
+// Peek for a message matching `predicate` without removing it. Used by
+// syncRequest to notice a queued teardown command (which sync_wait can't
+// safely dispatch mid-parse) so it can abort the blocking fetch instead
+// of stalling for the full per-request timeout.
+pub fn contains(self: *Inbox, predicate: *const fn (*Message) bool) bool {
+    self.mutex.lock();
+    defer self.mutex.unlock();
+    var it = self.queue.first;
+    while (it) |node| : (it = node.next) {
+        const msg: *Message = @fieldParentPtr("node", node);
+        if (predicate(msg)) return true;
+    }
+    return false;
+}
+
 // Cherry-pick the first message for which `predicate(msg)` returns
 // true, removing it from the queue. Walks the queue in FIFO order;
 // non-matching messages stay in place. Used to dispatch only the
diff --git a/src/browser/HttpClient.zig b/src/browser/HttpClient.zig
index cd978f2f..cd2e2543 100644
--- a/src/browser/HttpClient.zig
+++ b/src/browser/HttpClient.zig
@@ -802,6 +802,14 @@ pub fn syncRequest(self: *Client, allocator: Allocator, req: Request) !SyncRespo
             }
             return err;
         };
+        if (sync_ctx.completion == .in_progress and self.inbox.contains(isSyncWaitInterrupt)) {
+            // A teardown/close command is queued but sync_wait can't dispatch
+            // it mid-parse (it would free the Page/Frame this stack holds).
+            // Abort the blocking fetch so the parser unwinds to the next safe
+            // drain and the command runs there, instead of stalling for the
+            // full per-request timeout per blocking script.
+            transfer.abort(error.SyncWaitInterrupted);
+        }
     }
 
     switch (sync_ctx.completion) {
@@ -1019,6 +1027,24 @@ fn isFetchInterceptionMethod(method: []const u8) bool {
         std.mem.eql(u8, method, "Fetch.continueWithAuth");
 }
 
+// True for inbox messages that mean "this page/connection is going away".
+// syncRequest uses this to bail out of a blocking-script wait promptly
+// rather than holding the worker for the per-request timeout while a
+// teardown command sits undispatched behind the sync_wait allowlist.
+fn isSyncWaitInterrupt(msg: *Inbox.Message) bool {
+    return switch (msg.payload) {
+        .close, .disconnect => true,
+        .ping => false,
+        .cdp => |c| isTeardownMethod(c.input.method),
+    };
+}
+
+fn isTeardownMethod(method: []const u8) bool {
+    return std.mem.eql(u8, method, "Target.closeTarget") or
+        std.mem.eql(u8, method, "Target.disposeBrowserContext") or
+        std.mem.eql(u8, method, "Page.close");
+}
+
 fn processOneMessage(self: *Client, msg: http.Handles.MultiMessage, transfer: *Transfer) !bool {
     // State at entry: .inflight = conn (multi just delivered a completion).
     if (msg.err == null or msg.err.? == error.RecvError) {
@@ -2249,3 +2275,51 @@ test "HttpClient: allowDuringSyncWait denies non-Fetch CDP methods" {
         try testing.expect(!allowDuringSyncWait(&msg));
     }
 }
+
+test "HttpClient: isSyncWaitInterrupt matches teardown methods, close and disconnect" {
+    var raw_buf: [16]u8 = undefined;
+
+    inline for ([_][]const u8{
+        "Target.closeTarget",
+        "Target.disposeBrowserContext",
+        "Page.close",
+    }) |method| {
+        var msg = Inbox.Message{
+            .arena = testing.allocator,
+            .payload = .{ .cdp = .{
+                .raw = &raw_buf,
+                .input = .{ .method = method },
+            } },
+        };
+        try testing.expect(isSyncWaitInterrupt(&msg));
+    }
+
+    var close_msg = Inbox.Message{ .arena = testing.allocator, .payload = .close };
+    try testing.expect(isSyncWaitInterrupt(&close_msg));
+
+    var disconnect_msg = Inbox.Message{ .arena = testing.allocator, .payload = .{ .disconnect = null } };
+    try testing.expect(isSyncWaitInterrupt(&disconnect_msg));
+}
+
+test "HttpClient: isSyncWaitInterrupt ignores ping and non-teardown CDP methods" {
+    var ping_msg = Inbox.Message{ .arena = testing.allocator, .payload = .{ .ping = "" } };
+    try testing.expect(!isSyncWaitInterrupt(&ping_msg));
+
+    var raw_buf: [16]u8 = undefined;
+    inline for ([_][]const u8{
+        "Page.navigate",
+        "Runtime.evaluate",
+        "Target.createTarget",
+        "Fetch.continueRequest",
+        "",
+    }) |method| {
+        var msg = Inbox.Message{
+            .arena = testing.allocator,
+            .payload = .{ .cdp = .{
+                .raw = &raw_buf,
+                .input = .{ .method = method },
+            } },
+        };
+        try testing.expect(!isSyncWaitInterrupt(&msg));
+    }
+}