diff --git a/src/ArenaPool.zig b/src/ArenaPool.zig index 2f8de17f..998838df 100644 --- a/src/ArenaPool.zig +++ b/src/ArenaPool.zig @@ -17,12 +17,15 @@ // along with this program. If not, see . const std = @import("std"); +const builtin = @import("builtin"); const Allocator = std.mem.Allocator; const ArenaAllocator = std.heap.ArenaAllocator; const ArenaPool = @This(); +const IS_DEBUG = builtin.mode == .Debug; + allocator: Allocator, retain_bytes: usize, free_list_len: u16 = 0, @@ -30,10 +33,17 @@ free_list: ?*Entry = null, free_list_max: u16, entry_pool: std.heap.MemoryPool(Entry), mutex: std.Thread.Mutex = .{}, +// Debug mode: track acquire/release counts per debug name to detect leaks and double-frees +_leak_track: if (IS_DEBUG) std.StringHashMapUnmanaged(isize) else void = if (IS_DEBUG) .empty else {}, const Entry = struct { next: ?*Entry, arena: ArenaAllocator, + debug: if (IS_DEBUG) []const u8 else void = if (IS_DEBUG) "" else {}, +}; + +pub const DebugInfo = struct { + debug: []const u8 = "", }; pub fn init(allocator: Allocator, free_list_max: u16, retain_bytes: usize) ArenaPool { @@ -42,10 +52,26 @@ pub fn init(allocator: Allocator, free_list_max: u16, retain_bytes: usize) Arena .free_list_max = free_list_max, .retain_bytes = retain_bytes, .entry_pool = .init(allocator), + ._leak_track = if (IS_DEBUG) .empty else {}, }; } pub fn deinit(self: *ArenaPool) void { + if (IS_DEBUG) { + var has_leaks = false; + var it = self._leak_track.iterator(); + while (it.next()) |kv| { + if (kv.value_ptr.* != 0) { + std.debug.print("ArenaPool leak detected: '{s}' count={d}\n", .{ kv.key_ptr.*, kv.value_ptr.* }); + has_leaks = true; + } + } + if (has_leaks) { + @panic("ArenaPool: leaked arenas detected"); + } + self._leak_track.deinit(self.allocator); + } + var entry = self.free_list; while (entry) |e| { entry = e.next; @@ -54,13 +80,21 @@ pub fn deinit(self: *ArenaPool) void { self.entry_pool.deinit(); } -pub fn acquire(self: *ArenaPool) !Allocator { +pub fn acquire(self: *ArenaPool, dbg: DebugInfo) !Allocator { self.mutex.lock(); defer self.mutex.unlock(); if (self.free_list) |entry| { self.free_list = entry.next; self.free_list_len -= 1; + if (IS_DEBUG) { + entry.debug = dbg.debug; + const gop = try self._leak_track.getOrPut(self.allocator, dbg.debug); + if (!gop.found_existing) { + gop.value_ptr.* = 0; + } + gop.value_ptr.* += 1; + } return entry.arena.allocator(); } @@ -68,8 +102,17 @@ pub fn acquire(self: *ArenaPool) !Allocator { entry.* = .{ .next = null, .arena = ArenaAllocator.init(self.allocator), + .debug = if (IS_DEBUG) dbg.debug else {}, }; + if (IS_DEBUG) { + const gop = try self._leak_track.getOrPut(self.allocator, dbg.debug); + if (!gop.found_existing) { + gop.value_ptr.* = 0; + } + gop.value_ptr.* += 1; + } + return entry.arena.allocator(); } @@ -83,6 +126,19 @@ pub fn release(self: *ArenaPool, allocator: Allocator) void { self.mutex.lock(); defer self.mutex.unlock(); + if (IS_DEBUG) { + if (self._leak_track.getPtr(entry.debug)) |count| { + count.* -= 1; + if (count.* < 0) { + std.debug.print("ArenaPool double-free detected: '{s}'\n", .{entry.debug}); + @panic("ArenaPool: double-free detected"); + } + } else { + std.debug.print("ArenaPool release of untracked arena: '{s}'\n", .{entry.debug}); + @panic("ArenaPool: release of untracked arena"); + } + } + const free_list_len = self.free_list_len; if (free_list_len == self.free_list_max) { arena.deinit(); @@ -106,7 +162,7 @@ test "arena pool - basic acquire and use" { var pool = ArenaPool.init(testing.allocator, 512, 1024 * 16); defer pool.deinit(); - const alloc = try pool.acquire(); + const alloc = try pool.acquire(.{ .debug = "test" }); const buf = try alloc.alloc(u8, 64); @memset(buf, 0xAB); try testing.expectEqual(@as(u8, 0xAB), buf[0]); @@ -118,14 +174,14 @@ test "arena pool - reuse entry after release" { var pool = ArenaPool.init(testing.allocator, 512, 1024 * 16); defer pool.deinit(); - const alloc1 = try pool.acquire(); + const alloc1 = try pool.acquire(.{ .debug = "test" }); try testing.expectEqual(@as(u16, 0), pool.free_list_len); pool.release(alloc1); try testing.expectEqual(@as(u16, 1), pool.free_list_len); // The same entry should be returned from the free list. - const alloc2 = try pool.acquire(); + const alloc2 = try pool.acquire(.{ .debug = "test" }); try testing.expectEqual(@as(u16, 0), pool.free_list_len); try testing.expectEqual(alloc1.ptr, alloc2.ptr); @@ -136,9 +192,9 @@ test "arena pool - multiple concurrent arenas" { var pool = ArenaPool.init(testing.allocator, 512, 1024 * 16); defer pool.deinit(); - const a1 = try pool.acquire(); - const a2 = try pool.acquire(); - const a3 = try pool.acquire(); + const a1 = try pool.acquire(.{ .debug = "test1" }); + const a2 = try pool.acquire(.{ .debug = "test2" }); + const a3 = try pool.acquire(.{ .debug = "test3" }); // All three must be distinct arenas. try testing.expect(a1.ptr != a2.ptr); @@ -161,8 +217,8 @@ test "arena pool - free list respects max limit" { var pool = ArenaPool.init(testing.allocator, 1, 1024 * 16); defer pool.deinit(); - const a1 = try pool.acquire(); - const a2 = try pool.acquire(); + const a1 = try pool.acquire(.{ .debug = "test1" }); + const a2 = try pool.acquire(.{ .debug = "test2" }); pool.release(a1); try testing.expectEqual(@as(u16, 1), pool.free_list_len); @@ -176,7 +232,7 @@ test "arena pool - reset clears memory without releasing" { var pool = ArenaPool.init(testing.allocator, 512, 1024 * 16); defer pool.deinit(); - const alloc = try pool.acquire(); + const alloc = try pool.acquire(.{ .debug = "test" }); const buf = try alloc.alloc(u8, 128); @memset(buf, 0xFF); @@ -200,8 +256,8 @@ test "arena pool - deinit with entries in free list" { // detected by the test allocator). var pool = ArenaPool.init(testing.allocator, 512, 1024 * 16); - const a1 = try pool.acquire(); - const a2 = try pool.acquire(); + const a1 = try pool.acquire(.{ .debug = "test1" }); + const a2 = try pool.acquire(.{ .debug = "test2" }); _ = try a1.alloc(u8, 256); _ = try a2.alloc(u8, 512); pool.release(a1); diff --git a/src/browser/Page.zig b/src/browser/Page.zig index c3a6b5a3..148654a0 100644 --- a/src/browser/Page.zig +++ b/src/browser/Page.zig @@ -302,7 +302,10 @@ pub fn init(self: *Page, frame_id: u32, session: *Session, parent: ?*Page) !void self._script_manager = ScriptManager.init(browser.allocator, browser.http_client, self); errdefer self._script_manager.deinit(); - self.js = try browser.env.createContext(self); + self.js = try browser.env.createContext(self, .{ + .identity = &session.identity, + .identity_arena = session.page_arena, + }); errdefer self.js.deinit(); document._page = self; diff --git a/src/browser/Session.zig b/src/browser/Session.zig index 6376ff03..e318e486 100644 --- a/src/browser/Session.zig +++ b/src/browser/Session.zig @@ -66,30 +66,14 @@ page_arena: Allocator, // Origin map for same-origin context sharing. Scoped to the root page lifetime. origins: std.StringHashMapUnmanaged(*js.Origin) = .empty, -// Session-scoped identity tracking (for the lifetime of the root page). -// Maps Zig instance pointers to their v8::Global(Object) wrappers. -identity_map: std.AutoHashMapUnmanaged(usize, v8.Global) = .empty, - -// Tracked global v8 objects that need to be released when the page is reset. -globals: std.ArrayList(v8.Global) = .empty, - -// Temporary v8 globals that can be released early. Key is global.data_ptr. -temps: std.AutoHashMapUnmanaged(usize, v8.Global) = .empty, - -// Finalizer callbacks for weak references. Key is @intFromPtr of the Zig instance. -finalizer_callbacks: std.AutoHashMapUnmanaged(usize, *FinalizerCallback) = .empty, +// Identity tracking for the main world. All main world contexts share this, +// ensuring object identity works across same-origin frames. +identity: js.Identity = .{}, // 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), @@ -105,10 +89,10 @@ pub fn init(self: *Session, browser: *Browser, notification: *Notification) !voi const allocator = browser.app.allocator; const arena_pool = browser.arena_pool; - const arena = try arena_pool.acquire(); + const arena = try arena_pool.acquire(.{ .debug = "Session" }); errdefer arena_pool.release(arena); - const page_arena = try arena_pool.acquire(); + const page_arena = try arena_pool.acquire(.{ .debug = "Session.page_arena" }); errdefer arena_pool.release(page_arena); self.* = .{ @@ -183,32 +167,11 @@ pub const GetArenaOpts = struct { }; 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.page_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; + return self.arena_pool.acquire(.{ .debug = opts.debug }); } 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); + self.arena_pool.release(allocator); } pub fn getOrCreateOrigin(self: *Session, key_: ?[]const u8) !*js.Origin { @@ -249,44 +212,8 @@ pub fn releaseOrigin(self: *Session, origin: *js.Origin) void { /// Reset page_arena and factory for a clean slate. /// Called when root page is removed. fn resetPageResources(self: *Session) void { - { - var it = self.finalizer_callbacks.valueIterator(); - while (it.next()) |finalizer| { - finalizer.*.deinit(); - } - self.finalizer_callbacks = .empty; - } - - { - var it = self.identity_map.valueIterator(); - while (it.next()) |global| { - v8.v8__Global__Reset(global); - } - self.identity_map = .empty; - } - - for (self.globals.items) |*global| { - v8.v8__Global__Reset(global); - } - self.globals = .empty; - - { - var it = self.temps.valueIterator(); - while (it.next()) |global| { - v8.v8__Global__Reset(global); - } - self.temps = .empty; - } - - 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 = .empty; - } + self.identity.deinit(); + self.identity = .{}; if (comptime IS_DEBUG) { std.debug.assert(self.origins.count() == 0); @@ -670,16 +597,6 @@ fn processRootQueuedNavigation(self: *Session) !void { 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); @@ -711,77 +628,6 @@ pub fn nextPageId(self: *Session) u32 { return id; } -// These methods manage the mapping between Zig instances and v8 objects, -// scoped to the lifetime of the root page. -pub fn trackGlobal(self: *Session, global: v8.Global) !void { - return self.globals.append(self.page_arena, global); -} - -pub const IdentityResult = struct { - value_ptr: *v8.Global, - found_existing: bool, -}; - -pub fn addIdentity(self: *Session, ptr: usize) !IdentityResult { - const gop = try self.identity_map.getOrPut(self.page_arena, ptr); - return .{ - .value_ptr = gop.value_ptr, - .found_existing = gop.found_existing, - }; -} - -pub fn trackTemp(self: *Session, global: v8.Global) !void { - return self.temps.put(self.page_arena, global.data_ptr, global); -} - -pub fn releaseTemp(self: *Session, 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 releaseIdentity(self: *Session, 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 kv = self.finalizer_callbacks.fetchRemove(@intFromPtr(item)) orelse { - if (comptime IS_DEBUG) { - std.debug.assert(false); - } - return; - }; - const fc = kv.value; - self.releaseArena(fc.arena); -} - -pub fn createFinalizerCallback( - self: *Session, - global: v8.Global, - ptr: *anyopaque, - zig_finalizer: *const fn (ptr: *anyopaque, session: *Session) void, -) !*FinalizerCallback { - const arena = try self.getArena(.{ .debug = "FinalizerCallback" }); - errdefer self.releaseArena(arena); - const fc = try arena.create(FinalizerCallback); - fc.* = .{ - .arena = arena, - .session = self, - .ptr = ptr, - .global = global, - .zig_finalizer = zig_finalizer, - }; - return fc; -} - // 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 @@ -791,10 +637,26 @@ pub const FinalizerCallback = struct { session: *Session, ptr: *anyopaque, global: v8.Global, + identity: *js.Identity, 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); } + + /// Release this item from the identity tracking maps (called after finalizer runs from V8) + pub fn releaseIdentity(self: *FinalizerCallback) void { + const session = self.session; + const id = @intFromPtr(self.ptr); + + if (self.identity.identity_map.fetchRemove(id)) |kv| { + var global = kv.value; + v8.v8__Global__Reset(&global); + } + + _ = self.identity.finalizer_callbacks.remove(id); + + session.releaseArena(self.arena); + } }; diff --git a/src/browser/js/Context.zig b/src/browser/js/Context.zig index 5ea9454f..2b861e7a 100644 --- a/src/browser/js/Context.zig +++ b/src/browser/js/Context.zig @@ -79,6 +79,16 @@ local: ?*const js.Local = null, origin: *Origin, +// Identity tracking for this context. For main world contexts, this points to +// Session's Identity. For isolated world contexts (CDP inspector), this points +// to IsolatedWorld's Identity. This ensures same-origin frames share object +// identity while isolated worlds have separate identity tracking. +identity: *js.Identity, + +// Allocator to use for identity map operations. For main world contexts this is +// session.page_arena, for isolated worlds it's the isolated world's arena. +identity_arena: Allocator, + // Unlike other v8 types, like functions or objects, modules are not shared // across origins. global_modules: std.ArrayList(v8.Global) = .empty, @@ -202,16 +212,16 @@ pub fn setOrigin(self: *Context, key: ?[]const u8) !void { } pub fn trackGlobal(self: *Context, global: v8.Global) !void { - return self.session.trackGlobal(global); + return self.identity.globals.append(self.identity_arena, global); } pub fn trackTemp(self: *Context, global: v8.Global) !void { - return self.session.trackTemp(global); + return self.identity.temps.put(self.identity_arena, global.data_ptr, global); } pub fn weakRef(self: *Context, obj: anytype) void { const resolved = js.Local.resolveValue(obj); - const fc = self.session.finalizer_callbacks.get(@intFromPtr(resolved.ptr)) orelse { + const fc = self.identity.finalizer_callbacks.get(@intFromPtr(resolved.ptr)) orelse { if (comptime IS_DEBUG) { // should not be possible std.debug.assert(false); @@ -223,7 +233,7 @@ pub fn weakRef(self: *Context, obj: anytype) void { pub fn safeWeakRef(self: *Context, obj: anytype) void { const resolved = js.Local.resolveValue(obj); - const fc = self.session.finalizer_callbacks.get(@intFromPtr(resolved.ptr)) orelse { + const fc = self.identity.finalizer_callbacks.get(@intFromPtr(resolved.ptr)) orelse { if (comptime IS_DEBUG) { // should not be possible std.debug.assert(false); @@ -236,7 +246,7 @@ pub fn safeWeakRef(self: *Context, obj: anytype) void { pub fn strongRef(self: *Context, obj: anytype) void { const resolved = js.Local.resolveValue(obj); - const fc = self.session.finalizer_callbacks.get(@intFromPtr(resolved.ptr)) orelse { + const fc = self.identity.finalizer_callbacks.get(@intFromPtr(resolved.ptr)) orelse { if (comptime IS_DEBUG) { // should not be possible std.debug.assert(false); @@ -246,6 +256,48 @@ pub fn strongRef(self: *Context, obj: anytype) void { v8.v8__Global__ClearWeak(&fc.global); } +pub const IdentityResult = struct { + value_ptr: *v8.Global, + found_existing: bool, +}; + +pub fn addIdentity(self: *Context, ptr: usize) !IdentityResult { + const gop = try self.identity.identity_map.getOrPut(self.identity_arena, ptr); + return .{ + .value_ptr = gop.value_ptr, + .found_existing = gop.found_existing, + }; +} + +pub fn releaseTemp(self: *Context, global: v8.Global) void { + if (self.identity.temps.fetchRemove(global.data_ptr)) |kv| { + var g = kv.value; + v8.v8__Global__Reset(&g); + } +} + +pub fn createFinalizerCallback( + self: *Context, + global: v8.Global, + ptr: *anyopaque, + zig_finalizer: *const fn (ptr: *anyopaque, session: *Session) void, +) !*Session.FinalizerCallback { + const session = self.session; + const arena = try session.getArena(.{ .debug = "FinalizerCallback" }); + errdefer session.releaseArena(arena); + const fc = try arena.create(Session.FinalizerCallback); + fc.* = .{ + .arena = arena, + .session = session, + .ptr = ptr, + .global = global, + .zig_finalizer = zig_finalizer, + // Store identity pointer for cleanup when V8 GCs the object + .identity = self.identity, + }; + return fc; +} + // 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; diff --git a/src/browser/js/Env.zig b/src/browser/js/Env.zig index 07f1a479..d248a411 100644 --- a/src/browser/js/Env.zig +++ b/src/browser/js/Env.zig @@ -34,6 +34,7 @@ const Snapshot = @import("Snapshot.zig"); const Inspector = @import("Inspector.zig"); const Page = @import("../Page.zig"); +const Session = @import("../Session.zig"); const Window = @import("../webapi/Window.zig"); const JsApis = bridge.JsApis; @@ -254,8 +255,14 @@ pub fn deinit(self: *Env) void { allocator.destroy(self.isolate_params); } -pub fn createContext(self: *Env, page: *Page) !*Context { - const context_arena = try self.app.arena_pool.acquire(); +pub const ContextParams = struct { + identity: *js.Identity, + identity_arena: Allocator, + debug_name: []const u8 = "Context", +}; + +pub fn createContext(self: *Env, page: *Page, params: ContextParams) !*Context { + const context_arena = try self.app.arena_pool.acquire(.{ .debug = params.debug_name }); errdefer self.app.arena_pool.release(context_arena); const isolate = self.isolate; @@ -322,12 +329,14 @@ pub fn createContext(self: *Env, page: *Page) !*Context { .microtask_queue = microtask_queue, .script_manager = &page._script_manager, .scheduler = .init(context_arena), + .identity = params.identity, + .identity_arena = params.identity_arena, }; { // Multiple contexts can be created for the same Window (via CDP). We only // need to register the first one. - const gop = try session.identity_map.getOrPut(session.page_arena, @intFromPtr(page.window)); + const gop = try params.identity.identity_map.getOrPut(params.identity_arena, @intFromPtr(page.window)); if (gop.found_existing == false) { // our window wrapped in a v8::Global var global_global: v8.Global = undefined; diff --git a/src/browser/js/Function.zig b/src/browser/js/Function.zig index e2c10e9f..4c84a08f 100644 --- a/src/browser/js/Function.zig +++ b/src/browser/js/Function.zig @@ -211,10 +211,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); - return .{ .handle = global, .session = {} }; + return .{ .handle = global, .temps = {} }; } try ctx.trackTemp(global); - return .{ .handle = global, .session = ctx.session }; + return .{ .handle = global, .temps = &ctx.identity.temps }; } pub fn tempWithThis(self: *const Function, value: anytype) !Temp { @@ -238,7 +238,7 @@ const GlobalType = enum(u8) { fn G(comptime global_type: GlobalType) type { return struct { handle: v8.Global, - session: if (global_type == .temp) *Session else void, + temps: if (global_type == .temp) *std.AutoHashMapUnmanaged(usize, v8.Global) else void, const Self = @This(); @@ -258,7 +258,10 @@ fn G(comptime global_type: GlobalType) type { } pub fn release(self: *const Self) void { - self.session.releaseTemp(self.handle); + if (self.temps.fetchRemove(self.handle.data_ptr)) |kv| { + var g = kv.value; + v8.v8__Global__Reset(&g); + } } }; } diff --git a/src/browser/js/Identity.zig b/src/browser/js/Identity.zig new file mode 100644 index 00000000..323ae909 --- /dev/null +++ b/src/browser/js/Identity.zig @@ -0,0 +1,76 @@ +// Copyright (C) 2023-2026 Lightpanda (Selecy SAS) +// +// Francis Bouvier +// Pierre Tachoire +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as +// published by the Free Software Foundation, either version 3 of the +// License, or (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +// Identity manages the mapping between Zig instances and their v8::Object wrappers. +// This provides object identity semantics - the same Zig instance always maps to +// the same JS object within a given Identity scope. +// +// Main world contexts share a single Identity (on Session), ensuring that +// `window.top.document === top's document` works across same-origin frames. +// +// Isolated worlds (CDP inspector) have their own Identity, ensuring their +// v8::Global wrappers don't leak into the main world. + +const std = @import("std"); +const js = @import("js.zig"); + +const Session = @import("../Session.zig"); + +const v8 = js.v8; +const Allocator = std.mem.Allocator; + +const Identity = @This(); + +// Maps Zig instance pointers to their v8::Global(Object) wrappers. +identity_map: std.AutoHashMapUnmanaged(usize, v8.Global) = .empty, + +// Tracked global v8 objects that need to be released on cleanup. +globals: std.ArrayList(v8.Global) = .empty, + +// Temporary v8 globals that can be released early. Key is global.data_ptr. +temps: std.AutoHashMapUnmanaged(usize, v8.Global) = .empty, + +// Finalizer callbacks for weak references. Key is @intFromPtr of the Zig instance. +finalizer_callbacks: std.AutoHashMapUnmanaged(usize, *Session.FinalizerCallback) = .empty, + +pub fn deinit(self: *Identity) void { + { + var it = self.finalizer_callbacks.valueIterator(); + while (it.next()) |finalizer| { + finalizer.*.deinit(); + } + } + + { + 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); + } + } +} diff --git a/src/browser/js/Local.zig b/src/browser/js/Local.zig index ca09fc91..9080e3bc 100644 --- a/src/browser/js/Local.zig +++ b/src/browser/js/Local.zig @@ -202,21 +202,20 @@ pub fn compileAndRun(self: *const Local, src: []const u8, name: ?[]const u8) !js // we can just grab it from the identity_map) pub fn mapZigInstanceToJs(self: *const Local, js_obj_handle: ?*const v8.Object, value: anytype) !js.Object { const ctx = self.ctx; - const session = ctx.session; - const page_arena = session.page_arena; + const context_arena = ctx.arena; const T = @TypeOf(value); switch (@typeInfo(T)) { .@"struct" => { // Struct, has to be placed on the heap - const heap = try page_arena.create(T); + const heap = try context_arena.create(T); heap.* = value; return self.mapZigInstanceToJs(js_obj_handle, heap); }, .pointer => |ptr| { const resolved = resolveValue(value); - const gop = try session.addIdentity(@intFromPtr(resolved.ptr)); + const gop = try ctx.addIdentity(@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); @@ -245,7 +244,7 @@ pub fn mapZigInstanceToJs(self: *const Local, js_obj_handle: ?*const v8.Object, // The TAO contains the pointer to our Zig instance as // well as any meta data we'll need to use it later. // See the TaggedOpaque struct for more details. - const tao = try page_arena.create(TaggedOpaque); + const tao = try context_arena.create(TaggedOpaque); tao.* = .{ .value = resolved.ptr, .prototype_chain = resolved.prototype_chain.ptr, @@ -277,10 +276,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 session.createFinalizerCallback(gop.value_ptr.*, resolved.ptr, resolved.finalizer_from_zig.?); + const fc = try ctx.createFinalizerCallback(gop.value_ptr.*, resolved.ptr, resolved.finalizer_from_zig.?); { errdefer fc.deinit(); - try session.finalizer_callbacks.put(page_arena, @intFromPtr(resolved.ptr), fc); + try ctx.identity.finalizer_callbacks.put(ctx.identity_arena, @intFromPtr(resolved.ptr), fc); } conditionallyReference(value); diff --git a/src/browser/js/Origin.zig b/src/browser/js/Origin.zig index 6f79b005..c6c6bf81 100644 --- a/src/browser/js/Origin.zig +++ b/src/browser/js/Origin.zig @@ -20,8 +20,9 @@ // Multiple contexts (frames) from the same origin share a single Origin, // which provides the V8 SecurityToken that allows cross-context access. // -// Note: Identity tracking (mapping Zig instances to v8::Objects) is now -// handled at the Session level, scoped to the root page lifetime. +// Note: Identity tracking (mapping Zig instances to v8::Objects) is managed +// separately via js.Identity - Session has the main world Identity, and +// IsolatedWorlds have their own Identity instances. const std = @import("std"); const js = @import("js.zig"); @@ -44,7 +45,7 @@ key: []const u8, security_token: v8.Global, pub fn init(app: *App, isolate: js.Isolate, key: []const u8) !*Origin { - const arena = try app.arena_pool.acquire(); + const arena = try app.arena_pool.acquire(.{ .debug = "Origin" }); errdefer app.arena_pool.release(arena); var hs: js.HandleScope = undefined; diff --git a/src/browser/js/Promise.zig b/src/browser/js/Promise.zig index 0a08f424..4e83c098 100644 --- a/src/browser/js/Promise.zig +++ b/src/browser/js/Promise.zig @@ -16,6 +16,7 @@ // You should have received a copy of the GNU Affero General Public License // along with this program. If not, see . +const std = @import("std"); const js = @import("js.zig"); const v8 = js.v8; @@ -65,10 +66,10 @@ 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); - return .{ .handle = global, .session = {} }; + return .{ .handle = global, .temps = {} }; } try ctx.trackTemp(global); - return .{ .handle = global, .session = ctx.session }; + return .{ .handle = global, .temps = &ctx.identity.temps }; } pub const Temp = G(.temp); @@ -82,7 +83,7 @@ const GlobalType = enum(u8) { fn G(comptime global_type: GlobalType) type { return struct { handle: v8.Global, - session: if (global_type == .temp) *Session else void, + temps: if (global_type == .temp) *std.AutoHashMapUnmanaged(usize, v8.Global) else void, const Self = @This(); @@ -98,7 +99,10 @@ fn G(comptime global_type: GlobalType) type { } pub fn release(self: *const Self) void { - self.session.releaseTemp(self.handle); + if (self.temps.fetchRemove(self.handle.data_ptr)) |kv| { + var g = kv.value; + v8.v8__Global__Reset(&g); + } } }; } diff --git a/src/browser/js/Value.zig b/src/browser/js/Value.zig index edbbe4e3..d71e2f83 100644 --- a/src/browser/js/Value.zig +++ b/src/browser/js/Value.zig @@ -301,10 +301,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); - return .{ .handle = global, .session = {} }; + return .{ .handle = global, .temps = {} }; } try ctx.trackTemp(global); - return .{ .handle = global, .session = ctx.session }; + return .{ .handle = global, .temps = &ctx.identity.temps }; } pub fn toZig(self: Value, comptime T: type) !T { @@ -362,7 +362,7 @@ const GlobalType = enum(u8) { fn G(comptime global_type: GlobalType) type { return struct { handle: v8.Global, - session: if (global_type == .temp) *Session else void, + temps: if (global_type == .temp) *std.AutoHashMapUnmanaged(usize, v8.Global) else void, const Self = @This(); @@ -382,7 +382,10 @@ fn G(comptime global_type: GlobalType) type { } pub fn release(self: *const Self) void { - self.session.releaseTemp(self.handle); + if (self.temps.fetchRemove(self.handle.data_ptr)) |kv| { + var g = kv.value; + v8.v8__Global__Reset(&g); + } } }; } diff --git a/src/browser/js/bridge.zig b/src/browser/js/bridge.zig index 57cff1fa..8135e7d7 100644 --- a/src/browser/js/bridge.zig +++ b/src/browser/js/bridge.zig @@ -118,11 +118,10 @@ pub fn Builder(comptime T: type) type { const ptr = v8.v8__WeakCallbackInfo__GetParameter(handle.?).?; const fc: *Session.FinalizerCallback = @ptrCast(@alignCast(ptr)); - const session = fc.session; const value_ptr = fc.ptr; - if (session.finalizer_callbacks.contains(@intFromPtr(value_ptr))) { - func(@ptrCast(@alignCast(value_ptr)), false, session); - session.releaseIdentity(value_ptr); + if (fc.identity.finalizer_callbacks.contains(@intFromPtr(value_ptr))) { + func(@ptrCast(@alignCast(value_ptr)), false, fc.session); + fc.releaseIdentity(); } else { // A bit weird, but v8 _requires_ that we release it // If we don't. We'll 100% crash. diff --git a/src/browser/js/js.zig b/src/browser/js/js.zig index 0c196e5b..10867167 100644 --- a/src/browser/js/js.zig +++ b/src/browser/js/js.zig @@ -25,6 +25,7 @@ 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 Identity = @import("Identity.zig"); pub const Context = @import("Context.zig"); pub const Local = @import("Local.zig"); pub const Inspector = @import("Inspector.zig"); diff --git a/src/cdp/cdp.zig b/src/cdp/cdp.zig index 58ed11b9..d6b35d83 100644 --- a/src/cdp/cdp.zig +++ b/src/cdp/cdp.zig @@ -489,7 +489,7 @@ pub fn BrowserContext(comptime CDP_T: type) type { pub fn createIsolatedWorld(self: *Self, world_name: []const u8, grant_universal_access: bool) !*IsolatedWorld { const browser = &self.cdp.browser; - const arena = try browser.arena_pool.acquire(); + const arena = try browser.arena_pool.acquire(.{ .debug = "IsolatedWorld" }); errdefer browser.arena_pool.release(arena); const world = try arena.create(IsolatedWorld); @@ -750,8 +750,13 @@ const IsolatedWorld = struct { context: ?*js.Context = null, grant_universal_access: bool, + // Identity tracking for this isolated world (separate from main world). + // This ensures CDP inspector contexts don't share v8::Globals with main world. + identity: js.Identity = .{}, + pub fn deinit(self: *IsolatedWorld) void { self.removeContext() catch {}; + self.identity.deinit(); self.browser.arena_pool.release(self.arena); } @@ -768,7 +773,12 @@ const IsolatedWorld = struct { // Currently we have only 1 page/frame and thus also only 1 state in the isolate world. pub fn createContext(self: *IsolatedWorld, page: *Page) !*js.Context { if (self.context == null) { - self.context = try self.browser.env.createContext(page); + const ctx = try self.browser.env.createContext(page, .{ + .identity = &self.identity, + .identity_arena = self.arena, + .debug_name = "IsolatedContext", + }); + self.context = ctx; } else { log.warn(.cdp, "not implemented", .{ .feature = "createContext: Not implemented second isolated context creation",