From 381a18a40e83cc3f4fa5cb01667fdf56b7ac593e Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Mon, 6 Oct 2025 15:52:56 +0800 Subject: [PATCH] Move the call_arena to the page. The call_arena was previously owned by the js.Context, but it has to exist on the page, and the page is created before the context, so it's set to undefined on the page. While this has never caused an issue, there's no reason for the page not to own this, and the context to simply reference it. Also, renamed the js.Context.context_arena to simply `arena`, which is more consistent with other arena names (e.g. page.arena). --- src/browser/browser.zig | 3 +++ src/browser/js/Context.zig | 41 +++++++++++++++---------------- src/browser/js/Env.zig | 1 - src/browser/js/ExecutionWorld.zig | 14 ++--------- src/browser/js/Object.zig | 4 +-- src/browser/page.zig | 4 +-- src/browser/session.zig | 2 +- src/http/Http.zig | 1 - 8 files changed, 30 insertions(+), 40 deletions(-) diff --git a/src/browser/browser.zig b/src/browser/browser.zig index f787a6dbc..a8353ffd0 100644 --- a/src/browser/browser.zig +++ b/src/browser/browser.zig @@ -39,6 +39,7 @@ pub const Browser = struct { session: ?Session, allocator: Allocator, http_client: *HttpClient, + call_arena: ArenaAllocator, page_arena: ArenaAllocator, session_arena: ArenaAllocator, transfer_arena: ArenaAllocator, @@ -63,6 +64,7 @@ pub const Browser = struct { .allocator = allocator, .notification = notification, .http_client = app.http.client, + .call_arena = ArenaAllocator.init(allocator), .page_arena = ArenaAllocator.init(allocator), .session_arena = ArenaAllocator.init(allocator), .transfer_arena = ArenaAllocator.init(allocator), @@ -73,6 +75,7 @@ pub const Browser = struct { pub fn deinit(self: *Browser) void { self.closeSession(); self.env.deinit(); + self.call_arena.deinit(); self.page_arena.deinit(); self.session_arena.deinit(); self.transfer_arena.deinit(); diff --git a/src/browser/js/Context.zig b/src/browser/js/Context.zig index 3eb8a632b..ae1b54560 100644 --- a/src/browser/js/Context.zig +++ b/src/browser/js/Context.zig @@ -35,12 +35,11 @@ templates: []v8.FunctionTemplate, // references the Env.meta_lookup meta_lookup: []types.Meta, -// An arena for the lifetime of a call-group. Gets reset whenever -// call_depth reaches 0. -call_arena: Allocator, +// Arena for the lifetime of the context +arena: Allocator, -// An arena for the lifetime of the context -context_arena: Allocator, +// The page.call_arena +call_arena: Allocator, // Because calls can be nested (i.e.a function calling a callback), // we can only reset the call_arena when call_depth == 0. If we were @@ -179,7 +178,7 @@ pub fn deinit(self: *Context) void { } fn trackCallback(self: *Context, pf: PersistentFunction) !void { - return self.callbacks.append(self.context_arena, pf); + return self.callbacks.append(self.arena, pf); } // Given an anytype, turns it into a v8.Object. The anytype could be: @@ -236,7 +235,7 @@ pub fn module(self: *Context, comptime want_result: bool, src: []const u8, url: const m = try compileModule(self.isolate, src, url); - const arena = self.context_arena; + const arena = self.arena; const owned_url = try arena.dupe(u8, url); try self.module_identifier.putNoClobber(arena, m.getIdentityHash(), owned_url); @@ -258,9 +257,9 @@ pub fn module(self: *Context, comptime want_result: bool, src: []const u8, url: owned_url, .{ .alloc = .if_needed, .null_terminated = true }, ); - const gop = try self.module_cache.getOrPut(self.context_arena, normalized_specifier); + const gop = try self.module_cache.getOrPut(self.arena, normalized_specifier); if (!gop.found_existing) { - const owned_specifier = try self.context_arena.dupeZ(u8, normalized_specifier); + const owned_specifier = try self.arena.dupeZ(u8, normalized_specifier); gop.key_ptr.* = owned_specifier; gop.value_ptr.* = .{}; try self.script_manager.?.getModule(owned_specifier); @@ -522,18 +521,18 @@ pub fn zigValueToJs(self: *Context, value: anytype) !v8.Value { // we can just grab it from the identity_map) pub fn mapZigInstanceToJs(self: *Context, js_obj_or_template: anytype, value: anytype) !PersistentObject { const v8_context = self.v8_context; - const context_arena = self.context_arena; + const arena = self.arena; const T = @TypeOf(value); switch (@typeInfo(T)) { .@"struct" => { // Struct, has to be placed on the heap - const heap = try context_arena.create(T); + const heap = try arena.create(T); heap.* = value; return self.mapZigInstanceToJs(js_obj_or_template, heap); }, .pointer => |ptr| { - const gop = try self.identity_map.getOrPut(context_arena, @intFromPtr(value)); + const gop = try self.identity_map.getOrPut(arena, @intFromPtr(value)); if (gop.found_existing) { // we've seen this instance before, return the same // PersistentObject. @@ -541,7 +540,7 @@ pub fn mapZigInstanceToJs(self: *Context, js_obj_or_template: anytype, value: an } if (comptime @hasDecl(ptr.child, "destructor")) { - try self.destructor_callbacks.append(context_arena, DestructorCallback.init(value)); + try self.destructor_callbacks.append(arena, DestructorCallback.init(value)); } // Sometimes we're creating a new v8.Object, like when @@ -563,7 +562,7 @@ pub fn mapZigInstanceToJs(self: *Context, js_obj_or_template: anytype, value: an // The TAO contains the pointer ot our Zig instance as // well as any meta data we'll need to use it later. // See the TaggedAnyOpaque struct for more details. - const tao = try context_arena.create(TaggedAnyOpaque); + const tao = try arena.create(TaggedAnyOpaque); const meta_index = @field(types.LOOKUP, @typeName(ptr.child)); const meta = self.meta_lookup[meta_index]; @@ -768,7 +767,7 @@ fn jsValueToStruct(self: *Context, comptime named_function: NamedFunction, compt } if (T == js.String) { - return .{ .string = try self.valueToString(js_value, .{ .allocator = self.context_arena }) }; + return .{ .string = try self.valueToString(js_value, .{ .allocator = self.arena }) }; } const js_obj = js_value.castTo(v8.Object); @@ -1062,7 +1061,7 @@ pub fn createPromiseResolver(self: *Context, comptime lifetime: PromiseResolverL const persisted = v8.Persistent(v8.PromiseResolver).init(self.isolate, resolver); if (comptime lifetime == .page) { - try self.persisted_promise_resolvers.append(self.context_arena, persisted); + try self.persisted_promise_resolvers.append(self.arena, persisted); } return .{ @@ -1122,7 +1121,7 @@ pub fn dynamicModuleCallback( }; const normalized_specifier = @import("../../url.zig").stitch( - self.context_arena, // might need to survive until the module is loaded + self.arena, // might need to survive until the module is loaded specifier, resource, .{ .alloc = .if_needed, .null_terminated = true }, @@ -1172,7 +1171,7 @@ fn _resolveModuleCallback(self: *Context, referrer: v8.Module, specifier: []cons .{ .alloc = .if_needed, .null_terminated = true }, ); - const gop = try self.module_cache.getOrPut(self.context_arena, normalized_specifier); + const gop = try self.module_cache.getOrPut(self.arena, normalized_specifier); if (gop.found_existing) { if (gop.value_ptr.module) |m| { return m.handle; @@ -1229,7 +1228,7 @@ const DynamicModuleResolveState = struct { fn _dynamicModuleCallback(self: *Context, specifier: [:0]const u8) !v8.Promise { const isolate = self.isolate; - const gop = try self.module_cache.getOrPut(self.context_arena, specifier); + const gop = try self.module_cache.getOrPut(self.arena, specifier); if (gop.found_existing and gop.value_ptr.resolver_promise != null) { // This is easy, there's already something responsible // for loading the module. Maybe it's still loading, maybe @@ -1238,10 +1237,10 @@ fn _dynamicModuleCallback(self: *Context, specifier: [:0]const u8) !v8.Promise { } const persistent_resolver = v8.Persistent(v8.PromiseResolver).init(isolate, v8.PromiseResolver.init(self.v8_context)); - try self.persisted_promise_resolvers.append(self.context_arena, persistent_resolver); + try self.persisted_promise_resolvers.append(self.arena, persistent_resolver); var resolver = persistent_resolver.castToPromiseResolver(); - const state = try self.context_arena.create(DynamicModuleResolveState); + const state = try self.arena.create(DynamicModuleResolveState); state.* = .{ .module = null, .context = self, diff --git a/src/browser/js/Env.zig b/src/browser/js/Env.zig index 0aee15c27..370c6f7e5 100644 --- a/src/browser/js/Env.zig +++ b/src/browser/js/Env.zig @@ -178,7 +178,6 @@ pub fn newExecutionWorld(self: *Env) !ExecutionWorld { return .{ .env = self, .context = null, - .call_arena = ArenaAllocator.init(self.allocator), .context_arena = ArenaAllocator.init(self.allocator), }; } diff --git a/src/browser/js/ExecutionWorld.zig b/src/browser/js/ExecutionWorld.zig index fb20c928e..04522a044 100644 --- a/src/browser/js/ExecutionWorld.zig +++ b/src/browser/js/ExecutionWorld.zig @@ -21,13 +21,6 @@ const CONTEXT_ARENA_RETAIN = 1024 * 64; const ExecutionWorld = @This(); env: *Env, -// Arena whose lifetime is for a single getter/setter/function/etc. -// Largely used to get strings out of V8, like a stack trace from -// a TryCatch. The allocator will be owned by the Context, but the -// arena itself is owned by the ExecutionWorld so that we can re-use it -// from context to context. -call_arena: ArenaAllocator, - // Arena whose lifetime is for a single page load. Where // the call_arena lives for a single function call, the context_arena // lives for the lifetime of the entire page. The allocator will be @@ -48,7 +41,6 @@ pub fn deinit(self: *ExecutionWorld) void { self.removeContext(); } - self.call_arena.deinit(); self.context_arena.deinit(); } @@ -178,8 +170,8 @@ pub fn createContext(self: *ExecutionWorld, page: *Page, enter: bool, global_cal .meta_lookup = &env.meta_lookup, .handle_scope = handle_scope, .script_manager = &page.script_manager, - .call_arena = self.call_arena.allocator(), - .context_arena = self.context_arena.allocator(), + .call_arena = page.call_arena, + .arena = self.context_arena.allocator(), .global_callback = global_callback, }; @@ -191,8 +183,6 @@ pub fn createContext(self: *ExecutionWorld, page: *Page, enter: bool, global_cal v8_context.setEmbedderData(1, data); } - page.call_arena = context.call_arena; - // Custom exception // NOTE: there is no way in v8 to subclass the Error built-in type // TODO: this is an horrible hack diff --git a/src/browser/js/Object.zig b/src/browser/js/Object.zig index 24b2c0d42..36946c15d 100644 --- a/src/browser/js/Object.zig +++ b/src/browser/js/Object.zig @@ -22,7 +22,7 @@ pub fn setIndex(self: Object, index: u32, value: anytype, opts: SetOpts) !void { @setEvalBranchQuota(10000); const key = switch (index) { inline 0...20 => |i| std.fmt.comptimePrint("{d}", .{i}), - else => try std.fmt.allocPrint(self.context.context_arena, "{d}", .{index}), + else => try std.fmt.allocPrint(self.context.arena, "{d}", .{index}), }; return self.set(key, value, opts); } @@ -76,7 +76,7 @@ pub fn persist(self: Object) !Object { const js_obj = self.js_obj; const persisted = PersistentObject.init(context.isolate, js_obj); - try context.js_object_list.append(context.context_arena, persisted); + try context.js_object_list.append(context.arena, persisted); return .{ .context = context, diff --git a/src/browser/page.zig b/src/browser/page.zig index f0576d4b5..4141ab927 100644 --- a/src/browser/page.zig +++ b/src/browser/page.zig @@ -121,7 +121,7 @@ pub const Page = struct { complete, }; - pub fn init(self: *Page, arena: Allocator, session: *Session) !void { + pub fn init(self: *Page, arena: Allocator, call_arena: Allocator, session: *Session) !void { const browser = session.browser; const script_manager = ScriptManager.init(browser, self); @@ -131,7 +131,7 @@ pub const Page = struct { .window = try Window.create(null, null), .arena = arena, .session = session, - .call_arena = undefined, + .call_arena = call_arena, .renderer = Renderer.init(arena), .state_pool = &browser.state_pool, .cookie_jar = &session.cookie_jar, diff --git a/src/browser/session.zig b/src/browser/session.zig index 9b5641222..a5651d205 100644 --- a/src/browser/session.zig +++ b/src/browser/session.zig @@ -106,7 +106,7 @@ pub const Session = struct { self.page = @as(Page, undefined); const page = &self.page.?; - try Page.init(page, page_arena.allocator(), self); + try Page.init(page, page_arena.allocator(), self.browser.call_arena.allocator(), self); log.debug(.browser, "create page", .{}); // start JS env diff --git a/src/http/Http.zig b/src/http/Http.zig index 270bbd17d..17b481d09 100644 --- a/src/http/Http.zig +++ b/src/http/Http.zig @@ -443,7 +443,6 @@ const LineWriter = struct { } }; - pub fn debugCallback(_: *c.CURL, msg_type: c.curl_infotype, raw: [*c]u8, len: usize, _: *anyopaque) callconv(.c) void { const data = raw[0..len]; switch (msg_type) {