From 521c0f8460af483e035d3bfda1d6ed7f1f77c049 Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Thu, 16 Oct 2025 14:15:38 +0800 Subject: [PATCH 1/3] Fix a potential segfault on log message for failing to load module Using the `call_arena` here is unsafe in the case of a failure. It's possible for the call_arena to be reset during module processing, making the log crash. The issue is that the lifetime of a URL is often conditional. If the stitched URL has already been seen (i.e. is in the module_cache), then it can be short- lived. EXCEPT, URL.stitch might require an allocation..and then you start to think, well, if URL.stitch is going to allocate anyways...If we stitch with the `page.arena`, and end up not needing a long lifetime, we've wasted memory. If we stitch with `page.call_arena` and end up needing a long lifetime, we need to dupe. It's a bit messy, and I'd like to take a stab at improving it after: https://github.com/lightpanda-io/browser/pull/1127. I'm thinking that we need a URL intern pool. HashMap with a composite key of base + path -> resolved. Then all URLs are resolved using the page.arena, but we don't have any duplicates, so it isn't wasteful. --- src/browser/js/Context.zig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/browser/js/Context.zig b/src/browser/js/Context.zig index 142378b9b..aff83eb26 100644 --- a/src/browser/js/Context.zig +++ b/src/browser/js/Context.zig @@ -1171,7 +1171,7 @@ fn _resolveModuleCallback(self: *Context, referrer: v8.Module, specifier: []cons }; const normalized_specifier = try self.script_manager.?.resolveSpecifier( - self.call_arena, + self.arena, // might need to survive until the module is loaded specifier, referrer_path, ); From 7bb8581a9562689803a083eaf0233a35d9ba6ce9 Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Thu, 16 Oct 2025 14:31:09 +0800 Subject: [PATCH 2/3] Fix referrer in log (was printing using the src instead :/) --- src/browser/js/Context.zig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/browser/js/Context.zig b/src/browser/js/Context.zig index aff83eb26..93c036646 100644 --- a/src/browser/js/Context.zig +++ b/src/browser/js/Context.zig @@ -262,7 +262,7 @@ pub fn module(self: *Context, comptime want_result: bool, src: []const u8, url: 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, src); + try self.script_manager.?.getModule(owned_specifier, url); } } } From 55746f1a1d285d58169182ad3a7425eeac6d6c4f Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Thu, 16 Oct 2025 14:34:07 +0800 Subject: [PATCH 3/3] log the normalized specifier now that we've extended its lifetime to the page.arena --- src/browser/js/Context.zig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/browser/js/Context.zig b/src/browser/js/Context.zig index 93c036646..fbb77f700 100644 --- a/src/browser/js/Context.zig +++ b/src/browser/js/Context.zig @@ -1207,7 +1207,7 @@ fn _resolveModuleCallback(self: *Context, referrer: v8.Module, specifier: []cons const entry = self.module(true, fetch_result.src(), normalized_specifier, true) catch |err| { log.warn(.js, "compile resolved module", .{ - .specifier = specifier, + .specifier = normalized_specifier, .stack = try_catch.stack(self.call_arena) catch null, .src = try_catch.sourceLine(self.call_arena) catch "err", .line = try_catch.sourceLineNumber() orelse 0,