From 080c404581530d09b68d7f4b1896ac0da33b366e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Arrufat?= Date: Mon, 3 Aug 2026 11:12:59 +0200 Subject: [PATCH] c-api: heap-allocate fetch_browser to ensure pointer stability Ensure lp.Browser remains pointer-stable as it uses self-pointers. Also simplify ToolSession cancel hooks and pkgconfig file generation. --- build.zig | 14 +++++--------- src/c_api.zig | 45 +++++++++++++++++++++++++-------------------- src/lightpanda.zig | 26 +++----------------------- 3 files changed, 33 insertions(+), 52 deletions(-) diff --git a/build.zig b/build.zig index ee2b8c91e..dbe1376e0 100644 --- a/build.zig +++ b/build.zig @@ -225,7 +225,7 @@ pub fn build(b: *Build) !void { } { - // C API (src/c_api.zig): liblightpanda.so for embedders. + // c api const c_api_module = createCApiModule(b, lightpanda_module); const c_api_check = b.addLibrary(.{ @@ -290,9 +290,7 @@ pub fn build(b: *Build) !void { } lib_step.dependOn(&install_so.step); lib_step.dependOn(&install_header.step); - // The .so resolves its own dependencies, so the link line is - // just the library. - const shared_pc = pkgConfigFile(b, version_string, "-L${libdir} -llightpanda"); + const shared_pc = pkgConfigFile(b, version_string); lib_step.dependOn(&b.addInstallLibFile(shared_pc, "pkgconfig/lightpanda.pc").step); } else { lib_step.dependOn(&b.addFail("lib needs a source-built V8: drop -Dprebuilt_v8_path").step); @@ -319,8 +317,6 @@ pub fn build(b: *Build) !void { } } -/// Root module for a C-API artifact. The ABI tests get their own instance -/// so their test-only header import stays off the .so. fn createCApiModule(b: *Build, lightpanda: *Build.Module) *Build.Module { const mod = b.createModule(.{ .root_source_file = b.path("src/c_api.zig"), @@ -335,7 +331,7 @@ fn createCApiModule(b: *Build, lightpanda: *Build.Module) *Build.Module { return mod; } -fn pkgConfigFile(b: *Build, version: []const u8, libs: []const u8) Build.LazyPath { +fn pkgConfigFile(b: *Build, version: []const u8) Build.LazyPath { return b.addWriteFiles().add("lightpanda.pc", b.fmt( \\prefix=${{pcfiledir}}/../.. \\libdir=${{prefix}}/lib @@ -345,9 +341,9 @@ fn pkgConfigFile(b: *Build, version: []const u8, libs: []const u8) Build.LazyPat \\Description: Lightpanda headless browser C library \\Version: {s} \\Cflags: -I${{includedir}} - \\Libs: {s} + \\Libs: -L${{libdir}} -llightpanda \\ - , .{ version, libs })); + , .{version})); } fn linkV8( diff --git a/src/c_api.zig b/src/c_api.zig index f8584a558..913206d49 100644 --- a/src/c_api.zig +++ b/src/c_api.zig @@ -109,9 +109,9 @@ const BrowserHandle = struct { config_arena: std.heap.ArenaAllocator, // Owns the previous lp_fetch result; reset at the start of the next one. fetch_arena: std.heap.ArenaAllocator, - // lp_fetch's browser, created on first use and reused after — each call - // still gets a fresh session. Its isolate parks between calls. - fetch_browser: ?lp.Browser, + // lp_fetch's browser, created on first use; its isolate parks between + // calls. Heap-allocated: Browser registers self-pointers and must not move. + fetch_browser: ?*lp.Browser, sessions: std.ArrayList(*SessionHandle), // Static @errorName of the last failing lp_fetch/lp_session_new. last_error: []const u8, @@ -210,10 +210,11 @@ pub export fn lp_shutdown(handle_: ?*BrowserHandle) void { while (handle.sessions.pop()) |session| destroySession(session); handle.sessions.deinit(c_allocator); - if (handle.fetch_browser) |*browser| { + if (handle.fetch_browser) |browser| { // Browser.deinit's Env.deinit exit balances against this enter. browser.env.isolate.enter(); browser.deinit(); + c_allocator.destroy(browser); } handle.app.deinit(); handle.config.deinit(c_allocator); @@ -279,17 +280,18 @@ pub export fn lp_fetch( // Sessions park their isolate between calls, so entering this // browser's isolate here nests correctly. Browser.init leaves the // isolate entered; the exit below parks it either way. - if (handle.fetch_browser) |*browser| { + if (handle.fetch_browser) |browser| { browser.env.isolate.enter(); } else { - handle.fetch_browser = @as(lp.Browser, undefined); - (&handle.fetch_browser.?).init(handle.app, .{}, null) catch |err| { - handle.fetch_browser = null; + const browser = c_allocator.create(lp.Browser) catch return .out_of_memory; + browser.init(handle.app, .{}, null) catch |err| { + c_allocator.destroy(browser); handle.last_error = @errorName(err); return .internal; }; + handle.fetch_browser = browser; } - const browser = &handle.fetch_browser.?; + const browser = handle.fetch_browser.?; defer browser.env.isolate.exit(); lp.fetch(handle.app, browser, &.{url}, fetch_opts) catch |err| { @@ -356,7 +358,7 @@ fn createSession(handle: *BrowserHandle) !*SessionHandle { try entry.ts.init(handle.app); errdefer entry.ts.deinit(); - entry.ts.setCancelHook(.{ .context = entry, .check = cancelTrampoline }); + entry.ts.session.cancel_hook = .{ .context = entry, .check = cancelTrampoline }; try handle.sessions.append(c_allocator, entry); @@ -473,21 +475,24 @@ fn cancelTrampoline(ctx: *anyopaque) bool { /// Error name of the most recent failing `lp_call` on this session; empty /// when the last call succeeded. Static storage — do not free. pub export fn lp_last_error(entry_: ?*SessionHandle, len_: ?*usize) ?[*]const u8 { - if (len_) |len| len.* = 0; - const entry = entry_ orelse return null; - if (app_state != .live) return null; - if (len_) |len| len.* = entry.last_error.len; - return entry.last_error.ptr; + const entry = entry_ orelse return publishError(null, len_); + return publishError(if (app_state == .live) entry.last_error else null, len_); } /// Like `lp_last_error`, for the browser-level calls (`lp_fetch`, /// `lp_session_new`). pub export fn lp_browser_last_error(handle_: ?*BrowserHandle, len_: ?*usize) ?[*]const u8 { - if (len_) |len| len.* = 0; - const handle = handle_ orelse return null; - if (app_state != .live) return null; - if (len_) |len| len.* = handle.last_error.len; - return handle.last_error.ptr; + const handle = handle_ orelse return publishError(null, len_); + return publishError(if (app_state == .live) handle.last_error else null, len_); +} + +fn publishError(err: ?[]const u8, len_: ?*usize) ?[*]const u8 { + const e = err orelse { + if (len_) |len| len.* = 0; + return null; + }; + if (len_) |len| len.* = e.len; + return e.ptr; } /// JSON array describing every tool `lp_call` accepts, in the MCP diff --git a/src/lightpanda.zig b/src/lightpanda.zig index a5aa8d5ad..01769456f 100644 --- a/src/lightpanda.zig +++ b/src/lightpanda.zig @@ -178,25 +178,19 @@ pub fn Once(comptime f: fn () void) type { }; } -/// What a tool-driving embedder owns per isolated browsing context: a Browser -/// (its own V8 isolate), that browser's session, the notification hub, and -/// the node registry `tools.call` needs. Used by the C API. `self` must not -/// move after `init` — Browser registers self-pointers. +/// Everything a tool-driving embedder owns per isolated browsing context. +/// Used by the C API. `self` must not move after `init` — Browser registers +/// self-pointers. pub const ToolSession = struct { browser: Browser, session: *Session, notification: *Notification, registry: CDPNode.Registry, - // The hook is session-scoped; storing it here lets `reset` re-apply it - // so cancellation survives session replacement. - cancel_hook: ?Session.CancelHook, /// Leaves the browser's isolate entered, like `Browser.init`; callers /// sharing one thread between several isolates park it with /// `exitIsolate` afterwards. pub fn init(self: *ToolSession, app: *App) !void { - self.cancel_hook = null; - self.notification = try Notification.init(app.allocator); errdefer self.notification.deinit(); @@ -206,21 +200,7 @@ pub const ToolSession = struct { try self.browser.init(app, .{}, null); errdefer self.browser.deinit(); - try self.reset(); - } - - /// Install a cancellation probe on this and every future session. - pub fn setCancelHook(self: *ToolSession, hook: Session.CancelHook) void { - self.cancel_hook = hook; - self.session.cancel_hook = hook; - } - - /// Replace the browsing session with a fresh one (`Browser.newSession` - /// closes the old one, cookies and all); the stored cancel hook is - /// re-applied. - pub fn reset(self: *ToolSession) !void { self.session = try self.browser.newSession(self.notification); - self.session.cancel_hook = self.cancel_hook; try self.session.enableConsoleCapture(); }