Merge pull request #2647 from staylor/fix/synwait-teardown-interrupt

fix(http): abort blocking-script sync wait when a teardown command is queued (#2646)

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

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));
+    }
+}