public/browser
1
0
Fork 0
mirror of https://github.com/lightpanda-io/browser.git synced 2026-02-04 06:23:45 +00:00
Code Issues Actions 5 Packages Projects Releases Wiki Activity

When loading a module, make sure we have a module_promise

Browse Source 
Currently, when loading a module, if the module is found in the cache, it's
immediately returned. However, that can result in a timing issue where the
module is cached, but not evaluated, and doesn't have an associated promise.

This commit tries to ensure a module is always evaluated and that the cache
entry has a module promise.

This might fix an crash handler issue. I couldn't reproduce the issue though.
I believe it requires specific timing which is hard to reproduce in a test.
This commit is contained in:
Karl Seguin
2026-01-30 18:16:12 +08:00
parent 7c98a27c53
commit 1d03b688d9
1 changed files with 17 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files

31
src/browser/js/Context.zig
View File

@@ -322,7 +322,13 @@ pub fn module(self: *Context, comptime want_result: bool, local: *const js.Local
if (cacheable) { if (cacheable) {
gop = try self.module_cache.getOrPut(arena, url); gop = try self.module_cache.getOrPut(arena, url);
if (gop.found_existing) { if (gop.found_existing) {
if (gop.value_ptr.module != null) { if (gop.value_ptr.module) |cache_mod| {
if (gop.value_ptr.module_promise == null) {
const mod = local.toLocal(cache_mod);
if (mod.getStatus() == .kInstantiated) {
return self.evaluateModule(want_result, mod, url, true);
}
}
return if (comptime want_result) gop.value_ptr.* else {}; return if (comptime want_result) gop.value_ptr.* else {};
} }
} else { } else {
@@ -352,6 +358,10 @@ pub fn module(self: *Context, comptime want_result: bool, local: *const js.Local
return error.ModuleInstantiationError; return error.ModuleInstantiationError;
} }
return self.evaluateModule(want_result, mod, owned_url, cacheable);
}
fn evaluateModule(self: *Context, comptime want_result: bool, mod: js.Module, url: []const u8, cacheable: bool) !(if (want_result) ModuleEntry else void) {
const evaluated = mod.evaluate() catch { const evaluated = mod.evaluate() catch {
if (comptime IS_DEBUG) { if (comptime IS_DEBUG) {
std.debug.assert(mod.getStatus() == .kErrored); std.debug.assert(mod.getStatus() == .kErrored);
@@ -365,7 +375,7 @@ pub fn module(self: *Context, comptime want_result: bool, local: *const js.Local
}; };
log.warn(.js, "evaluate module", .{ log.warn(.js, "evaluate module", .{
.message = message, .message = message,
.specifier = owned_url, .specifier = url,
}); });
return error.EvaluationError; return error.EvaluationError;
}; };
@@ -374,24 +384,15 @@ pub fn module(self: *Context, comptime want_result: bool, local: *const js.Local
// Must be a promise that gets returned here. // Must be a promise that gets returned here.
lp.assert(evaluated.isPromise(), "Context.module non-promise", .{}); lp.assert(evaluated.isPromise(), "Context.module non-promise", .{});
if (comptime !want_result) {
// avoid creating a bunch of persisted objects if it isn't
// cacheable and the caller doesn't care about results.
// This is pretty common, i.e. every <script type=module>
// within the html page.
if (!cacheable) { if (!cacheable) {
return; switch (comptime want_result) {
false => return,
true => unreachable,
} }
} }
// anyone who cares about the result, should also want it to
// be cached
if (comptime IS_DEBUG) {
std.debug.assert(cacheable);
}
// entry has to have been created atop this function // entry has to have been created atop this function
const entry = self.module_cache.getPtr(owned_url).?; const entry = self.module_cache.getPtr(url).?;
// and the module must have been set after we compiled it // and the module must have been set after we compiled it
lp.assert(entry.module != null, "Context.module with module", .{}); lp.assert(entry.module != null, "Context.module with module", .{});