From a68697921e25ec62c8b8aa7efbaa67caae64bcfc Mon Sep 17 00:00:00 2001
From: Karl Seguin <k@openmymind.io>
Date: Tue, 1 Sep 2026 12:08:11 +0800
Subject: [PATCH] cdp: remove race between terminate/disconnect

A client that disconnects might get treated as a harsher terminate failure (e.g.
watchdog). This doesn't have a huge impact, but it makes the CI flaky and it
produces more logs than is necessary.

In a terminate state, the driver will now check its inbox to see if this is a
client disconnection.
---
 src/network/HttpClient.zig | 17 ++++++++++++++++-
 src/server/Driver.zig      | 12 +++++++++---
 2 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/src/network/HttpClient.zig b/src/network/HttpClient.zig
index 344865a21..f2c04fce3 100644
--- a/src/network/HttpClient.zig
+++ b/src/network/HttpClient.zig
@@ -566,7 +566,10 @@ pub fn activity(self: *const Client) Activity {
 //                refs to page / session / V8 state; dispatching a
 //                command that frees that state would UAF on unwind.
 //                Cherry-pick only Fetch interception responses
-const DrainMode = enum { all, sync_wait };
+// .terminal    - pops only close/disconnect: the connection is going away and
+//                nothing else may be dispatched, but the peer still gets its
+//                close reason.
+const DrainMode = enum { all, sync_wait, terminal };
 
 // One-shot convenience: create and submit in a single call.
 pub fn request(self: *Client, req: Request, owner: ?*Owner) anyerror!void {
@@ -1310,6 +1313,10 @@ fn makeRequest(self: *Client, conn: *http.Connection, transfer: *Transfer) anyer
     _ = try self.handles.perform();
 }
 
+pub fn drainTerminal(self: *Client) !void {
+    return self.drainInbox(.terminal);
+}
+
 // Drain any client messages the Network thread pushed into our inbox
 // and dispatch them via the driver callbacks. Returns
 // error.ClientDisconnected if the inbox surfaced a disconnect message,
@@ -1322,6 +1329,7 @@ fn drainInbox(self: *Client, mode: DrainMode) !void {
         const msg = switch (mode) {
             .all => self.inbox.pop(),
             .sync_wait => self.inbox.popIf(allowDuringSyncWait),
+            .terminal => self.inbox.popIf(isTerminal),
         } orelse return;
 
         defer msg.deinit();
@@ -1369,6 +1377,13 @@ fn allowDuringSyncWait(msg: *Inbox.Message) bool {
     };
 }
 
+fn isTerminal(msg: *Inbox.Message) bool {
+    return switch (msg.payload) {
+        .close, .disconnect => true,
+        .ping, .cdp, .bidi => false,
+    };
+}
+
 fn isFetchInterceptionMethod(method: []const u8) bool {
     return std.mem.eql(u8, method, "Fetch.continueRequest") or
         std.mem.eql(u8, method, "Fetch.failRequest") or
diff --git a/src/server/Driver.zig b/src/server/Driver.zig
index 2e41315e2..0832dde5e 100644
--- a/src/server/Driver.zig
+++ b/src/server/Driver.zig
@@ -77,11 +77,13 @@ pub fn onData(self: *const Driver, data: []const u8) anyerror!bool {
 
 // Server run loop. Called when it drops the link unsolicited (peer EOF, ...)
 pub fn onLinkDisconnect(self: *const Driver, err: ?anyerror) void {
-    self.browser.env.requestTerminate();
     const arena = self.browser.arena_pool.acquire(.tiny, "driver disconnect") catch |e| switch (e) {
         error.OutOfMemory => @panic("OOM"),
     };
+    // order matters, this ensures that the disconnect message is in the inbox
+    // when tick() discovers the terminatePending flag is set.
     self.browser.http_client.inbox.push(arena, .{ .disconnect = err });
+    self.browser.env.requestTerminate();
 }
 
 // Worker thread. We're processing messages from the inbox.
@@ -134,8 +136,12 @@ pub fn run(self: *const Driver) void {
 // One iteration of the worker loop. Returns false to disconnect.
 fn tick(self: *const Driver) !bool {
     if (self.browser.env.terminatePending()) {
-        // terminatePending means someone decided this browser must die
-        // (e.g. the heap limit was reached).
+        // Maybe something bad happened (e.g. watchdog) or maybe the client
+        // just disconnected. Check the inbox to see if there's a disconnect
+        // message and, if so, it'll handle it directly.
+        self.browser.http_client.drainTerminal() catch |err| switch (err) {
+            error.ClientDisconnected => return false,
+        };
         log.warn(self.scope, "closing connection", .{ .reason = "pending terminate" });
         // The worker thread is the sole writer of this socket, so sending
         // the close frame here can't interleave with another write.