From 87320a506d8cbae2350db90edd90d4f06f8a5c5d Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Thu, 3 Sep 2026 16:51:12 +0800 Subject: [PATCH] chore: Move ownership of the Inbox from HttpClient to WebSocker Driver Currently, the HttpClient owns the inbox and its borrowed by the Link. This is a bit backwards, but it also means that we can't eagerly create a Link: the Link needs the inbox, so it needs the HttpClient, which is created by the Browser (which creates an Isolate). Remember, the Inbox is one of the few things shared between the main thread and the worker, so either end can own it and the other can borrow it. This switches the ownership so that the HttpClient now borrows the Inbox from the Server's side of the Link (the WebSocket). The main goal of this change is to prepare for more advanced HTTP WebDriver flows. The more we can create _without_ a Browser, the fewer edge cases we have to deal with (Browser because it's expensive and has to be created on the Worker thread due to how V8::Isolate works). --- src/Inbox.zig | 7 ---- src/ToolSession.zig | 2 +- src/browser/Browser.zig | 5 ++- src/browser/ScriptManager.zig | 9 +++-- src/browser/ScriptManagerBase.zig | 9 +++-- src/main.zig | 2 +- src/network/HttpClient.zig | 60 ++++++++++++++----------------- src/server/Driver.zig | 27 ++++++++++---- src/server/Server.zig | 20 +++++------ src/server/bidi/BiDi.zig | 10 +++--- src/server/bidi/browser.zig | 6 ++-- src/server/bidi/session.zig | 6 ++-- src/server/bidi/testing.zig | 10 +++++- src/server/cdp/CDP.zig | 18 ++++------ src/server/cdp/testing.zig | 14 ++++++-- src/testing.zig | 2 +- 16 files changed, 111 insertions(+), 96 deletions(-) diff --git a/src/Inbox.zig b/src/Inbox.zig index 24cdd80dc..353e7585d 100644 --- a/src/Inbox.zig +++ b/src/Inbox.zig @@ -41,13 +41,6 @@ queue: DoublyLinkedList = .{}, // fallen too far behind (largely to protect against a misbehaving client) queued_bytes: usize = 0, -// One-way latch, set by the worker's drainInbox the first time it -// observes a .disconnect (or .close) and never cleared. Ensures that, on -// multiple drains, the terminated state is preserved / communicated. This is -// specifically meant to handle the case where a disconnect is captured during -// a syncRequest and we want the following non-nested tick to pick it up again. -terminated: bool = false, - pub fn deinit(self: *Inbox) void { self.mutex.lockUncancelable(lp.io); defer self.mutex.unlock(lp.io); diff --git a/src/ToolSession.zig b/src/ToolSession.zig index fa0a2e397..7d5b5b888 100644 --- a/src/ToolSession.zig +++ b/src/ToolSession.zig @@ -43,7 +43,7 @@ pub fn init(self: *ToolSession, app: *App) !void { self.registry = .init(app.allocator); errdefer self.registry.deinit(); - try self.browser.init(app, .{}, null); + try self.browser.init(app, .{}); errdefer self.browser.deinit(); try self.restartSession(); diff --git a/src/browser/Browser.zig b/src/browser/Browser.zig index 79cad999b..6dd8d0d87 100644 --- a/src/browser/Browser.zig +++ b/src/browser/Browser.zig @@ -21,7 +21,6 @@ const lp = @import("lightpanda"); const App = @import("../App.zig"); const Watchdog = @import("../Watchdog.zig"); -const Driver = @import("../server/Driver.zig"); const Notification = @import("../Notification.zig"); const HttpClient = @import("../network/HttpClient.zig"); @@ -112,7 +111,7 @@ pub fn nextFrameId(self: *Browser) u32 { return id; } -pub fn init(self: *Browser, app: *App, opts: InitOpts, driver: ?Driver) !void { +pub fn init(self: *Browser, app: *App, opts: InitOpts) !void { const allocator = app.allocator; var env = try js.Env.init(app, opts.env); @@ -131,7 +130,7 @@ pub fn init(self: *Browser, app: *App, opts: InitOpts, driver: ?Driver) !void { .watchdog_entry = undefined, }; self.env.protectHeapLimit(); - try self.http_client.init(app, driver); + try self.http_client.init(app); self.watchdog_entry = .{ .env = &self.env, diff --git a/src/browser/ScriptManager.zig b/src/browser/ScriptManager.zig index 98a245a8a..bb0d9db08 100644 --- a/src/browser/ScriptManager.zig +++ b/src/browser/ScriptManager.zig @@ -560,6 +560,7 @@ const PreloadedScript = struct { }; const testing = @import("../testing.zig"); +const Inbox = @import("../Inbox.zig"); test "ScriptManager: PreloadedScript.shutdownCallback drops a .loading preload" { const page = try testing.pageTest("mcp_nav.html", .{}); @@ -616,12 +617,16 @@ test "ScriptManager: waitForPreload stops when teardown is pending" { try sm.preloaded_scripts.put(sm.base.allocator, url, .{ .state = .{ .loading = script } }); defer sm.takePreload(url).?.deinit(); + var inbox: Inbox = .{}; + defer inbox.deinit(); + client.test_inbox = &inbox; + defer client.test_inbox = null; + const message_arena = try client.arena_pool.acquire(.tiny, "test teardown message"); - client.inbox.push(message_arena, .{ .cdp = .{ + inbox.push(message_arena, .{ .cdp = .{ .raw = try message_arena.dupe(u8, "{}"), .input = .{ .method = "Target.closeTarget" }, } }); - defer client.inbox.pop().?.deinit(); try testing.expect(sm.waitForPreload(url) == null); } diff --git a/src/browser/ScriptManagerBase.zig b/src/browser/ScriptManagerBase.zig index 1e90c2fb5..1f6a8ce97 100644 --- a/src/browser/ScriptManagerBase.zig +++ b/src/browser/ScriptManagerBase.zig @@ -1061,6 +1061,7 @@ pub const ImportedModule = struct { }; const testing = @import("../testing.zig"); +const Inbox = @import("../Inbox.zig"); test "ScriptManagerBase: shutdownCallback fails a .loading module" { const page = try testing.pageTest("mcp_nav.html", .{}); @@ -1172,12 +1173,16 @@ test "ScriptManagerBase: waitForImport stops when teardown is pending" { try sm.imported_modules.put(sm.allocator, url, .{ .state = .{ .loading = script } }); sm.async_scripts.append(&script.node); + var inbox: Inbox = .{}; + defer inbox.deinit(); + client.test_inbox = &inbox; + defer client.test_inbox = null; + const message_arena = try client.arena_pool.acquire(.tiny, "test teardown message"); - client.inbox.push(message_arena, .{ .cdp = .{ + inbox.push(message_arena, .{ .cdp = .{ .raw = try message_arena.dupe(u8, "{}"), .input = .{ .method = "Target.disposeBrowserContext" }, } }); - defer client.inbox.pop().?.deinit(); try testing.expectError(error.SyncWaitInterrupted, sm.waitForImport(url)); } diff --git a/src/main.zig b/src/main.zig index 35a2073ba..ba9ef9be2 100644 --- a/src/main.zig +++ b/src/main.zig @@ -357,7 +357,7 @@ const FetchTerminator = struct { fn fetchThread(app: *App, ft: *FetchTerminator, urls: []const [:0]const u8, fetch_opts: lp.FetchOpts, err_out: *?anyerror) void { var browser: lp.Browser = undefined; - browser.init(app, .{}, null) catch |err| { + browser.init(app, .{}) catch |err| { err_out.* = err; log.fatal(.app, "browser init error", .{ .err = err }); return; diff --git a/src/network/HttpClient.zig b/src/network/HttpClient.zig index 082c296d3..f4abac866 100644 --- a/src/network/HttpClient.zig +++ b/src/network/HttpClient.zig @@ -151,30 +151,17 @@ test_fail_submit: if (lp.IS_TEST) ?anyerror else void = if (lp.IS_TEST) null els // Allocated from self.allocator when set, null otherwise. user_agent_override: ?[:0]const u8 = null, -// The protocol layer we dispatch inbox messages to. Set in CDP.init / -// BiDi.init for `serve` mode; null in all other modes. Since this is set -// early, BEFORE the socket is registered with the network thread, we also -// have the `driver_link_active` boolean. +// The driver (CDP / BiDi) attached to us. If there's a driver, then there's +// an inbox for us to process (and there's someone to wake us up from a poll) driver: ?Driver = null, -// True iff a producer (Server.handleConnection, after the worker -// handshake completes) has registered the client socket with the Network -// thread and Network will fire curl_multi_wakeup on our multi handle -// when it pushes to the inbox. perform uses this — NOT `driver != null` -// — to decide whether to block in poll without any in-flight curl -// work. driver is set in the driver's init, well before the link is -// wired; tests and the pre-handshake window have a driver but no -// producer, so polling -// there would just eat the timeout waiting for a wakeup that's never -// coming. -driver_link_active: bool = false, +// If there's a driver, it can tell us to disconnect and that has to stick +// until _tick is called and picks it up. +disconnected: bool = false, -// Client messages read off the WS socket by the Network thread land -// here. perform drains the inbox at each safe point and dispatches -// via driver.onMessage / onPing / onClose / onDisconnect. Always present -// even in non-serve mode — the empty-queue drain is one mutex lock plus -// a linked-list head check, cheaper than nullability everywhere. -inbox: Inbox, +// Test-only: an inbox for a client with no driver (the shared test browser), +// so the pending-teardown checks can be exercised without a CDP connection. +test_inbox: if (lp.IS_TEST) ?*Inbox else void = if (lp.IS_TEST) null else {}, max_response_size: usize, @@ -201,7 +188,7 @@ http_version: lp.Config.HttpVersion, robots: RobotsGate, url_blocklist: ?UrlBlocklist, -pub fn init(self: *Client, app: *lp.App, driver: ?Driver) !void { +pub fn init(self: *Client, app: *lp.App) !void { const config = app.config; const allocator = app.allocator; @@ -231,8 +218,6 @@ pub fn init(self: *Client, app: *lp.App, driver: ?Driver) !void { .handles = handles, .network = network, .allocator = app.allocator, - .driver = driver, - .inbox = .{}, .cache = &network.cache, .use_proxy = http_proxy != null, @@ -280,7 +265,6 @@ pub fn deinit(self: *Client) void { self.robots.deinit(); self.blocking_requests.deinit(self.allocator); self.transfers.deinit(self.allocator); - self.inbox.deinit(); self.cache.maintenance(lp.datetime.timestamp(.real)); } @@ -701,14 +685,21 @@ pub fn tickSync(self: *Client, timeout_ms: u32) !void { } fn hasPendingTeardown(self: *Client) bool { - return self.inbox.contains(isSyncWaitInterrupt); + const inbox = blk: { + if (comptime lp.IS_TEST) { + if (self.test_inbox) |test_inbox| break :blk test_inbox; + } + const driver = &(self.driver orelse return false); + break :blk driver.inbox; + }; + return inbox.contains(isSyncWaitInterrupt); } // Returns false iff the tick was a no-op. When false is returned, immediately // calling this again will almost [instantly] return false again, potentially // causing a spin. pub fn _tick(self: *Client, timeout_ms: u32, mode: DrainMode) !bool { - if (self.inbox.terminated) { + if (self.disconnected) { return error.ClientDisconnected; } @@ -723,7 +714,7 @@ pub fn _tick(self: *Client, timeout_ms: u32, mode: DrainMode) !bool { if (dispatched == false and processed == false and self.dispatch_queue.first == null and self.ws_dispatch_queue.first == null) { // Nothing was dispatched, no messages were processed and nothing is // waiting for dispatch. We need to wait for I/O. - if (running > 0 or self.driver_link_active or self.delayed_queue.first != null) { + if (running > 0 or self.driver != null or self.delayed_queue.first != null) { { self.heartbeat.enterWait(); defer self.heartbeat.exitWait(); @@ -1362,11 +1353,12 @@ pub fn drainTerminal(self: *Client) !void { // processOneMessage's redirect path. fn drainInbox(self: *Client, mode: DrainMode) !void { const driver = &(self.driver orelse return); + const inbox = driver.inbox; while (true) { const msg = switch (mode) { - .all => self.inbox.pop(), - .sync_wait => self.inbox.popIf(allowDuringSyncWait), - .terminal => self.inbox.popIf(isTerminal), + .all => inbox.pop(), + .sync_wait => inbox.popIf(allowDuringSyncWait), + .terminal => inbox.popIf(isTerminal), } orelse return; defer msg.deinit(); @@ -1380,12 +1372,12 @@ fn drainInbox(self: *Client, mode: DrainMode) !void { .ping => |body| driver.onPing(body), .close => { driver.onClose(); - self.inbox.terminated = true; + self.disconnected = true; return error.ClientDisconnected; }, .disconnect => |err| { driver.onDisconnect(err); - self.inbox.terminated = true; + self.disconnected = true; return error.ClientDisconnected; }, } @@ -2332,7 +2324,7 @@ pub const Transfer = struct { pub fn submitSync(self: *Transfer) !SyncResponse { const client = self.client; - if (client.inbox.terminated) { + if (client.disconnected) { self.deinit(); return error.ClientDisconnected; } diff --git a/src/server/Driver.zig b/src/server/Driver.zig index 747560c57..0fbf56243 100644 --- a/src/server/Driver.zig +++ b/src/server/Driver.zig @@ -50,18 +50,20 @@ impl: Impl, conn: *Link, browser: *Browser, +// The worker's mailbox, owned by the loop's connection slot (it outlives +// the link). The loop pushes, the worker's HttpClient drains through us. +inbox: *Inbox, + // The protocol's log scope, so shared code still logs as .cdp / .bidi. scope: log.Scope, -// Called from CDP.init / BiDi.init, where conn and browser are both still -// undefined: we only take their addresses, which the impl's own allocation -// already fixed. -pub fn init(impl: Impl) Driver { +pub fn init(impl: Impl, inbox: *Inbox) Driver { return switch (impl) { inline else => |d, tag| .{ .impl = impl, .conn = &d.conn, .browser = &d.browser, + .inbox = inbox, .scope = @field(log.Scope, @tagName(tag)), // The tag names line up with the log scopes of the same name. }, }; @@ -83,7 +85,7 @@ pub fn onLinkDisconnect(self: *const Driver, err: ?anyerror) void { }; // 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.inbox.push(arena, .{ .disconnect = err }); self.browser.env.requestTerminate(); self.wakeup(); } @@ -137,9 +139,10 @@ pub fn onDisconnect(self: *const Driver, err: ?anyerror) void { log.info(self.scope, "disconnect", .{ .err = err }); } -// Worker thread. Once the websocket connection is established, the server -// calls this (from the worker thread) and it becomes the driving loop. +// Worker thread. pub fn run(self: *const Driver) void { + self.attach(); // make HttpClient aware of our inbox + defer self.detach(); // make HttpClient forget our inbox while (true) { const alive = self.tick() catch |err| { log.err(self.scope, "tick", .{ .err = err }); @@ -151,6 +154,16 @@ pub fn run(self: *const Driver) void { } } +// Worker thread. Tell the http_client about us (so it can monitor our inbox) +pub fn attach(self: *const Driver) void { + self.browser.http_client.driver = self.*; +} + +// Worker thread. +pub fn detach(self: *const Driver) void { + self.browser.http_client.driver = null; +} + // One iteration of the worker loop. Returns false to disconnect. fn tick(self: *const Driver) !bool { if (self.browser.env.terminatePending()) { diff --git a/src/server/Server.zig b/src/server/Server.zig index ae4815618..5eb48b610 100644 --- a/src/server/Server.zig +++ b/src/server/Server.zig @@ -30,6 +30,7 @@ const BiDi = @import("bidi/BiDi.zig"); const WS = @import("WS.zig"); const http = @import("http.zig"); const Driver = @import("Driver.zig"); +const Inbox = @import("../Inbox.zig"); const log = lp.log; const posix = std.posix; @@ -60,6 +61,8 @@ const WebSocket = struct { // threads `websockets` while live, the pool's free list otherwise node: DoublyLinkedList.Node, protocol: Driver.Protocol, + // The worker's mailbox, which is how the main loop communicates with the worker. + inbox: Inbox = .{}, // null until the worker attaches driver: ?Driver = null, // whether or not socket is in the poll set. Makes sure we don't double-remove @@ -582,6 +585,7 @@ fn releaseWorker(self: *Server, ws: *WebSocket, notify: *std.Io.Event) void { // or upgradeConnection when there never was a worker. fn releaseWebSocket(self: *Server, ws: *WebSocket) void { self.websockets.remove(&ws.node); + ws.inbox.deinit(); lp.metrics.serve_active_connections.decr(ws.protocol); self.websocket_pool.release(ws); self.slotFreed(); @@ -672,31 +676,23 @@ const Worker = struct { .cdp => { const cdp = try allocator.create(CDP); defer allocator.destroy(cdp); - try cdp.init(server.app, ws.socket); + try cdp.init(server.app, ws.socket, &ws.inbox); defer cdp.deinit(); - Worker.run(server, ws, .init(.{ .cdp = cdp })); + Worker.run(server, ws, .init(.{ .cdp = cdp }, &ws.inbox)); }, .bidi => { const bidi = try allocator.create(BiDi); defer allocator.destroy(bidi); - try bidi.init(server.app, ws.socket, session_id); + try bidi.init(server.app, ws.socket, &ws.inbox, session_id); defer bidi.deinit(); - Worker.run(server, ws, .init(.{ .bidi = bidi })); + Worker.run(server, ws, .init(.{ .bidi = bidi }, &ws.inbox)); }, } } fn run(server: *Server, ws: *WebSocket, driver: Driver) void { - // Gates HttpClient's block in curl_multi_poll: false (tests, before - // the attach) means "nobody will wake us, don't sleep on it". From - // here the loop is about to feed our inbox and wake us, so the - // worker parks in poll instead of spinning through tick(). - driver.browser.http_client.driver_link_active = true; Worker.notifyLoopOfChange(server, .{ .ws = ws, .op = .{ .attach = driver } }); driver.run(); - // The loop is done with us once releaseConnection returns; the - // driver's deinit may still tick the client, without a producer. - defer driver.browser.http_client.driver_link_active = false; // Release first: until the loop has let go of this websocket it can // still drop the link, and onLinkDisconnect requests a terminate. Doing // it the other way round left that request landing after the cancel, diff --git a/src/server/bidi/BiDi.zig b/src/server/bidi/BiDi.zig index bd26bd7e8..4a007d74e 100644 --- a/src/server/bidi/BiDi.zig +++ b/src/server/bidi/BiDi.zig @@ -28,7 +28,7 @@ const Notification = @import("../../Notification.zig"); const NodeRegistry = @import("../../NodeRegistry.zig"); const Link = @import("../Link.zig"); -const Driver = @import("../Driver.zig"); +const Inbox = @import("../../Inbox.zig"); const script = @import("script.zig"); const remote_value = @import("remote_value.zig"); @@ -84,7 +84,7 @@ const InputMessage = struct { method: ?[]const u8 = null, }; -pub fn init(self: *BiDi, app: *App, socket: posix.socket_t, session_id: ?[36]u8) !void { +pub fn init(self: *BiDi, app: *App, socket: posix.socket_t, inbox: *Inbox, session_id: ?[36]u8) !void { const allocator = app.allocator; self.* = .{ .app = app, @@ -99,12 +99,10 @@ pub fn init(self: *BiDi, app: *App, socket: posix.socket_t, session_id: ?[36]u8) .session_arena = std.heap.ArenaAllocator.init(allocator), }; - const driver = Driver.init(.{ .bidi = self }); - - try self.browser.init(app, .{}, driver); + try self.browser.init(app, .{}); errdefer self.browser.deinit(); - try self.conn.init(app, socket, .bidi, &self.browser.http_client.inbox); + try self.conn.init(app, socket, .bidi, inbox); errdefer self.conn.deinit(); self.notification = try Notification.init(allocator); diff --git a/src/server/bidi/browser.zig b/src/server/bidi/browser.zig index d5909783e..b4049047d 100644 --- a/src/server/bidi/browser.zig +++ b/src/server/bidi/browser.zig @@ -45,9 +45,9 @@ pub fn processMessage(cmd: *const BiDi.Command) !void { fn close(cmd: *const BiDi.Command) !void { try cmd.sendResult(struct {}{}); - const browser = &cmd.bidi.browser; - const arena = try browser.arena_pool.acquire(.tiny, "bidi browser close"); - browser.http_client.inbox.push(arena, .close); + const bidi = cmd.bidi; + const arena = try bidi.browser.arena_pool.acquire(.tiny, "bidi browser close"); + bidi.conn.inbox.push(arena, .close); } const UserContextInfo = struct { userContext: []const u8 }; diff --git a/src/server/bidi/session.zig b/src/server/bidi/session.zig index bb93e0b22..e11e4d167 100644 --- a/src/server/bidi/session.zig +++ b/src/server/bidi/session.zig @@ -106,9 +106,9 @@ pub const Capabilities = struct { fn end(cmd: *const BiDi.Command) !void { try cmd.sendResult(struct {}{}); - const browser = &cmd.bidi.browser; - const arena = try browser.arena_pool.acquire(.tiny, "bidi session end"); - browser.http_client.inbox.push(arena, .close); + const bidi = cmd.bidi; + const arena = try bidi.browser.arena_pool.acquire(.tiny, "bidi session end"); + bidi.conn.inbox.push(arena, .close); } // Subscriptions are global (per-context filtering is not supported yet). diff --git a/src/server/bidi/testing.zig b/src/server/bidi/testing.zig index 68cfe0ce8..76c41bcc1 100644 --- a/src/server/bidi/testing.zig +++ b/src/server/bidi/testing.zig @@ -22,6 +22,8 @@ const base = @import("../../testing.zig"); const Frame = @import("../../browser/Frame.zig"); const BiDi = @import("BiDi.zig"); +const Inbox = @import("../../Inbox.zig"); +const Driver = @import("../Driver.zig"); const json = std.json; const posix = std.posix; @@ -44,6 +46,8 @@ pub const TestContext = struct { read_buf: [1024 * 32]u8 = undefined, bidi_: BiDi = undefined, bidi_initialized: bool = false, + inbox: Inbox = .{}, + driver: Driver = undefined, bidi_socket: posix.socket_t, socket: posix.socket_t, received: std.ArrayList(json.Value) = .empty, @@ -51,8 +55,10 @@ pub const TestContext = struct { pub fn deinit(self: *TestContext) void { if (self.bidi_initialized) { + self.driver.detach(); self.bidi_.deinit(); } + self.inbox.deinit(); _ = std.c.close(self.socket); _ = std.c.close(self.bidi_socket); base.reset(); @@ -60,8 +66,10 @@ pub const TestContext = struct { pub fn bidi(self: *TestContext) *BiDi { if (!self.bidi_initialized) { - self.bidi_.init(base.test_app, self.bidi_socket, null) catch |err| @panic(@errorName(err)); + self.bidi_.init(base.test_app, self.bidi_socket, &self.inbox, null) catch |err| @panic(@errorName(err)); self.bidi_initialized = true; + self.driver = .init(.{ .bidi = &self.bidi_ }, &self.inbox); + self.driver.attach(); } return &self.bidi_; } diff --git a/src/server/cdp/CDP.zig b/src/server/cdp/CDP.zig index b82491b53..e9c431b65 100644 --- a/src/server/cdp/CDP.zig +++ b/src/server/cdp/CDP.zig @@ -37,7 +37,6 @@ const Label = @import("../../browser/webapi/element/html/Label.zig"); const WS = @import("../WS.zig"); const Link = @import("../Link.zig"); const Server = @import("../Server.zig"); -const Driver = @import("../Driver.zig"); const Incrementing = @import("id.zig").Incrementing; const fetch = @import("domains/fetch.zig"); @@ -98,7 +97,7 @@ browser_context_arena: std.heap.ArenaAllocator, // Files handed out as IO stream handles (Page.printToPDF ReturnAsStream). streams: @import("domains/io.zig").Streams, -pub fn init(self: *CDP, app: *App, socket: posix.socket_t) !void { +pub fn init(self: *CDP, app: *App, socket: posix.socket_t, inbox: *Inbox) !void { const allocator = app.allocator; self.* = .{ @@ -114,12 +113,10 @@ pub fn init(self: *CDP, app: *App, socket: posix.socket_t) !void { .streams = .{ .allocator = allocator }, }; - const driver = Driver.init(.{ .cdp = self }); - - try self.browser.init(app, .{ .env = .{ .with_inspector = true } }, driver); + try self.browser.init(app, .{ .env = .{ .with_inspector = true } }); errdefer self.browser.deinit(); - try self.conn.init(app, socket, .cdp, &self.browser.http_client.inbox); + try self.conn.init(app, socket, .cdp, inbox); } pub fn deinit(self: *CDP) void { @@ -1463,7 +1460,7 @@ test "cdp: disconnect latches so the worker keeps exiting" { // since #2510, on shutdown via shutdownLinks. { const arena = try client.arena_pool.acquire(.tiny, "test disconnect"); - client.inbox.push(arena, .{ .disconnect = null }); + ctx.inbox.push(arena, .{ .disconnect = null }); } // First tick drains the .disconnect and tears the link down. @@ -1489,8 +1486,7 @@ test "cdp: run sends a close frame on pending terminate" { defer cdp.browser.env.cancelTerminate(); // The pending terminate makes the first tick the last one, so run returns. - const driver: Driver = .init(.{ .cdp = cdp }); - driver.run(); + ctx.driver.run(); // The client should receive a close frame (code 1001, going away), not // just an abrupt socket close. @@ -1505,10 +1501,10 @@ test "cdp: syncRequest short-circuits after disconnect" { const client = &ctx.cdp().browser.http_client; - // Latch terminated via a drained disconnect (as above). + // Latch disconnected via a drained disconnect (as above). { const arena = try client.arena_pool.acquire(.tiny, "test disconnect"); - client.inbox.push(arena, .{ .disconnect = null }); + ctx.inbox.push(arena, .{ .disconnect = null }); } try testing.expectError(error.ClientDisconnected, client.tick(0)); diff --git a/src/server/cdp/testing.zig b/src/server/cdp/testing.zig index f8cb9822d..57bfc54d0 100644 --- a/src/server/cdp/testing.zig +++ b/src/server/cdp/testing.zig @@ -19,6 +19,8 @@ const std = @import("std"); const CDP = @import("CDP.zig"); +const Inbox = @import("../../Inbox.zig"); +const Driver = @import("../Driver.zig"); pub const base = @import("../../testing.zig"); @@ -43,13 +45,19 @@ pub const TestContext = struct { read_buf: [1024 * 32]u8 = undefined, cdp_: CDP = undefined, cdp_initialized: bool = false, + inbox: Inbox = .{}, + driver: Driver = undefined, cdp_socket: posix.socket_t, socket: posix.socket_t, received: std.ArrayList(json.Value) = .empty, received_raw: std.ArrayList([]const u8) = .empty, pub fn deinit(self: *TestContext) void { - if (self.cdp_initialized) self.cdp_.deinit(); + if (self.cdp_initialized) { + self.driver.detach(); + self.cdp_.deinit(); + } + self.inbox.deinit(); _ = std.c.close(self.socket); _ = std.c.close(self.cdp_socket); base.reset(); @@ -57,8 +65,10 @@ pub const TestContext = struct { pub fn cdp(self: *TestContext) *CDP { if (!self.cdp_initialized) { - self.cdp_.init(base.test_app, self.cdp_socket) catch |err| @panic(@errorName(err)); + self.cdp_.init(base.test_app, self.cdp_socket, &self.inbox) catch |err| @panic(@errorName(err)); self.cdp_initialized = true; + self.driver = .init(.{ .cdp = &self.cdp_ }, &self.inbox); + self.driver.attach(); } return &self.cdp_; } diff --git a/src/testing.zig b/src/testing.zig index bfc8c7f50..4e39058a0 100644 --- a/src/testing.zig +++ b/src/testing.zig @@ -545,7 +545,7 @@ test "tests:beforeAll" { test_app = try App.init(test_allocator, &test_config); errdefer test_app.deinit(); - try test_browser.init(test_app, .{}, null); + try test_browser.init(test_app, .{}); errdefer test_browser.deinit(); // Create notification for testing