From 94ce5edd20458b2dcdf03e55f828b7afaa108729 Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Mon, 9 Mar 2026 07:47:33 +0800 Subject: [PATCH 1/8] Frames on the same origin share v8 data Depends on: https://github.com/lightpanda-io/zig-v8-fork/pull/153 In some ways this is an extension of https://github.com/lightpanda-io/browser/pull/1635 but it has more implications with respect to correctness. A js.Context wraps a v8::Context. One of the important thing it adds is the identity_map so that, given a Zig instance we always return the same v8::Object. But imagine code running in a frame. This frame has its own Context, and thus its own identity_map. What happens when that frame does: ```js window.top.frame_loaded = true; ``` From Zig's point of view, `Window.getTop` will return the correct Zig instance. It will return the *Window references by the "root" page. When that instance is passed to the bridge, we'll look for the v8::Object in the Context's `identity_map` but wont' find it. The mapping exists in the root context `identity_map`, but not within this frame. So we create a new v8::Object and now our 1 zig instance has N v8::Objects for every page/frame that tries to access it. This breaks cross-frame scripting which should work, at least to some degree, even when frames are on the same origin. This commit adds a `js.Origin` which contains the `identity_map`, along with our other `v8::Global` storage. The `Env` now contains a `*js.Origin` lookup, mapping an origin string (e.g. lightpanda.io:443) to an *Origin. When a Page's URL is changed, we call `self.js.setOrigin(new_url)` which will then either get or create an origin from the Env's origin lookup map. js.Origin is reference counted so that it remains valid so long as at least 1 frame references them. There's some special handling for null-origins (i.e. about:blank). At the root, null origins get a distinct/isolated Origin. For a frame, the parent's origin is used. Above, we talked about `identity_map`, but a `js.Context` has 8 other fields to track v8 values, e.g. `global_objects`, `global_functions`, `global_values_temp`, etc. These all must be shared by frames on the same origin. So all of these have also been moved to js.Origin. They've also been merged so that we now have 3 fields: `identity_map`, `globals` and `temps`. Finally, when the origin of a context is changed, we set the v8::Context's SecurityToken (to that origin). This is a key part of how v8 allows cross- context access. --- build.zig.zon | 6 +- src/browser/Page.zig | 27 +++-- src/browser/js/Context.zig | 124 ++++++++-------------- src/browser/js/Env.zig | 62 ++++++++++- src/browser/js/Function.zig | 4 +- src/browser/js/Local.zig | 2 +- src/browser/js/Object.zig | 2 +- src/browser/js/Origin.zig | 148 +++++++++++++++++++++++++++ src/browser/js/Promise.zig | 4 +- src/browser/js/PromiseResolver.zig | 2 +- src/browser/js/Value.zig | 4 +- src/browser/js/js.zig | 2 +- src/browser/tests/frames/frames.html | 11 +- src/browser/tests/testing.js | 2 +- src/browser/webapi/URL.zig | 3 +- src/cdp/domains/page.zig | 2 +- src/testing.zig | 9 -- 17 files changed, 295 insertions(+), 119 deletions(-) create mode 100644 src/browser/js/Origin.zig diff --git a/build.zig.zon b/build.zig.zon index eb3812a8..b7f9cf3b 100644 --- a/build.zig.zon +++ b/build.zig.zon @@ -5,10 +5,10 @@ .minimum_zig_version = "0.15.2", .dependencies = .{ .v8 = .{ - .url = "https://github.com/lightpanda-io/zig-v8-fork/archive/refs/tags/v0.3.1.tar.gz", - .hash = "v8-0.0.0-xddH64J7BAC81mkf6G9RbEJxS-W3TIRl5iFnShwbqCqy", + .url = "https://github.com/lightpanda-io/zig-v8-fork/archive/refs/tags/v0.3.2.tar.gz", + .hash = "v8-0.0.0-xddH6wx-BABNgL7YIDgbnFgKZuXZ68yZNngNSrV6OjrY", }, - //.v8 = .{ .path = "../zig-v8-fork" }, + // .v8 = .{ .path = "../zig-v8-fork" }, .brotli = .{ // v1.2.0 .url = "https://github.com/google/brotli/archive/028fb5a23661f123017c060daa546b55cf4bde29.tar.gz", diff --git a/src/browser/Page.zig b/src/browser/Page.zig index 47c63904..9b05c39b 100644 --- a/src/browser/Page.zig +++ b/src/browser/Page.zig @@ -190,6 +190,8 @@ _queued_navigation: ?*QueuedNavigation = null, // The URL of the current page url: [:0]const u8 = "about:blank", +origin: ?[]const u8 = null, + // The base url specifies the base URL used to resolve the relative urls. // It is set by a tag. // If null the url must be used. @@ -388,10 +390,6 @@ pub fn getTitle(self: *Page) !?[]const u8 { return null; } -pub fn getOrigin(self: *Page, allocator: Allocator) !?[]const u8 { - return try URL.getOrigin(allocator, self.url); -} - // Add comon headers for a request: // * cookies // * referer @@ -449,7 +447,7 @@ pub fn releaseArena(self: *Page, allocator: Allocator) void { } 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; + const current_origin = self.origin orelse return false; return std.mem.startsWith(u8, url, current_origin); } @@ -472,6 +470,14 @@ pub fn navigate(self: *Page, request_url: [:0]const u8, opts: NavigateOpts) !voi // page and dispatch the events. if (std.mem.eql(u8, "about:blank", request_url)) { self.url = "about:blank"; + + if (self.parent) |parent| { + self.origin = parent.origin; + } else { + self.origin = null; + } + try self.js.setOrigin(self.origin); + // Assume we parsed the document. // It's important to force a reset during the following navigation. self._parse_state = .complete; @@ -518,6 +524,7 @@ pub fn navigate(self: *Page, request_url: [:0]const u8, opts: NavigateOpts) !voi var http_client = session.browser.http_client; self.url = try self.arena.dupeZ(u8, request_url); + self.origin = try URL.getOrigin(self.arena, self.url); self._req_id = req_id; self._navigated_options = .{ @@ -825,9 +832,15 @@ fn notifyParentLoadComplete(self: *Page) void { fn pageHeaderDoneCallback(transfer: *HttpClient.Transfer) !bool { var self: *Page = @ptrCast(@alignCast(transfer.ctx)); - // would be different than self.url in the case of a redirect const header = &transfer.response_header.?; - self.url = try self.arena.dupeZ(u8, std.mem.span(header.url)); + + const response_url = std.mem.span(header.url); + if (std.mem.eql(u8, response_url, self.url) == false) { + // would be different than self.url in the case of a redirect + self.url = try self.arena.dupeZ(u8, response_url); + self.origin = try URL.getOrigin(self.arena, self.url); + } + try self.js.setOrigin(self.origin); self.window._location = try Location.init(self.url, self); self.document._location = self.window._location; diff --git a/src/browser/js/Context.zig b/src/browser/js/Context.zig index 9180223c..065b5e28 100644 --- a/src/browser/js/Context.zig +++ b/src/browser/js/Context.zig @@ -23,6 +23,7 @@ const log = @import("../../log.zig"); const js = @import("js.zig"); const Env = @import("Env.zig"); const bridge = @import("bridge.zig"); +const Origin = @import("Origin.zig"); const Scheduler = @import("Scheduler.zig"); const Page = @import("../Page.zig"); @@ -74,12 +75,7 @@ call_depth: usize = 0, // context.localScope local: ?*const js.Local = null, -// Serves two purposes. Like `global_objects`, this is used to free -// every Global(Object) we've created during the lifetime of the context. -// More importantly, it serves as an identity map - for a given Zig -// instance, we map it to the same Global(Object). -// The key is the @intFromPtr of the Zig value -identity_map: std.AutoHashMapUnmanaged(usize, v8.Global) = .empty, +origin: *Origin, // 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 @@ -87,26 +83,9 @@ identity_map: std.AutoHashMapUnmanaged(usize, v8.Global) = .empty, finalizer_callbacks: std.AutoHashMapUnmanaged(usize, *FinalizerCallback) = .empty, finalizer_callback_pool: std.heap.MemoryPool(FinalizerCallback), -// 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 -// a `Global(Object)`. We need to track these to free them. -// This used to be a map and acted like identity_map; the key was -// the @intFromPtr(js_obj.handle). But v8 can re-use address. Without -// a reliable way to know if an object has already been persisted, -// we now simply persist every time persist() is called. -global_values: std.ArrayList(v8.Global) = .empty, -global_objects: std.ArrayList(v8.Global) = .empty, +// Unlike other v8 types, like functions or objects, modules are not shared +// across origins. global_modules: std.ArrayList(v8.Global) = .empty, -global_promises: std.ArrayList(v8.Global) = .empty, -global_functions: std.ArrayList(v8.Global) = .empty, -global_promise_resolvers: std.ArrayList(v8.Global) = .empty, - -// Temp variants stored in HashMaps for O(1) early cleanup. -// Key is global.data_ptr. -global_values_temp: std.AutoHashMapUnmanaged(usize, v8.Global) = .empty, -global_promises_temp: std.AutoHashMapUnmanaged(usize, v8.Global) = .empty, -global_functions_temp: std.AutoHashMapUnmanaged(usize, v8.Global) = .empty, // Our module cache: normalized module specifier => module. module_cache: std.StringHashMapUnmanaged(ModuleEntry) = .empty, @@ -174,12 +153,6 @@ pub fn deinit(self: *Context) void { // this can release objects self.scheduler.deinit(); - { - var it = self.identity_map.valueIterator(); - while (it.next()) |global| { - v8.v8__Global__Reset(global); - } - } { var it = self.finalizer_callbacks.valueIterator(); while (it.next()) |finalizer| { @@ -188,50 +161,11 @@ pub fn deinit(self: *Context) void { self.finalizer_callback_pool.deinit(); } - for (self.global_values.items) |*global| { - v8.v8__Global__Reset(global); - } - - for (self.global_objects.items) |*global| { - v8.v8__Global__Reset(global); - } - for (self.global_modules.items) |*global| { v8.v8__Global__Reset(global); } - for (self.global_functions.items) |*global| { - v8.v8__Global__Reset(global); - } - - for (self.global_promises.items) |*global| { - v8.v8__Global__Reset(global); - } - - for (self.global_promise_resolvers.items) |*global| { - v8.v8__Global__Reset(global); - } - - { - var it = self.global_values_temp.valueIterator(); - while (it.next()) |global| { - v8.v8__Global__Reset(global); - } - } - - { - var it = self.global_promises_temp.valueIterator(); - while (it.next()) |global| { - v8.v8__Global__Reset(global); - } - } - - { - var it = self.global_functions_temp.valueIterator(); - while (it.next()) |global| { - v8.v8__Global__Reset(global); - } - } + env.releaseOrigin(self.origin); v8.v8__Global__Reset(&self.handle); env.isolate.notifyContextDisposed(); @@ -241,6 +175,38 @@ pub fn deinit(self: *Context) void { v8.v8__MicrotaskQueue__DELETE(self.microtask_queue); } +pub fn setOrigin(self: *Context, key: ?[]const u8) !void { + const env = self.env; + const isolate = env.isolate; + + const origin = try env.getOrCreateOrigin(key); + errdefer env.releaseOrigin(origin); + + try self.origin.transferTo(origin); + self.origin.deinit(env.app); + + self.origin = origin; + + { + var ls: js.Local.Scope = undefined; + self.localScope(&ls); + defer ls.deinit(); + + // Set the V8::Context SecurityToken, which is a big part of what allows + // one context to access another. + const token_local = v8.v8__Global__Get(&origin.security_token, isolate.handle); + v8.v8__Context__SetSecurityToken(ls.local.handle, token_local); + } +} + +pub fn trackGlobal(self: *Context, global: v8.Global) !void { + return self.origin.trackGlobal(global); +} + +pub fn trackTemp(self: *Context, global: v8.Global) !void { + return self.origin.trackTemp(global); +} + pub fn weakRef(self: *Context, obj: anytype) void { const fc = self.finalizer_callbacks.get(@intFromPtr(obj)) orelse { if (comptime IS_DEBUG) { @@ -279,7 +245,7 @@ pub fn release(self: *Context, item: anytype) void { if (@TypeOf(item) == *anyopaque) { // Existing *anyopaque path for identity_map. Called internally from // finalizers - var global = self.identity_map.fetchRemove(@intFromPtr(item)) orelse { + var global = self.origin.identity_map.fetchRemove(@intFromPtr(item)) orelse { if (comptime IS_DEBUG) { // should not be possible std.debug.assert(false); @@ -301,14 +267,14 @@ pub fn release(self: *Context, item: anytype) void { return; } - var map = switch (@TypeOf(item)) { - js.Value.Temp => &self.global_values_temp, - js.Promise.Temp => &self.global_promises_temp, - js.Function.Temp => &self.global_functions_temp, - else => |T| @compileError("Context.release cannot be called with a " ++ @typeName(T)), - }; + if (comptime IS_DEBUG) { + switch (@TypeOf(item)) { + js.Value.Temp, js.Promise.Temp, js.Function.Temp => {}, + else => |T| @compileError("Context.release cannot be called with a " ++ @typeName(T)), + } + } - if (map.fetchRemove(item.handle.data_ptr)) |kv| { + if (self.origin.temps.fetchRemove(item.handle.data_ptr)) |kv| { var global = kv.value; v8.v8__Global__Reset(&global); } diff --git a/src/browser/js/Env.zig b/src/browser/js/Env.zig index 1a86ffd5..9bacc088 100644 --- a/src/browser/js/Env.zig +++ b/src/browser/js/Env.zig @@ -26,6 +26,7 @@ const App = @import("../../App.zig"); const log = @import("../../log.zig"); const bridge = @import("bridge.zig"); +const Origin = @import("Origin.zig"); const Context = @import("Context.zig"); const Isolate = @import("Isolate.zig"); const Platform = @import("Platform.zig"); @@ -57,6 +58,8 @@ const Env = @This(); app: *App, +allocator: Allocator, + platform: *const Platform, // the global isolate @@ -70,6 +73,9 @@ isolate_params: *v8.CreateParams, context_id: usize, +// Maps origin -> shared Origin contains, for v8 values shared across same-origin Contexts +origins: std.StringHashMapUnmanaged(*Origin) = .empty, + // Global handles that need to be freed on deinit eternal_function_templates: []v8.Eternal, @@ -206,6 +212,7 @@ pub fn init(app: *App, opts: InitOpts) !Env { return .{ .app = app, .context_id = 0, + .allocator = allocator, .contexts = undefined, .context_count = 0, .isolate = isolate, @@ -228,7 +235,17 @@ pub fn deinit(self: *Env) void { ctx.deinit(); } - const allocator = self.app.allocator; + const app = self.app; + const allocator = app.allocator; + + { + var it = self.origins.valueIterator(); + while (it.next()) |value| { + value.*.deinit(app); + } + self.origins.deinit(allocator); + } + if (self.inspector) |i| { i.deinit(allocator); } @@ -272,6 +289,7 @@ pub fn createContext(self: *Env, page: *Page) !*Context { // get the global object for the context, this maps to our Window const global_obj = v8.v8__Context__Global(v8_context).?; + { // Store our TAO inside the internal field of the global object. This // maps the v8::Object -> Zig instance. Almost all objects have this, and @@ -287,6 +305,7 @@ pub fn createContext(self: *Env, page: *Page) !*Context { }; v8.v8__Object__SetAlignedPointerInInternalField(global_obj, 0, tao); } + // our window wrapped in a v8::Global var global_global: v8.Global = undefined; v8.v8__Global__New(isolate.handle, global_obj, &global_global); @@ -294,10 +313,14 @@ pub fn createContext(self: *Env, page: *Page) !*Context { const context_id = self.context_id; self.context_id = context_id + 1; + const origin = try self.getOrCreateOrigin(null); + errdefer self.releaseOrigin(origin); + const context = try context_arena.create(Context); context.* = .{ .env = self, .page = page, + .origin = origin, .id = context_id, .isolate = isolate, .arena = context_arena, @@ -309,7 +332,7 @@ pub fn createContext(self: *Env, page: *Page) !*Context { .scheduler = .init(context_arena), .finalizer_callback_pool = std.heap.MemoryPool(Context.FinalizerCallback).init(self.app.allocator), }; - try context.identity_map.putNoClobber(context_arena, @intFromPtr(page.window), global_global); + try context.origin.identity_map.putNoClobber(context_arena, @intFromPtr(page.window), global_global); // Store a pointer to our context inside the v8 context so that, given // a v8 context, we can get our context out @@ -350,6 +373,41 @@ pub fn destroyContext(self: *Env, context: *Context) void { context.deinit(); } +pub fn getOrCreateOrigin(self: *Env, key_: ?[]const u8) !*Origin { + const key = key_ orelse { + var opaque_origin: [36]u8 = undefined; + @import("../../id.zig").uuidv4(&opaque_origin); + // Origin.init will dupe opaque_origin. It's fine that this doesn't + // get added to self.origins. In fact, it further isolates it. When the + // context is freed, it'll call env.releaseOrigin which will free it. + return Origin.init(self.app, self.isolate, &opaque_origin); + }; + + const gop = try self.origins.getOrPut(self.allocator, key); + if (gop.found_existing) { + const origin = gop.value_ptr.*; + origin.rc += 1; + return origin; + } + + errdefer _ = self.origins.remove(key); + + const origin = try Origin.init(self.app, self.isolate, key); + gop.key_ptr.* = origin.key; + gop.value_ptr.* = origin; + return origin; +} + +pub fn releaseOrigin(self: *Env, origin: *Origin) void { + const rc = origin.rc; + if (rc == 1) { + _ = self.origins.remove(origin.key); + origin.deinit(self.app); + } else { + origin.rc = rc - 1; + } +} + pub fn runMicrotasks(self: *Env) void { if (self.microtask_queues_are_running == false) { const v8_isolate = self.isolate.handle; diff --git a/src/browser/js/Function.zig b/src/browser/js/Function.zig index 01243d35..203fd9ab 100644 --- a/src/browser/js/Function.zig +++ b/src/browser/js/Function.zig @@ -209,9 +209,9 @@ fn _persist(self: *const Function, comptime is_global: bool) !(if (is_global) Gl var global: v8.Global = undefined; v8.v8__Global__New(ctx.isolate.handle, self.handle, &global); if (comptime is_global) { - try ctx.global_functions.append(ctx.arena, global); + try ctx.trackGlobal(global); } else { - try ctx.global_functions_temp.put(ctx.arena, global.data_ptr, global); + try ctx.trackTemp(global); } return .{ .handle = global }; } diff --git a/src/browser/js/Local.zig b/src/browser/js/Local.zig index 6a68b332..305391f5 100644 --- a/src/browser/js/Local.zig +++ b/src/browser/js/Local.zig @@ -171,7 +171,7 @@ pub fn mapZigInstanceToJs(self: *const Local, js_obj_handle: ?*const v8.Object, .pointer => |ptr| { const resolved = resolveValue(value); - const gop = try ctx.identity_map.getOrPut(arena, @intFromPtr(resolved.ptr)); + const gop = try ctx.origin.identity_map.getOrPut(arena, @intFromPtr(resolved.ptr)); if (gop.found_existing) { // we've seen this instance before, return the same object return (js.Object.Global{ .handle = gop.value_ptr.* }).local(self); diff --git a/src/browser/js/Object.zig b/src/browser/js/Object.zig index 981f4a2b..fbf036e4 100644 --- a/src/browser/js/Object.zig +++ b/src/browser/js/Object.zig @@ -97,7 +97,7 @@ pub fn persist(self: Object) !Global { var global: v8.Global = undefined; v8.v8__Global__New(ctx.isolate.handle, self.handle, &global); - try ctx.global_objects.append(ctx.arena, global); + try ctx.trackGlobal(global); return .{ .handle = global }; } diff --git a/src/browser/js/Origin.zig b/src/browser/js/Origin.zig new file mode 100644 index 00000000..6cb0b356 --- /dev/null +++ b/src/browser/js/Origin.zig @@ -0,0 +1,148 @@ +// Copyright (C) 2023-2025 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 . + +// Origin represents the shared Zig<->JS bridge state for all contexts within +// the same origin. Multiple contexts (frames) from the same origin share a +// single Origin, ensuring that JS objects maintain their identity across frames. + +const std = @import("std"); +const js = @import("js.zig"); + +const App = @import("../../App.zig"); + +const v8 = js.v8; +const Allocator = std.mem.Allocator; +const IS_DEBUG = @import("build").mode == .Debug; + +const Origin = @This(); + +rc: usize = 1, +arena: Allocator, + +// The key, e.g. lightpanda.io:443 +key: []const u8, + +// Security token - all contexts in this realm must use the same v8::Value instance +// as their security token for V8 to allow cross-context access +security_token: v8.Global, + +// Serves two purposes. Like `global_objects`, this is used to free +// every Global(Object) we've created during the lifetime of the realm. +// More importantly, it serves as an identity map - for a given Zig +// instance, we map it to the same Global(Object). +// The key is the @intFromPtr of the Zig value +identity_map: std.AutoHashMapUnmanaged(usize, v8.Global) = .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 +// a `Global(Object)`. We need to track these to free them. +// This used to be a map and acted like identity_map; the key was +// the @intFromPtr(js_obj.handle). But v8 can re-use address. Without +// a reliable way to know if an object has already been persisted, +// we now simply persist every time persist() is called. +globals: std.ArrayList(v8.Global) = .empty, + +// Temp variants stored in HashMaps for O(1) early cleanup. +// Key is global.data_ptr. +temps: std.AutoHashMapUnmanaged(usize, v8.Global) = .empty, + +pub fn init(app: *App, isolate: js.Isolate, key: []const u8) !*Origin { + const arena = try app.arena_pool.acquire(); + errdefer app.arena_pool.release(arena); + + var hs: js.HandleScope = undefined; + hs.init(isolate); + defer hs.deinit(); + + const owned_key = try arena.dupe(u8, key); + const token_local = isolate.initStringHandle(owned_key); + var token_global: v8.Global = undefined; + v8.v8__Global__New(isolate.handle, token_local, &token_global); + + const self = try arena.create(Origin); + self.* = .{ + .rc = 1, + .arena = arena, + .key = owned_key, + .globals = .empty, + .temps = .empty, + .security_token = token_global, + }; + return self; +} + +pub fn deinit(self: *Origin, app: *App) void { + v8.v8__Global__Reset(&self.security_token); + + { + var it = self.identity_map.valueIterator(); + while (it.next()) |global| { + v8.v8__Global__Reset(global); + } + } + + for (self.globals.items) |*global| { + v8.v8__Global__Reset(global); + } + + { + var it = self.temps.valueIterator(); + while (it.next()) |global| { + v8.v8__Global__Reset(global); + } + } + + app.arena_pool.release(self.arena); +} + +pub fn trackGlobal(self: *Origin, global: v8.Global) !void { + return self.globals.append(self.arena, global); +} + +pub fn trackTemp(self: *Origin, global: v8.Global) !void { + return self.temps.put(self.arena, global.data_ptr, global); +} + +pub fn transferTo(self: *Origin, dest: *Origin) !void { + const arena = dest.arena; + + try dest.globals.ensureUnusedCapacity(arena, self.globals.items.len); + for (self.globals.items) |obj| { + dest.globals.appendAssumeCapacity(obj); + } + self.globals.clearRetainingCapacity(); + + { + try dest.temps.ensureUnusedCapacity(arena, self.temps.count()); + var it = self.temps.iterator(); + while (it.next()) |kv| { + try dest.temps.put(arena, kv.key_ptr.*, kv.value_ptr.*); + } + self.temps.clearRetainingCapacity(); + } + + { + try dest.identity_map.ensureUnusedCapacity(arena, self.identity_map.count()); + var it = self.identity_map.iterator(); + while (it.next()) |kv| { + try dest.identity_map.put(arena, kv.key_ptr.*, kv.value_ptr.*); + } + self.identity_map.clearRetainingCapacity(); + } +} diff --git a/src/browser/js/Promise.zig b/src/browser/js/Promise.zig index afadbe82..98520d4b 100644 --- a/src/browser/js/Promise.zig +++ b/src/browser/js/Promise.zig @@ -62,9 +62,9 @@ fn _persist(self: *const Promise, comptime is_global: bool) !(if (is_global) Glo var global: v8.Global = undefined; v8.v8__Global__New(ctx.isolate.handle, self.handle, &global); if (comptime is_global) { - try ctx.global_promises.append(ctx.arena, global); + try ctx.trackGlobal(global); } else { - try ctx.global_promises_temp.put(ctx.arena, global.data_ptr, global); + try ctx.trackTemp(global); } return .{ .handle = global }; } diff --git a/src/browser/js/PromiseResolver.zig b/src/browser/js/PromiseResolver.zig index 183effee..f2aac0e0 100644 --- a/src/browser/js/PromiseResolver.zig +++ b/src/browser/js/PromiseResolver.zig @@ -79,7 +79,7 @@ pub fn persist(self: PromiseResolver) !Global { var ctx = self.local.ctx; var global: v8.Global = undefined; v8.v8__Global__New(ctx.isolate.handle, self.handle, &global); - try ctx.global_promise_resolvers.append(ctx.arena, global); + try ctx.trackGlobal(global); return .{ .handle = global }; } diff --git a/src/browser/js/Value.zig b/src/browser/js/Value.zig index 7963ae7c..fbc961ed 100644 --- a/src/browser/js/Value.zig +++ b/src/browser/js/Value.zig @@ -259,9 +259,9 @@ fn _persist(self: *const Value, comptime is_global: bool) !(if (is_global) Globa var global: v8.Global = undefined; v8.v8__Global__New(ctx.isolate.handle, self.handle, &global); if (comptime is_global) { - try ctx.global_values.append(ctx.arena, global); + try ctx.trackGlobal(global); } else { - try ctx.global_values_temp.put(ctx.arena, global.data_ptr, global); + try ctx.trackTemp(global); } return .{ .handle = global }; } diff --git a/src/browser/js/js.zig b/src/browser/js/js.zig index 9415b717..22651c39 100644 --- a/src/browser/js/js.zig +++ b/src/browser/js/js.zig @@ -161,7 +161,7 @@ pub fn ArrayBufferRef(comptime kind: ArrayType) type { var ctx = self.local.ctx; var global: v8.Global = undefined; v8.v8__Global__New(ctx.isolate.handle, self.handle, &global); - try ctx.global_values.append(ctx.arena, global); + try ctx.trackGlobal(global); return .{ .handle = global }; } diff --git a/src/browser/tests/frames/frames.html b/src/browser/tests/frames/frames.html index 4e614de9..97bed281 100644 --- a/src/browser/tests/frames/frames.html +++ b/src/browser/tests/frames/frames.html @@ -64,11 +64,12 @@ // child frame's top.parent is itself (root has no parent) testing.expectEqual(window, window[0].top.parent); - // Todo: Context security tokens - // testing.expectEqual(true, window.sub1_loaded); - // testing.expectEqual(true, window.sub2_loaded); - // testing.expectEqual(1, window.sub1_count); - // testing.expectEqual(2, window.sub2_count); + // Cross-frame property access + testing.expectEqual(true, window.sub1_loaded); + testing.expectEqual(true, window.sub2_loaded); + testing.expectEqual(1, window.sub1_count); + // depends on how far the initial load got before it was cancelled. + testing.expectEqual(true, window.sub2_count == 1 || window.sub2_count == 2); }); diff --git a/src/browser/tests/testing.js b/src/browser/tests/testing.js index 90434f0f..987ba042 100644 --- a/src/browser/tests/testing.js +++ b/src/browser/tests/testing.js @@ -118,7 +118,7 @@ BASE_URL: 'http://127.0.0.1:9582/src/browser/tests/', }; - if (!IS_TEST_RUNNER) { + if (window.navigator.userAgent.startsWith("Lightpanda/") == false) { // The page is running in a different browser. Probably a developer making sure // a test is correct. There are a few tweaks we need to do to make this a // seemless, namely around adapting paths/urls. diff --git a/src/browser/webapi/URL.zig b/src/browser/webapi/URL.zig index 3bc6f586..fda7d2a5 100644 --- a/src/browser/webapi/URL.zig +++ b/src/browser/webapi/URL.zig @@ -243,11 +243,10 @@ pub fn createObjectURL(blob: *Blob, page: *Page) ![]const u8 { var uuid_buf: [36]u8 = undefined; @import("../../id.zig").uuidv4(&uuid_buf); - const origin = (try page.getOrigin(page.call_arena)) orelse "null"; const blob_url = try std.fmt.allocPrint( page.arena, "blob:{s}/{s}", - .{ origin, uuid_buf }, + .{ page.origin orelse "null", uuid_buf }, ); try page._blob_urls.put(page.arena, blob_url, blob); return blob_url; diff --git a/src/cdp/domains/page.zig b/src/cdp/domains/page.zig index a96ac2b6..6e406c05 100644 --- a/src/cdp/domains/page.zig +++ b/src/cdp/domains/page.zig @@ -414,7 +414,7 @@ pub fn pageNavigated(arena: Allocator, bc: anytype, event: *const Notification.P bc.inspector_session.inspector.contextCreated( &ls.local, "", - try page.getOrigin(arena) orelse "", + page.origin orelse "", aux_data, true, ); diff --git a/src/testing.zig b/src/testing.zig index a398f824..774f76e4 100644 --- a/src/testing.zig +++ b/src/testing.zig @@ -414,15 +414,6 @@ fn runWebApiTest(test_file: [:0]const u8) !void { try_catch.init(&ls.local); defer try_catch.deinit(); - // by default, on load, testing.js will call testing.assertOk(). This makes our - // tests work well in a browser. But, for our test runner, we disable that - // and call it explicitly. This gives us better error messages. - ls.local.eval("window._lightpanda_skip_auto_assert = true;", "auto_assert") catch |err| { - const caught = try_catch.caughtOrError(arena_allocator, err); - std.debug.print("disable auto assert failure\nError: {f}\n", .{caught}); - return err; - }; - try page.navigate(url, .{}); _ = test_session.wait(2000); From 753391b7e26d113544f302eb64e7b79adcf350ba Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Mon, 9 Mar 2026 08:47:43 +0800 Subject: [PATCH 2/8] Add origins safety cleanup when destroying the context for the root page --- src/browser/Page.zig | 5 +++-- src/browser/js/Env.zig | 32 ++++++++++++++++++++++++++++++-- src/cdp/cdp.zig | 2 +- 3 files changed, 34 insertions(+), 5 deletions(-) diff --git a/src/browser/Page.zig b/src/browser/Page.zig index 9b05c39b..dbb93792 100644 --- a/src/browser/Page.zig +++ b/src/browser/Page.zig @@ -345,11 +345,12 @@ pub fn deinit(self: *Page, abort_http: bool) void { } const session = self._session; - session.browser.env.destroyContext(self.js); + const is_root = self.parent == null; + session.browser.env.destroyContext(self.js, is_root); self._script_manager.shutdown = true; - if (self.parent == null) { + if (is_root) { session.browser.http_client.abort(); } else if (abort_http) { // a small optimization, it's faster to abort _everything_ on the root diff --git a/src/browser/js/Env.zig b/src/browser/js/Env.zig index 9bacc088..d64ef649 100644 --- a/src/browser/js/Env.zig +++ b/src/browser/js/Env.zig @@ -73,7 +73,17 @@ isolate_params: *v8.CreateParams, context_id: usize, -// Maps origin -> shared Origin contains, for v8 values shared across same-origin Contexts +// Maps origin -> shared Origin contains, for v8 values shared across +// same-origin Contexts. There's a mismatch here between our JS model and our +// Browser model. Origins only live as long as the root page of a session exists. +// It would be wrong/dangerous to re-use an Origin across root page navigations. +// But we have no mechanism to capture that lifetime in js. We used to have a +// js.BrowserContext which mapped to a Session (oops, I took it out), but even +// that wouldn't match correctly, because 1 session can have have muliple non- +// concurrent pages. We deal with this in destroyContext by checking if we're +// destroying the root context and, if so, making sure origins is empty. But, if +// we ever add multiple Sessions to a Browser or mulitple Pages to a Session, +// this map will have to live in a new, better scoped, container. origins: std.StringHashMapUnmanaged(*Origin) = .empty, // Global handles that need to be freed on deinit @@ -348,7 +358,7 @@ pub fn createContext(self: *Env, page: *Page) !*Context { return context; } -pub fn destroyContext(self: *Env, context: *Context) void { +pub fn destroyContext(self: *Env, context: *Context, is_root: bool) void { for (self.contexts[0..self.context_count], 0..) |ctx, i| { if (ctx == context) { // Swap with last element and decrement count @@ -371,6 +381,24 @@ pub fn destroyContext(self: *Env, context: *Context) void { } context.deinit(); + + if (is_root) { + // When the root is destroyed, the all of our contexts should be gone + // and with them, all of our origins. Keep origins around longer than + // intended would cause issues, so we're going to be defensive here and + // clean things up. + if (comptime IS_DEBUG) { + std.debug.assert(self.context_count == 0); + std.debug.assert(self.origins.count() == 0); + } + + const app = self.app; + var it = self.origins.valueIterator(); + while (it.next()) |value| { + value.*.deinit(app); + } + self.origins.clearRetainingCapacity(); + } } pub fn getOrCreateOrigin(self: *Env, key_: ?[]const u8) !*Origin { diff --git a/src/cdp/cdp.zig b/src/cdp/cdp.zig index 78e5ab50..40057b8b 100644 --- a/src/cdp/cdp.zig +++ b/src/cdp/cdp.zig @@ -759,7 +759,7 @@ const IsolatedWorld = struct { pub fn removeContext(self: *IsolatedWorld) !void { const ctx = self.context orelse return error.NoIsolatedContextToRemove; - self.browser.env.destroyContext(ctx); + self.browser.env.destroyContext(ctx, false); self.context = null; } From 2a103fc94ab3d50aa777ea9fa113b4d7ec58707c Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Mon, 9 Mar 2026 13:14:57 +0800 Subject: [PATCH 3/8] Use Session as a container for cross-frame resources The introduction of frames means that data is no longer tied to a specific Page or Context. 255b9a91cc409adc685806878b716dc834fb991a introduced Origins for v8 values shared across frames of the same origin. The commit highlighted the lifetime mismatched that we now have with data that can outlive 1 frame. A specific issue with that commit was the finalizers were still Context-owned. But like any other piece of data, that isn't right; aside from modules, nothing should be context-owned. This commit continues where the last left off and moves finalizers from Context to Origin. This is done in a separate commit because it introduces significant changes. Currently, finalizers take a *Page, but that's no longer correct. A value created in one Page, can outlive the Page. We need another container. I original thought to use Origin, but that isn't know to CDP/MCP. Instead, I decide to enhance the Session. Session is now the owner of the page.arena, the page.factory and the page.arena_pool. Finalizers are given a *Session which they can use to release their arena. --- src/browser/EventManager.zig | 4 +- src/browser/Factory.zig | 6 +- src/browser/Page.zig | 94 +++--------- src/browser/Session.zig | 142 +++++++++++++++--- src/browser/js/Context.zig | 89 +---------- src/browser/js/Env.zig | 2 +- src/browser/js/Function.zig | 25 +-- src/browser/js/Local.zig | 7 +- src/browser/js/Origin.zig | 82 +++++++++- src/browser/js/Promise.zig | 25 +-- src/browser/js/Value.zig | 25 +-- src/browser/js/bridge.zig | 26 ++-- src/browser/js/js.zig | 1 + src/browser/webapi/Event.zig | 7 +- src/browser/webapi/EventTarget.zig | 2 +- src/browser/webapi/FileReader.zig | 19 ++- src/browser/webapi/IntersectionObserver.zig | 15 +- src/browser/webapi/MutationObserver.zig | 13 +- src/browser/webapi/Window.zig | 4 +- src/browser/webapi/animation/Animation.zig | 5 +- src/browser/webapi/collections/ChildNodes.zig | 5 +- src/browser/webapi/collections/NodeList.zig | 11 +- src/browser/webapi/collections/iterator.zig | 5 +- src/browser/webapi/encoding/TextDecoder.zig | 5 +- src/browser/webapi/event/CompositionEvent.zig | 5 +- src/browser/webapi/event/CustomEvent.zig | 7 +- src/browser/webapi/event/ErrorEvent.zig | 7 +- src/browser/webapi/event/FocusEvent.zig | 5 +- src/browser/webapi/event/KeyboardEvent.zig | 5 +- src/browser/webapi/event/MessageEvent.zig | 7 +- src/browser/webapi/event/MouseEvent.zig | 5 +- .../NavigationCurrentEntryChangeEvent.zig | 5 +- .../webapi/event/PageTransitionEvent.zig | 5 +- src/browser/webapi/event/PointerEvent.zig | 5 +- src/browser/webapi/event/PopStateEvent.zig | 5 +- src/browser/webapi/event/ProgressEvent.zig | 5 +- .../webapi/event/PromiseRejectionEvent.zig | 9 +- src/browser/webapi/event/TextEvent.zig | 5 +- src/browser/webapi/event/UIEvent.zig | 5 +- src/browser/webapi/event/WheelEvent.zig | 5 +- src/browser/webapi/net/Fetch.zig | 6 +- src/browser/webapi/net/Response.zig | 5 +- src/browser/webapi/net/XMLHttpRequest.zig | 23 +-- src/browser/webapi/selector/List.zig | 5 +- src/cdp/Node.zig | 6 +- src/cdp/domains/dom.zig | 4 +- src/mcp/tools.zig | 3 +- 47 files changed, 427 insertions(+), 334 deletions(-) diff --git a/src/browser/EventManager.zig b/src/browser/EventManager.zig index 573aa4f9..5588b704 100644 --- a/src/browser/EventManager.zig +++ b/src/browser/EventManager.zig @@ -205,7 +205,7 @@ pub fn dispatch(self: *EventManager, target: *EventTarget, event: *Event) Dispat pub fn dispatchOpts(self: *EventManager, target: *EventTarget, event: *Event, comptime opts: DispatchOpts) DispatchError!void { event.acquireRef(); - defer event.deinit(false, self.page); + defer event.deinit(false, self.page._session); if (comptime IS_DEBUG) { log.debug(.event, "eventManager.dispatch", .{ .type = event._type_string.str(), .bubbles = event._bubbles }); @@ -234,7 +234,7 @@ pub fn dispatchDirect(self: *EventManager, target: *EventTarget, event: *Event, const page = self.page; event.acquireRef(); - defer event.deinit(false, page); + defer event.deinit(false, page._session); if (comptime IS_DEBUG) { log.debug(.event, "dispatchDirect", .{ .type = event._type_string, .context = opts.context }); diff --git a/src/browser/Factory.zig b/src/browser/Factory.zig index cbc2170d..c5ede22b 100644 --- a/src/browser/Factory.zig +++ b/src/browser/Factory.zig @@ -48,13 +48,11 @@ const Factory = @This(); _arena: Allocator, _slab: SlabAllocator, -pub fn init(arena: Allocator) !*Factory { - const self = try arena.create(Factory); - self.* = .{ +pub fn init(arena: Allocator) Factory { + return .{ ._arena = arena, ._slab = SlabAllocator.init(arena, 128), }; - return self; } // this is a root object diff --git a/src/browser/Page.zig b/src/browser/Page.zig index dbb93792..60a3f968 100644 --- a/src/browser/Page.zig +++ b/src/browser/Page.zig @@ -214,14 +214,6 @@ 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, struct { - owner: []const u8, - count: usize, -}) else void) = if (IS_DEBUG) .empty else {}, - parent: ?*Page, window: *Window, document: *Document, @@ -248,17 +240,11 @@ pub fn init(self: *Page, frame_id: u32, session: *Session, parent: ?*Page) !void if (comptime IS_DEBUG) { log.debug(.page, "page.init", .{}); } - const browser = session.browser; - const arena_pool = browser.arena_pool; - const page_arena = if (parent) |p| p.arena else try arena_pool.acquire(); - errdefer if (parent == null) arena_pool.release(page_arena); - - var factory = if (parent) |p| p._factory else try Factory.init(page_arena); - - const call_arena = try arena_pool.acquire(); - errdefer arena_pool.release(call_arena); + const call_arena = try session.getArena(.{ .debug = "call_arena" }); + errdefer session.releaseArena(call_arena); + const factory = &session.factory; const document = (try factory.document(Node.Document.HTMLDocument{ ._proto = undefined, })).asDocument(); @@ -266,10 +252,9 @@ pub fn init(self: *Page, frame_id: u32, session: *Session, parent: ?*Page) !void self.* = .{ .js = undefined, .parent = parent, - .arena = page_arena, + .arena = session.page_arena, .document = document, .window = undefined, - .arena_pool = arena_pool, .call_arena = call_arena, ._frame_id = frame_id, ._session = session, @@ -277,7 +262,7 @@ pub fn init(self: *Page, frame_id: u32, session: *Session, parent: ?*Page) !void ._pending_loads = 1, // always 1 for the ScriptManager ._type = if (parent == null) .root else .frame, ._script_manager = undefined, - ._event_manager = EventManager.init(page_arena, self), + ._event_manager = EventManager.init(session.page_arena, self), }; var screen: *Screen = undefined; @@ -305,6 +290,7 @@ pub fn init(self: *Page, frame_id: u32, session: *Session, parent: ?*Page) !void ._visual_viewport = visual_viewport, }); + const browser = session.browser; self._script_manager = ScriptManager.init(browser.allocator, browser.http_client, self); errdefer self._script_manager.deinit(); @@ -340,11 +326,12 @@ pub fn deinit(self: *Page, abort_http: bool) void { // stats.print(&stream) catch unreachable; } + const session = self._session; + if (self._queued_navigation) |qn| { - self.arena_pool.release(qn.arena); + session.releaseArena(qn.arena); } - const session = self._session; const is_root = self.parent == null; session.browser.env.destroyContext(self.js, is_root); @@ -361,23 +348,7 @@ pub fn deinit(self: *Page, abort_http: bool) void { self._script_manager.deinit(); - if (comptime IS_DEBUG) { - var it = self._arena_pool_leak_track.valueIterator(); - while (it.next()) |value_ptr| { - if (value_ptr.count > 0) { - log.err(.bug, "ArenaPool Leak", .{ .owner = value_ptr.owner, .type = self._type, .url = self.url }); - if (comptime builtin.is_test) { - @panic("ArenaPool Leak"); - } - } - } - } - - self.arena_pool.release(self.call_arena); - - if (self.parent == null) { - self.arena_pool.release(self.arena); - } + session.releaseArena(self.call_arena); } pub fn base(self: *const Page) [:0]const u8 { @@ -417,34 +388,12 @@ pub fn headersForRequest(self: *Page, temp: Allocator, url: [:0]const u8, header } } -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) { - const gop = try self._arena_pool_leak_track.getOrPut(self.arena, @intFromPtr(allocator.ptr)); - if (gop.found_existing) { - std.debug.assert(gop.value_ptr.count == 0); - } - gop.value_ptr.* = .{ .owner = opts.debug, .count = 1 }; - } - return allocator; +pub fn getArena(self: *Page, comptime opts: Session.GetArenaOpts) !Allocator { + return self._session.getArena(opts); } pub fn releaseArena(self: *Page, allocator: Allocator) void { - if (comptime IS_DEBUG) { - const found = self._arena_pool_leak_track.getPtr(@intFromPtr(allocator.ptr)).?; - if (found.count != 1) { - log.err(.bug, "ArenaPool Double Free", .{ .owner = found.owner, .count = found.count, .type = self._type, .url = self.url }); - if (comptime builtin.is_test) { - @panic("ArenaPool Double Free"); - } - return; - } - found.count = 0; - } - return self.arena_pool.release(allocator); + return self._session.releaseArena(allocator); } pub fn isSameOrigin(self: *const Page, url: [:0]const u8) !bool { @@ -586,8 +535,8 @@ pub fn scheduleNavigation(self: *Page, request_url: []const u8, opts: NavigateOp if (self.canScheduleNavigation(std.meta.activeTag(nt)) == false) { return; } - const arena = try self.arena_pool.acquire(); - errdefer self.arena_pool.release(arena); + const arena = try self._session.getArena(.{ .debug = "scheduleNavigation" }); + errdefer self._session.releaseArena(arena); return self.scheduleNavigationWithArena(arena, request_url, opts, nt); } @@ -626,9 +575,8 @@ fn scheduleNavigationWithArena(originator: *Page, arena: Allocator, request_url: if (target.parent == null) { try session.navigation.updateEntries(target.url, opts.kind, target, true); } - // doin't defer this, the caller, the caller is responsible for freeing - // it on error - target.arena_pool.release(arena); + // don't defer this, the caller is responsible for freeing it on error + session.releaseArena(arena); return; } @@ -3177,7 +3125,7 @@ pub fn handleClick(self: *Page, target: *Node) !void { pub fn triggerKeyboard(self: *Page, keyboard_event: *KeyboardEvent) !void { const event = keyboard_event.asEvent(); const element = self.window._document._active_element orelse { - keyboard_event.deinit(false, self); + keyboard_event.deinit(false, self._session); return; }; @@ -3253,7 +3201,7 @@ pub fn submitForm(self: *Page, submitter_: ?*Element, form_: ?*Element.Html.Form // so submit_event is still valid when we check _prevent_default submit_event.acquireRef(); - defer submit_event.deinit(false, self); + defer submit_event.deinit(false, self._session); try self._event_manager.dispatch(form_element.asEventTarget(), submit_event); // If the submit event was prevented, don't submit the form @@ -3267,8 +3215,8 @@ pub fn submitForm(self: *Page, submitter_: ?*Element, form_: ?*Element.Html.Form // I don't think this is technically correct, but FormData handles it ok const form_data = try FormData.init(form, submitter_, self); - const arena = try self.arena_pool.acquire(); - errdefer self.arena_pool.release(arena); + const arena = try self._session.getArena(.{ .debug = "submitForm" }); + errdefer self._session.releaseArena(arena); const encoding = form_element.getAttributeSafe(comptime .wrap("enctype")); diff --git a/src/browser/Session.zig b/src/browser/Session.zig index d8a85fa2..42fe8ef5 100644 --- a/src/browser/Session.zig +++ b/src/browser/Session.zig @@ -21,6 +21,7 @@ const lp = @import("lightpanda"); const builtin = @import("builtin"); const log = @import("../log.zig"); +const App = @import("../App.zig"); const js = @import("js/js.zig"); const storage = @import("webapi/storage/storage.zig"); @@ -29,20 +30,50 @@ const History = @import("webapi/History.zig"); const Page = @import("Page.zig"); const Browser = @import("Browser.zig"); +const Factory = @import("Factory.zig"); const Notification = @import("../Notification.zig"); const QueuedNavigation = Page.QueuedNavigation; const Allocator = std.mem.Allocator; +const ArenaPool = App.ArenaPool; const IS_DEBUG = builtin.mode == .Debug; -// Session is like a browser's tab. -// It owns the js env and the loader for all the pages of the session. // You can create successively multiple pages for a session, but you must -// deinit a page before running another one. +// deinit a page before running another one. It manages two distinct lifetimes. +// +// The first is the lifetime of the Session itself, where pages are created and +// removed, but share the same cookie jar and navigation history (etc...) +// +// The second is as a container the data needed by the full page hierarchy, i.e. \ +// the root page and all of its frames (and all of their frames.) const Session = @This(); +// These are the fields that remain intact for the duration of the Session browser: *Browser, +arena: Allocator, +history: History, +navigation: Navigation, +storage_shed: storage.Shed, notification: *Notification, +cookie_jar: storage.Cookie.Jar, + +// These are the fields that get reset whenever the Session's page (the root) is reset. +factory: Factory, + +page_arena: Allocator, + +// Shared resources for all pages in this session. +// These live for the duration of the page tree (root + frames). +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, struct { + owner: []const u8, + count: usize, +}) else void = if (IS_DEBUG) .empty else {}, + +page: ?Page, queued_navigation: std.ArrayList(*Page), // Temporary buffer for about:blank navigations during processing. @@ -50,27 +81,24 @@ queued_navigation: std.ArrayList(*Page), // about:blank navigations (which may add to queued_navigation). queued_queued_navigation: std.ArrayList(*Page), -// Used to create our Inspector and in the BrowserContext. -arena: Allocator, - -cookie_jar: storage.Cookie.Jar, -storage_shed: storage.Shed, - -history: History, -navigation: Navigation, - -page: ?Page, - frame_id_gen: u32, pub fn init(self: *Session, browser: *Browser, notification: *Notification) !void { const allocator = browser.app.allocator; - const arena = try browser.arena_pool.acquire(); - errdefer browser.arena_pool.release(arena); + const arena_pool = browser.arena_pool; + + const arena = try arena_pool.acquire(); + errdefer arena_pool.release(arena); + + const page_arena = try arena_pool.acquire(); + errdefer arena_pool.release(page_arena); self.* = .{ .page = null, .arena = arena, + .arena_pool = arena_pool, + .page_arena = page_arena, + .factory = Factory.init(page_arena), .history = .{}, .frame_id_gen = 0, // The prototype (EventTarget) for Navigation is created when a Page is created. @@ -90,9 +118,9 @@ pub fn deinit(self: *Session) void { } self.cookie_jar.deinit(); - const browser = self.browser; - self.storage_shed.deinit(browser.app.allocator); - browser.arena_pool.release(self.arena); + self.storage_shed.deinit(self.browser.app.allocator); + self.arena_pool.release(self.page_arena); + self.arena_pool.release(self.arena); } // NOTE: the caller is not the owner of the returned value, @@ -126,29 +154,84 @@ pub fn removePage(self: *Session) void { self.page = null; self.navigation.onRemovePage(); + self.resetPageResources(); if (comptime IS_DEBUG) { log.debug(.browser, "remove page", .{}); } } +pub const GetArenaOpts = struct { + debug: []const u8, +}; + +pub fn getArena(self: *Session, opts: GetArenaOpts) !Allocator { + const allocator = try self.arena_pool.acquire(); + if (comptime IS_DEBUG) { + // Use session's arena (not page_arena) since page_arena gets reset between pages + const gop = try self._arena_pool_leak_track.getOrPut(self.arena, @intFromPtr(allocator.ptr)); + if (gop.found_existing and gop.value_ptr.count != 0) { + log.err(.bug, "ArenaPool Double Use", .{ .owner = gop.value_ptr.*.owner }); + @panic("ArenaPool Double Use"); + } + gop.value_ptr.* = .{ .owner = opts.debug, .count = 1 }; + } + return allocator; +} + +pub fn releaseArena(self: *Session, allocator: Allocator) void { + if (comptime IS_DEBUG) { + const found = self._arena_pool_leak_track.getPtr(@intFromPtr(allocator.ptr)).?; + if (found.count != 1) { + log.err(.bug, "ArenaPool Double Free", .{ .owner = found.owner, .count = found.count }); + if (comptime builtin.is_test) { + @panic("ArenaPool Double Free"); + } + return; + } + found.count = 0; + } + return self.arena_pool.release(allocator); +} + +/// Reset page_arena and factory for a clean slate. +/// Called when root page is removed. +fn resetPageResources(self: *Session) void { + // Check for arena leaks before releasing + if (comptime IS_DEBUG) { + var it = self._arena_pool_leak_track.valueIterator(); + while (it.next()) |value_ptr| { + if (value_ptr.count > 0) { + log.err(.bug, "ArenaPool Leak", .{ .owner = value_ptr.owner }); + } + } + self._arena_pool_leak_track.clearRetainingCapacity(); + } + + // Release old page_arena and acquire fresh one + self.frame_id_gen = 0; + self.arena_pool.reset(self.page_arena, 64 * 1024); + self.factory = Factory.init(self.page_arena); +} + pub fn replacePage(self: *Session) !*Page { if (comptime IS_DEBUG) { log.debug(.browser, "replace page", .{}); } lp.assert(self.page != null, "Session.replacePage null page", .{}); + lp.assert(self.page.?.parent == null, "Session.replacePage with parent", .{}); var current = self.page.?; const frame_id = current._frame_id; - const parent = current.parent; current.deinit(false); + self.resetPageResources(); self.browser.env.memoryPressureNotification(.moderate); self.page = @as(Page, undefined); const page = &self.page.?; - try Page.init(page, frame_id, self, parent); + try Page.init(page, frame_id, self, null); return page; } @@ -428,12 +511,11 @@ fn processQueuedNavigation(self: *Session) !void { fn processFrameNavigation(self: *Session, page: *Page, qn: *QueuedNavigation) !void { lp.assert(page.parent != null, "root queued navigation", .{}); - const browser = self.browser; const iframe = page.iframe.?; const parent = page.parent.?; page._queued_navigation = null; - defer browser.arena_pool.release(qn.arena); + defer self.releaseArena(qn.arena); errdefer iframe._window = null; @@ -465,9 +547,21 @@ fn processRootQueuedNavigation(self: *Session) !void { // create a copy before the page is cleared const qn = current_page._queued_navigation.?; current_page._queued_navigation = null; - defer self.browser.arena_pool.release(qn.arena); + + defer self.arena_pool.release(qn.arena); + + // HACK + // Mark as released in tracking BEFORE removePage clears the map. + // We can't call releaseArena() because that would also return the arena + // to the pool, making the memory invalid before we use qn.url/qn.opts. + if (comptime IS_DEBUG) { + if (self._arena_pool_leak_track.getPtr(@intFromPtr(qn.arena.ptr))) |found| { + found.count = 0; + } + } self.removePage(); + self.page = @as(Page, undefined); const new_page = &self.page.?; try Page.init(new_page, frame_id, self, null); diff --git a/src/browser/js/Context.zig b/src/browser/js/Context.zig index 065b5e28..750cd809 100644 --- a/src/browser/js/Context.zig +++ b/src/browser/js/Context.zig @@ -27,6 +27,7 @@ const Origin = @import("Origin.zig"); const Scheduler = @import("Scheduler.zig"); const Page = @import("../Page.zig"); +const Session = @import("../Session.zig"); const ScriptManager = @import("../ScriptManager.zig"); const v8 = js.v8; @@ -42,6 +43,7 @@ const Context = @This(); id: usize, env: *Env, page: *Page, +session: *Session, isolate: js.Isolate, // Per-context microtask queue for isolation between contexts @@ -77,12 +79,6 @@ local: ?*const js.Local = null, origin: *Origin, -// 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, -finalizer_callback_pool: std.heap.MemoryPool(FinalizerCallback), - // Unlike other v8 types, like functions or objects, modules are not shared // across origins. global_modules: std.ArrayList(v8.Global) = .empty, @@ -153,14 +149,6 @@ pub fn deinit(self: *Context) void { // this can release objects self.scheduler.deinit(); - { - var it = self.finalizer_callbacks.valueIterator(); - while (it.next()) |finalizer| { - finalizer.*.deinit(); - } - self.finalizer_callback_pool.deinit(); - } - for (self.global_modules.items) |*global| { v8.v8__Global__Reset(global); } @@ -208,7 +196,7 @@ pub fn trackTemp(self: *Context, global: v8.Global) !void { } pub fn weakRef(self: *Context, obj: anytype) void { - const fc = self.finalizer_callbacks.get(@intFromPtr(obj)) orelse { + const fc = self.origin.finalizer_callbacks.get(@intFromPtr(obj)) orelse { if (comptime IS_DEBUG) { // should not be possible std.debug.assert(false); @@ -219,7 +207,7 @@ pub fn weakRef(self: *Context, obj: anytype) void { } pub fn safeWeakRef(self: *Context, obj: anytype) void { - const fc = self.finalizer_callbacks.get(@intFromPtr(obj)) orelse { + const fc = self.origin.finalizer_callbacks.get(@intFromPtr(obj)) orelse { if (comptime IS_DEBUG) { // should not be possible std.debug.assert(false); @@ -231,7 +219,7 @@ pub fn safeWeakRef(self: *Context, obj: anytype) void { } pub fn strongRef(self: *Context, obj: anytype) void { - const fc = self.finalizer_callbacks.get(@intFromPtr(obj)) orelse { + const fc = self.origin.finalizer_callbacks.get(@intFromPtr(obj)) orelse { if (comptime IS_DEBUG) { // should not be possible std.debug.assert(false); @@ -241,45 +229,6 @@ pub fn strongRef(self: *Context, obj: anytype) void { v8.v8__Global__ClearWeak(&fc.global); } -pub fn release(self: *Context, item: anytype) void { - if (@TypeOf(item) == *anyopaque) { - // Existing *anyopaque path for identity_map. Called internally from - // finalizers - var global = self.origin.identity_map.fetchRemove(@intFromPtr(item)) 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. - const fc = self.finalizer_callbacks.fetchRemove(@intFromPtr(item)) orelse { - if (comptime IS_DEBUG) { - // should not be possible - std.debug.assert(false); - } - return; - }; - self.finalizer_callback_pool.destroy(fc.value); - return; - } - - if (comptime IS_DEBUG) { - switch (@TypeOf(item)) { - js.Value.Temp, js.Promise.Temp, js.Function.Temp => {}, - else => |T| @compileError("Context.release cannot be called with a " ++ @typeName(T)), - } - } - - if (self.origin.temps.fetchRemove(item.handle.data_ptr)) |kv| { - var global = kv.value; - v8.v8__Global__Reset(&global); - } -} - // 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; @@ -1005,34 +954,6 @@ pub fn queueMicrotaskFunc(self: *Context, cb: js.Function) void { v8.v8__MicrotaskQueue__EnqueueMicrotaskFunc(self.microtask_queue, self.isolate.handle, cb.handle); } -pub fn createFinalizerCallback(self: *Context, global: v8.Global, ptr: *anyopaque, finalizerFn: *const fn (ptr: *anyopaque, page: *Page) void) !*FinalizerCallback { - const fc = try self.finalizer_callback_pool.create(); - fc.* = .{ - .ctx = self, - .ptr = ptr, - .global = global, - .finalizerFn = finalizerFn, - }; - return fc; -} - -// == Misc == -// 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. -pub const FinalizerCallback = struct { - ctx: *Context, - ptr: *anyopaque, - global: v8.Global, - finalizerFn: *const fn (ptr: *anyopaque, page: *Page) void, - - pub fn deinit(self: *FinalizerCallback) void { - self.finalizerFn(self.ptr, self.ctx.page); - self.ctx.finalizer_callback_pool.destroy(self); - } -}; - // == Profiler == pub fn startCpuProfiler(self: *Context) void { if (comptime !IS_DEBUG) { diff --git a/src/browser/js/Env.zig b/src/browser/js/Env.zig index d64ef649..4318394d 100644 --- a/src/browser/js/Env.zig +++ b/src/browser/js/Env.zig @@ -330,6 +330,7 @@ pub fn createContext(self: *Env, page: *Page) !*Context { context.* = .{ .env = self, .page = page, + .session = page._session, .origin = origin, .id = context_id, .isolate = isolate, @@ -340,7 +341,6 @@ pub fn createContext(self: *Env, page: *Page) !*Context { .microtask_queue = microtask_queue, .script_manager = &page._script_manager, .scheduler = .init(context_arena), - .finalizer_callback_pool = std.heap.MemoryPool(Context.FinalizerCallback).init(self.app.allocator), }; try context.origin.identity_map.putNoClobber(context_arena, @intFromPtr(page.window), global_global); diff --git a/src/browser/js/Function.zig b/src/browser/js/Function.zig index 203fd9ab..bfb5e53d 100644 --- a/src/browser/js/Function.zig +++ b/src/browser/js/Function.zig @@ -210,10 +210,10 @@ fn _persist(self: *const Function, comptime is_global: bool) !(if (is_global) Gl v8.v8__Global__New(ctx.isolate.handle, self.handle, &global); if (comptime is_global) { try ctx.trackGlobal(global); - } else { - try ctx.trackTemp(global); + return .{ .handle = global, .origin = {} }; } - return .{ .handle = global }; + try ctx.trackTemp(global); + return .{ .handle = global, .origin = ctx.origin }; } pub fn tempWithThis(self: *const Function, value: anytype) !Temp { @@ -226,15 +226,18 @@ pub fn persistWithThis(self: *const Function, value: anytype) !Global { return with_this.persist(); } -pub const Temp = G(0); -pub const Global = G(1); +pub const Temp = G(.temp); +pub const Global = G(.global); -fn G(comptime discriminator: u8) type { +const GlobalType = enum(u8) { + temp, + global, +}; + +fn G(comptime global_type: GlobalType) type { return struct { handle: v8.Global, - - // makes the types different (G(0) != G(1)), without taking up space - comptime _: u8 = discriminator, + origin: if (global_type == .temp) *js.Origin else void, const Self = @This(); @@ -252,5 +255,9 @@ fn G(comptime discriminator: u8) type { pub fn isEqual(self: *const Self, other: Function) bool { return v8.v8__Global__IsEqual(&self.handle, other.handle); } + + pub fn release(self: *const Self) void { + self.origin.releaseTemp(self.handle); + } }; } diff --git a/src/browser/js/Local.zig b/src/browser/js/Local.zig index 305391f5..84e83b49 100644 --- a/src/browser/js/Local.zig +++ b/src/browser/js/Local.zig @@ -18,6 +18,7 @@ const std = @import("std"); const Page = @import("../Page.zig"); +const Session = @import("../Session.zig"); const log = @import("../../log.zig"); const string = @import("../../string.zig"); @@ -231,10 +232,10 @@ pub fn mapZigInstanceToJs(self: *const Local, js_obj_handle: ?*const v8.Object, // Instead, we check if the base has finalizer. The assumption // here is that if a resolve type has a finalizer, then the base // should have a finalizer too. - const fc = try ctx.createFinalizerCallback(gop.value_ptr.*, resolved.ptr, resolved.finalizer_from_zig.?); + const fc = try ctx.origin.createFinalizerCallback(ctx.session, gop.value_ptr.*, resolved.ptr, resolved.finalizer_from_zig.?); { errdefer fc.deinit(); - try ctx.finalizer_callbacks.put(ctx.arena, @intFromPtr(resolved.ptr), fc); + try ctx.origin.finalizer_callbacks.put(ctx.origin.arena, @intFromPtr(resolved.ptr), fc); } conditionallyReference(value); @@ -1083,7 +1084,7 @@ const Resolved = struct { class_id: u16, prototype_chain: []const @import("TaggedOpaque.zig").PrototypeChainEntry, finalizer_from_v8: ?*const fn (handle: ?*const v8.WeakCallbackInfo) callconv(.c) void = null, - finalizer_from_zig: ?*const fn (ptr: *anyopaque, page: *Page) void = null, + finalizer_from_zig: ?*const fn (ptr: *anyopaque, session: *Session) void = null, }; pub fn resolveValue(value: anytype) Resolved { const T = bridge.Struct(@TypeOf(value)); diff --git a/src/browser/js/Origin.zig b/src/browser/js/Origin.zig index 6cb0b356..356b75c8 100644 --- a/src/browser/js/Origin.zig +++ b/src/browser/js/Origin.zig @@ -24,10 +24,11 @@ const std = @import("std"); const js = @import("js.zig"); const App = @import("../../App.zig"); +const Session = @import("../Session.zig"); const v8 = js.v8; const Allocator = std.mem.Allocator; -const IS_DEBUG = @import("build").mode == .Debug; +const IS_DEBUG = @import("builtin").mode == .Debug; const Origin = @This(); @@ -62,6 +63,29 @@ globals: std.ArrayList(v8.Global) = .empty, // Key is global.data_ptr. temps: 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, +finalizer_callback_pool: std.heap.MemoryPool(FinalizerCallback), + +// 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 finalizer_callbacks and call them on +// origin shutdown. +pub const FinalizerCallback = struct { + origin: *Origin, + session: *Session, + ptr: *anyopaque, + global: v8.Global, + finalizerFn: *const fn (ptr: *anyopaque, session: *Session) void, + + pub fn deinit(self: *FinalizerCallback) void { + self.finalizerFn(self.ptr, self.session); + self.origin.finalizer_callback_pool.destroy(self); + } +}; + pub fn init(app: *App, isolate: js.Isolate, key: []const u8) !*Origin { const arena = try app.arena_pool.acquire(); errdefer app.arena_pool.release(arena); @@ -83,11 +107,21 @@ pub fn init(app: *App, isolate: js.Isolate, key: []const u8) !*Origin { .globals = .empty, .temps = .empty, .security_token = token_global, + .finalizer_callback_pool = .init(arena), }; return self; } pub fn deinit(self: *Origin, app: *App) void { + // Call finalizers before releasing anything + { + var it = self.finalizer_callbacks.valueIterator(); + while (it.next()) |finalizer| { + finalizer.*.deinit(); + } + self.finalizer_callback_pool.deinit(); + } + v8.v8__Global__Reset(&self.security_token); { @@ -119,6 +153,52 @@ pub fn trackTemp(self: *Origin, global: v8.Global) !void { return self.temps.put(self.arena, global.data_ptr, global); } +pub fn releaseTemp(self: *Origin, global: v8.Global) void { + if (self.temps.fetchRemove(global.data_ptr)) |kv| { + var g = kv.value; + v8.v8__Global__Reset(&g); + } +} + +/// Release an item from the identity_map (called after finalizer runs from V8) +pub fn release(self: *Origin, item: *anyopaque) void { + var global = self.identity_map.fetchRemove(@intFromPtr(item)) orelse { + if (comptime IS_DEBUG) { + std.debug.assert(false); + } + return; + }; + v8.v8__Global__Reset(&global.value); + + // The item has been finalized, remove it from the finalizer callback so that + // we don't try to call it again on shutdown. + const fc = self.finalizer_callbacks.fetchRemove(@intFromPtr(item)) orelse { + if (comptime IS_DEBUG) { + std.debug.assert(false); + } + return; + }; + self.finalizer_callback_pool.destroy(fc.value); +} + +pub fn createFinalizerCallback( + self: *Origin, + session: *Session, + global: v8.Global, + ptr: *anyopaque, + finalizerFn: *const fn (ptr: *anyopaque, session: *Session) void, +) !*FinalizerCallback { + const fc = try self.finalizer_callback_pool.create(); + fc.* = .{ + .origin = self, + .session = session, + .ptr = ptr, + .global = global, + .finalizerFn = finalizerFn, + }; + return fc; +} + pub fn transferTo(self: *Origin, dest: *Origin) !void { const arena = dest.arena; diff --git a/src/browser/js/Promise.zig b/src/browser/js/Promise.zig index 98520d4b..372d2578 100644 --- a/src/browser/js/Promise.zig +++ b/src/browser/js/Promise.zig @@ -63,21 +63,24 @@ fn _persist(self: *const Promise, comptime is_global: bool) !(if (is_global) Glo v8.v8__Global__New(ctx.isolate.handle, self.handle, &global); if (comptime is_global) { try ctx.trackGlobal(global); - } else { - try ctx.trackTemp(global); + return .{ .handle = global, .origin = {} }; } - return .{ .handle = global }; + try ctx.trackTemp(global); + return .{ .handle = global, .origin = ctx.origin }; } -pub const Temp = G(0); -pub const Global = G(1); +pub const Temp = G(.temp); +pub const Global = G(.global); -fn G(comptime discriminator: u8) type { +const GlobalType = enum(u8) { + temp, + global, +}; + +fn G(comptime global_type: GlobalType) type { return struct { handle: v8.Global, - - // makes the types different (G(0) != G(1)), without taking up space - comptime _: u8 = discriminator, + origin: if (global_type == .temp) *js.Origin else void, const Self = @This(); @@ -91,5 +94,9 @@ fn G(comptime discriminator: u8) type { .handle = @ptrCast(v8.v8__Global__Get(&self.handle, l.isolate.handle)), }; } + + pub fn release(self: *const Self) void { + self.origin.releaseTemp(self.handle); + } }; } diff --git a/src/browser/js/Value.zig b/src/browser/js/Value.zig index fbc961ed..309bdb6b 100644 --- a/src/browser/js/Value.zig +++ b/src/browser/js/Value.zig @@ -260,10 +260,10 @@ fn _persist(self: *const Value, comptime is_global: bool) !(if (is_global) Globa v8.v8__Global__New(ctx.isolate.handle, self.handle, &global); if (comptime is_global) { try ctx.trackGlobal(global); - } else { - try ctx.trackTemp(global); + return .{ .handle = global, .origin = {} }; } - return .{ .handle = global }; + try ctx.trackTemp(global); + return .{ .handle = global, .origin = ctx.origin }; } pub fn toZig(self: Value, comptime T: type) !T { @@ -310,15 +310,18 @@ pub fn format(self: Value, writer: *std.Io.Writer) !void { return js_str.format(writer); } -pub const Temp = G(0); -pub const Global = G(1); +pub const Temp = G(.temp); +pub const Global = G(.global); -fn G(comptime discriminator: u8) type { +const GlobalType = enum(u8) { + temp, + global, +}; + +fn G(comptime global_type: GlobalType) type { return struct { handle: v8.Global, - - // makes the types different (G(0) != G(1)), without taking up space - comptime _: u8 = discriminator, + origin: if (global_type == .temp) *js.Origin else void, const Self = @This(); @@ -336,5 +339,9 @@ fn G(comptime discriminator: u8) type { pub fn isEqual(self: *const Self, other: Value) bool { return v8.v8__Global__IsEqual(&self.handle, other.handle); } + + pub fn release(self: *const Self) void { + self.origin.releaseTemp(self.handle); + } }; } diff --git a/src/browser/js/bridge.zig b/src/browser/js/bridge.zig index fcbbea21..f95329dc 100644 --- a/src/browser/js/bridge.zig +++ b/src/browser/js/bridge.zig @@ -21,11 +21,13 @@ const js = @import("js.zig"); const lp = @import("lightpanda"); const log = @import("../../log.zig"); const Page = @import("../Page.zig"); +const Session = @import("../Session.zig"); const v8 = js.v8; const Caller = @import("Caller.zig"); const Context = @import("Context.zig"); +const Origin = @import("Origin.zig"); const IS_DEBUG = @import("builtin").mode == .Debug; @@ -104,24 +106,24 @@ pub fn Builder(comptime T: type) type { return entries; } - pub fn finalizer(comptime func: *const fn (self: *T, shutdown: bool, page: *Page) void) Finalizer { + pub fn finalizer(comptime func: *const fn (self: *T, shutdown: bool, session: *Session) void) Finalizer { return .{ .from_zig = struct { - fn wrap(ptr: *anyopaque, page: *Page) void { - func(@ptrCast(@alignCast(ptr)), true, page); + fn wrap(ptr: *anyopaque, session: *Session) void { + func(@ptrCast(@alignCast(ptr)), true, session); } }.wrap, .from_v8 = struct { fn wrap(handle: ?*const v8.WeakCallbackInfo) callconv(.c) void { const ptr = v8.v8__WeakCallbackInfo__GetParameter(handle.?).?; - const fc: *Context.FinalizerCallback = @ptrCast(@alignCast(ptr)); + const fc: *Origin.FinalizerCallback = @ptrCast(@alignCast(ptr)); - const ctx = fc.ctx; + const origin = fc.origin; const value_ptr = fc.ptr; - if (ctx.finalizer_callbacks.contains(@intFromPtr(value_ptr))) { - func(@ptrCast(@alignCast(value_ptr)), false, ctx.page); - ctx.release(value_ptr); + if (origin.finalizer_callbacks.contains(@intFromPtr(value_ptr))) { + func(@ptrCast(@alignCast(value_ptr)), false, fc.session); + origin.release(value_ptr); } else { // A bit weird, but v8 _requires_ that we release it // If we don't. We'll 100% crash. @@ -413,12 +415,12 @@ pub const Property = struct { }; const Finalizer = struct { - // The finalizer wrapper when called fro Zig. This is only called on - // Context.deinit - from_zig: *const fn (ctx: *anyopaque, page: *Page) void, + // The finalizer wrapper when called from Zig. This is only called on + // Origin.deinit + from_zig: *const fn (ctx: *anyopaque, session: *Session) 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, + // (hence why we fallback to calling in Origin.deinit). 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, }; diff --git a/src/browser/js/js.zig b/src/browser/js/js.zig index 22651c39..0c196e5b 100644 --- a/src/browser/js/js.zig +++ b/src/browser/js/js.zig @@ -24,6 +24,7 @@ const string = @import("../../string.zig"); pub const Env = @import("Env.zig"); pub const bridge = @import("bridge.zig"); pub const Caller = @import("Caller.zig"); +pub const Origin = @import("Origin.zig"); pub const Context = @import("Context.zig"); pub const Local = @import("Local.zig"); pub const Inspector = @import("Inspector.zig"); diff --git a/src/browser/webapi/Event.zig b/src/browser/webapi/Event.zig index 2821e9f1..d70042f8 100644 --- a/src/browser/webapi/Event.zig +++ b/src/browser/webapi/Event.zig @@ -20,6 +20,7 @@ const std = @import("std"); const js = @import("../js/js.zig"); const Page = @import("../Page.zig"); +const Session = @import("../Session.zig"); const EventTarget = @import("EventTarget.zig"); const Node = @import("Node.zig"); const String = @import("../../string.zig").String; @@ -139,9 +140,9 @@ pub fn acquireRef(self: *Event) void { self._rc += 1; } -pub fn deinit(self: *Event, shutdown: bool, page: *Page) void { +pub fn deinit(self: *Event, shutdown: bool, session: *Session) void { if (shutdown) { - page.releaseArena(self._arena); + session.releaseArena(self._arena); return; } @@ -151,7 +152,7 @@ pub fn deinit(self: *Event, shutdown: bool, page: *Page) void { } if (rc == 1) { - page.releaseArena(self._arena); + session.releaseArena(self._arena); } else { self._rc = rc - 1; } diff --git a/src/browser/webapi/EventTarget.zig b/src/browser/webapi/EventTarget.zig index 31f472c3..715c209a 100644 --- a/src/browser/webapi/EventTarget.zig +++ b/src/browser/webapi/EventTarget.zig @@ -59,7 +59,7 @@ pub fn dispatchEvent(self: *EventTarget, event: *Event, page: *Page) !bool { event._is_trusted = false; event.acquireRef(); - defer event.deinit(false, page); + defer event.deinit(false, page._session); try page._event_manager.dispatch(self, event); return !event._cancelable or !event._prevent_default; } diff --git a/src/browser/webapi/FileReader.zig b/src/browser/webapi/FileReader.zig index 90e26aa0..da7ec71c 100644 --- a/src/browser/webapi/FileReader.zig +++ b/src/browser/webapi/FileReader.zig @@ -20,6 +20,7 @@ const std = @import("std"); const js = @import("../js/js.zig"); const Page = @import("../Page.zig"); +const Session = @import("../Session.zig"); const EventTarget = @import("EventTarget.zig"); const ProgressEvent = @import("event/ProgressEvent.zig"); const Blob = @import("Blob.zig"); @@ -69,17 +70,15 @@ pub fn init(page: *Page) !*FileReader { }); } -pub fn deinit(self: *FileReader, _: bool, page: *Page) void { - const js_ctx = page.js; +pub fn deinit(self: *FileReader, _: bool, session: *Session) void { + if (self._on_abort) |func| func.release(); + if (self._on_error) |func| func.release(); + if (self._on_load) |func| func.release(); + if (self._on_load_end) |func| func.release(); + if (self._on_load_start) |func| func.release(); + if (self._on_progress) |func| func.release(); - if (self._on_abort) |func| js_ctx.release(func); - if (self._on_error) |func| js_ctx.release(func); - if (self._on_load) |func| js_ctx.release(func); - if (self._on_load_end) |func| js_ctx.release(func); - if (self._on_load_start) |func| js_ctx.release(func); - if (self._on_progress) |func| js_ctx.release(func); - - page.releaseArena(self._arena); + session.releaseArena(self._arena); } fn asEventTarget(self: *FileReader) *EventTarget { diff --git a/src/browser/webapi/IntersectionObserver.zig b/src/browser/webapi/IntersectionObserver.zig index 9d64de3c..74a5d79e 100644 --- a/src/browser/webapi/IntersectionObserver.zig +++ b/src/browser/webapi/IntersectionObserver.zig @@ -24,6 +24,7 @@ const IS_DEBUG = @import("builtin").mode == .Debug; const Allocator = std.mem.Allocator; const Page = @import("../Page.zig"); +const Session = @import("../Session.zig"); const Element = @import("Element.zig"); const DOMRect = @import("DOMRect.zig"); @@ -91,13 +92,13 @@ pub fn init(callback: js.Function.Temp, options: ?ObserverInit, page: *Page) !*I return self; } -pub fn deinit(self: *IntersectionObserver, shutdown: bool, page: *Page) void { - page.js.release(self._callback); +pub fn deinit(self: *IntersectionObserver, shutdown: bool, session: *Session) void { + self._callback.release(); if ((comptime IS_DEBUG) and !shutdown) { std.debug.assert(self._observing.items.len == 0); } - page.releaseArena(self._arena); + session.releaseArena(self._arena); } pub fn observe(self: *IntersectionObserver, target: *Element, page: *Page) !void { @@ -137,7 +138,7 @@ pub fn unobserve(self: *IntersectionObserver, target: *Element, page: *Page) voi while (j < self._pending_entries.items.len) { if (self._pending_entries.items[j]._target == target) { const entry = self._pending_entries.swapRemove(j); - entry.deinit(false, page); + entry.deinit(false, page._session); } else { j += 1; } @@ -157,7 +158,7 @@ pub fn disconnect(self: *IntersectionObserver, page: *Page) void { self._previous_states.clearRetainingCapacity(); for (self._pending_entries.items) |entry| { - entry.deinit(false, page); + entry.deinit(false, page._session); } self._pending_entries.clearRetainingCapacity(); page.js.safeWeakRef(self); @@ -302,8 +303,8 @@ pub const IntersectionObserverEntry = struct { _intersection_ratio: f64, _is_intersecting: bool, - pub fn deinit(self: *const IntersectionObserverEntry, _: bool, page: *Page) void { - page.releaseArena(self._arena); + pub fn deinit(self: *IntersectionObserverEntry, _: bool, session: *Session) void { + session.releaseArena(self._arena); } pub fn getTarget(self: *const IntersectionObserverEntry) *Element { diff --git a/src/browser/webapi/MutationObserver.zig b/src/browser/webapi/MutationObserver.zig index a5dd15dd..b8608381 100644 --- a/src/browser/webapi/MutationObserver.zig +++ b/src/browser/webapi/MutationObserver.zig @@ -21,6 +21,7 @@ const String = @import("../../string.zig").String; const js = @import("../js/js.zig"); const Page = @import("../Page.zig"); +const Session = @import("../Session.zig"); const Node = @import("Node.zig"); const Element = @import("Element.zig"); const log = @import("../../log.zig"); @@ -84,13 +85,13 @@ pub fn init(callback: js.Function.Temp, page: *Page) !*MutationObserver { return self; } -pub fn deinit(self: *MutationObserver, shutdown: bool, page: *Page) void { - page.js.release(self._callback); +pub fn deinit(self: *MutationObserver, shutdown: bool, session: *Session) void { + self._callback.release(); if ((comptime IS_DEBUG) and !shutdown) { std.debug.assert(self._observing.items.len == 0); } - page.releaseArena(self._arena); + session.releaseArena(self._arena); } pub fn observe(self: *MutationObserver, target: *Node, options: ObserveOptions, page: *Page) !void { @@ -171,7 +172,7 @@ pub fn disconnect(self: *MutationObserver, page: *Page) void { page.unregisterMutationObserver(self); self._observing.clearRetainingCapacity(); for (self._pending_records.items) |record| { - record.deinit(false, page); + record.deinit(false, page._session); } self._pending_records.clearRetainingCapacity(); page.js.safeWeakRef(self); @@ -363,8 +364,8 @@ pub const MutationRecord = struct { characterData, }; - pub fn deinit(self: *const MutationRecord, _: bool, page: *Page) void { - page.releaseArena(self._arena); + pub fn deinit(self: *MutationRecord, _: bool, session: *Session) void { + session.releaseArena(self._arena); } pub fn getType(self: *const MutationRecord) []const u8 { diff --git a/src/browser/webapi/Window.zig b/src/browser/webapi/Window.zig index 40265bb5..2bc9fbb0 100644 --- a/src/browser/webapi/Window.zig +++ b/src/browser/webapi/Window.zig @@ -646,9 +646,9 @@ const ScheduleCallback = struct { } fn deinit(self: *ScheduleCallback) void { - self.page.js.release(self.cb); + self.cb.release(); for (self.params) |param| { - self.page.js.release(param); + param.release(); } self.page.releaseArena(self.arena); } diff --git a/src/browser/webapi/animation/Animation.zig b/src/browser/webapi/animation/Animation.zig index 96143952..8d445733 100644 --- a/src/browser/webapi/animation/Animation.zig +++ b/src/browser/webapi/animation/Animation.zig @@ -20,6 +20,7 @@ const std = @import("std"); const log = @import("../../../log.zig"); const js = @import("../../js/js.zig"); const Page = @import("../../Page.zig"); +const Session = @import("../../Session.zig"); const Allocator = std.mem.Allocator; @@ -61,8 +62,8 @@ pub fn init(page: *Page) !*Animation { return self; } -pub fn deinit(self: *Animation, _: bool, page: *Page) void { - page.releaseArena(self._arena); +pub fn deinit(self: *Animation, _: bool, session: *Session) void { + session.releaseArena(self._arena); } pub fn play(self: *Animation, page: *Page) !void { diff --git a/src/browser/webapi/collections/ChildNodes.zig b/src/browser/webapi/collections/ChildNodes.zig index 5ba6256a..9c2bde91 100644 --- a/src/browser/webapi/collections/ChildNodes.zig +++ b/src/browser/webapi/collections/ChildNodes.zig @@ -20,6 +20,7 @@ const std = @import("std"); const Node = @import("../Node.zig"); const Page = @import("../../Page.zig"); +const Session = @import("../../Session.zig"); const GenericIterator = @import("iterator.zig").Entry; // Optimized for node.childNodes, which has to be a live list. @@ -53,8 +54,8 @@ pub fn init(node: *Node, page: *Page) !*ChildNodes { return self; } -pub fn deinit(self: *const ChildNodes, page: *Page) void { - page.releaseArena(self._arena); +pub fn deinit(self: *const ChildNodes, session: *Session) void { + session.releaseArena(self._arena); } pub fn length(self: *ChildNodes, page: *Page) !u32 { diff --git a/src/browser/webapi/collections/NodeList.zig b/src/browser/webapi/collections/NodeList.zig index 0237a76c..a61cc598 100644 --- a/src/browser/webapi/collections/NodeList.zig +++ b/src/browser/webapi/collections/NodeList.zig @@ -21,6 +21,7 @@ const std = @import("std"); const log = @import("../../../log.zig"); const js = @import("../../js/js.zig"); const Page = @import("../../Page.zig"); +const Session = @import("../../Session.zig"); const Node = @import("../Node.zig"); const ChildNodes = @import("ChildNodes.zig"); @@ -38,7 +39,7 @@ _data: union(enum) { }, _rc: usize = 0, -pub fn deinit(self: *NodeList, _: bool, page: *Page) void { +pub fn deinit(self: *NodeList, _: bool, session: *Session) void { const rc = self._rc; if (rc > 1) { self._rc = rc - 1; @@ -46,8 +47,8 @@ pub fn deinit(self: *NodeList, _: bool, page: *Page) void { } switch (self._data) { - .selector_list => |list| list.deinit(page), - .child_nodes => |cn| cn.deinit(page), + .selector_list => |list| list.deinit(session), + .child_nodes => |cn| cn.deinit(session), else => {}, } } @@ -118,8 +119,8 @@ const Iterator = struct { const Entry = struct { u32, *Node }; - pub fn deinit(self: *Iterator, shutdown: bool, page: *Page) void { - self.list.deinit(shutdown, page); + pub fn deinit(self: *Iterator, shutdown: bool, session: *Session) void { + self.list.deinit(shutdown, session); } pub fn acquireRef(self: *Iterator) void { diff --git a/src/browser/webapi/collections/iterator.zig b/src/browser/webapi/collections/iterator.zig index 3c43f3f8..9fe3354d 100644 --- a/src/browser/webapi/collections/iterator.zig +++ b/src/browser/webapi/collections/iterator.zig @@ -19,6 +19,7 @@ const std = @import("std"); const js = @import("../../js/js.zig"); const Page = @import("../../Page.zig"); +const Session = @import("../../Session.zig"); pub fn Entry(comptime Inner: type, comptime field: ?[]const u8) type { const R = reflect(Inner, field); @@ -39,9 +40,9 @@ pub fn Entry(comptime Inner: type, comptime field: ?[]const u8) type { return page._factory.create(Self{ .inner = inner }); } - pub fn deinit(self: *Self, shutdown: bool, page: *Page) void { + pub fn deinit(self: *Self, shutdown: bool, session: *Session) void { if (@hasDecl(Inner, "deinit")) { - self.inner.deinit(shutdown, page); + self.inner.deinit(shutdown, session); } } diff --git a/src/browser/webapi/encoding/TextDecoder.zig b/src/browser/webapi/encoding/TextDecoder.zig index 4f23cfcc..736a4008 100644 --- a/src/browser/webapi/encoding/TextDecoder.zig +++ b/src/browser/webapi/encoding/TextDecoder.zig @@ -20,6 +20,7 @@ const std = @import("std"); const js = @import("../../js/js.zig"); const Page = @import("../../Page.zig"); +const Session = @import("../../Session.zig"); const Allocator = std.mem.Allocator; const TextDecoder = @This(); @@ -59,8 +60,8 @@ pub fn init(label_: ?[]const u8, opts_: ?InitOpts, page: *Page) !*TextDecoder { return self; } -pub fn deinit(self: *TextDecoder, _: bool, page: *Page) void { - page.releaseArena(self._arena); +pub fn deinit(self: *TextDecoder, _: bool, session: *Session) void { + session.releaseArena(self._arena); } pub fn getIgnoreBOM(self: *const TextDecoder) bool { diff --git a/src/browser/webapi/event/CompositionEvent.zig b/src/browser/webapi/event/CompositionEvent.zig index b98fdd6f..a761e3ba 100644 --- a/src/browser/webapi/event/CompositionEvent.zig +++ b/src/browser/webapi/event/CompositionEvent.zig @@ -20,6 +20,7 @@ const String = @import("../../../string.zig").String; const js = @import("../../js/js.zig"); const Page = @import("../../Page.zig"); +const Session = @import("../../Session.zig"); const Event = @import("../Event.zig"); const Allocator = std.mem.Allocator; @@ -53,8 +54,8 @@ pub fn init(typ: []const u8, opts_: ?Options, page: *Page) !*CompositionEvent { return event; } -pub fn deinit(self: *CompositionEvent, shutdown: bool, page: *Page) void { - self._proto.deinit(shutdown, page); +pub fn deinit(self: *CompositionEvent, shutdown: bool, session: *Session) void { + self._proto.deinit(shutdown, session); } pub fn asEvent(self: *CompositionEvent) *Event { diff --git a/src/browser/webapi/event/CustomEvent.zig b/src/browser/webapi/event/CustomEvent.zig index e303d901..eaba2e7d 100644 --- a/src/browser/webapi/event/CustomEvent.zig +++ b/src/browser/webapi/event/CustomEvent.zig @@ -21,6 +21,7 @@ const String = @import("../../../string.zig").String; const js = @import("../../js/js.zig"); const Page = @import("../../Page.zig"); +const Session = @import("../../Session.zig"); const Event = @import("../Event.zig"); const Allocator = std.mem.Allocator; @@ -72,11 +73,11 @@ pub fn initCustomEvent( self._detail = detail_; } -pub fn deinit(self: *CustomEvent, shutdown: bool, page: *Page) void { +pub fn deinit(self: *CustomEvent, shutdown: bool, session: *Session) void { if (self._detail) |d| { - page.js.release(d); + d.release(); } - self._proto.deinit(shutdown, page); + self._proto.deinit(shutdown, session); } pub fn asEvent(self: *CustomEvent) *Event { diff --git a/src/browser/webapi/event/ErrorEvent.zig b/src/browser/webapi/event/ErrorEvent.zig index 5dd12a26..2d3b857f 100644 --- a/src/browser/webapi/event/ErrorEvent.zig +++ b/src/browser/webapi/event/ErrorEvent.zig @@ -21,6 +21,7 @@ const String = @import("../../../string.zig").String; const js = @import("../../js/js.zig"); const Page = @import("../../Page.zig"); +const Session = @import("../../Session.zig"); const Event = @import("../Event.zig"); const Allocator = std.mem.Allocator; @@ -79,11 +80,11 @@ fn initWithTrusted(arena: Allocator, typ: String, opts_: ?Options, trusted: bool return event; } -pub fn deinit(self: *ErrorEvent, shutdown: bool, page: *Page) void { +pub fn deinit(self: *ErrorEvent, shutdown: bool, session: *Session) void { if (self._error) |e| { - page.js.release(e); + e.release(); } - self._proto.deinit(shutdown, page); + self._proto.deinit(shutdown, session); } pub fn asEvent(self: *ErrorEvent) *Event { diff --git a/src/browser/webapi/event/FocusEvent.zig b/src/browser/webapi/event/FocusEvent.zig index 37065936..f6823c23 100644 --- a/src/browser/webapi/event/FocusEvent.zig +++ b/src/browser/webapi/event/FocusEvent.zig @@ -20,6 +20,7 @@ const std = @import("std"); const Allocator = std.mem.Allocator; const String = @import("../../../string.zig").String; const Page = @import("../../Page.zig"); +const Session = @import("../../Session.zig"); const js = @import("../../js/js.zig"); const Event = @import("../Event.zig"); @@ -69,8 +70,8 @@ fn initWithTrusted(arena: Allocator, typ: String, _opts: ?Options, trusted: bool return event; } -pub fn deinit(self: *FocusEvent, shutdown: bool, page: *Page) void { - self._proto.deinit(shutdown, page); +pub fn deinit(self: *FocusEvent, shutdown: bool, session: *Session) void { + self._proto.deinit(shutdown, session); } pub fn asEvent(self: *FocusEvent) *Event { diff --git a/src/browser/webapi/event/KeyboardEvent.zig b/src/browser/webapi/event/KeyboardEvent.zig index 6e391812..1d4494b3 100644 --- a/src/browser/webapi/event/KeyboardEvent.zig +++ b/src/browser/webapi/event/KeyboardEvent.zig @@ -21,6 +21,7 @@ const String = @import("../../../string.zig").String; const js = @import("../../js/js.zig"); const Page = @import("../../Page.zig"); +const Session = @import("../../Session.zig"); const Event = @import("../Event.zig"); const UIEvent = @import("UIEvent.zig"); @@ -221,8 +222,8 @@ fn initWithTrusted(arena: Allocator, typ: String, _opts: ?Options, trusted: bool return event; } -pub fn deinit(self: *KeyboardEvent, shutdown: bool, page: *Page) void { - self._proto.deinit(shutdown, page); +pub fn deinit(self: *KeyboardEvent, shutdown: bool, session: *Session) void { + self._proto.deinit(shutdown, session); } pub fn asEvent(self: *KeyboardEvent) *Event { diff --git a/src/browser/webapi/event/MessageEvent.zig b/src/browser/webapi/event/MessageEvent.zig index 34e04518..32ced1d8 100644 --- a/src/browser/webapi/event/MessageEvent.zig +++ b/src/browser/webapi/event/MessageEvent.zig @@ -22,6 +22,7 @@ const String = @import("../../../string.zig").String; const js = @import("../../js/js.zig"); const Page = @import("../../Page.zig"); +const Session = @import("../../Session.zig"); const Event = @import("../Event.zig"); const Window = @import("../Window.zig"); const Allocator = std.mem.Allocator; @@ -72,11 +73,11 @@ fn initWithTrusted(arena: Allocator, typ: String, opts_: ?Options, trusted: bool return event; } -pub fn deinit(self: *MessageEvent, shutdown: bool, page: *Page) void { +pub fn deinit(self: *MessageEvent, shutdown: bool, session: *Session) void { if (self._data) |d| { - page.js.release(d); + d.release(); } - self._proto.deinit(shutdown, page); + self._proto.deinit(shutdown, session); } pub fn asEvent(self: *MessageEvent) *Event { diff --git a/src/browser/webapi/event/MouseEvent.zig b/src/browser/webapi/event/MouseEvent.zig index c94a37d1..6b032433 100644 --- a/src/browser/webapi/event/MouseEvent.zig +++ b/src/browser/webapi/event/MouseEvent.zig @@ -19,6 +19,7 @@ const std = @import("std"); const String = @import("../../../string.zig").String; const Page = @import("../../Page.zig"); +const Session = @import("../../Session.zig"); const js = @import("../../js/js.zig"); const Event = @import("../Event.zig"); @@ -109,8 +110,8 @@ pub fn init(typ: []const u8, _opts: ?Options, page: *Page) !*MouseEvent { return event; } -pub fn deinit(self: *MouseEvent, shutdown: bool, page: *Page) void { - self._proto.deinit(shutdown, page); +pub fn deinit(self: *MouseEvent, shutdown: bool, session: *Session) void { + self._proto.deinit(shutdown, session); } pub fn asEvent(self: *MouseEvent) *Event { diff --git a/src/browser/webapi/event/NavigationCurrentEntryChangeEvent.zig b/src/browser/webapi/event/NavigationCurrentEntryChangeEvent.zig index 59e32b06..98cba330 100644 --- a/src/browser/webapi/event/NavigationCurrentEntryChangeEvent.zig +++ b/src/browser/webapi/event/NavigationCurrentEntryChangeEvent.zig @@ -21,6 +21,7 @@ const String = @import("../../../string.zig").String; const js = @import("../../js/js.zig"); const Page = @import("../../Page.zig"); +const Session = @import("../../Session.zig"); const Event = @import("../Event.zig"); const NavigationHistoryEntry = @import("../navigation/NavigationHistoryEntry.zig"); @@ -82,8 +83,8 @@ fn initWithTrusted( return event; } -pub fn deinit(self: *NavigationCurrentEntryChangeEvent, shutdown: bool, page: *Page) void { - self._proto.deinit(shutdown, page); +pub fn deinit(self: *NavigationCurrentEntryChangeEvent, shutdown: bool, session: *Session) void { + self._proto.deinit(shutdown, session); } pub fn asEvent(self: *NavigationCurrentEntryChangeEvent) *Event { diff --git a/src/browser/webapi/event/PageTransitionEvent.zig b/src/browser/webapi/event/PageTransitionEvent.zig index eceab4f2..bf747c9a 100644 --- a/src/browser/webapi/event/PageTransitionEvent.zig +++ b/src/browser/webapi/event/PageTransitionEvent.zig @@ -21,6 +21,7 @@ const String = @import("../../../string.zig").String; const js = @import("../../js/js.zig"); const Page = @import("../../Page.zig"); +const Session = @import("../../Session.zig"); const Event = @import("../Event.zig"); const Allocator = std.mem.Allocator; @@ -65,8 +66,8 @@ fn initWithTrusted(arena: Allocator, typ: String, _opts: ?Options, trusted: bool return event; } -pub fn deinit(self: *PageTransitionEvent, shutdown: bool, page: *Page) void { - self._proto.deinit(shutdown, page); +pub fn deinit(self: *PageTransitionEvent, shutdown: bool, session: *Session) void { + self._proto.deinit(shutdown, session); } pub fn asEvent(self: *PageTransitionEvent) *Event { diff --git a/src/browser/webapi/event/PointerEvent.zig b/src/browser/webapi/event/PointerEvent.zig index 82f4874f..c5178faf 100644 --- a/src/browser/webapi/event/PointerEvent.zig +++ b/src/browser/webapi/event/PointerEvent.zig @@ -21,6 +21,7 @@ const String = @import("../../../string.zig").String; const js = @import("../../js/js.zig"); const Page = @import("../../Page.zig"); +const Session = @import("../../Session.zig"); const Event = @import("../Event.zig"); const MouseEvent = @import("MouseEvent.zig"); @@ -127,8 +128,8 @@ pub fn init(typ: []const u8, _opts: ?Options, page: *Page) !*PointerEvent { return event; } -pub fn deinit(self: *PointerEvent, shutdown: bool, page: *Page) void { - self._proto.deinit(shutdown, page); +pub fn deinit(self: *PointerEvent, shutdown: bool, session: *Session) void { + self._proto.deinit(shutdown, session); } pub fn asEvent(self: *PointerEvent) *Event { diff --git a/src/browser/webapi/event/PopStateEvent.zig b/src/browser/webapi/event/PopStateEvent.zig index 45774998..f26c17b6 100644 --- a/src/browser/webapi/event/PopStateEvent.zig +++ b/src/browser/webapi/event/PopStateEvent.zig @@ -21,6 +21,7 @@ const String = @import("../../../string.zig").String; const js = @import("../../js/js.zig"); const Page = @import("../../Page.zig"); +const Session = @import("../../Session.zig"); const Event = @import("../Event.zig"); const Allocator = std.mem.Allocator; @@ -66,8 +67,8 @@ fn initWithTrusted(arena: Allocator, typ: String, _opts: ?Options, trusted: bool return event; } -pub fn deinit(self: *PopStateEvent, shutdown: bool, page: *Page) void { - self._proto.deinit(shutdown, page); +pub fn deinit(self: *PopStateEvent, shutdown: bool, session: *Session) void { + self._proto.deinit(shutdown, session); } pub fn asEvent(self: *PopStateEvent) *Event { diff --git a/src/browser/webapi/event/ProgressEvent.zig b/src/browser/webapi/event/ProgressEvent.zig index a78982a1..b257f12b 100644 --- a/src/browser/webapi/event/ProgressEvent.zig +++ b/src/browser/webapi/event/ProgressEvent.zig @@ -20,6 +20,7 @@ const std = @import("std"); const String = @import("../../../string.zig").String; const Page = @import("../../Page.zig"); +const Session = @import("../../Session.zig"); const Event = @import("../Event.zig"); const Allocator = std.mem.Allocator; @@ -67,8 +68,8 @@ fn initWithTrusted(arena: Allocator, typ: String, _opts: ?Options, trusted: bool return event; } -pub fn deinit(self: *ProgressEvent, shutdown: bool, page: *Page) void { - self._proto.deinit(shutdown, page); +pub fn deinit(self: *ProgressEvent, shutdown: bool, session: *Session) void { + self._proto.deinit(shutdown, session); } pub fn asEvent(self: *ProgressEvent) *Event { diff --git a/src/browser/webapi/event/PromiseRejectionEvent.zig b/src/browser/webapi/event/PromiseRejectionEvent.zig index 957228df..f0a195b9 100644 --- a/src/browser/webapi/event/PromiseRejectionEvent.zig +++ b/src/browser/webapi/event/PromiseRejectionEvent.zig @@ -20,6 +20,7 @@ const String = @import("../../../string.zig").String; const js = @import("../../js/js.zig"); const Page = @import("../../Page.zig"); +const Session = @import("../../Session.zig"); const Event = @import("../Event.zig"); const Allocator = std.mem.Allocator; @@ -56,14 +57,14 @@ pub fn init(typ: []const u8, opts_: ?Options, page: *Page) !*PromiseRejectionEve return event; } -pub fn deinit(self: *PromiseRejectionEvent, shutdown: bool, page: *Page) void { +pub fn deinit(self: *PromiseRejectionEvent, shutdown: bool, session: *Session) void { if (self._reason) |r| { - page.js.release(r); + r.release(); } if (self._promise) |p| { - page.js.release(p); + p.release(); } - self._proto.deinit(shutdown, page); + self._proto.deinit(shutdown, session); } pub fn asEvent(self: *PromiseRejectionEvent) *Event { diff --git a/src/browser/webapi/event/TextEvent.zig b/src/browser/webapi/event/TextEvent.zig index 54789c13..fd5e32fb 100644 --- a/src/browser/webapi/event/TextEvent.zig +++ b/src/browser/webapi/event/TextEvent.zig @@ -19,6 +19,7 @@ const std = @import("std"); const String = @import("../../../string.zig").String; const Page = @import("../../Page.zig"); +const Session = @import("../../Session.zig"); const js = @import("../../js/js.zig"); const Event = @import("../Event.zig"); @@ -58,8 +59,8 @@ pub fn init(typ: []const u8, _opts: ?Options, page: *Page) !*TextEvent { return event; } -pub fn deinit(self: *TextEvent, shutdown: bool, page: *Page) void { - self._proto.deinit(shutdown, page); +pub fn deinit(self: *TextEvent, shutdown: bool, session: *Session) void { + self._proto.deinit(shutdown, session); } pub fn asEvent(self: *TextEvent) *Event { diff --git a/src/browser/webapi/event/UIEvent.zig b/src/browser/webapi/event/UIEvent.zig index 6d329221..0aa2943b 100644 --- a/src/browser/webapi/event/UIEvent.zig +++ b/src/browser/webapi/event/UIEvent.zig @@ -18,6 +18,7 @@ const String = @import("../../../string.zig").String; const Page = @import("../../Page.zig"); +const Session = @import("../../Session.zig"); const js = @import("../../js/js.zig"); const Event = @import("../Event.zig"); @@ -69,8 +70,8 @@ pub fn init(typ: []const u8, _opts: ?Options, page: *Page) !*UIEvent { return event; } -pub fn deinit(self: *UIEvent, shutdown: bool, page: *Page) void { - self._proto.deinit(shutdown, page); +pub fn deinit(self: *UIEvent, shutdown: bool, session: *Session) void { + self._proto.deinit(shutdown, session); } pub fn as(self: *UIEvent, comptime T: type) *T { diff --git a/src/browser/webapi/event/WheelEvent.zig b/src/browser/webapi/event/WheelEvent.zig index ee725941..831c4e02 100644 --- a/src/browser/webapi/event/WheelEvent.zig +++ b/src/browser/webapi/event/WheelEvent.zig @@ -19,6 +19,7 @@ const std = @import("std"); const String = @import("../../../string.zig").String; const Page = @import("../../Page.zig"); +const Session = @import("../../Session.zig"); const js = @import("../../js/js.zig"); const Event = @import("../Event.zig"); @@ -86,8 +87,8 @@ pub fn init(typ: []const u8, _opts: ?Options, page: *Page) !*WheelEvent { return event; } -pub fn deinit(self: *WheelEvent, shutdown: bool, page: *Page) void { - self._proto.deinit(shutdown, page); +pub fn deinit(self: *WheelEvent, shutdown: bool, session: *Session) void { + self._proto.deinit(shutdown, session); } pub fn asEvent(self: *WheelEvent) *Event { diff --git a/src/browser/webapi/net/Fetch.zig b/src/browser/webapi/net/Fetch.zig index 699cc9c4..9b0f2f98 100644 --- a/src/browser/webapi/net/Fetch.zig +++ b/src/browser/webapi/net/Fetch.zig @@ -45,7 +45,7 @@ pub const InitOpts = Request.InitOpts; pub fn init(input: Input, options: ?InitOpts, page: *Page) !js.Promise { const request = try Request.init(input, options, page); const response = try Response.init(null, .{ .status = 0 }, page); - errdefer response.deinit(true, page); + errdefer response.deinit(true, page._session); const resolver = page.js.local.?.createPromiseResolver(); @@ -184,7 +184,7 @@ fn httpErrorCallback(ctx: *anyopaque, err: anyerror) void { // clear this. (defer since `self is in the response's arena). defer if (self._owns_response) { - response.deinit(err == error.Abort, self._page); + response.deinit(err == error.Abort, self._page._session); self._owns_response = false; }; @@ -205,7 +205,7 @@ fn httpShutdownCallback(ctx: *anyopaque) void { if (self._owns_response) { var response = self._response; response._transfer = null; - response.deinit(true, self._page); + response.deinit(true, self._page._session); // Do not access `self` after this point: the Fetch struct was // allocated from response._arena which has been released. } diff --git a/src/browser/webapi/net/Response.zig b/src/browser/webapi/net/Response.zig index 13048f33..6a926369 100644 --- a/src/browser/webapi/net/Response.zig +++ b/src/browser/webapi/net/Response.zig @@ -21,6 +21,7 @@ const js = @import("../../js/js.zig"); const HttpClient = @import("../../HttpClient.zig"); const Page = @import("../../Page.zig"); +const Session = @import("../../Session.zig"); const Headers = @import("Headers.zig"); const ReadableStream = @import("../streams/ReadableStream.zig"); const Blob = @import("../Blob.zig"); @@ -77,7 +78,7 @@ pub fn init(body_: ?[]const u8, opts_: ?InitOpts, page: *Page) !*Response { return self; } -pub fn deinit(self: *Response, shutdown: bool, page: *Page) void { +pub fn deinit(self: *Response, shutdown: bool, session: *Session) void { if (self._transfer) |transfer| { if (shutdown) { transfer.terminate(); @@ -86,7 +87,7 @@ pub fn deinit(self: *Response, shutdown: bool, page: *Page) void { } self._transfer = null; } - page.releaseArena(self._arena); + session.releaseArena(self._arena); } pub fn getStatus(self: *const Response) u16 { diff --git a/src/browser/webapi/net/XMLHttpRequest.zig b/src/browser/webapi/net/XMLHttpRequest.zig index b9f053a8..d8d5e369 100644 --- a/src/browser/webapi/net/XMLHttpRequest.zig +++ b/src/browser/webapi/net/XMLHttpRequest.zig @@ -26,6 +26,8 @@ const net_http = @import("../../../network/http.zig"); const URL = @import("../../URL.zig"); const Mime = @import("../../Mime.zig"); const Page = @import("../../Page.zig"); +const Session = @import("../../Session.zig"); + const Node = @import("../Node.zig"); const Event = @import("../Event.zig"); const Headers = @import("Headers.zig"); @@ -93,7 +95,7 @@ pub fn init(page: *Page) !*XMLHttpRequest { }); } -pub fn deinit(self: *XMLHttpRequest, shutdown: bool, page: *Page) void { +pub fn deinit(self: *XMLHttpRequest, shutdown: bool, session: *Session) void { if (self._transfer) |transfer| { if (shutdown) { transfer.terminate(); @@ -103,37 +105,36 @@ pub fn deinit(self: *XMLHttpRequest, shutdown: bool, page: *Page) void { self._transfer = null; } - const js_ctx = page.js; if (self._on_ready_state_change) |func| { - js_ctx.release(func); + func.release(); } { const proto = self._proto; if (proto._on_abort) |func| { - js_ctx.release(func); + func.release(); } if (proto._on_error) |func| { - js_ctx.release(func); + func.release(); } if (proto._on_load) |func| { - js_ctx.release(func); + func.release(); } if (proto._on_load_end) |func| { - js_ctx.release(func); + func.release(); } if (proto._on_load_start) |func| { - js_ctx.release(func); + func.release(); } if (proto._on_progress) |func| { - js_ctx.release(func); + func.release(); } if (proto._on_timeout) |func| { - js_ctx.release(func); + func.release(); } } - page.releaseArena(self._arena); + session.releaseArena(self._arena); } fn asEventTarget(self: *XMLHttpRequest) *EventTarget { diff --git a/src/browser/webapi/selector/List.zig b/src/browser/webapi/selector/List.zig index 04055910..1061320d 100644 --- a/src/browser/webapi/selector/List.zig +++ b/src/browser/webapi/selector/List.zig @@ -19,6 +19,7 @@ const std = @import("std"); const Page = @import("../../Page.zig"); +const Session = @import("../../Session.zig"); const Node = @import("../Node.zig"); const Part = @import("Selector.zig").Part; @@ -40,8 +41,8 @@ pub const EntryIterator = GenericIterator(Iterator, null); pub const KeyIterator = GenericIterator(Iterator, "0"); pub const ValueIterator = GenericIterator(Iterator, "1"); -pub fn deinit(self: *const List, page: *Page) void { - page.releaseArena(self._arena); +pub fn deinit(self: *const List, session: *Session) void { + session.releaseArena(self._arena); } pub fn collect( diff --git a/src/cdp/Node.zig b/src/cdp/Node.zig index 34c58e3a..1260217b 100644 --- a/src/cdp/Node.zig +++ b/src/cdp/Node.zig @@ -406,7 +406,7 @@ test "cdp Node: search list" { { const l1 = try doc.querySelectorAll(.wrap("a"), page); - defer l1.deinit(page); + defer l1.deinit(page._session); const s1 = try search_list.create(l1._nodes); try testing.expectEqual("1", s1.name); try testing.expectEqualSlices(u32, &.{ 1, 2 }, s1.node_ids); @@ -417,7 +417,7 @@ test "cdp Node: search list" { { const l2 = try doc.querySelectorAll(.wrap("#a1"), page); - defer l2.deinit(page); + defer l2.deinit(page._session); const s2 = try search_list.create(l2._nodes); try testing.expectEqual("2", s2.name); try testing.expectEqualSlices(u32, &.{1}, s2.node_ids); @@ -425,7 +425,7 @@ test "cdp Node: search list" { { const l3 = try doc.querySelectorAll(.wrap("#a2"), page); - defer l3.deinit(page); + defer l3.deinit(page._session); const s3 = try search_list.create(l3._nodes); try testing.expectEqual("3", s3.name); try testing.expectEqualSlices(u32, &.{2}, s3.node_ids); diff --git a/src/cdp/domains/dom.zig b/src/cdp/domains/dom.zig index 2f2befa5..df1d37c2 100644 --- a/src/cdp/domains/dom.zig +++ b/src/cdp/domains/dom.zig @@ -98,7 +98,7 @@ fn performSearch(cmd: anytype) !void { const bc = cmd.browser_context orelse return error.BrowserContextNotLoaded; const page = bc.session.currentPage() orelse return error.PageNotLoaded; const list = try Selector.querySelectorAll(page.window._document.asNode(), params.query, page); - defer list.deinit(page); + defer list.deinit(page._session); const search = try bc.node_search_list.create(list._nodes); @@ -249,7 +249,7 @@ fn querySelectorAll(cmd: anytype) !void { }; const selected_nodes = try Selector.querySelectorAll(node.dom, params.selector, page); - defer selected_nodes.deinit(page); + defer selected_nodes.deinit(page._session); const nodes = selected_nodes._nodes; diff --git a/src/mcp/tools.zig b/src/mcp/tools.zig index 1ca9d120..53d29757 100644 --- a/src/mcp/tools.zig +++ b/src/mcp/tools.zig @@ -92,7 +92,8 @@ const ToolStreamingText = struct { }, .links => { if (Selector.querySelectorAll(self.page.document.asNode(), "a[href]", self.page)) |list| { - defer list.deinit(self.page); + defer list.deinit(self.page._session); + var first = true; for (list._nodes) |node| { if (node.is(Element.Html.Anchor)) |anchor| { From 7d90c3f582eefb574b1be878c588504d08f28f48 Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Mon, 9 Mar 2026 14:16:46 +0800 Subject: [PATCH 4/8] Move origin lookup to Session With the last commit, this becomes the more logical place to hold this as it ties into the Session's awareness of the root page's lifetime. --- src/browser/Session.zig | 52 ++++++++++++++++++++++++++++ src/browser/js/Context.zig | 6 ++-- src/browser/js/Env.zig | 69 +++----------------------------------- 3 files changed, 59 insertions(+), 68 deletions(-) diff --git a/src/browser/Session.zig b/src/browser/Session.zig index 42fe8ef5..9cce0ccf 100644 --- a/src/browser/Session.zig +++ b/src/browser/Session.zig @@ -62,6 +62,9 @@ factory: Factory, page_arena: Allocator, +// Origin map for same-origin context sharing. Scoped to the root page lifetime. +origins: std.StringHashMapUnmanaged(*js.Origin) = .empty, + // Shared resources for all pages in this session. // These live for the duration of the page tree (root + frames). arena_pool: *ArenaPool, @@ -194,6 +197,41 @@ pub fn releaseArena(self: *Session, allocator: Allocator) void { return self.arena_pool.release(allocator); } +pub fn getOrCreateOrigin(self: *Session, key_: ?[]const u8) !*js.Origin { + const key = key_ orelse { + var opaque_origin: [36]u8 = undefined; + @import("../id.zig").uuidv4(&opaque_origin); + // Origin.init will dupe opaque_origin. It's fine that this doesn't + // get added to self.origins. In fact, it further isolates it. When the + // context is freed, it'll call session.releaseOrigin which will free it. + return js.Origin.init(self.browser.app, self.browser.env.isolate, &opaque_origin); + }; + + const gop = try self.origins.getOrPut(self.arena, key); + if (gop.found_existing) { + const origin = gop.value_ptr.*; + origin.rc += 1; + return origin; + } + + errdefer _ = self.origins.remove(key); + + const origin = try js.Origin.init(self.browser.app, self.browser.env.isolate, key); + gop.key_ptr.* = origin.key; + gop.value_ptr.* = origin; + return origin; +} + +pub fn releaseOrigin(self: *Session, origin: *js.Origin) void { + const rc = origin.rc; + if (rc == 1) { + _ = self.origins.remove(origin.key); + origin.deinit(self.browser.app); + } else { + origin.rc = rc - 1; + } +} + /// Reset page_arena and factory for a clean slate. /// Called when root page is removed. fn resetPageResources(self: *Session) void { @@ -208,6 +246,20 @@ fn resetPageResources(self: *Session) void { self._arena_pool_leak_track.clearRetainingCapacity(); } + // All origins should have been released when contexts were destroyed + if (comptime IS_DEBUG) { + std.debug.assert(self.origins.count() == 0); + } + // Defensive cleanup in case origins leaked + { + const app = self.browser.app; + var it = self.origins.valueIterator(); + while (it.next()) |value| { + value.*.deinit(app); + } + self.origins.clearRetainingCapacity(); + } + // Release old page_arena and acquire fresh one self.frame_id_gen = 0; self.arena_pool.reset(self.page_arena, 64 * 1024); diff --git a/src/browser/js/Context.zig b/src/browser/js/Context.zig index 750cd809..ac6ed3c1 100644 --- a/src/browser/js/Context.zig +++ b/src/browser/js/Context.zig @@ -153,7 +153,7 @@ pub fn deinit(self: *Context) void { v8.v8__Global__Reset(global); } - env.releaseOrigin(self.origin); + self.session.releaseOrigin(self.origin); v8.v8__Global__Reset(&self.handle); env.isolate.notifyContextDisposed(); @@ -167,8 +167,8 @@ pub fn setOrigin(self: *Context, key: ?[]const u8) !void { const env = self.env; const isolate = env.isolate; - const origin = try env.getOrCreateOrigin(key); - errdefer env.releaseOrigin(origin); + const origin = try self.session.getOrCreateOrigin(key); + errdefer self.session.releaseOrigin(origin); try self.origin.transferTo(origin); self.origin.deinit(env.app); diff --git a/src/browser/js/Env.zig b/src/browser/js/Env.zig index 4318394d..5bc2fded 100644 --- a/src/browser/js/Env.zig +++ b/src/browser/js/Env.zig @@ -77,14 +77,6 @@ context_id: usize, // same-origin Contexts. There's a mismatch here between our JS model and our // Browser model. Origins only live as long as the root page of a session exists. // It would be wrong/dangerous to re-use an Origin across root page navigations. -// But we have no mechanism to capture that lifetime in js. We used to have a -// js.BrowserContext which mapped to a Session (oops, I took it out), but even -// that wouldn't match correctly, because 1 session can have have muliple non- -// concurrent pages. We deal with this in destroyContext by checking if we're -// destroying the root context and, if so, making sure origins is empty. But, if -// we ever add multiple Sessions to a Browser or mulitple Pages to a Session, -// this map will have to live in a new, better scoped, container. -origins: std.StringHashMapUnmanaged(*Origin) = .empty, // Global handles that need to be freed on deinit eternal_function_templates: []v8.Eternal, @@ -248,14 +240,6 @@ pub fn deinit(self: *Env) void { const app = self.app; const allocator = app.allocator; - { - var it = self.origins.valueIterator(); - while (it.next()) |value| { - value.*.deinit(app); - } - self.origins.deinit(allocator); - } - if (self.inspector) |i| { i.deinit(allocator); } @@ -323,8 +307,8 @@ pub fn createContext(self: *Env, page: *Page) !*Context { const context_id = self.context_id; self.context_id = context_id + 1; - const origin = try self.getOrCreateOrigin(null); - errdefer self.releaseOrigin(origin); + const origin = try page._session.getOrCreateOrigin(null); + errdefer page._session.releaseOrigin(origin); const context = try context_arena.create(Context); context.* = .{ @@ -383,56 +367,11 @@ pub fn destroyContext(self: *Env, context: *Context, is_root: bool) void { context.deinit(); if (is_root) { - // When the root is destroyed, the all of our contexts should be gone - // and with them, all of our origins. Keep origins around longer than - // intended would cause issues, so we're going to be defensive here and - // clean things up. + // When the root is destroyed, all of our contexts should be gone. + // Origin cleanup happens in Session.resetPageResources. if (comptime IS_DEBUG) { std.debug.assert(self.context_count == 0); - std.debug.assert(self.origins.count() == 0); } - - const app = self.app; - var it = self.origins.valueIterator(); - while (it.next()) |value| { - value.*.deinit(app); - } - self.origins.clearRetainingCapacity(); - } -} - -pub fn getOrCreateOrigin(self: *Env, key_: ?[]const u8) !*Origin { - const key = key_ orelse { - var opaque_origin: [36]u8 = undefined; - @import("../../id.zig").uuidv4(&opaque_origin); - // Origin.init will dupe opaque_origin. It's fine that this doesn't - // get added to self.origins. In fact, it further isolates it. When the - // context is freed, it'll call env.releaseOrigin which will free it. - return Origin.init(self.app, self.isolate, &opaque_origin); - }; - - const gop = try self.origins.getOrPut(self.allocator, key); - if (gop.found_existing) { - const origin = gop.value_ptr.*; - origin.rc += 1; - return origin; - } - - errdefer _ = self.origins.remove(key); - - const origin = try Origin.init(self.app, self.isolate, key); - gop.key_ptr.* = origin.key; - gop.value_ptr.* = origin; - return origin; -} - -pub fn releaseOrigin(self: *Env, origin: *Origin) void { - const rc = origin.rc; - if (rc == 1) { - _ = self.origins.remove(origin.key); - origin.deinit(self.app); - } else { - origin.rc = rc - 1; } } From 7348a68c84301ce3c5d6b10138aff925a3d2b4dc Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Mon, 9 Mar 2026 17:18:50 +0800 Subject: [PATCH 5/8] merge main --- src/browser/Page.zig | 2 +- src/browser/webapi/css/FontFace.zig | 5 +++-- src/browser/webapi/css/FontFaceSet.zig | 5 +++-- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/browser/Page.zig b/src/browser/Page.zig index 60a3f968..10fc49a3 100644 --- a/src/browser/Page.zig +++ b/src/browser/Page.zig @@ -608,7 +608,7 @@ fn scheduleNavigationWithArena(originator: *Page, arena: Allocator, request_url: }; if (target._queued_navigation) |existing| { - target.arena_pool.release(existing.arena); + session.releaseArena(existing.arena); } target._queued_navigation = qn; diff --git a/src/browser/webapi/css/FontFace.zig b/src/browser/webapi/css/FontFace.zig index f824259a..f3c4059d 100644 --- a/src/browser/webapi/css/FontFace.zig +++ b/src/browser/webapi/css/FontFace.zig @@ -19,6 +19,7 @@ const std = @import("std"); const js = @import("../../js/js.zig"); const Page = @import("../../Page.zig"); +const Session = @import("../../Session.zig"); const Allocator = std.mem.Allocator; @@ -41,8 +42,8 @@ pub fn init(family: []const u8, source: []const u8, page: *Page) !*FontFace { return self; } -pub fn deinit(self: *FontFace, _: bool, page: *Page) void { - page.releaseArena(self._arena); +pub fn deinit(self: *FontFace, _: bool, session: *Session) void { + session.releaseArena(self._arena); } pub fn getFamily(self: *const FontFace) []const u8 { diff --git a/src/browser/webapi/css/FontFaceSet.zig b/src/browser/webapi/css/FontFaceSet.zig index 6e5cd941..2a4a000d 100644 --- a/src/browser/webapi/css/FontFaceSet.zig +++ b/src/browser/webapi/css/FontFaceSet.zig @@ -19,6 +19,7 @@ const std = @import("std"); const js = @import("../../js/js.zig"); const Page = @import("../../Page.zig"); +const Session = @import("../../Session.zig"); const FontFace = @import("FontFace.zig"); const Allocator = std.mem.Allocator; @@ -38,8 +39,8 @@ pub fn init(page: *Page) !*FontFaceSet { return self; } -pub fn deinit(self: *FontFaceSet, _: bool, page: *Page) void { - page.releaseArena(self._arena); +pub fn deinit(self: *FontFaceSet, _: bool, session: *Session) void { + session.releaseArena(self._arena); } // FontFaceSet.ready - returns an already-resolved Promise. From 4cea9aba3c08e45d3eed77c81018d669b83b9dce Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Tue, 10 Mar 2026 06:43:22 +0800 Subject: [PATCH 6/8] update v8 dep --- .github/actions/install/action.yml | 2 +- Dockerfile | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/actions/install/action.yml b/.github/actions/install/action.yml index 6c98f2e5..d9d7b803 100644 --- a/.github/actions/install/action.yml +++ b/.github/actions/install/action.yml @@ -13,7 +13,7 @@ inputs: zig-v8: description: 'zig v8 version to install' required: false - default: 'v0.3.1' + default: 'v0.3.2' v8: description: 'v8 version to install' required: false diff --git a/Dockerfile b/Dockerfile index 8729f992..e30fbdbe 100644 --- a/Dockerfile +++ b/Dockerfile @@ -3,7 +3,7 @@ FROM debian:stable-slim ARG MINISIG=0.12 ARG ZIG_MINISIG=RWSGOq2NVecA2UPNdBUZykf1CCb147pkmdtYxgb3Ti+JO/wCYvhbAb/U ARG V8=14.0.365.4 -ARG ZIG_V8=v0.3.1 +ARG ZIG_V8=v0.3.2 ARG TARGETPLATFORM RUN apt-get update -yq && \ From f6d0e484b022e72b03f48a9f129b99b02919691e Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Wed, 11 Mar 2026 07:19:59 +0800 Subject: [PATCH 7/8] transfer finalizers on origin change --- src/browser/js/Local.zig | 1 + src/browser/js/Origin.zig | 62 +++++++++++++++++++++++---------------- 2 files changed, 38 insertions(+), 25 deletions(-) diff --git a/src/browser/js/Local.zig b/src/browser/js/Local.zig index 84e83b49..78ab9d15 100644 --- a/src/browser/js/Local.zig +++ b/src/browser/js/Local.zig @@ -226,6 +226,7 @@ pub fn mapZigInstanceToJs(self: *const Local, js_obj_handle: ?*const v8.Object, // can't figure out how to make that work, since it depends on // the [runtime] `value`. // We need the resolved finalizer, which we have in resolved. + // // The above if statement would be more clear as: // if (resolved.finalizer_from_v8) |finalizer| { // But that's a runtime check. diff --git a/src/browser/js/Origin.zig b/src/browser/js/Origin.zig index 356b75c8..486888f1 100644 --- a/src/browser/js/Origin.zig +++ b/src/browser/js/Origin.zig @@ -67,24 +67,6 @@ temps: std.AutoHashMapUnmanaged(usize, v8.Global) = .empty, // 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, -finalizer_callback_pool: std.heap.MemoryPool(FinalizerCallback), - -// 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 finalizer_callbacks and call them on -// origin shutdown. -pub const FinalizerCallback = struct { - origin: *Origin, - session: *Session, - ptr: *anyopaque, - global: v8.Global, - finalizerFn: *const fn (ptr: *anyopaque, session: *Session) void, - - pub fn deinit(self: *FinalizerCallback) void { - self.finalizerFn(self.ptr, self.session); - self.origin.finalizer_callback_pool.destroy(self); - } -}; pub fn init(app: *App, isolate: js.Isolate, key: []const u8) !*Origin { const arena = try app.arena_pool.acquire(); @@ -107,7 +89,6 @@ pub fn init(app: *App, isolate: js.Isolate, key: []const u8) !*Origin { .globals = .empty, .temps = .empty, .security_token = token_global, - .finalizer_callback_pool = .init(arena), }; return self; } @@ -119,7 +100,6 @@ pub fn deinit(self: *Origin, app: *App) void { while (it.next()) |finalizer| { finalizer.*.deinit(); } - self.finalizer_callback_pool.deinit(); } v8.v8__Global__Reset(&self.security_token); @@ -172,13 +152,14 @@ pub fn release(self: *Origin, item: *anyopaque) void { // The item has been finalized, remove it from the finalizer callback so that // we don't try to call it again on shutdown. - const fc = self.finalizer_callbacks.fetchRemove(@intFromPtr(item)) orelse { + const kv = self.finalizer_callbacks.fetchRemove(@intFromPtr(item)) orelse { if (comptime IS_DEBUG) { std.debug.assert(false); } return; }; - self.finalizer_callback_pool.destroy(fc.value); + const fc = kv.value; + fc.session.releaseArena(fc.arena); } pub fn createFinalizerCallback( @@ -186,15 +167,18 @@ pub fn createFinalizerCallback( session: *Session, global: v8.Global, ptr: *anyopaque, - finalizerFn: *const fn (ptr: *anyopaque, session: *Session) void, + zig_finalizer: *const fn (ptr: *anyopaque, session: *Session) void, ) !*FinalizerCallback { - const fc = try self.finalizer_callback_pool.create(); + const arena = try session.getArena(.{ .debug = "FinalizerCallback" }); + errdefer session.releaseArena(arena); + const fc = try arena.create(FinalizerCallback); fc.* = .{ + .arena = arena, .origin = self, .session = session, .ptr = ptr, .global = global, - .finalizerFn = finalizerFn, + .zig_finalizer = zig_finalizer, }; return fc; } @@ -217,6 +201,16 @@ pub fn transferTo(self: *Origin, dest: *Origin) !void { self.temps.clearRetainingCapacity(); } + { + try dest.finalizer_callbacks.ensureUnusedCapacity(arena, self.finalizer_callbacks.count()); + var it = self.finalizer_callbacks.iterator(); + while (it.next()) |kv| { + kv.value_ptr.*.origin = dest; + try dest.finalizer_callbacks.put(arena, kv.key_ptr.*, kv.value_ptr.*); + } + self.finalizer_callbacks.clearRetainingCapacity(); + } + { try dest.identity_map.ensureUnusedCapacity(arena, self.identity_map.count()); var it = self.identity_map.iterator(); @@ -226,3 +220,21 @@ pub fn transferTo(self: *Origin, dest: *Origin) !void { self.identity_map.clearRetainingCapacity(); } } + +// 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 finalizer_callbacks and call them on +// origin shutdown. +pub const FinalizerCallback = struct { + arena: Allocator, + origin: *Origin, + session: *Session, + ptr: *anyopaque, + global: v8.Global, + zig_finalizer: *const fn (ptr: *anyopaque, session: *Session) void, + + pub fn deinit(self: *FinalizerCallback) void { + self.zig_finalizer(self.ptr, self.session); + self.session.releaseArena(self.arena); + } +}; From dc3d2e97906e3d0b6df0b9917c4ca4fbaa7e8bf1 Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Wed, 11 Mar 2026 08:21:35 +0800 Subject: [PATCH 8/8] Remove root context check from Env This was only added [very briefly] when Env managed Origins, which it no longer does. --- src/browser/Page.zig | 5 ++--- src/browser/Session.zig | 2 +- src/browser/js/Env.zig | 10 +--------- src/cdp/cdp.zig | 2 +- 4 files changed, 5 insertions(+), 14 deletions(-) diff --git a/src/browser/Page.zig b/src/browser/Page.zig index 10fc49a3..811e7e5c 100644 --- a/src/browser/Page.zig +++ b/src/browser/Page.zig @@ -332,12 +332,11 @@ pub fn deinit(self: *Page, abort_http: bool) void { session.releaseArena(qn.arena); } - const is_root = self.parent == null; - session.browser.env.destroyContext(self.js, is_root); + session.browser.env.destroyContext(self.js); self._script_manager.shutdown = true; - if (is_root) { + if (self.parent == null) { session.browser.http_client.abort(); } else if (abort_http) { // a small optimization, it's faster to abort _everything_ on the root diff --git a/src/browser/Session.zig b/src/browser/Session.zig index 9cce0ccf..529f0847 100644 --- a/src/browser/Session.zig +++ b/src/browser/Session.zig @@ -276,7 +276,7 @@ pub fn replacePage(self: *Session) !*Page { var current = self.page.?; const frame_id = current._frame_id; - current.deinit(false); + current.deinit(true); self.resetPageResources(); self.browser.env.memoryPressureNotification(.moderate); diff --git a/src/browser/js/Env.zig b/src/browser/js/Env.zig index 5bc2fded..e8488541 100644 --- a/src/browser/js/Env.zig +++ b/src/browser/js/Env.zig @@ -342,7 +342,7 @@ pub fn createContext(self: *Env, page: *Page) !*Context { return context; } -pub fn destroyContext(self: *Env, context: *Context, is_root: bool) void { +pub fn destroyContext(self: *Env, context: *Context) void { for (self.contexts[0..self.context_count], 0..) |ctx, i| { if (ctx == context) { // Swap with last element and decrement count @@ -365,14 +365,6 @@ pub fn destroyContext(self: *Env, context: *Context, is_root: bool) void { } context.deinit(); - - if (is_root) { - // When the root is destroyed, all of our contexts should be gone. - // Origin cleanup happens in Session.resetPageResources. - if (comptime IS_DEBUG) { - std.debug.assert(self.context_count == 0); - } - } } pub fn runMicrotasks(self: *Env) void { diff --git a/src/cdp/cdp.zig b/src/cdp/cdp.zig index 40057b8b..78e5ab50 100644 --- a/src/cdp/cdp.zig +++ b/src/cdp/cdp.zig @@ -759,7 +759,7 @@ const IsolatedWorld = struct { pub fn removeContext(self: *IsolatedWorld) !void { const ctx = self.context orelse return error.NoIsolatedContextToRemove; - self.browser.env.destroyContext(ctx, false); + self.browser.env.destroyContext(ctx); self.context = null; }