From 03b999c592759eadc9b06c133837ce25b70e0b2f Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Mon, 2 Mar 2026 10:24:07 +0800 Subject: [PATCH 01/10] Remove redundant CDP v8 shutdown https://github.com/lightpanda-io/browser/pull/1614 improved our shutdown behavior so that microtasks associated with a context wouldn't fire after the context was disposed of. This involved having context-specific microtasks, pumping the message loop, and prevent re-entry. The shutdown code in CDP already had much of this behavior built-in, but it has now become redundant. Most importantly the CDP shutdown logic did not prevent re-entry. Removing this code fixes a flaky WPT crash. I didn't seem to be tied to a specific test, but rather a cross-context/page use-after-free that was saw prior to 1614. I could reproduce it reliably by running `/wasm/core/`. I'll be honest, it isn't clear to me why _removing_ the CDP cleanup helps. Running the message loop and microtask _before_ our normal shutdown might be unnecessary, but why would it crash? I don't know, but the CDP path is slightly different in that it also involves Inspector shutdown. So there's still something about this flow I don't quite understand. And, at least for this case the current flow seems "correct". --- src/cdp/cdp.zig | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/cdp/cdp.zig b/src/cdp/cdp.zig index a4faaa17..6d7c31e5 100644 --- a/src/cdp/cdp.zig +++ b/src/cdp/cdp.zig @@ -438,15 +438,10 @@ pub fn BrowserContext(comptime CDP_T: type) type { const browser = &self.cdp.browser; const env = &browser.env; - // Drain microtasks makes sure we don't have inspector's callback - // in progress before deinit. - env.runMicrotasks(); - // resetContextGroup detach the inspector from all contexts. - // It append async tasks, so we make sure we run the message loop + // It appends async tasks, so we make sure we run the message loop // before deinit it. env.inspector.?.resetContextGroup(); - _ = env.pumpMessageLoop(); env.inspector.?.stopSession(); // abort all intercepted requests before closing the sesion/page From 82e3f126ff38d4401bd806465c25fff724f1467e Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Mon, 2 Mar 2026 11:44:42 +0800 Subject: [PATCH 02/10] Protect against transfer.abort() being called during callback This was already handled in most cases, but not for a body-less response. It's safe to call transfer.abort() during a callback, so long as the performing flag is set to true. This was set during the normal libcurl callbacks, but for a body-less response, we manually invoke the header_done_callback and were not setting the performing flag. --- src/browser/tests/net/xhr.html | 31 +++++++++++++++++++++++++++ src/http/Client.zig | 39 +++++++++++++++++----------------- src/testing.zig | 8 +++++++ 3 files changed, 59 insertions(+), 19 deletions(-) diff --git a/src/browser/tests/net/xhr.html b/src/browser/tests/net/xhr.html index 7dc89a23..64fac5c3 100644 --- a/src/browser/tests/net/xhr.html +++ b/src/browser/tests/net/xhr.html @@ -252,3 +252,34 @@ testing.expectEqual(XMLHttpRequest.UNSENT, req.readyState); }); + + + diff --git a/src/http/Client.zig b/src/http/Client.zig index 1670883c..db3ec161 100644 --- a/src/http/Client.zig +++ b/src/http/Client.zig @@ -789,25 +789,30 @@ fn processMessages(self: *Client) !bool { if (msg.err) |err| { requestFailed(transfer, err, true); } else blk: { - // In case of request w/o data, we need to call the header done - // callback now. - if (!transfer._header_done_called) { - const proceed = transfer.headerDoneCallback(&msg.conn) catch |err| { - log.err(.http, "header_done_callback", .{ .err = err }); + { + self.handles.performing = true; + defer self.handles.performing = false; + + // In case of request w/o data, we need to call the header done + // callback now. + if (!transfer._header_done_called) { + const proceed = transfer.headerDoneCallback(&msg.conn) catch |err| { + log.err(.http, "header_done_callback", .{ .err = err }); + requestFailed(transfer, err, true); + continue; + }; + if (!proceed) { + requestFailed(transfer, error.Abort, true); + break :blk; + } + } + transfer.req.done_callback(transfer.ctx) catch |err| { + // transfer isn't valid at this point, don't use it. + log.err(.http, "done_callback", .{ .err = err }); requestFailed(transfer, err, true); continue; }; - if (!proceed) { - requestFailed(transfer, error.Abort, true); - break :blk; - } } - transfer.req.done_callback(transfer.ctx) catch |err| { - // transfer isn't valid at this point, don't use it. - log.err(.http, "done_callback", .{ .err = err }); - requestFailed(transfer, err, true); - continue; - }; transfer.req.notification.dispatch(.http_request_done, &.{ .transfer = transfer, @@ -1041,10 +1046,6 @@ pub const Transfer = struct { pub fn abort(self: *Transfer, err: anyerror) void { requestFailed(self, err, true); - if (self._conn == null) { - self.deinit(); - return; - } const client = self.client; if (client.handles.performing) { diff --git a/src/testing.zig b/src/testing.zig index 62ec8870..16b06a35 100644 --- a/src/testing.zig +++ b/src/testing.zig @@ -561,6 +561,14 @@ fn testHTTPHandler(req: *std.http.Server.Request) !void { }); } + if (std.mem.eql(u8, path, "/xhr_empty")) { + return req.respond("", .{ + .extra_headers = &.{ + .{ .name = "Content-Type", .value = "text/html; charset=utf-8" }, + }, + }); + } + if (std.mem.eql(u8, path, "/xhr/json")) { return req.respond("{\"over\":\"9000!!!\",\"updated_at\":1765867200000}", .{ .extra_headers = &.{ From b104c3bfe889355ffa070be31345fdd2c506fc19 Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Mon, 2 Mar 2026 12:04:02 +0800 Subject: [PATCH 03/10] Don't start request during callback Fixes a separate but similar issue to https://github.com/lightpanda-io/browser/pull/1689 Specifically, it prevents starting a request from within a libcurl handler, thus avoiding an illegal recursive call. (This commit also removes the failed function call debug logging for DOMExceptions, as these aren't particularly abnormal / log-worthy) --- src/browser/js/Caller.zig | 12 ++++++++---- src/http/Client.zig | 6 ++++-- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/src/browser/js/Caller.zig b/src/browser/js/Caller.zig index 6ab014c4..5f34a5b9 100644 --- a/src/browser/js/Caller.zig +++ b/src/browser/js/Caller.zig @@ -328,9 +328,13 @@ fn nameToString(local: *const Local, comptime T: type, name: *const v8.Name) !T fn handleError(comptime T: type, comptime F: type, local: *const Local, err: anyerror, info: anytype, comptime opts: CallOpts) void { const isolate = local.isolate; - if (comptime @import("builtin").mode == .Debug and @TypeOf(info) == FunctionCallbackInfo) { - if (log.enabled(.js, .warn)) { - logFunctionCallError(local, @typeName(T), @typeName(F), err, info); + if (comptime IS_DEBUG and @TypeOf(info) == FunctionCallbackInfo) { + if (log.enabled(.js, .debug)) { + const DOMException = @import("../webapi/DOMException.zig"); + if (DOMException.fromError(err) == null) { + // This isn't a DOMException, let's log it + logFunctionCallError(local, @typeName(T), @typeName(F), err, info); + } } } @@ -360,7 +364,7 @@ fn handleError(comptime T: type, comptime F: type, local: *const Local, err: any // this can add as much as 10 seconds of compilation time. fn logFunctionCallError(local: *const Local, type_name: []const u8, func: []const u8, err: anyerror, info: FunctionCallbackInfo) void { const args_dump = serializeFunctionArgs(local, info) catch "failed to serialize args"; - log.info(.js, "function call error", .{ + log.debug(.js, "function call error", .{ .type = type_name, .func = func, .err = err, diff --git a/src/http/Client.zig b/src/http/Client.zig index db3ec161..3d8ddc3c 100644 --- a/src/http/Client.zig +++ b/src/http/Client.zig @@ -496,8 +496,10 @@ fn waitForInterceptedResponse(self: *Client, transfer: *Transfer) !bool { // cases, the interecptor is expected to call resume to continue the transfer // or transfer.abort() to abort it. fn process(self: *Client, transfer: *Transfer) !void { - if (self.handles.get()) |conn| { - return self.makeRequest(conn, transfer); + if (self.handles.performing == false) { + if (self.handles.get()) |conn| { + return self.makeRequest(conn, transfer); + } } self.queue.append(&transfer._node); From ce73f7ac5a8c27ed2bfd1a7bd1db276b9ae3e592 Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Mon, 2 Mar 2026 15:11:02 +0800 Subject: [PATCH 04/10] Attempt to improve non-DOM EventTarget dispatching There are two main ways to dispatch events, both via the EventManager: dispatch and dispatchWithFunction. dispatchWithFunction came about from having to dispatch to function callbacks in addition to event listeners. Specifically, firing the window.onload callback. Since that original design, much has changed. Most significantly, with https://github.com/lightpanda-io/browser/pull/1524 callbacks defined via attributes became properly (I hope) integrated with the event dispatching. Furthermore, the number of non-tree event targets (e.g. AbortSignal) has grown significantly. Finally, dispatching an event is DOM-based event is pretty complex, involving multiple phases and capturing the path. The current design is largely correct, but non-obvious. This commit attempts to improve the ergonomics of event dispatching. `dispatchWithFunction` has been renamed to `dispatchDirect`. This function is meant to be used with non-DOM event targets. It is optimized for having an event path with a single target andh no bubbling/capture phase. In addition to being a little more streamlined, `dispatchDirect` will internally turn a `js.Function.Global` or `js.Function.Temp` into a local. This makes the callsite simpler, but also provides optimization opportunity - not having to create a new scope for the common case of having no callback/listener. This lays the groundwork for having a `hasDirect` guard clause at the callsite to avoid unnecessary event creation (todo in a follow up commit). `dispatch` remains unchanged. While `dispatch` is primarily meant to handle the DOM-based EventTarget, it will forward non-DOM EventTargets to `dispatchDirect`. This is necessary since JS code can call `signal.dispatchEvent(....)`. Two notes: 1 - The flow of dispatchDirect is an optimization. The spec makes no distinction between DOM and non-DOM based EventTargets. 2 - While the window (as an EventTarget) should probably be thought of as a DOM-based EventTarget, we use `dispatchDirect with it. This is because it sits at the root and thus can safely go through the faster `dispatchDirect`. --- src/browser/EventManager.zig | 188 +++++++++++++----- src/browser/Page.zig | 29 +-- src/browser/webapi/AbortController.zig | 2 +- src/browser/webapi/AbortSignal.zig | 14 +- src/browser/webapi/FileReader.zig | 28 +-- src/browser/webapi/History.zig | 4 +- src/browser/webapi/MessagePort.zig | 8 +- src/browser/webapi/Window.zig | 4 +- src/browser/webapi/element/Html.zig | 8 +- src/browser/webapi/navigation/Navigation.zig | 8 +- src/browser/webapi/net/XMLHttpRequest.zig | 45 ++--- .../webapi/net/XMLHttpRequestEventTarget.zig | 6 +- 12 files changed, 189 insertions(+), 155 deletions(-) diff --git a/src/browser/EventManager.zig b/src/browser/EventManager.zig index 1cf82bfc..aeeb2248 100644 --- a/src/browser/EventManager.zig +++ b/src/browser/EventManager.zig @@ -211,38 +211,9 @@ pub fn dispatchOpts(self: *EventManager, target: *EventTarget, event: *Event, co log.debug(.event, "eventManager.dispatch", .{ .type = event._type_string.str(), .bubbles = event._bubbles }); } - event._target = target; - event._dispatch_target = target; // Store original target for composedPath() - var was_handled = false; - - defer if (was_handled) { - var ls: js.Local.Scope = undefined; - self.page.js.localScope(&ls); - defer ls.deinit(); - ls.local.runMicrotasks(); - }; - switch (target._type) { - .node => |node| try self.dispatchNode(node, event, &was_handled, opts), - .xhr, - .window, - .abort_signal, - .media_query_list, - .message_port, - .text_track_cue, - .navigation, - .screen, - .screen_orientation, - .visual_viewport, - .file_reader, - .generic, - => { - const list = self.lookup.get(.{ - .event_target = @intFromPtr(target), - .type_string = event._type_string, - }) orelse return; - try self.dispatchAll(list, target, event, &was_handled, opts); - }, + .node => |node| try self.dispatchNode(node, event, opts), + else => try self.dispatchDirect(target, event, null, .{ .context = "dispatch" }), } } @@ -251,16 +222,22 @@ pub fn dispatchOpts(self: *EventManager, target: *EventTarget, event: *Event, co // property is just a shortcut for calling addEventListener, but they are distinct. // An event set via property cannot be removed by removeEventListener. If you // set both the property and add a listener, they both execute. -const DispatchWithFunctionOptions = struct { +const DispatchDirectOptions = struct { context: []const u8, inject_target: bool = true, }; -pub fn dispatchWithFunction(self: *EventManager, target: *EventTarget, event: *Event, function_: ?js.Function, comptime opts: DispatchWithFunctionOptions) !void { + +// Direct dispatch for non-DOM targets (Window, XHR, AbortSignal) or DOM nodes with +// property handlers. No propagation - just calls the handler and registered listeners. +// Handler can be: null, ?js.Function.Global, ?js.Function.Temp, or js.Function +pub fn dispatchDirect(self: *EventManager, target: *EventTarget, event: *Event, handler: anytype, comptime opts: DispatchDirectOptions) !void { + const page = self.page; + event.acquireRef(); - defer event.deinit(false, self.page); + defer event.deinit(false, page); if (comptime IS_DEBUG) { - log.debug(.event, "dispatchWithFunction", .{ .type = event._type_string.str(), .context = opts.context, .has_function = function_ != null }); + log.debug(.event, "dispatchDirect", .{ .type = event._type_string, .context = opts.context }); } if (comptime opts.inject_target) { @@ -269,14 +246,15 @@ pub fn dispatchWithFunction(self: *EventManager, target: *EventTarget, event: *E } var was_dispatched = false; - defer if (was_dispatched) { - var ls: js.Local.Scope = undefined; - self.page.js.localScope(&ls); - defer ls.deinit(); - ls.local.runMicrotasks(); - }; - if (function_) |func| { + var ls: js.Local.Scope = undefined; + page.js.localScope(&ls); + defer { + ls.local.runMicrotasks(); + ls.deinit(); + } + + if (getFunction(handler, &ls.local)) |func| { event._current_target = target; if (func.callWithThis(void, target, .{event})) { was_dispatched = true; @@ -286,17 +264,126 @@ pub fn dispatchWithFunction(self: *EventManager, target: *EventTarget, event: *E } } + // listeners reigstered via addEventListener const list = self.lookup.get(.{ .event_target = @intFromPtr(target), .type_string = event._type_string, }) orelse return; - try self.dispatchAll(list, target, event, &was_dispatched, .{}); + + // This is a slightly simplified version of what you'll find in dispatchPhase + // It is simpler because, for direct dispatching, we know there's no ancestors + // and only the single target phase. + + // Track dispatch depth for deferred removal + self.dispatch_depth += 1; + defer { + const dispatch_depth = self.dispatch_depth; + // Only destroy deferred listeners when we exit the outermost dispatch + if (dispatch_depth == 1) { + for (self.deferred_removals.items) |removal| { + removal.list.remove(&removal.listener.node); + self.listener_pool.destroy(removal.listener); + } + self.deferred_removals.clearRetainingCapacity(); + } else { + self.dispatch_depth = dispatch_depth - 1; + } + } + + // Use the last listener in the list as sentinel - listeners added during dispatch will be after it + const last_node = list.last orelse return; + const last_listener: *Listener = @alignCast(@fieldParentPtr("node", last_node)); + + // Iterate through the list, stopping after we've encountered the last_listener + var node = list.first; + var is_done = false; + while (node) |n| { + if (is_done) { + break; + } + + const listener: *Listener = @alignCast(@fieldParentPtr("node", n)); + is_done = (listener == last_listener); + node = n.next; + + // Skip removed listeners + if (listener.removed) { + continue; + } + + // If the listener has an aborted signal, remove it and skip + if (listener.signal) |signal| { + if (signal.getAborted()) { + self.removeListener(list, listener); + continue; + } + } + + // Remove "once" listeners BEFORE calling them so nested dispatches don't see them + if (listener.once) { + self.removeListener(list, listener); + } + + was_dispatched = true; + event._current_target = target; + + switch (listener.function) { + .value => |value| try ls.toLocal(value).callWithThis(void, target, .{event}), + .string => |string| { + const str = try page.call_arena.dupeZ(u8, string.str()); + try ls.local.eval(str, null); + }, + .object => |obj_global| { + const obj = ls.toLocal(obj_global); + if (try obj.getFunction("handleEvent")) |handleEvent| { + try handleEvent.callWithThis(void, obj, .{event}); + } + }, + } + + if (event._stop_immediate_propagation) { + return; + } + } } -fn dispatchNode(self: *EventManager, target: *Node, event: *Event, was_handled: *bool, comptime opts: DispatchOpts) !void { +fn getFunction(handler: anytype, local: *const js.Local) ?js.Function { + const T = @TypeOf(handler); + const ti = @typeInfo(T); + + if (ti == .null) { + return null; + } + if (ti == .optional) { + return getFunction(handler orelse return null, local); + } + return switch (T) { + js.Function => handler, + js.Function.Temp => local.toLocal(handler), + js.Function.Global => local.toLocal(handler), + else => @compileError("handler must be null or \\??js.Function(\\.(Temp|Global))?"), + }; +} + +fn dispatchNode(self: *EventManager, target: *Node, event: *Event, comptime opts: DispatchOpts) !void { const ShadowRoot = @import("webapi/ShadowRoot.zig"); + { + const et = target.asEventTarget(); + event._target = et; + event._dispatch_target = et; // Store original target for composedPath() + } + const page = self.page; + var was_handled = false; + + defer if (was_handled) { + var ls: js.Local.Scope = undefined; + page.js.localScope(&ls); + defer ls.deinit(); + ls.local.runMicrotasks(); + }; + const activation_state = ActivationState.create(event, target, page); // Defer runs even on early return - ensures event phase is reset @@ -374,7 +461,7 @@ fn dispatchNode(self: *EventManager, target: *Node, event: *Event, was_handled: .event_target = @intFromPtr(current_target), .type_string = event._type_string, })) |list| { - try self.dispatchPhase(list, current_target, event, was_handled, comptime .init(true, opts)); + try self.dispatchPhase(list, current_target, event, &was_handled, comptime .init(true, opts)); } } @@ -386,7 +473,7 @@ fn dispatchNode(self: *EventManager, target: *Node, event: *Event, was_handled: blk: { // Get inline handler (e.g., onclick property) for this target if (self.getInlineHandler(target_et, event)) |inline_handler| { - was_handled.* = true; + was_handled = true; event._current_target = target_et; var ls: js.Local.Scope = undefined; @@ -408,7 +495,7 @@ fn dispatchNode(self: *EventManager, target: *Node, event: *Event, was_handled: .type_string = event._type_string, .event_target = @intFromPtr(target_et), })) |list| { - try self.dispatchPhase(list, target_et, event, was_handled, comptime .init(null, opts)); + try self.dispatchPhase(list, target_et, event, &was_handled, comptime .init(null, opts)); if (event._stop_propagation) { return; } @@ -425,7 +512,7 @@ fn dispatchNode(self: *EventManager, target: *Node, event: *Event, was_handled: .type_string = event._type_string, .event_target = @intFromPtr(current_target), })) |list| { - try self.dispatchPhase(list, current_target, event, was_handled, comptime .init(false, opts)); + try self.dispatchPhase(list, current_target, event, &was_handled, comptime .init(false, opts)); } } } @@ -549,11 +636,6 @@ fn dispatchPhase(self: *EventManager, list: *std.DoublyLinkedList, current_targe } } -// Non-Node dispatching (XHR, Window without propagation) -fn dispatchAll(self: *EventManager, list: *std.DoublyLinkedList, current_target: *EventTarget, event: *Event, was_handled: *bool, comptime opts: DispatchOpts) !void { - return self.dispatchPhase(list, current_target, event, was_handled, comptime .init(null, opts)); -} - fn getInlineHandler(self: *EventManager, target: *EventTarget, event: *Event) ?js.Function.Global { const global_event_handlers = @import("webapi/global_event_handlers.zig"); const handler_type = global_event_handlers.fromEventType(event._type_string.str()) orelse return null; diff --git a/src/browser/Page.zig b/src/browser/Page.zig index e2ec7885..c1622140 100644 --- a/src/browser/Page.zig +++ b/src/browser/Page.zig @@ -131,7 +131,7 @@ _element_namespace_uris: Element.NamespaceUriLookup = .empty, /// ```js /// img.setAttribute("onload", "(() => { ... })()"); /// ``` -_element_attr_listeners: GlobalEventHandlersLookup = .empty, +_event_target_attr_listeners: GlobalEventHandlersLookup = .empty, // Blob URL registry for URL.createObjectURL/revokeObjectURL _blob_urls: std.StringHashMapUnmanaged(*Blob) = .{}, @@ -726,27 +726,23 @@ fn _documentIsComplete(self: *Page) !void { // Run load events before window.load. try self.dispatchLoad(); - var ls: JS.Local.Scope = undefined; - self.js.localScope(&ls); - defer ls.deinit(); - // Dispatch window.load event. const event = try Event.initTrusted(comptime .wrap("load"), .{}, self); // This event is weird, it's dispatched directly on the window, but // with the document as the target. event._target = self.document.asEventTarget(); - try self._event_manager.dispatchWithFunction( + try self._event_manager.dispatchDirect( self.window.asEventTarget(), event, - ls.toLocal(self.window._on_load), + self.window._on_load, .{ .inject_target = false, .context = "page load" }, ); const pageshow_event = (try PageTransitionEvent.initTrusted(comptime .wrap("pageshow"), .{}, self)).asEvent(); - try self._event_manager.dispatchWithFunction( + try self._event_manager.dispatchDirect( self.window.asEventTarget(), pageshow_event, - ls.toLocal(self.window._on_pageshow), + self.window._on_pageshow, .{ .context = "page show" }, ); @@ -3127,20 +3123,13 @@ pub fn submitForm(self: *Page, submitter_: ?*Element, form_: ?*Element.Html.Form const form_element = form.asElement(); if (submit_opts.fire_event) { - const onsubmit_handler = try form.asHtmlElement().getOnSubmit(self); const submit_event = try Event.initTrusted(comptime .wrap("submit"), .{ .bubbles = true, .cancelable = true }, self); - var ls: JS.Local.Scope = undefined; - self.js.localScope(&ls); - defer ls.deinit(); - - try self._event_manager.dispatchWithFunction( - form_element.asEventTarget(), - submit_event, - ls.toLocal(onsubmit_handler), - .{ .context = "form submit" }, - ); + // so submit_event is still valid when we check _prevent_default + submit_event.acquireRef(); + defer submit_event.deinit(false, self); + try self._event_manager.dispatch(form_element.asEventTarget(), submit_event); // If the submit event was prevented, don't submit the form if (submit_event._prevent_default) { return; diff --git a/src/browser/webapi/AbortController.zig b/src/browser/webapi/AbortController.zig index 0c22bd4d..9e26c4ad 100644 --- a/src/browser/webapi/AbortController.zig +++ b/src/browser/webapi/AbortController.zig @@ -38,7 +38,7 @@ pub fn getSignal(self: *const AbortController) *AbortSignal { } pub fn abort(self: *AbortController, reason_: ?js.Value.Global, page: *Page) !void { - try self._signal.abort(if (reason_) |r| .{ .js_val = r } else null, page.js.local.?, page); + try self._signal.abort(if (reason_) |r| .{ .js_val = r } else null, page); } pub const JsApi = struct { diff --git a/src/browser/webapi/AbortSignal.zig b/src/browser/webapi/AbortSignal.zig index 186a6cad..de685efc 100644 --- a/src/browser/webapi/AbortSignal.zig +++ b/src/browser/webapi/AbortSignal.zig @@ -57,7 +57,7 @@ pub fn asEventTarget(self: *AbortSignal) *EventTarget { return self._proto; } -pub fn abort(self: *AbortSignal, reason_: ?Reason, local: *const js.Local, page: *Page) !void { +pub fn abort(self: *AbortSignal, reason_: ?Reason, page: *Page) !void { if (self._aborted) { return; } @@ -77,10 +77,10 @@ pub fn abort(self: *AbortSignal, reason_: ?Reason, local: *const js.Local, page: // Dispatch abort event const event = try Event.initTrusted(comptime .wrap("abort"), .{}, page); - try page._event_manager.dispatchWithFunction( + try page._event_manager.dispatchDirect( self.asEventTarget(), event, - local.toLocal(self._on_abort), + self._on_abort, .{ .context = "abort signal" }, ); } @@ -88,7 +88,7 @@ pub fn abort(self: *AbortSignal, reason_: ?Reason, local: *const js.Local, page: // Static method to create an already-aborted signal pub fn createAborted(reason_: ?js.Value.Global, page: *Page) !*AbortSignal { const signal = try init(page); - try signal.abort(if (reason_) |r| .{ .js_val = r } else null, page.js.local.?, page); + try signal.abort(if (reason_) |r| .{ .js_val = r } else null, page); return signal; } @@ -136,11 +136,7 @@ const TimeoutCallback = struct { fn run(ctx: *anyopaque) !?u32 { const self: *TimeoutCallback = @ptrCast(@alignCast(ctx)); - var ls: js.Local.Scope = undefined; - self.page.js.localScope(&ls); - defer ls.deinit(); - - self.signal.abort(.{ .string = "TimeoutError" }, &ls.local, self.page) catch |err| { + self.signal.abort(.{ .string = "TimeoutError" }, self.page) catch |err| { log.warn(.app, "abort signal timeout", .{ .err = err }); }; return null; diff --git a/src/browser/webapi/FileReader.zig b/src/browser/webapi/FileReader.zig index 3d189089..90e26aa0 100644 --- a/src/browser/webapi/FileReader.zig +++ b/src/browser/webapi/FileReader.zig @@ -183,12 +183,7 @@ fn readInternal(self: *FileReader, blob: *Blob, read_type: ReadType) !void { const page = self._page; - var ls: js.Local.Scope = undefined; - page.js.localScope(&ls); - defer ls.deinit(); - const local = &ls.local; - - try self.dispatch(.load_start, .{ .loaded = 0, .total = blob.getSize() }, local, page); + try self.dispatch(.load_start, .{ .loaded = 0, .total = blob.getSize() }, page); if (self._aborted) { return; } @@ -196,7 +191,7 @@ fn readInternal(self: *FileReader, blob: *Blob, read_type: ReadType) !void { // Perform the read (synchronous since data is in memory) const data = blob._slice; const size = data.len; - try self.dispatch(.progress, .{ .loaded = size, .total = size }, local, page); + try self.dispatch(.progress, .{ .loaded = size, .total = size }, page); if (self._aborted) { return; } @@ -216,8 +211,8 @@ fn readInternal(self: *FileReader, blob: *Blob, read_type: ReadType) !void { self._ready_state = .done; - try self.dispatch(.load, .{ .loaded = size, .total = size }, local, page); - try self.dispatch(.load_end, .{ .loaded = size, .total = size }, local, page); + try self.dispatch(.load, .{ .loaded = size, .total = size }, page); + try self.dispatch(.load_end, .{ .loaded = size, .total = size }, page); } pub fn abort(self: *FileReader) !void { @@ -231,17 +226,12 @@ pub fn abort(self: *FileReader) !void { const page = self._page; - var ls: js.Local.Scope = undefined; - page.js.localScope(&ls); - defer ls.deinit(); - const local = &ls.local; + try self.dispatch(.abort, null, page); - try self.dispatch(.abort, null, local, page); - - try self.dispatch(.load_end, null, local, page); + try self.dispatch(.load_end, null, page); } -fn dispatch(self: *FileReader, comptime event_type: DispatchType, progress_: ?Progress, local: *const js.Local, page: *Page) !void { +fn dispatch(self: *FileReader, comptime event_type: DispatchType, progress_: ?Progress, page: *Page) !void { const field, const typ = comptime blk: { break :blk switch (event_type) { .abort => .{ "_on_abort", "abort" }, @@ -260,10 +250,10 @@ fn dispatch(self: *FileReader, comptime event_type: DispatchType, progress_: ?Pr page, )).asEvent(); - return page._event_manager.dispatchWithFunction( + return page._event_manager.dispatchDirect( self.asEventTarget(), event, - local.toLocal(@field(self, field)), + @field(self, field), .{ .context = "FileReader " ++ typ }, ); } diff --git a/src/browser/webapi/History.zig b/src/browser/webapi/History.zig index 4e6bb348..b8819708 100644 --- a/src/browser/webapi/History.zig +++ b/src/browser/webapi/History.zig @@ -80,10 +80,10 @@ fn goInner(delta: i32, page: *Page) !void { if (entry._url) |url| { if (try page.isSameOrigin(url)) { const event = (try PopStateEvent.initTrusted(comptime .wrap("popstate"), .{ .state = entry._state.value }, page)).asEvent(); - try page._event_manager.dispatchWithFunction( + try page._event_manager.dispatchDirect( page.window.asEventTarget(), event, - page.js.toLocal(page.window._on_popstate), + page.window._on_popstate, .{ .context = "Pop State" }, ); } diff --git a/src/browser/webapi/MessagePort.zig b/src/browser/webapi/MessagePort.zig index ffddf30c..dfe031f7 100644 --- a/src/browser/webapi/MessagePort.zig +++ b/src/browser/webapi/MessagePort.zig @@ -131,14 +131,10 @@ const PostMessageCallback = struct { return null; }).asEvent(); - var ls: js.Local.Scope = undefined; - page.js.localScope(&ls); - defer ls.deinit(); - - page._event_manager.dispatchWithFunction( + page._event_manager.dispatchDirect( self.port.asEventTarget(), event, - ls.toLocal(self.port._on_message), + self.port._on_message, .{ .context = "MessagePort message" }, ) catch |err| { log.err(.dom, "MessagePort.postMessage", .{ .err = err }); diff --git a/src/browser/webapi/Window.zig b/src/browser/webapi/Window.zig index 394aa004..91f55b60 100644 --- a/src/browser/webapi/Window.zig +++ b/src/browser/webapi/Window.zig @@ -556,10 +556,10 @@ pub fn unhandledPromiseRejection(self: *Window, rejection: js.PromiseRejection, .promise = try rejection.promise().temp(), }, page)).asEvent(); - try page._event_manager.dispatchWithFunction( + try page._event_manager.dispatchDirect( self.asEventTarget(), event, - rejection.local.toLocal(self._on_unhandled_rejection), + self._on_unhandled_rejection, .{ .inject_target = true, .context = "window.unhandledrejection" }, ); } diff --git a/src/browser/webapi/element/Html.zig b/src/browser/webapi/element/Html.zig index 07187384..a4e6484d 100644 --- a/src/browser/webapi/element/Html.zig +++ b/src/browser/webapi/element/Html.zig @@ -380,7 +380,7 @@ pub fn getAttributeFunction( page: *Page, ) !?js.Function.Global { const element = self.asElement(); - if (page._element_attr_listeners.get(.{ .target = element.asEventTarget(), .handler = listener_type })) |cached| { + if (page._event_target_attr_listeners.get(.{ .target = element.asEventTarget(), .handler = listener_type })) |cached| { return cached; } @@ -404,7 +404,7 @@ pub fn getAttributeFunction( } pub fn hasAttributeFunction(self: *HtmlElement, listener_type: GlobalEventHandler, page: *const Page) bool { - return page._element_attr_listeners.contains(.{ .target = self.asEventTarget(), .handler = listener_type }); + return page._event_target_attr_listeners.contains(.{ .target = self.asEventTarget(), .handler = listener_type }); } fn setAttributeListener( @@ -421,7 +421,7 @@ fn setAttributeListener( } if (listener_callback) |cb| { - try page._element_attr_listeners.put(page.arena, .{ + try page._event_target_attr_listeners.put(page.arena, .{ .target = self.asEventTarget(), .handler = listener_type, }, cb); @@ -429,7 +429,7 @@ fn setAttributeListener( } // The listener is null, remove existing listener. - _ = page._element_attr_listeners.remove(.{ + _ = page._event_target_attr_listeners.remove(.{ .target = self.asEventTarget(), .handler = listener_type, }); diff --git a/src/browser/webapi/navigation/Navigation.zig b/src/browser/webapi/navigation/Navigation.zig index 6facfee9..e81ff46e 100644 --- a/src/browser/webapi/navigation/Navigation.zig +++ b/src/browser/webapi/navigation/Navigation.zig @@ -440,14 +440,10 @@ pub fn updateCurrentEntry(self: *Navigation, options: UpdateCurrentEntryOptions, } pub fn dispatch(self: *Navigation, func: js.Function.Global, event: *Event, page: *Page) !void { - var ls: js.Local.Scope = undefined; - page.js.localScope(&ls); - defer ls.deinit(); - - return page._event_manager.dispatchWithFunction( + return page._event_manager.dispatchDirect( self.asEventTarget(), event, - ls.toLocal(func), + func, .{ .context = "Navigation" }, ); } diff --git a/src/browser/webapi/net/XMLHttpRequest.zig b/src/browser/webapi/net/XMLHttpRequest.zig index 8e839ff6..0eb32cad 100644 --- a/src/browser/webapi/net/XMLHttpRequest.zig +++ b/src/browser/webapi/net/XMLHttpRequest.zig @@ -184,7 +184,7 @@ pub fn open(self: *XMLHttpRequest, method_: []const u8, url: [:0]const u8) !void const page = self._page; self._method = try parseMethod(method_); self._url = try URL.resolve(self._arena, page.base(), url, .{ .always_dupe = true, .encode = true }); - try self.stateChanged(.opened, page.js.local.?, page); + try self.stateChanged(.opened, page); } pub fn setRequestHeader(self: *XMLHttpRequest, name: []const u8, value: []const u8, page: *Page) !void { @@ -397,11 +397,10 @@ fn httpHeaderDoneCallback(transfer: *Http.Transfer) !bool { var ls: js.Local.Scope = undefined; page.js.localScope(&ls); defer ls.deinit(); - const local = &ls.local; - try self.stateChanged(.headers_received, local, page); - try self._proto.dispatch(.load_start, .{ .loaded = 0, .total = self._response_len orelse 0 }, local, page); - try self.stateChanged(.loading, local, page); + try self.stateChanged(.headers_received, page); + try self._proto.dispatch(.load_start, .{ .loaded = 0, .total = self._response_len orelse 0 }, page); + try self.stateChanged(.loading, page); return true; } @@ -412,14 +411,10 @@ fn httpDataCallback(transfer: *Http.Transfer, data: []const u8) !void { const page = self._page; - var ls: js.Local.Scope = undefined; - page.js.localScope(&ls); - defer ls.deinit(); - try self._proto.dispatch(.progress, .{ .total = self._response_len orelse 0, .loaded = self._response_data.items.len, - }, &ls.local, page); + }, page); } fn httpDoneCallback(ctx: *anyopaque) !void { @@ -438,22 +433,17 @@ fn httpDoneCallback(ctx: *anyopaque) !void { const page = self._page; - var ls: js.Local.Scope = undefined; - page.js.localScope(&ls); - defer ls.deinit(); - const local = &ls.local; - - try self.stateChanged(.done, local, page); + try self.stateChanged(.done, page); const loaded = self._response_data.items.len; try self._proto.dispatch(.load, .{ .total = loaded, .loaded = loaded, - }, local, page); + }, page); try self._proto.dispatch(.load_end, .{ .total = loaded, .loaded = loaded, - }, local, page); + }, page); page.js.weakRef(self); } @@ -495,17 +485,12 @@ fn _handleError(self: *XMLHttpRequest, err: anyerror) !void { if (new_state != self._ready_state) { const page = self._page; - var ls: js.Local.Scope = undefined; - page.js.localScope(&ls); - defer ls.deinit(); - const local = &ls.local; - - try self.stateChanged(new_state, local, page); + try self.stateChanged(new_state, page); if (is_abort) { - try self._proto.dispatch(.abort, null, local, page); + try self._proto.dispatch(.abort, null, page); } - try self._proto.dispatch(.err, null, local, page); - try self._proto.dispatch(.load_end, null, local, page); + try self._proto.dispatch(.err, null, page); + try self._proto.dispatch(.load_end, null, page); } const level: log.Level = if (err == error.Abort) .debug else .err; @@ -516,7 +501,7 @@ fn _handleError(self: *XMLHttpRequest, err: anyerror) !void { }); } -fn stateChanged(self: *XMLHttpRequest, state: ReadyState, local: *const js.Local, page: *Page) !void { +fn stateChanged(self: *XMLHttpRequest, state: ReadyState, page: *Page) !void { if (state == self._ready_state) { return; } @@ -524,10 +509,10 @@ fn stateChanged(self: *XMLHttpRequest, state: ReadyState, local: *const js.Local self._ready_state = state; const event = try Event.initTrusted(.wrap("readystatechange"), .{}, page); - try page._event_manager.dispatchWithFunction( + try page._event_manager.dispatchDirect( self.asEventTarget(), event, - local.toLocal(self._on_ready_state_change), + self._on_ready_state_change, .{ .context = "XHR state change" }, ); } diff --git a/src/browser/webapi/net/XMLHttpRequestEventTarget.zig b/src/browser/webapi/net/XMLHttpRequestEventTarget.zig index ae268a59..ad51c10e 100644 --- a/src/browser/webapi/net/XMLHttpRequestEventTarget.zig +++ b/src/browser/webapi/net/XMLHttpRequestEventTarget.zig @@ -43,7 +43,7 @@ pub fn asEventTarget(self: *XMLHttpRequestEventTarget) *EventTarget { return self._proto; } -pub fn dispatch(self: *XMLHttpRequestEventTarget, comptime event_type: DispatchType, progress_: ?Progress, local: *const js.Local, page: *Page) !void { +pub fn dispatch(self: *XMLHttpRequestEventTarget, comptime event_type: DispatchType, progress_: ?Progress, page: *Page) !void { const field, const typ = comptime blk: { break :blk switch (event_type) { .abort => .{ "_on_abort", "abort" }, @@ -63,10 +63,10 @@ pub fn dispatch(self: *XMLHttpRequestEventTarget, comptime event_type: DispatchT page, )).asEvent(); - return page._event_manager.dispatchWithFunction( + return page._event_manager.dispatchDirect( self.asEventTarget(), event, - local.toLocal(@field(self, field)), + @field(self, field), .{ .context = "XHR " ++ typ }, ); } From 5c228ae0a10628925c97538247ae7242d2bcfeb0 Mon Sep 17 00:00:00 2001 From: Pierre Tachoire Date: Mon, 2 Mar 2026 08:58:03 +0100 Subject: [PATCH 05/10] adjust WPT timeout on CI --- .github/workflows/wpt.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/wpt.yml b/.github/workflows/wpt.yml index 38405591..361196eb 100644 --- a/.github/workflows/wpt.yml +++ b/.github/workflows/wpt.yml @@ -45,7 +45,7 @@ jobs: name: build wpt runner runs-on: ubuntu-latest - timeout-minutes: 90 + timeout-minutes: 15 steps: - uses: actions/checkout@v6 @@ -73,7 +73,7 @@ jobs: # use a self host runner. runs-on: lpd-bench-hetzner - timeout-minutes: 90 + timeout-minutes: 120 steps: - uses: actions/checkout@v6 From 10ad5d763e48ad3a60f8c49d000fce5ae0df3d2a Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Mon, 2 Mar 2026 16:21:29 +0800 Subject: [PATCH 06/10] Rename page.id to page._frame_id This field was recently added and is used to generate correct frameIds in CDP messages. They remain the same during a navigation event, so calling them page.id might cause surprises since navigation events create new pages, but retain the original id. Hence, frame_id is more accurate and hopefully less surprising. (This is a small cleanup prior to doing some iframe navigation work). --- src/Notification.zig | 40 +++++++++++------------ src/browser/Page.zig | 28 ++++++++-------- src/browser/ScriptManager.zig | 6 ++-- src/browser/Session.zig | 28 ++++++++-------- src/browser/webapi/net/Fetch.zig | 2 +- src/browser/webapi/net/XMLHttpRequest.zig | 2 +- src/cdp/domains/fetch.zig | 4 +-- src/cdp/domains/network.zig | 8 ++--- src/cdp/domains/page.zig | 14 ++++---- src/cdp/domains/target.zig | 4 +-- src/http/Client.zig | 4 +-- 11 files changed, 70 insertions(+), 70 deletions(-) diff --git a/src/Notification.zig b/src/Notification.zig index d01492c8..186cc04e 100644 --- a/src/Notification.zig +++ b/src/Notification.zig @@ -105,7 +105,7 @@ pub const PageRemove = struct {}; pub const PageNavigate = struct { req_id: u32, - page_id: u32, + frame_id: u32, timestamp: u64, url: [:0]const u8, opts: Page.NavigateOpts, @@ -113,7 +113,7 @@ pub const PageNavigate = struct { pub const PageNavigated = struct { req_id: u32, - page_id: u32, + frame_id: u32, timestamp: u64, url: [:0]const u8, opts: Page.NavigatedOpts, @@ -121,18 +121,18 @@ pub const PageNavigated = struct { pub const PageNetworkIdle = struct { req_id: u32, - page_id: u32, + frame_id: u32, timestamp: u64, }; pub const PageNetworkAlmostIdle = struct { req_id: u32, - page_id: u32, + frame_id: u32, timestamp: u64, }; pub const PageFrameCreated = struct { - page_id: u32, + frame_id: u32, parent_id: u32, timestamp: u64, }; @@ -319,7 +319,7 @@ test "Notification" { // noop notifier.dispatch(.page_navigate, &.{ - .page_id = 0, + .frame_id = 0, .req_id = 1, .timestamp = 4, .url = undefined, @@ -330,7 +330,7 @@ test "Notification" { try notifier.register(.page_navigate, &tc, TestClient.pageNavigate); notifier.dispatch(.page_navigate, &.{ - .page_id = 0, + .frame_id = 0, .req_id = 1, .timestamp = 4, .url = undefined, @@ -340,7 +340,7 @@ test "Notification" { notifier.unregisterAll(&tc); notifier.dispatch(.page_navigate, &.{ - .page_id = 0, + .frame_id = 0, .req_id = 1, .timestamp = 10, .url = undefined, @@ -351,25 +351,25 @@ test "Notification" { try notifier.register(.page_navigate, &tc, TestClient.pageNavigate); try notifier.register(.page_navigated, &tc, TestClient.pageNavigated); notifier.dispatch(.page_navigate, &.{ - .page_id = 0, + .frame_id = 0, .req_id = 1, .timestamp = 10, .url = undefined, .opts = .{}, }); - notifier.dispatch(.page_navigated, &.{ .page_id = 0, .req_id = 1, .timestamp = 6, .url = undefined, .opts = .{} }); + notifier.dispatch(.page_navigated, &.{ .frame_id = 0, .req_id = 1, .timestamp = 6, .url = undefined, .opts = .{} }); try testing.expectEqual(14, tc.page_navigate); try testing.expectEqual(6, tc.page_navigated); notifier.unregisterAll(&tc); notifier.dispatch(.page_navigate, &.{ - .page_id = 0, + .frame_id = 0, .req_id = 1, .timestamp = 100, .url = undefined, .opts = .{}, }); - notifier.dispatch(.page_navigated, &.{ .page_id = 0, .req_id = 1, .timestamp = 100, .url = undefined, .opts = .{} }); + notifier.dispatch(.page_navigated, &.{ .frame_id = 0, .req_id = 1, .timestamp = 100, .url = undefined, .opts = .{} }); try testing.expectEqual(14, tc.page_navigate); try testing.expectEqual(6, tc.page_navigated); @@ -377,27 +377,27 @@ test "Notification" { // unregister try notifier.register(.page_navigate, &tc, TestClient.pageNavigate); try notifier.register(.page_navigated, &tc, TestClient.pageNavigated); - notifier.dispatch(.page_navigate, &.{ .page_id = 0, .req_id = 1, .timestamp = 100, .url = undefined, .opts = .{} }); - notifier.dispatch(.page_navigated, &.{ .page_id = 0, .req_id = 1, .timestamp = 1000, .url = undefined, .opts = .{} }); + notifier.dispatch(.page_navigate, &.{ .frame_id = 0, .req_id = 1, .timestamp = 100, .url = undefined, .opts = .{} }); + notifier.dispatch(.page_navigated, &.{ .frame_id = 0, .req_id = 1, .timestamp = 1000, .url = undefined, .opts = .{} }); try testing.expectEqual(114, tc.page_navigate); try testing.expectEqual(1006, tc.page_navigated); notifier.unregister(.page_navigate, &tc); - notifier.dispatch(.page_navigate, &.{ .page_id = 0, .req_id = 1, .timestamp = 100, .url = undefined, .opts = .{} }); - notifier.dispatch(.page_navigated, &.{ .page_id = 0, .req_id = 1, .timestamp = 1000, .url = undefined, .opts = .{} }); + notifier.dispatch(.page_navigate, &.{ .frame_id = 0, .req_id = 1, .timestamp = 100, .url = undefined, .opts = .{} }); + notifier.dispatch(.page_navigated, &.{ .frame_id = 0, .req_id = 1, .timestamp = 1000, .url = undefined, .opts = .{} }); try testing.expectEqual(114, tc.page_navigate); try testing.expectEqual(2006, tc.page_navigated); notifier.unregister(.page_navigated, &tc); - notifier.dispatch(.page_navigate, &.{ .page_id = 0, .req_id = 1, .timestamp = 100, .url = undefined, .opts = .{} }); - notifier.dispatch(.page_navigated, &.{ .page_id = 0, .req_id = 1, .timestamp = 1000, .url = undefined, .opts = .{} }); + notifier.dispatch(.page_navigate, &.{ .frame_id = 0, .req_id = 1, .timestamp = 100, .url = undefined, .opts = .{} }); + notifier.dispatch(.page_navigated, &.{ .frame_id = 0, .req_id = 1, .timestamp = 1000, .url = undefined, .opts = .{} }); try testing.expectEqual(114, tc.page_navigate); try testing.expectEqual(2006, tc.page_navigated); // already unregistered, try anyways notifier.unregister(.page_navigated, &tc); - notifier.dispatch(.page_navigate, &.{ .page_id = 0, .req_id = 1, .timestamp = 100, .url = undefined, .opts = .{} }); - notifier.dispatch(.page_navigated, &.{ .page_id = 0, .req_id = 1, .timestamp = 1000, .url = undefined, .opts = .{} }); + notifier.dispatch(.page_navigate, &.{ .frame_id = 0, .req_id = 1, .timestamp = 100, .url = undefined, .opts = .{} }); + notifier.dispatch(.page_navigated, &.{ .frame_id = 0, .req_id = 1, .timestamp = 1000, .url = undefined, .opts = .{} }); try testing.expectEqual(114, tc.page_navigate); try testing.expectEqual(2006, tc.page_navigated); } diff --git a/src/browser/Page.zig b/src/browser/Page.zig index e2ec7885..5e0e4110 100644 --- a/src/browser/Page.zig +++ b/src/browser/Page.zig @@ -81,7 +81,7 @@ const Page = @This(); // This is the "id" of the frame. It can be re-used from page-to-page, e.g. // when navigating. -id: u32, +_frame_id: u32, _session: *Session, @@ -242,7 +242,7 @@ _type: enum { root, frame }, // only used for logs right now _req_id: u32 = 0, _navigated_options: ?NavigatedOpts = null, -pub fn init(self: *Page, id: u32, session: *Session, parent: ?*Page) !void { +pub fn init(self: *Page, frame_id: u32, session: *Session, parent: ?*Page) !void { if (comptime IS_DEBUG) { log.debug(.page, "page.init", .{}); } @@ -262,7 +262,6 @@ pub fn init(self: *Page, id: u32, session: *Session, parent: ?*Page) !void { })).asDocument(); self.* = .{ - .id = id, .js = undefined, .parent = parent, .arena = page_arena, @@ -270,6 +269,7 @@ pub fn init(self: *Page, id: u32, session: *Session, parent: ?*Page) !void { .window = undefined, .arena_pool = arena_pool, .call_arena = call_arena, + ._frame_id = frame_id, ._session = session, ._factory = factory, ._pending_loads = 1, // always 1 for the ScriptManager @@ -465,7 +465,7 @@ pub fn navigate(self: *Page, request_url: [:0]const u8, opts: NavigateOpts) !voi self.documentIsComplete(); session.notification.dispatch(.page_navigate, &.{ - .page_id = self.id, + .frame_id = self._frame_id, .req_id = req_id, .opts = opts, .url = request_url, @@ -481,7 +481,7 @@ pub fn navigate(self: *Page, request_url: [:0]const u8, opts: NavigateOpts) !voi }); session.notification.dispatch(.page_navigated, &.{ - .page_id = self.id, + .frame_id = self._frame_id, .req_id = req_id, .opts = .{ .cdp_id = opts.cdp_id, @@ -517,7 +517,7 @@ pub fn navigate(self: *Page, request_url: [:0]const u8, opts: NavigateOpts) !voi // We dispatch page_navigate event before sending the request. // It ensures the event page_navigated is not dispatched before this one. session.notification.dispatch(.page_navigate, &.{ - .page_id = self.id, + .frame_id = self._frame_id, .req_id = req_id, .opts = opts, .url = self.url, @@ -535,7 +535,7 @@ pub fn navigate(self: *Page, request_url: [:0]const u8, opts: NavigateOpts) !voi http_client.request(.{ .ctx = self, .url = self.url, - .page_id = self.id, + .frame_id = self._frame_id, .method = opts.method, .headers = headers, .body = opts.body, @@ -712,7 +712,7 @@ pub fn documentIsComplete(self: *Page) void { } self._session.notification.dispatch(.page_navigated, &.{ - .page_id = self.id, + .frame_id = self._frame_id, .req_id = self._req_id, .opts = self._navigated_options.?, .url = self.url, @@ -968,17 +968,17 @@ pub fn iframeAddedCallback(self: *Page, iframe: *Element.Html.IFrame) !void { iframe._executed = true; const session = self._session; - const page_id = session.nextPageId(); + const frame_id = session.nextFrameId(); const page_frame = try self.arena.create(Page); - try Page.init(page_frame, page_id, session, self); + try Page.init(page_frame, frame_id, session, self); self._pending_loads += 1; page_frame.iframe = iframe; iframe._content_window = page_frame.window; self._session.notification.dispatch(.page_frame_created, &.{ - .page_id = page_id, - .parent_id = self.id, + .frame_id = frame_id, + .parent_id = self._frame_id, .timestamp = timestamp(.monotonic), }); @@ -1331,8 +1331,8 @@ pub fn deliverSlotchangeEvents(self: *Page) void { pub fn notifyNetworkIdle(self: *Page) void { lp.assert(self._notified_network_idle == .done, "Page.notifyNetworkIdle", .{}); self._session.notification.dispatch(.page_network_idle, &.{ - .page_id = self.id, .req_id = self._req_id, + .frame_id = self._frame_id, .timestamp = timestamp(.monotonic), }); } @@ -1340,8 +1340,8 @@ pub fn notifyNetworkIdle(self: *Page) void { pub fn notifyNetworkAlmostIdle(self: *Page) void { lp.assert(self._notified_network_almost_idle == .done, "Page.notifyNetworkAlmostIdle", .{}); self._session.notification.dispatch(.page_network_almost_idle, &.{ - .page_id = self.id, .req_id = self._req_id, + .frame_id = self._frame_id, .timestamp = timestamp(.monotonic), }); } diff --git a/src/browser/ScriptManager.zig b/src/browser/ScriptManager.zig index 61d4aef0..4b5316e7 100644 --- a/src/browser/ScriptManager.zig +++ b/src/browser/ScriptManager.zig @@ -265,7 +265,7 @@ pub fn addFromElement(self: *ScriptManager, comptime from_parser: bool, script_e .url = url, .ctx = script, .method = .GET, - .page_id = page.id, + .frame_id = page._frame_id, .headers = try self.getHeaders(url), .blocking = is_blocking, .cookie_jar = &page._session.cookie_jar, @@ -384,7 +384,7 @@ pub fn preloadImport(self: *ScriptManager, url: [:0]const u8, referrer: []const .url = url, .ctx = script, .method = .GET, - .page_id = page.id, + .frame_id = page._frame_id, .headers = try self.getHeaders(url), .cookie_jar = &page._session.cookie_jar, .resource_type = .script, @@ -487,7 +487,7 @@ pub fn getAsyncImport(self: *ScriptManager, url: [:0]const u8, cb: ImportAsync.C try self.client.request(.{ .url = url, .method = .GET, - .page_id = page.id, + .frame_id = page._frame_id, .headers = try self.getHeaders(url), .ctx = script, .resource_type = .script, diff --git a/src/browser/Session.zig b/src/browser/Session.zig index 540ba520..0cf17d70 100644 --- a/src/browser/Session.zig +++ b/src/browser/Session.zig @@ -54,7 +54,7 @@ navigation: Navigation, page: ?Page, -page_id_gen: u32, +frame_id_gen: u32, pub fn init(self: *Session, browser: *Browser, notification: *Notification) !void { const allocator = browser.app.allocator; @@ -65,7 +65,7 @@ pub fn init(self: *Session, browser: *Browser, notification: *Notification) !voi .page = null, .arena = arena, .history = .{}, - .page_id_gen = 0, + .frame_id_gen = 0, // The prototype (EventTarget) for Navigation is created when a Page is created. .navigation = .{ ._proto = undefined }, .storage_shed = .{}, @@ -93,7 +93,7 @@ pub fn createPage(self: *Session) !*Page { self.page = @as(Page, undefined); const page = &self.page.?; - try Page.init(page, self.nextPageId(), self, null); + try Page.init(page, self.nextFrameId(), self, null); // Creates a new NavigationEventTarget for this page. try self.navigation.onNewPage(page); @@ -131,7 +131,7 @@ pub fn replacePage(self: *Session) !*Page { lp.assert(self.page != null, "Session.replacePage null page", .{}); var current = self.page.?; - const page_id = current.id; + const frame_id = current._frame_id; const parent = current.parent; current.deinit(); @@ -139,7 +139,7 @@ pub fn replacePage(self: *Session) !*Page { self.page = @as(Page, undefined); const page = &self.page.?; - try Page.init(page, page_id, self, parent); + try Page.init(page, frame_id, self, parent); return page; } @@ -153,9 +153,9 @@ pub const WaitResult = enum { cdp_socket, }; -pub fn findPage(self: *Session, id: u32) ?*Page { +pub fn findPage(self: *Session, frame_id: u32) ?*Page { const page = self.currentPage() orelse return null; - return if (page.id == id) page else null; + return if (page._frame_id == frame_id) page else null; } pub fn wait(self: *Session, wait_ms: u32) WaitResult { @@ -347,20 +347,20 @@ fn processScheduledNavigation(self: *Session, current_page: *Page) !*Page { current_page._queued_navigation = null; defer browser.arena_pool.release(qn.arena); - const page_id, const parent = blk: { + const frame_id, const parent = blk: { const page = &self.page.?; - const page_id = page.id; + const frame_id = page._frame_id; const parent = page.parent; browser.http_client.abort(); self.removePage(); - break :blk .{ page_id, parent }; + break :blk .{ frame_id, parent }; }; self.page = @as(Page, undefined); const page = &self.page.?; - try Page.init(page, page_id, self, parent); + try Page.init(page, frame_id, self, parent); // Creates a new NavigationEventTarget for this page. try self.navigation.onNewPage(page); @@ -377,8 +377,8 @@ fn processScheduledNavigation(self: *Session, current_page: *Page) !*Page { return page; } -pub fn nextPageId(self: *Session) u32 { - const id = self.page_id_gen +% 1; - self.page_id_gen = id; +pub fn nextFrameId(self: *Session) u32 { + const id = self.frame_id_gen +% 1; + self.frame_id_gen = id; return id; } diff --git a/src/browser/webapi/net/Fetch.zig b/src/browser/webapi/net/Fetch.zig index 161e88c2..35fce366 100644 --- a/src/browser/webapi/net/Fetch.zig +++ b/src/browser/webapi/net/Fetch.zig @@ -72,9 +72,9 @@ pub fn init(input: Input, options: ?InitOpts, page: *Page) !js.Promise { try http_client.request(.{ .ctx = fetch, - .page_id = page.id, .url = request._url, .method = request._method, + .frame_id = page._frame_id, .body = request._body, .headers = headers, .resource_type = .fetch, diff --git a/src/browser/webapi/net/XMLHttpRequest.zig b/src/browser/webapi/net/XMLHttpRequest.zig index 8e839ff6..a5e34233 100644 --- a/src/browser/webapi/net/XMLHttpRequest.zig +++ b/src/browser/webapi/net/XMLHttpRequest.zig @@ -223,9 +223,9 @@ pub fn send(self: *XMLHttpRequest, body_: ?[]const u8) !void { try http_client.request(.{ .ctx = self, .url = self._url, - .page_id = page.id, .method = self._method, .headers = headers, + .frame_id = page._frame_id, .body = self._request_body, .cookie_jar = if (cookie_support) &page._session.cookie_jar else null, .resource_type = .xhr, diff --git a/src/cdp/domains/fetch.zig b/src/cdp/domains/fetch.zig index d6e96f2f..8a54881b 100644 --- a/src/cdp/domains/fetch.zig +++ b/src/cdp/domains/fetch.zig @@ -193,7 +193,7 @@ pub fn requestIntercept(bc: anytype, intercept: *const Notification.RequestInter try bc.cdp.sendEvent("Fetch.requestPaused", .{ .requestId = &id.toInterceptId(transfer.id), - .frameId = &id.toFrameId(transfer.req.page_id), + .frameId = &id.toFrameId(transfer.req.frame_id), .request = network.TransferAsRequestWriter.init(transfer), .resourceType = switch (transfer.req.resource_type) { .script => "Script", @@ -397,7 +397,7 @@ pub fn requestAuthRequired(bc: anytype, intercept: *const Notification.RequestAu try bc.cdp.sendEvent("Fetch.authRequired", .{ .requestId = &id.toInterceptId(transfer.id), - .frameId = &id.toFrameId(transfer.req.page_id), + .frameId = &id.toFrameId(transfer.req.frame_id), .request = network.TransferAsRequestWriter.init(transfer), .resourceType = switch (transfer.req.resource_type) { .script => "Script", diff --git a/src/cdp/domains/network.zig b/src/cdp/domains/network.zig index bad29b2d..b353dc76 100644 --- a/src/cdp/domains/network.zig +++ b/src/cdp/domains/network.zig @@ -237,8 +237,8 @@ pub fn httpRequestStart(bc: anytype, msg: *const Notification.RequestStart) !voi const transfer = msg.transfer; const req = &transfer.req; - const page_id = req.page_id; - const page = bc.session.findPage(page_id) orelse return; + const frame_id = req.frame_id; + const page = bc.session.findPage(frame_id) orelse return; // Modify request with extra CDP headers for (bc.extra_headers.items) |extra| { @@ -249,7 +249,7 @@ pub fn httpRequestStart(bc: anytype, msg: *const Notification.RequestStart) !voi try bc.cdp.sendEvent("Network.requestWillBeSent", .{ .loaderId = &id.toLoaderId(transfer.id), .requestId = &id.toRequestId(transfer.id), - .frameId = &id.toFrameId(page_id), + .frameId = &id.toFrameId(frame_id), .type = req.resource_type.string(), .documentURL = page.url, .request = TransferAsRequestWriter.init(transfer), @@ -270,7 +270,7 @@ pub fn httpResponseHeaderDone(arena: Allocator, bc: anytype, msg: *const Notific try bc.cdp.sendEvent("Network.responseReceived", .{ .loaderId = &id.toLoaderId(transfer.id), .requestId = &id.toRequestId(transfer.id), - .frameId = &id.toFrameId(transfer.req.page_id), + .frameId = &id.toFrameId(transfer.req.frame_id), .response = TransferAsResponseWriter.init(arena, msg.transfer), .hasExtraInfo = false, // TODO change after adding Network.responseReceivedExtraInfo }, .{ .session_id = session_id }); diff --git a/src/cdp/domains/page.zig b/src/cdp/domains/page.zig index 3db5d67c..6a9af8b0 100644 --- a/src/cdp/domains/page.zig +++ b/src/cdp/domains/page.zig @@ -105,7 +105,7 @@ fn setLifecycleEventsEnabled(cmd: anytype) !void { const page = bc.session.currentPage() orelse return error.PageNotLoaded; if (page._load_state == .complete) { - const frame_id = &id.toFrameId(page.id); + const frame_id = &id.toFrameId(page._frame_id); const loader_id = &id.toLoaderId(page._req_id); const now = timestampF(.monotonic); @@ -239,7 +239,7 @@ pub fn pageNavigate(bc: anytype, event: *const Notification.PageNavigate) !void const session_id = bc.session_id orelse return; bc.reset(); - const frame_id = &id.toFrameId(event.page_id); + const frame_id = &id.toFrameId(event.frame_id); const loader_id = &id.toLoaderId(event.req_id); var cdp = bc.cdp; @@ -308,7 +308,7 @@ pub fn pageFrameCreated(bc: anytype, event: *const Notification.PageFrameCreated const session_id = bc.session_id orelse return; const cdp = bc.cdp; - const frame_id = &id.toFrameId(event.page_id); + const frame_id = &id.toFrameId(event.frame_id); try cdp.sendEvent("Page.frameAttached", .{ .params = .{ .frameId = frame_id, @@ -319,7 +319,7 @@ pub fn pageFrameCreated(bc: anytype, event: *const Notification.PageFrameCreated try cdp.sendEvent("Page.lifecycleEvent", LifecycleEvent{ .name = "init", .frameId = frame_id, - .loaderId = &id.toLoaderId(event.page_id), + .loaderId = &id.toLoaderId(event.frame_id), .timestamp = event.timestamp, }, .{ .session_id = session_id }); } @@ -331,7 +331,7 @@ pub fn pageNavigated(arena: Allocator, bc: anytype, event: *const Notification.P const session_id = bc.session_id orelse return; const timestamp = event.timestamp; - const frame_id = &id.toFrameId(event.page_id); + const frame_id = &id.toFrameId(event.frame_id); const loader_id = &id.toLoaderId(event.req_id); var cdp = bc.cdp; @@ -478,11 +478,11 @@ pub fn pageNavigated(arena: Allocator, bc: anytype, event: *const Notification.P } pub fn pageNetworkIdle(bc: anytype, event: *const Notification.PageNetworkIdle) !void { - return sendPageLifecycle(bc, "networkIdle", event.timestamp, &id.toFrameId(event.page_id), &id.toLoaderId(event.req_id)); + return sendPageLifecycle(bc, "networkIdle", event.timestamp, &id.toFrameId(event.frame_id), &id.toLoaderId(event.req_id)); } pub fn pageNetworkAlmostIdle(bc: anytype, event: *const Notification.PageNetworkAlmostIdle) !void { - return sendPageLifecycle(bc, "networkAlmostIdle", event.timestamp, &id.toFrameId(event.page_id), &id.toLoaderId(event.req_id)); + return sendPageLifecycle(bc, "networkAlmostIdle", event.timestamp, &id.toFrameId(event.frame_id), &id.toLoaderId(event.req_id)); } fn sendPageLifecycle(bc: anytype, name: []const u8, timestamp: u64, frame_id: []const u8, loader_id: []const u8) !void { diff --git a/src/cdp/domains/target.zig b/src/cdp/domains/target.zig index 8a818a27..243ddc8f 100644 --- a/src/cdp/domains/target.zig +++ b/src/cdp/domains/target.zig @@ -177,7 +177,7 @@ fn createTarget(cmd: anytype) !void { const page = try bc.session.createPage(); // the target_id == the frame_id of the "root" page - const frame_id = id.toFrameId(page.id); + const frame_id = id.toFrameId(page._frame_id); bc.target_id = frame_id; const target_id = &bc.target_id.?; { @@ -421,7 +421,7 @@ fn setAutoAttach(cmd: anytype) !void { if (bc.target_id == null) { if (bc.session.currentPage()) |page| { // the target_id == the frame_id of the "root" page - bc.target_id = id.toFrameId(page.id); + bc.target_id = id.toFrameId(page._frame_id); try doAttachtoTarget(cmd, &bc.target_id.?); } } diff --git a/src/http/Client.zig b/src/http/Client.zig index 1670883c..b1380e49 100644 --- a/src/http/Client.zig +++ b/src/http/Client.zig @@ -320,7 +320,7 @@ fn fetchRobotsThenProcessRequest(self: *Client, robots_url: [:0]const u8, req: R .method = .GET, .headers = headers, .blocking = false, - .page_id = req.page_id, + .frame_id = req.frame_id, .cookie_jar = req.cookie_jar, .notification = req.notification, .resource_type = .fetch, @@ -855,7 +855,7 @@ pub const RequestCookie = struct { }; pub const Request = struct { - page_id: u32, + frame_id: u32, method: Method, url: [:0]const u8, headers: Net.Headers, From 328c681a8fa3a7a8016cc1983a0cd0392b8f285c Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Mon, 2 Mar 2026 17:29:47 +0800 Subject: [PATCH 07/10] Add transfer-specific "performing" flag In the previous commits, two separte crash resolution conspired to introduce 1 tick delay in request handling. When we're in a libcurl perform, we can't re-enter libcurl. So we added a check before processing new requests to make sure we weren't "performing" and, if we were, we'd queue the request (hence the 1 tick delay). But for another issue, we set the same "performing" check when manually triggering callbacks. This extended the situations where the above check fired thus causing the 1-tick delay to happen under more (and even common) situation. This commit improves this - instead of relying on the global "performing" check when processing 1 transfer explicitly, we now have a per-transfer performing check. This prevents the transfer from being deinitialized during a callback but does not block requests from being started immediately. --- src/http/Client.zig | 39 +++++++++++++++++++++------------------ 1 file changed, 21 insertions(+), 18 deletions(-) diff --git a/src/http/Client.zig b/src/http/Client.zig index 3d8ddc3c..6405c8ae 100644 --- a/src/http/Client.zig +++ b/src/http/Client.zig @@ -496,6 +496,8 @@ fn waitForInterceptedResponse(self: *Client, transfer: *Transfer) !bool { // cases, the interecptor is expected to call resume to continue the transfer // or transfer.abort() to abort it. fn process(self: *Client, transfer: *Transfer) !void { + // libcurl doesn't allow recursive calls, if we're in a `perform()` operation + // then we _have_ to queue this. if (self.handles.performing == false) { if (self.handles.get()) |conn| { return self.makeRequest(conn, transfer); @@ -791,30 +793,30 @@ fn processMessages(self: *Client) !bool { if (msg.err) |err| { requestFailed(transfer, err, true); } else blk: { - { - self.handles.performing = true; - defer self.handles.performing = false; + // make sure the transfer can't be immediately aborted from a callback + // since we still need it here. + transfer._performing = true; + defer transfer._performing = false; + if (!transfer._header_done_called) { // In case of request w/o data, we need to call the header done // callback now. - if (!transfer._header_done_called) { - const proceed = transfer.headerDoneCallback(&msg.conn) catch |err| { - log.err(.http, "header_done_callback", .{ .err = err }); - requestFailed(transfer, err, true); - continue; - }; - if (!proceed) { - requestFailed(transfer, error.Abort, true); - break :blk; - } - } - transfer.req.done_callback(transfer.ctx) catch |err| { - // transfer isn't valid at this point, don't use it. - log.err(.http, "done_callback", .{ .err = err }); + const proceed = transfer.headerDoneCallback(&msg.conn) catch |err| { + log.err(.http, "header_done_callback", .{ .err = err }); requestFailed(transfer, err, true); continue; }; + if (!proceed) { + requestFailed(transfer, error.Abort, true); + break :blk; + } } + transfer.req.done_callback(transfer.ctx) catch |err| { + // transfer isn't valid at this point, don't use it. + log.err(.http, "done_callback", .{ .err = err }); + requestFailed(transfer, err, true); + continue; + }; transfer.req.notification.dispatch(.http_request_done, &.{ .transfer = transfer, @@ -944,6 +946,7 @@ pub const Transfer = struct { // number of times the transfer has been tried. // incremented by reset func. _tries: u8 = 0, + _performing: bool = false, // for when a Transfer is queued in the client.queue _node: std.DoublyLinkedList.Node = .{}, @@ -1050,7 +1053,7 @@ pub const Transfer = struct { requestFailed(self, err, true); const client = self.client; - if (client.handles.performing) { + if (self._performing or client.handles.performing) { // We're currently in a curl_multi_perform. We cannot call endTransfer // as that calls curl_multi_remove_handle, and you can't do that // from a curl callback. Instead, we flag this transfer and all of From ad87573d092bd002c03471cb0760b21016823a26 Mon Sep 17 00:00:00 2001 From: Halil Durak Date: Mon, 2 Mar 2026 12:55:55 +0300 Subject: [PATCH 08/10] `ArenaPool`: make `init` configurable --- src/App.zig | 2 +- src/ArenaPool.zig | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/App.zig b/src/App.zig index a4ed0e8f..bb797ec5 100644 --- a/src/App.zig +++ b/src/App.zig @@ -66,7 +66,7 @@ pub fn init(allocator: Allocator, config: *const Config) !*App { app.telemetry = try Telemetry.init(app, config.mode); errdefer app.telemetry.deinit(); - app.arena_pool = ArenaPool.init(allocator); + app.arena_pool = ArenaPool.init(allocator, 512, 1024 * 16); errdefer app.arena_pool.deinit(); return app; diff --git a/src/ArenaPool.zig b/src/ArenaPool.zig index 7f1d38dc..6743a552 100644 --- a/src/ArenaPool.zig +++ b/src/ArenaPool.zig @@ -36,12 +36,12 @@ const Entry = struct { arena: ArenaAllocator, }; -pub fn init(allocator: Allocator) ArenaPool { +pub fn init(allocator: Allocator, free_list_max: u16, retain_bytes: usize) ArenaPool { return .{ .allocator = allocator, - .free_list_max = 512, // TODO make configurable - .retain_bytes = 1024 * 16, // TODO make configurable - .entry_pool = std.heap.MemoryPool(Entry).init(allocator), + .free_list_max = free_list_max, + .retain_bytes = retain_bytes, + .entry_pool = .init(allocator), }; } From 52c3aadd243d81864f6d696cb4f91311b4be46f1 Mon Sep 17 00:00:00 2001 From: Halil Durak Date: Mon, 2 Mar 2026 12:56:10 +0300 Subject: [PATCH 09/10] `ArenaPool`: add tests --- src/ArenaPool.zig | 111 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 111 insertions(+) diff --git a/src/ArenaPool.zig b/src/ArenaPool.zig index 6743a552..2f8de17f 100644 --- a/src/ArenaPool.zig +++ b/src/ArenaPool.zig @@ -99,3 +99,114 @@ pub fn reset(_: *const ArenaPool, allocator: Allocator, retain: usize) void { const arena: *std.heap.ArenaAllocator = @ptrCast(@alignCast(allocator.ptr)); _ = arena.reset(.{ .retain_with_limit = retain }); } + +const testing = std.testing; + +test "arena pool - basic acquire and use" { + var pool = ArenaPool.init(testing.allocator, 512, 1024 * 16); + defer pool.deinit(); + + const alloc = try pool.acquire(); + const buf = try alloc.alloc(u8, 64); + @memset(buf, 0xAB); + try testing.expectEqual(@as(u8, 0xAB), buf[0]); + + pool.release(alloc); +} + +test "arena pool - reuse entry after release" { + var pool = ArenaPool.init(testing.allocator, 512, 1024 * 16); + defer pool.deinit(); + + const alloc1 = try pool.acquire(); + try testing.expectEqual(@as(u16, 0), pool.free_list_len); + + pool.release(alloc1); + try testing.expectEqual(@as(u16, 1), pool.free_list_len); + + // The same entry should be returned from the free list. + const alloc2 = try pool.acquire(); + try testing.expectEqual(@as(u16, 0), pool.free_list_len); + try testing.expectEqual(alloc1.ptr, alloc2.ptr); + + pool.release(alloc2); +} + +test "arena pool - multiple concurrent arenas" { + var pool = ArenaPool.init(testing.allocator, 512, 1024 * 16); + defer pool.deinit(); + + const a1 = try pool.acquire(); + const a2 = try pool.acquire(); + const a3 = try pool.acquire(); + + // All three must be distinct arenas. + try testing.expect(a1.ptr != a2.ptr); + try testing.expect(a2.ptr != a3.ptr); + try testing.expect(a1.ptr != a3.ptr); + + _ = try a1.alloc(u8, 16); + _ = try a2.alloc(u8, 32); + _ = try a3.alloc(u8, 48); + + pool.release(a1); + pool.release(a2); + pool.release(a3); + + try testing.expectEqual(@as(u16, 3), pool.free_list_len); +} + +test "arena pool - free list respects max limit" { + // Cap the free list at 1 so the second release discards its arena. + var pool = ArenaPool.init(testing.allocator, 1, 1024 * 16); + defer pool.deinit(); + + const a1 = try pool.acquire(); + const a2 = try pool.acquire(); + + pool.release(a1); + try testing.expectEqual(@as(u16, 1), pool.free_list_len); + + // The free list is full; a2's arena should be destroyed, not queued. + pool.release(a2); + try testing.expectEqual(@as(u16, 1), pool.free_list_len); +} + +test "arena pool - reset clears memory without releasing" { + var pool = ArenaPool.init(testing.allocator, 512, 1024 * 16); + defer pool.deinit(); + + const alloc = try pool.acquire(); + + const buf = try alloc.alloc(u8, 128); + @memset(buf, 0xFF); + + // reset() frees arena memory but keeps the allocator in-flight. + pool.reset(alloc, 0); + + // The free list must stay empty; the allocator was not released. + try testing.expectEqual(@as(u16, 0), pool.free_list_len); + + // Allocating again through the same arena must still work. + const buf2 = try alloc.alloc(u8, 64); + @memset(buf2, 0x00); + try testing.expectEqual(@as(u8, 0x00), buf2[0]); + + pool.release(alloc); +} + +test "arena pool - deinit with entries in free list" { + // Verifies that deinit properly cleans up free-listed arenas (no leaks + // detected by the test allocator). + var pool = ArenaPool.init(testing.allocator, 512, 1024 * 16); + + const a1 = try pool.acquire(); + const a2 = try pool.acquire(); + _ = try a1.alloc(u8, 256); + _ = try a2.alloc(u8, 512); + pool.release(a1); + pool.release(a2); + try testing.expectEqual(@as(u16, 2), pool.free_list_len); + + pool.deinit(); +} From 1f81b6ddc43c060bba1a18d9cfcffbefaa7474a2 Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Mon, 2 Mar 2026 17:46:36 +0800 Subject: [PATCH 10/10] Allow frame-specific HTTP abort Needed for frame navigation. Implemented using some ugly comptime to avoid duplication and avoid an runtime frame check when doing a full abort. --- src/http/Client.zig | 62 ++++++++++++++++++++++++++++++++++----------- 1 file changed, 47 insertions(+), 15 deletions(-) diff --git a/src/http/Client.zig b/src/http/Client.zig index 242ec0aa..26284b8a 100644 --- a/src/http/Client.zig +++ b/src/http/Client.zig @@ -174,27 +174,59 @@ pub fn newHeaders(self: *const Client) !Net.Headers { } pub fn abort(self: *Client) void { - while (self.handles.in_use.first) |node| { - const conn: *Net.Connection = @fieldParentPtr("node", node); - var transfer = Transfer.fromConnection(conn) catch |err| { - log.err(.http, "get private info", .{ .err = err, .source = "abort" }); - continue; - }; - transfer.kill(); + self._abort(true, 0); +} + +pub fn abortFrame(self: *Client, frame_id: u32) void { + self._abort(false, frame_id); +} + +// Written this way so that both abort and abortFrame can share the same code +// but abort can avoid the frame_id check at comptime. +fn _abort(self: *Client, comptime abort_all: bool, frame_id: u32) void { + { + var q = &self.handles.in_use; + var n = q.first; + while (n) |node| { + n = node.next; + const conn: *Net.Connection = @fieldParentPtr("node", node); + var transfer = Transfer.fromConnection(conn) catch |err| { + log.err(.http, "get private info", .{ .err = err, .source = "abort" }); + continue; + }; + if (comptime abort_all) { + transfer.kill(); + } else if (transfer.req.frame_id == frame_id) { + q.remove(node); + transfer.kill(); + } + } } - if (comptime IS_DEBUG) { + + if (comptime IS_DEBUG and abort_all) { std.debug.assert(self.active == 0); } - var n = self.queue.first; - while (n) |node| { - n = node.next; - const transfer: *Transfer = @fieldParentPtr("_node", node); - transfer.kill(); + { + var q = &self.queue; + var n = q.first; + while (n) |node| { + n = node.next; + const transfer: *Transfer = @fieldParentPtr("_node", node); + if (comptime abort_all) { + transfer.kill(); + } else if (transfer.req.frame_id == frame_id) { + q.remove(node); + transfer.kill(); + } + } } - self.queue = .{}; - if (comptime IS_DEBUG) { + if (comptime abort_all) { + self.queue = .{}; + } + + if (comptime IS_DEBUG and abort_all) { std.debug.assert(self.handles.in_use.first == null); std.debug.assert(self.handles.available.len() == self.handles.connections.len);