From 3c0c75be10f410b6f45c36a503ae571b3c7bf392 Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Mon, 19 Jan 2026 19:09:20 +0800 Subject: [PATCH 1/7] Add XHR finalizer and ArenaPool Any object we return from Zig to V8 becomes a v8::Global that we track in our `ctx.identity_map`. V8 will not free such objects. On the flip side, on its own, our Zig code never knows if the underlying v8::Object of a global can still be used from JS. Imagine an XHR request where we fire the last readyStateChange event..we might think we no longer need that XHR instance, but nothing stops the JavaScript code from holding a reference to it and calling a property on it, e.g. `xhr.status`. What we can do is tell v8 that we're done with the global and register a callback. We make our reference to the global weak. When v8 determines that this object cannot be reached from JavaScript, it _may_ call our registered callback. We can then clean things up on our side and free the global (we actually _have_ to free the global). v8 makes no guarantee that our callback will ever be called, so we need to track these finalizable objects and free them ourselves on context shutdown. Furthermore there appears to be some possible timing issues, especially during context shutdown, so we need to be defensive and make sure we don't double-free (we can use the existing identity_map for this). An type like XMLHttpRequest can be re-used. After a request succeeds or fails, it can be re-opened and a new request sent. So we also need a way to revert a "weak" reference back into a "strong" reference. These are simple v8 calls on the v8::Global, but it highlights how sensitive all this is. We need to mark it as weak when we're 100% sure we're done with it, and we need to switch it to strong under any circumstances where we might need it again on our side. Finally, none of this makes sense if there isn't something to free. Of course, the finalizer lets us release the v8::Global, and we can free the memory for the object itself (i.e. the `*XMLHttpRequest`). This PR also adds an ArenaPool. This allows the XMLHTTPRequest to be self-contained and not need the `page.arena`. On init, the `XMLHTTPRequest` acquires an arena from the pool. On finalization it releases it back to the pool. So we now have: - page.call_arena: short, guaranteed for 1 v8 -> zig -> v8 flow - page.arena long: lives for the duration of the entire page - page.arena_pool: ideally lives for as long as needed by its instance (but no guarantees from v8 about this, or the script might leak a lot of global, so worst case, same as page.arena) --- src/App.zig | 12 +++- src/ArenaPool.zig | 84 ++++++++++++++++++++++ src/browser/Browser.zig | 8 ++- src/browser/Factory.zig | 54 ++------------ src/browser/Page.zig | 47 ++++++++++++- src/browser/js/Context.zig | 86 ++++++++++++++++++----- src/browser/js/Local.zig | 18 +++++ src/browser/js/bridge.zig | 41 +++++++++++ src/browser/webapi/net/XMLHttpRequest.zig | 23 ++++-- src/slab.zig | 1 - 10 files changed, 293 insertions(+), 81 deletions(-) create mode 100644 src/ArenaPool.zig diff --git a/src/App.zig b/src/App.zig index 38283e10..fca528cd 100644 --- a/src/App.zig +++ b/src/App.zig @@ -21,13 +21,14 @@ const std = @import("std"); const Allocator = std.mem.Allocator; const log = @import("log.zig"); -const Http = @import("http/Http.zig"); const Snapshot = @import("browser/js/Snapshot.zig"); const Platform = @import("browser/js/Platform.zig"); - -const Notification = @import("Notification.zig"); const Telemetry = @import("telemetry/telemetry.zig").Telemetry; +pub const Http = @import("http/Http.zig"); +pub const ArenaPool = @import("ArenaPool.zig"); +pub const Notification = @import("Notification.zig"); + // Container for global state / objects that various parts of the system // might need. const App = @This(); @@ -38,6 +39,7 @@ platform: Platform, snapshot: Snapshot, telemetry: Telemetry, allocator: Allocator, +arena_pool: ArenaPool, app_dir_path: ?[]const u8, notification: *Notification, shutdown: bool = false, @@ -96,6 +98,9 @@ pub fn init(allocator: Allocator, config: Config) !*App { try app.telemetry.register(app.notification); + app.arena_pool = ArenaPool.init(allocator); + errdefer app.arena_pool.deinit(); + return app; } @@ -114,6 +119,7 @@ pub fn deinit(self: *App) void { self.http.deinit(); self.snapshot.deinit(); self.platform.deinit(); + self.arena_pool.deinit(); allocator.destroy(self); } diff --git a/src/ArenaPool.zig b/src/ArenaPool.zig new file mode 100644 index 00000000..75adad36 --- /dev/null +++ b/src/ArenaPool.zig @@ -0,0 +1,84 @@ +// Copyright (C) 2023-2026 Lightpanda (Selecy SAS) +// +// Francis Bouvier +// Pierre Tachoire +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as +// published by the Free Software Foundation, either version 3 of the +// License, or (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +const std = @import("std"); + +const Allocator = std.mem.Allocator; +const ArenaAllocator = std.heap.ArenaAllocator; + +const ArenaPool = @This(); + +allocator: Allocator, +retain_bytes: usize, +free_list_len: u16 = 0, +free_list: ?*Entry = null, +free_list_max: u16, +entry_pool: std.heap.MemoryPool(Entry), + +const Entry = struct { + next: ?*Entry, + arena: ArenaAllocator, +}; + +pub fn init(allocator: Allocator) 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), + }; +} + +pub fn deinit(self: *ArenaPool) void { + var entry = self.free_list; + while (entry) |e| { + entry = e.next; + e.arena.deinit(); + } + self.entry_pool.deinit(); +} + +pub fn acquire(self: *ArenaPool) !Allocator { + if (self.free_list) |entry| { + self.free_list = entry.next; + return entry.arena.allocator(); + } + + const entry = try self.entry_pool.create(); + entry.* = .{ + .next = null, + .arena = ArenaAllocator.init(self.allocator), + }; + + return entry.arena.allocator(); +} + +pub fn release(self: *ArenaPool, allocator: Allocator) void { + const arena: *std.heap.ArenaAllocator = @ptrCast(@alignCast(allocator.ptr)); + const entry: *Entry = @fieldParentPtr("arena", arena); + + if (self.free_list_len == self.free_list_max) { + arena.deinit(); + self.entry_pool.destroy(entry); + return; + } + + _ = arena.reset(.{ .retain_with_limit = self.retain_bytes }); + entry.next = self.free_list; + self.free_list = entry; +} diff --git a/src/browser/Browser.zig b/src/browser/Browser.zig index 1a74468b..70b04429 100644 --- a/src/browser/Browser.zig +++ b/src/browser/Browser.zig @@ -24,8 +24,10 @@ const ArenaAllocator = std.heap.ArenaAllocator; const js = @import("js/js.zig"); const log = @import("../log.zig"); const App = @import("../App.zig"); -const HttpClient = @import("../http/Client.zig"); -const Notification = @import("../Notification.zig"); + +const ArenaPool = App.ArenaPool; +const HttpClient = App.Http.Client; +const Notification = App.Notification; const IS_DEBUG = @import("builtin").mode == .Debug; @@ -40,6 +42,7 @@ env: js.Env, app: *App, session: ?Session, allocator: Allocator, +arena_pool: *ArenaPool, http_client: *HttpClient, call_arena: ArenaAllocator, page_arena: ArenaAllocator, @@ -64,6 +67,7 @@ pub fn init(app: *App) !Browser { .session = null, .allocator = allocator, .notification = notification, + .arena_pool = &app.arena_pool, .http_client = app.http.client, .call_arena = ArenaAllocator.init(allocator), .page_arena = ArenaAllocator.init(allocator), diff --git a/src/browser/Factory.zig b/src/browser/Factory.zig index 60a622b4..205e70b7 100644 --- a/src/browser/Factory.zig +++ b/src/browser/Factory.zig @@ -361,32 +361,6 @@ pub fn textTrackCue(self: *Factory, child: anytype) !*@TypeOf(child) { ).create(allocator, child); } -fn hasChainRoot(comptime T: type) bool { - // Check if this is a root - if (@hasDecl(T, "_prototype_root")) { - return true; - } - - // If no _proto field, we're at the top but not a recognized root - if (!@hasField(T, "_proto")) return false; - - // Get the _proto field's type and recurse - const fields = @typeInfo(T).@"struct".fields; - inline for (fields) |field| { - if (std.mem.eql(u8, field.name, "_proto")) { - const ProtoType = reflect.Struct(field.type); - return hasChainRoot(ProtoType); - } - } - - return false; -} - -fn isChainType(comptime T: type) bool { - if (@hasField(T, "_proto")) return false; - return comptime hasChainRoot(T); -} - pub fn destroy(self: *Factory, value: anytype) void { const S = reflect.Struct(@TypeOf(value)); @@ -403,7 +377,7 @@ pub fn destroy(self: *Factory, value: anytype) void { } } - if (comptime isChainType(S)) { + if (comptime @hasField(S, "_proto")) { self.destroyChain(value, true, 0, std.mem.Alignment.@"1"); } else { self.destroyStandalone(value); @@ -411,20 +385,7 @@ pub fn destroy(self: *Factory, value: anytype) void { } pub fn destroyStandalone(self: *Factory, value: anytype) void { - const S = reflect.Struct(@TypeOf(value)); - assert(!@hasDecl(S, "_prototype_root")); - const allocator = self._slab.allocator(); - - if (@hasDecl(S, "deinit")) { - // And it has a deinit, we'll call it - switch (@typeInfo(@TypeOf(S.deinit)).@"fn".params.len) { - 1 => value.deinit(), - 2 => value.deinit(self._page), - else => @compileLog(@typeName(S) ++ " has an invalid deinit function"), - } - } - allocator.destroy(value); } @@ -440,10 +401,8 @@ fn destroyChain( // aligns the old size to the alignment of this element const current_size = std.mem.alignForward(usize, old_size, @alignOf(S)); - const alignment = std.mem.Alignment.fromByteUnits(@alignOf(S)); - - const new_align = std.mem.Alignment.max(old_align, alignment); const new_size = current_size + @sizeOf(S); + const new_align = std.mem.Alignment.max(old_align, std.mem.Alignment.of(S)); // This is initially called from a deinit. We don't want to call that // same deinit. So when this is the first time destroyChain is called @@ -462,20 +421,15 @@ fn destroyChain( if (@hasField(S, "_proto")) { self.destroyChain(value._proto, false, new_size, new_align); - } else if (@hasDecl(S, "JsApi")) { - // Doesn't have a _proto, but has a JsApi. - if (self._page.js.removeTaggedMapping(@intFromPtr(value))) |tagged| { - allocator.destroy(tagged); - } } else { // no proto so this is the head of the chain. // we use this as the ptr to the start of the chain. // and we have summed up the length. assert(@hasDecl(S, "_prototype_root")); - const memory_ptr: [*]const u8 = @ptrCast(value); + const memory_ptr: [*]u8 = @ptrCast(@constCast(value)); const len = std.mem.alignForward(usize, new_size, new_align.toByteUnits()); - allocator.free(memory_ptr[0..len]); + allocator.rawFree(memory_ptr[0..len], new_align, @returnAddress()); } } diff --git a/src/browser/Page.zig b/src/browser/Page.zig index 9348aacc..344af081 100644 --- a/src/browser/Page.zig +++ b/src/browser/Page.zig @@ -27,7 +27,7 @@ const IS_DEBUG = builtin.mode == .Debug; const log = @import("../log.zig"); -const Http = @import("../http/Http.zig"); +const App = @import("../App.zig"); const String = @import("../string.zig").String; const Mime = @import("Mime.zig"); @@ -59,6 +59,9 @@ const PageTransitionEvent = @import("webapi/event/PageTransitionEvent.zig"); const NavigationKind = @import("webapi/navigation/root.zig").NavigationKind; const KeyboardEvent = @import("webapi/event/KeyboardEvent.zig"); +const Http = App.Http; +const ArenaPool = App.ArenaPool; + const timestamp = @import("../datetime.zig").timestamp; const milliTimestamp = @import("../datetime.zig").milliTimestamp; @@ -168,6 +171,11 @@ arena: Allocator, // from JS. Best arena to use, when possible. call_arena: Allocator, +arena_pool: *ArenaPool, +// In Debug, we use this to see if anything fails to release an arena back to +// the pool. +_arena_pool_leak_track: (if (IS_DEBUG) std.AutoHashMapUnmanaged(usize, []const u8) else void), + window: *Window, document: *Document, @@ -185,10 +193,14 @@ pub fn init(arena: Allocator, call_arena: Allocator, session: *Session) !*Page { } const page = try session.browser.allocator.create(Page); + page._session = session; page.arena = arena; page.call_arena = call_arena; - page._session = session; + page.arena_pool = session.browser.arena_pool; + if (comptime IS_DEBUG) { + page._arena_pool_leak_track = .empty; + } try page.reset(true); return page; @@ -220,6 +232,14 @@ pub fn deinit(self: *Page) void { self._script_manager.shutdown = true; session.browser.http_client.abort(); self._script_manager.deinit(); + + if (comptime IS_DEBUG) { + var it = self._arena_pool_leak_track.valueIterator(); + while (it.next()) |value_ptr| { + log.err(.bug, "ArenaPool Leak", .{ .owner = value_ptr.* }); + } + } + session.browser.allocator.destroy(self); } @@ -294,6 +314,14 @@ fn reset(self: *Page, comptime initializing: bool) !void { self._undefined_custom_elements = .{}; try self.registerBackgroundTasks(); + + if (comptime IS_DEBUG) { + var it = self._arena_pool_leak_track.valueIterator(); + while (it.next()) |value_ptr| { + log.err(.bug, "ArenaPool Leak", .{ .owner = value_ptr.* }); + } + self._arena_pool_leak_track.clearRetainingCapacity(); + } } pub fn base(self: *const Page) [:0]const u8 { @@ -328,6 +356,21 @@ pub fn getOrigin(self: *Page, allocator: Allocator) !?[]const u8 { return try URL.getOrigin(allocator, self.url); } +pub fn getArena(self: *Page, comptime owner: []const u8) !Allocator { + const allocator = try self.arena_pool.acquire(); + if (comptime IS_DEBUG) { + try self._arena_pool_leak_track.put(self.arena, @intFromPtr(allocator.ptr), owner); + } + return allocator; +} + +pub fn releaseArena(self: *Page, allocator: Allocator) void { + if (comptime IS_DEBUG) { + _ = self._arena_pool_leak_track.remove(@intFromPtr(allocator.ptr)); + } + return self.arena_pool.release(allocator); +} + pub fn isSameOrigin(self: *const Page, url: [:0]const u8) !bool { const current_origin = (try URL.getOrigin(self.call_arena, self.url)) orelse return false; return std.mem.startsWith(u8, url, current_origin); diff --git a/src/browser/js/Context.zig b/src/browser/js/Context.zig index de24fdb1..497ba09e 100644 --- a/src/browser/js/Context.zig +++ b/src/browser/js/Context.zig @@ -79,6 +79,11 @@ local: ?*const js.Local = null, // The key is the @intFromPtr of the Zig value identity_map: std.AutoHashMapUnmanaged(usize, v8.Global) = .empty, +// Any type that is stored in the identity_map which has a finalizer declared +// will have its finalizer stored here. This is only used when shutting down +// if v8 hasn't called the finalizer directly itself. +finalizer_callbacks: std.AutoHashMapUnmanaged(usize, FinalizerCallback) = .empty, + // Some web APIs have to manage opaque values. Ideally, they use an // js.Object, but the js.Object has no lifetime guarantee beyond the // current call. They can call .persist() on their js.Object to get @@ -145,6 +150,12 @@ pub fn deinit(self: *Context) void { v8.v8__Global__Reset(global); } } + { + var it = self.finalizer_callbacks.valueIterator(); + while (it.next()) |finalizer| { + finalizer.deinit(); + } + } for (self.global_values.items) |*global| { v8.v8__Global__Reset(global); @@ -179,6 +190,48 @@ pub fn deinit(self: *Context) void { v8.v8__Global__Reset(&self.handle); } +pub fn weakRef(self: *Context, obj: anytype) void { + const global = self.identity_map.getPtr(@intFromPtr(obj)) orelse { + if (comptime IS_DEBUG) { + // should not be possible + std.debug.assert(false); + } + return; + }; + v8.v8__Global__SetWeakFinalizer(global, obj, bridge.Struct(@TypeOf(obj)).JsApi.Meta.finalizer.from_v8, v8.kParameter); +} + +pub fn strongRef(self: *Context, obj: anytype) void { + const global = self.identity_map.getPtr(@intFromPtr(obj)) orelse { + if (comptime IS_DEBUG) { + // should not be possible + std.debug.assert(false); + } + return; + }; + v8.v8__Global__ClearWeak(global); +} + +pub fn release(self: *Context, obj: *anyopaque) void { + var global = self.identity_map.fetchRemove(@intFromPtr(obj)) orelse { + if (comptime IS_DEBUG) { + // should not be possible + std.debug.assert(false); + } + return; + }; + v8.v8__Global__Reset(&global.value); + + // The item has been fianalized, remove it for the finalizer callback so that + // we don't try to call it again on shutdown. + _ = self.finalizer_callbacks.fetchRemove(@intFromPtr(obj)) orelse { + if (comptime IS_DEBUG) { + // should not be possible + std.debug.assert(false); + } + }; +} + // Any operation on the context have to be made from a local. pub fn localScope(self: *Context, ls: *js.Local.Scope) void { const isolate = self.isolate; @@ -793,31 +846,28 @@ pub fn queueMicrotaskFunc(self: *Context, cb: js.Function) void { } // == Misc == -// An interface for types that want to have their jsDeinit function to be -// called when the call context ends -const DestructorCallback = struct { +// A type that has a finalizer can have its finalizer called one of two ways. +// The first is from V8 via the WeakCallback we give to weakRef. But that isn't +// guaranteed to fire, so we track this in ctx._finalizers and call them on +// context shutdown. +const FinalizerCallback = struct { ptr: *anyopaque, - destructorFn: *const fn (ptr: *anyopaque) void, - - fn init(ptr: anytype) DestructorCallback { - const T = @TypeOf(ptr); - const ptr_info = @typeInfo(T); - - const gen = struct { - pub fn destructor(pointer: *anyopaque) void { - const self: T = @ptrCast(@alignCast(pointer)); - return ptr_info.pointer.child.destructor(self); - } - }; + finalizerFn: *const fn (ptr: *anyopaque) void, + pub fn init(ptr: anytype) FinalizerCallback { + const T = bridge.Struct(@TypeOf(ptr)); return .{ .ptr = ptr, - .destructorFn = gen.destructor, + .finalizerFn = struct { + pub fn wrap(self: *anyopaque) void { + T.JsApi.Meta.finalizer.from_zig(self); + } + }.wrap, }; } - pub fn destructor(self: DestructorCallback) void { - self.destructorFn(self.ptr); + pub fn deinit(self: FinalizerCallback) void { + self.finalizerFn(self.ptr); } }; diff --git a/src/browser/js/Local.zig b/src/browser/js/Local.zig index 80b885f0..b7b54857 100644 --- a/src/browser/js/Local.zig +++ b/src/browser/js/Local.zig @@ -26,6 +26,8 @@ const Context = @import("Context.zig"); const Isolate = @import("Isolate.zig"); const TaggedOpaque = @import("TaggedOpaque.zig"); +const IS_DEBUG = @import("builtin").mode == .Debug; + const v8 = js.v8; const CallOpts = Caller.CallOpts; const Allocator = std.mem.Allocator; @@ -194,6 +196,22 @@ pub fn mapZigInstanceToJs(self: *const Local, js_obj_handle: ?*const v8.Object, // dont' use js_obj.persist(), because we don't want to track this in // context.global_objects, we want to track it in context.identity_map. v8.v8__Global__New(isolate.handle, js_obj.handle, gop.value_ptr); + if (@hasDecl(JsApi.Meta, "finalizer")) { + if (comptime IS_DEBUG) { + // You can normally return a "*Node" and we'll correctly + // handle it as what it really is, e.g. an HTMLScriptElement. + // But for finalizers, we can't do that. I think this + // limitation will be OK - this auto-resolution is largely + // limited to Node -> HtmlElement, none of which has finalizers + std.debug.assert(resolved.class_id == JsApi.Meta.class_id); + } + + try ctx.finalizer_callbacks.put(ctx.arena, @intFromPtr(resolved.ptr), .init(value)); + if (@hasDecl(JsApi.Meta, "weak")) { + std.debug.assert(JsApi.Meta.weak == true); + v8.v8__Global__SetWeakFinalizer(gop.value_ptr, resolved.ptr, JsApi.Meta.finalizer.from_v8, v8.kParameter); + } + } return js_obj; }, else => @compileError("Expected a struct or pointer, got " ++ @typeName(T) ++ " (constructors must return struct or pointers)"), diff --git a/src/browser/js/bridge.zig b/src/browser/js/bridge.zig index 889b74d1..b58da3f9 100644 --- a/src/browser/js/bridge.zig +++ b/src/browser/js/bridge.zig @@ -91,6 +91,36 @@ pub fn Builder(comptime T: type) type { } return entries; } + + pub fn finalizer(comptime func: *const fn (self: *T) void) Finalizer { + return .{ + .from_zig = struct { + fn wrap(ptr: *anyopaque) void { + func(@ptrCast(@alignCast(ptr))); + } + }.wrap, + + .from_v8 = struct { + fn wrap(handle: ?*const v8.WeakCallbackInfo) callconv(.c) void { + const ptr = v8.v8__WeakCallbackInfo__GetParameter(handle.?).?; + const self: *T = @ptrCast(@alignCast(ptr)); + // This is simply a requirement of any type that Finalizes: + // It must have a _page: *Page field. We need it because + // we need to check the item has already been cleared + // (There are all types of weird timing issues that seem + // to be possible between finalization and context shutdown, + // we need to be defensive). + // There _ARE_ alternatives to this. But this is simple. + const ctx = self._page.js; + if (!ctx.identity_map.contains(@intFromPtr(ptr))) { + return; + } + func(self); + ctx.release(ptr); + } + }.wrap, + }; + } }; } @@ -369,6 +399,17 @@ pub const Property = union(enum) { int: i64, }; +const Finalizer = struct { + // The finalizer wrapper when called fro Zig. This is only called on + // Context.deinit + from_zig: *const fn (ctx: *anyopaque) void, + + // The finalizer wrapper when called from V8. This may never be called + // (hence why we fallback to calling in Context.denit). If it is called, + // it is only ever called after we SetWeak on the Global. + from_v8: *const fn (?*const v8.WeakCallbackInfo) callconv(.c) void, +}; + pub fn unknownPropertyCallback(c_name: ?*const v8.Name, handle: ?*const v8.PropertyCallbackInfo) callconv(.c) u8 { const v8_isolate = v8.v8__PropertyCallbackInfo__GetIsolate(handle).?; var caller: Caller = undefined; diff --git a/src/browser/webapi/net/XMLHttpRequest.zig b/src/browser/webapi/net/XMLHttpRequest.zig index c61a2760..83e7db0e 100644 --- a/src/browser/webapi/net/XMLHttpRequest.zig +++ b/src/browser/webapi/net/XMLHttpRequest.zig @@ -79,19 +79,25 @@ const ResponseType = enum { }; pub fn init(page: *Page) !*XMLHttpRequest { - return page._factory.xhrEventTarget(XMLHttpRequest{ + const arena = try page.getArena("XMLHttpRequest"); + errdefer page.releaseArena(arena); + + const xx = try page._factory.xhrEventTarget(XMLHttpRequest{ ._page = page, + ._arena = arena, ._proto = undefined, - ._arena = page.arena, ._request_headers = try Headers.init(null, page), }); + return xx; } pub fn deinit(self: *XMLHttpRequest) void { - if (self.transfer) |transfer| { + if (self._transfer) |transfer| { transfer.abort(error.Abort); - self.transfer = null; + self._transfer = null; } + self._page.releaseArena(self._arena); + self._page._factory.destroy(self); } fn asEventTarget(self: *XMLHttpRequest) *EventTarget { @@ -110,7 +116,7 @@ pub fn setOnReadyStateChange(self: *XMLHttpRequest, cb_: ?js.Function) !void { } } -// TODO: this takes an opitonal 3 more parameters +// TODO: this takes an optional 3 more parameters // TODO: url should be a union, as it can be multiple things pub fn open(self: *XMLHttpRequest, method_: []const u8, url: [:0]const u8) !void { // Abort any in-progress request @@ -148,6 +154,7 @@ pub fn send(self: *XMLHttpRequest, body_: ?[]const u8) !void { if (self._ready_state != .opened) { return error.InvalidStateError; } + self._page.js.strongRef(self); if (body_) |b| { if (self._method != .GET and self._method != .HEAD) { @@ -385,6 +392,8 @@ fn httpDoneCallback(ctx: *anyopaque) !void { .total = loaded, .loaded = loaded, }, local, page); + + page.js.weakRef(self); } fn httpErrorCallback(ctx: *anyopaque, err: anyerror) void { @@ -392,6 +401,7 @@ fn httpErrorCallback(ctx: *anyopaque, err: anyerror) void { // http client will close it after an error, it isn't safe to keep around self._transfer = null; self.handleError(err); + self._page.js.weakRef(self); } pub fn abort(self: *XMLHttpRequest) void { @@ -400,6 +410,7 @@ pub fn abort(self: *XMLHttpRequest) void { transfer.abort(error.Abort); self._transfer = null; } + self._page.js.weakRef(self); } fn handleError(self: *XMLHttpRequest, err: anyerror) void { @@ -477,6 +488,8 @@ pub const JsApi = struct { pub const name = "XMLHttpRequest"; pub const prototype_chain = bridge.prototypeChain(); pub var class_id: bridge.ClassId = undefined; + pub const weak = true; + pub const finalizer = bridge.finalizer(XMLHttpRequest.deinit); }; pub const constructor = bridge.constructor(XMLHttpRequest.init, .{}); diff --git a/src/slab.zig b/src/slab.zig index dbc07b0c..d8cb78f1 100644 --- a/src/slab.zig +++ b/src/slab.zig @@ -404,7 +404,6 @@ pub const SlabAllocator = struct { const ptr = memory.ptr; const len = memory.len; const aligned_len = std.mem.alignForward(usize, len, alignment.toByteUnits()); - const list = self.slabs.getPtr(.{ .size = aligned_len, .alignment = alignment }).?; list.free(ptr); } From 9f0c90203067f9aaab750d8d207dadf45435aa1b Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Mon, 19 Jan 2026 22:43:00 +0800 Subject: [PATCH 2/7] more explicit arena pool debug parameter --- src/browser/Page.zig | 7 +++++-- src/browser/webapi/net/XMLHttpRequest.zig | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/browser/Page.zig b/src/browser/Page.zig index 344af081..06840c05 100644 --- a/src/browser/Page.zig +++ b/src/browser/Page.zig @@ -356,10 +356,13 @@ pub fn getOrigin(self: *Page, allocator: Allocator) !?[]const u8 { return try URL.getOrigin(allocator, self.url); } -pub fn getArena(self: *Page, comptime owner: []const u8) !Allocator { +const GetArenaOpts = struct { + debug: []const u8, +}; +pub fn getArena(self: *Page, comptime opts: GetArenaOpts) !Allocator { const allocator = try self.arena_pool.acquire(); if (comptime IS_DEBUG) { - try self._arena_pool_leak_track.put(self.arena, @intFromPtr(allocator.ptr), owner); + try self._arena_pool_leak_track.put(self.arena, @intFromPtr(allocator.ptr), opts.debug); } return allocator; } diff --git a/src/browser/webapi/net/XMLHttpRequest.zig b/src/browser/webapi/net/XMLHttpRequest.zig index 83e7db0e..796fa973 100644 --- a/src/browser/webapi/net/XMLHttpRequest.zig +++ b/src/browser/webapi/net/XMLHttpRequest.zig @@ -79,7 +79,7 @@ const ResponseType = enum { }; pub fn init(page: *Page) !*XMLHttpRequest { - const arena = try page.getArena("XMLHttpRequest"); + const arena = try page.getArena(.{.debug = "XMLHttpRequest"}); errdefer page.releaseArena(arena); const xx = try page._factory.xhrEventTarget(XMLHttpRequest{ From 81378d4353f07b22053fd4db5205103b89f2abfd Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Tue, 20 Jan 2026 07:54:34 +0800 Subject: [PATCH 3/7] Simplify XHR lifetime Keep this a weak reference (the default now).And rely on transfer abort to ensure reference isn't needed after finalizer. --- src/browser/js/Local.zig | 3 +-- src/browser/webapi/net/XMLHttpRequest.zig | 6 ------ 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/src/browser/js/Local.zig b/src/browser/js/Local.zig index b7b54857..c065f421 100644 --- a/src/browser/js/Local.zig +++ b/src/browser/js/Local.zig @@ -207,8 +207,7 @@ pub fn mapZigInstanceToJs(self: *const Local, js_obj_handle: ?*const v8.Object, } try ctx.finalizer_callbacks.put(ctx.arena, @intFromPtr(resolved.ptr), .init(value)); - if (@hasDecl(JsApi.Meta, "weak")) { - std.debug.assert(JsApi.Meta.weak == true); + if (@hasDecl(JsApi.Meta, "finalizer")) { v8.v8__Global__SetWeakFinalizer(gop.value_ptr, resolved.ptr, JsApi.Meta.finalizer.from_v8, v8.kParameter); } } diff --git a/src/browser/webapi/net/XMLHttpRequest.zig b/src/browser/webapi/net/XMLHttpRequest.zig index 796fa973..30da5d6e 100644 --- a/src/browser/webapi/net/XMLHttpRequest.zig +++ b/src/browser/webapi/net/XMLHttpRequest.zig @@ -154,7 +154,6 @@ pub fn send(self: *XMLHttpRequest, body_: ?[]const u8) !void { if (self._ready_state != .opened) { return error.InvalidStateError; } - self._page.js.strongRef(self); if (body_) |b| { if (self._method != .GET and self._method != .HEAD) { @@ -392,8 +391,6 @@ fn httpDoneCallback(ctx: *anyopaque) !void { .total = loaded, .loaded = loaded, }, local, page); - - page.js.weakRef(self); } fn httpErrorCallback(ctx: *anyopaque, err: anyerror) void { @@ -401,7 +398,6 @@ fn httpErrorCallback(ctx: *anyopaque, err: anyerror) void { // http client will close it after an error, it isn't safe to keep around self._transfer = null; self.handleError(err); - self._page.js.weakRef(self); } pub fn abort(self: *XMLHttpRequest) void { @@ -410,7 +406,6 @@ pub fn abort(self: *XMLHttpRequest) void { transfer.abort(error.Abort); self._transfer = null; } - self._page.js.weakRef(self); } fn handleError(self: *XMLHttpRequest, err: anyerror) void { @@ -488,7 +483,6 @@ pub const JsApi = struct { pub const name = "XMLHttpRequest"; pub const prototype_chain = bridge.prototypeChain(); pub var class_id: bridge.ClassId = undefined; - pub const weak = true; pub const finalizer = bridge.finalizer(XMLHttpRequest.deinit); }; From 97f9c2991bc78f2644ea7c08807e76f474195b34 Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Wed, 21 Jan 2026 14:46:34 +0800 Subject: [PATCH 4/7] on XHR shutdown, use terminate to prevent any client callbacks into the XHR --- src/browser/webapi/net/XMLHttpRequest.zig | 2 +- src/http/Client.zig | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/browser/webapi/net/XMLHttpRequest.zig b/src/browser/webapi/net/XMLHttpRequest.zig index 30da5d6e..789d0b52 100644 --- a/src/browser/webapi/net/XMLHttpRequest.zig +++ b/src/browser/webapi/net/XMLHttpRequest.zig @@ -93,7 +93,7 @@ pub fn init(page: *Page) !*XMLHttpRequest { pub fn deinit(self: *XMLHttpRequest) void { if (self._transfer) |transfer| { - transfer.abort(error.Abort); + transfer.terminate(); self._transfer = null; } self._page.releaseArena(self._arena); diff --git a/src/http/Client.zig b/src/http/Client.zig index a0f5ee81..9614d218 100644 --- a/src/http/Client.zig +++ b/src/http/Client.zig @@ -979,6 +979,13 @@ pub const Transfer = struct { self.deinit(); } + pub fn terminate(self: *Transfer) void { + if (self._handle != null) { + self.client.endTransfer(self); + } + self.deinit(); + } + // internal, when the page is shutting down. Doesn't have the same ceremony // as abort (doesn't send a notification, doesn't invoke an error callback) fn kill(self: *Transfer) void { From d5f26f6d15c61d9f811f227a51138214195aab7f Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Wed, 21 Jan 2026 15:30:39 +0800 Subject: [PATCH 5/7] remove temp variable --- src/browser/webapi/net/XMLHttpRequest.zig | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/browser/webapi/net/XMLHttpRequest.zig b/src/browser/webapi/net/XMLHttpRequest.zig index 789d0b52..a25bd6b8 100644 --- a/src/browser/webapi/net/XMLHttpRequest.zig +++ b/src/browser/webapi/net/XMLHttpRequest.zig @@ -82,13 +82,12 @@ pub fn init(page: *Page) !*XMLHttpRequest { const arena = try page.getArena(.{.debug = "XMLHttpRequest"}); errdefer page.releaseArena(arena); - const xx = try page._factory.xhrEventTarget(XMLHttpRequest{ + return try page._factory.xhrEventTarget(XMLHttpRequest{ ._page = page, ._arena = arena, ._proto = undefined, ._request_headers = try Headers.init(null, page), }); - return xx; } pub fn deinit(self: *XMLHttpRequest) void { From fc64abee8fc09552a9b44df986e1e98e3b39a4f5 Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Thu, 22 Jan 2026 10:58:28 +0800 Subject: [PATCH 6/7] Add finalizer mode When a type is finalized by V8, it's because it's fallen out of scope. When a type is finalized by Zig, it's because the Context is being shutdown. Those are two different environments and might require distinct cleanup logic. Specifically, a zig-initiated finalization needs to consider that the page and context are being shutdown. It isn't necessarily safe to execute JavaScript at this point, and thus, not safe to execute a callback (on_error, on_abort, ready_state_change, ...). --- src/browser/js/bridge.zig | 6 +++--- src/browser/webapi/net/XMLHttpRequest.zig | 8 ++++++-- src/http/Client.zig | 15 +++++++++------ 3 files changed, 18 insertions(+), 11 deletions(-) diff --git a/src/browser/js/bridge.zig b/src/browser/js/bridge.zig index b58da3f9..27359f88 100644 --- a/src/browser/js/bridge.zig +++ b/src/browser/js/bridge.zig @@ -92,11 +92,11 @@ pub fn Builder(comptime T: type) type { return entries; } - pub fn finalizer(comptime func: *const fn (self: *T) void) Finalizer { + pub fn finalizer(comptime func: *const fn (self: *T, comptime shutdown: bool) void) Finalizer { return .{ .from_zig = struct { fn wrap(ptr: *anyopaque) void { - func(@ptrCast(@alignCast(ptr))); + func(@ptrCast(@alignCast(ptr)), true); } }.wrap, @@ -115,7 +115,7 @@ pub fn Builder(comptime T: type) type { if (!ctx.identity_map.contains(@intFromPtr(ptr))) { return; } - func(self); + func(self, false); ctx.release(ptr); } }.wrap, diff --git a/src/browser/webapi/net/XMLHttpRequest.zig b/src/browser/webapi/net/XMLHttpRequest.zig index a25bd6b8..85b961c5 100644 --- a/src/browser/webapi/net/XMLHttpRequest.zig +++ b/src/browser/webapi/net/XMLHttpRequest.zig @@ -90,9 +90,13 @@ pub fn init(page: *Page) !*XMLHttpRequest { }); } -pub fn deinit(self: *XMLHttpRequest) void { +pub fn deinit(self: *XMLHttpRequest, comptime shutdown: bool) void { if (self._transfer) |transfer| { - transfer.terminate(); + if (shutdown) { + transfer.terminate(); + } else { + transfer.abort(error.Abort); + } self._transfer = null; } self._page.releaseArena(self._arena); diff --git a/src/http/Client.zig b/src/http/Client.zig index 9614d218..14ff21c7 100644 --- a/src/http/Client.zig +++ b/src/http/Client.zig @@ -371,7 +371,7 @@ fn makeTransfer(self: *Client, req: Request) !*Transfer { return transfer; } -fn requestFailed(self: *Client, transfer: *Transfer, err: anyerror) void { +fn requestFailed(self: *Client, transfer: *Transfer, err: anyerror, comptime execute_callback: bool) void { // this shouldn't happen, we'll crash in debug mode. But in release, we'll // just noop this state. if (comptime IS_DEBUG) { @@ -390,7 +390,9 @@ fn requestFailed(self: *Client, transfer: *Transfer, err: anyerror) void { }); } - transfer.req.error_callback(transfer.ctx, err); + if (execute_callback) { + transfer.req.error_callback(transfer.ctx, err); + } } // Restrictive since it'll only work if there are no inflight requests. In some @@ -600,7 +602,7 @@ fn processMessages(self: *Client) !bool { if (!transfer._header_done_called) { const proceed = transfer.headerDoneCallback(easy) catch |err| { log.err(.http, "header_done_callback", .{ .err = err }); - self.requestFailed(transfer, err); + self.requestFailed(transfer, err, true); continue; }; if (!proceed) { @@ -611,7 +613,7 @@ fn processMessages(self: *Client) !bool { 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 }); - self.requestFailed(transfer, err); + self.requestFailed(transfer, err, true); continue; }; @@ -622,7 +624,7 @@ fn processMessages(self: *Client) !bool { } processed = true; } else |err| { - self.requestFailed(transfer, err); + self.requestFailed(transfer, err, true); } } return processed; @@ -972,7 +974,7 @@ pub const Transfer = struct { } pub fn abort(self: *Transfer, err: anyerror) void { - self.client.requestFailed(self, err); + self.client.requestFailed(self, err, true); if (self._handle != null) { self.client.endTransfer(self); } @@ -980,6 +982,7 @@ pub const Transfer = struct { } pub fn terminate(self: *Transfer) void { + self.client.requestFailed(self, error.Shutdown, false); if (self._handle != null) { self.client.endTransfer(self); } From 9a57c2a0d48f8364a6bcedbe93147be7a0f003ac Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Sat, 24 Jan 2026 08:28:26 +0800 Subject: [PATCH 7/7] fix merge --- src/http/Client.zig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/http/Client.zig b/src/http/Client.zig index 14ff21c7..68a2d919 100644 --- a/src/http/Client.zig +++ b/src/http/Client.zig @@ -606,7 +606,7 @@ fn processMessages(self: *Client) !bool { continue; }; if (!proceed) { - self.requestFailed(transfer, error.Abort); + self.requestFailed(transfer, error.Abort, true); break :blk; } }