diff --git a/src/browser/ScriptManagerBase.zig b/src/browser/ScriptManagerBase.zig index b26f118dc..5d46a85ef 100644 --- a/src/browser/ScriptManagerBase.zig +++ b/src/browser/ScriptManagerBase.zig @@ -212,24 +212,30 @@ pub fn preloadImport(self: *ScriptManagerBase, url: [:0]const u8, referrer: []co self.async_scripts.append(&script.node); const owner = self.owner; - owner.makeRequest(.{ - .ctx = script, - .url = url, - .method = .GET, - .origin = owner.origin(), - .request_mode = .cors, - .credentials_mode = .same_origin, - .resource_type = .script, - .start_callback = if (log.enabled(.http, .debug)) Script.startCallback else null, - .header_callback = Script.headerCallback, - .data_callback = Script.dataCallback, - .done_callback = Script.doneCallback, - .error_callback = Script.errorCallback, - .shutdown_callback = Script.shutdownCallback, - }) catch |err| { - self.async_scripts.remove(&script.node); - return err; + const transfer = blk: { + errdefer self.async_scripts.remove(&script.node); + const transfer = try owner.newRequest(.{ + .ctx = script, + .url = url, + .method = .GET, + .origin = owner.origin(), + .request_mode = .cors, + .credentials_mode = .same_origin, + .resource_type = .script, + .start_callback = if (log.enabled(.http, .debug)) Script.startCallback else null, + .header_callback = Script.headerCallback, + .data_callback = Script.dataCallback, + .done_callback = Script.doneCallback, + .error_callback = Script.errorCallback, + .shutdown_callback = Script.shutdownCallback, + }); + errdefer transfer.deinit(); + try owner.headersForRequest(transfer); + break :blk transfer; }; + gop.value_ptr.transfer_id = transfer.id; + // A synchronous failure is delivered through Script.errorCallback. + transfer.submit() catch {}; } // (element set) or the prescan finding a @@ -277,6 +283,24 @@ pub fn waitForImport(self: *ScriptManagerBase, url: [:0]const u8) !ModuleSource defer self.endEvaluationWindow(was_evaluating); var client = self.client; + + // We're inside V8's module instantiation. Nothing but this module's + // transfer may be delivered: any other callback can run JS (e.g. a fetch() + // resolving), and JS that import()s a module of the graph V8 is still + // linking re-enters instantiation and crashes V8. + const frame_id = self.owner.frameId(); + const blocked = blk: { + const entry = self.imported_modules.get(url) orelse break :blk false; + if (entry.state != .loading) { + break :blk false; + } + try client.blockOn(frame_id, entry.transfer_id); + break :blk true; + }; + defer if (blocked) { + client.releaseBlocking(frame_id); + }; + while (true) { // imported_modules can be mutated by client.tick, so we need to lookup // the entry on each iteration. @@ -1018,6 +1042,9 @@ const ImportedModule = struct { // will never collect it (see preloadModuleHint). A dynamic import may // adopt a hint entry outright (see getAsyncImport). hint: bool = false, + // The transfer fetching the module, which waitForImport lets through the + // HttpClient's gate. + transfer_id: u32 = 0, state: State, buffer: std.ArrayList(u8) = .empty, diff --git a/src/browser/tests/page/module_reentrant_import.html b/src/browser/tests/page/module_reentrant_import.html new file mode 100644 index 000000000..9a776b319 --- /dev/null +++ b/src/browser/tests/page/module_reentrant_import.html @@ -0,0 +1,25 @@ + + + + + + diff --git a/src/browser/tests/page/modules/reentrant-a.js b/src/browser/tests/page/modules/reentrant-a.js new file mode 100644 index 000000000..38ef85b4d --- /dev/null +++ b/src/browser/tests/page/modules/reentrant-a.js @@ -0,0 +1,2 @@ +import { c } from "./reentrant-c.js?delay_ms=400"; +export const a = c; diff --git a/src/browser/tests/page/modules/reentrant-c.js b/src/browser/tests/page/modules/reentrant-c.js new file mode 100644 index 000000000..93c974371 --- /dev/null +++ b/src/browser/tests/page/modules/reentrant-c.js @@ -0,0 +1 @@ +export const c = 'c'; diff --git a/src/network/HttpClient.zig b/src/network/HttpClient.zig index 24357b236..aa5f22b48 100644 --- a/src/network/HttpClient.zig +++ b/src/network/HttpClient.zig @@ -172,9 +172,10 @@ test_inbox: if (lp.IS_TEST) ?*Inbox else void = if (lp.IS_TEST) null else {}, max_response_size: usize, -// While a frame has a blocking (synchronous) request in flight, dispatch -// holds back every other transfer for that frame so their callbacks can't -// run JS while the parser is on the stack. frame_id -> blocking transfer id. +// While a frame has a blocking (synchronous) request in flight, or waits for +// a module import, dispatch holds back every other transfer for that frame so +// their callbacks can't run JS while the parser is on the stack. +// frame_id -> blocking transfer id. blocking_requests: std.AutoHashMapUnmanaged(u32, u32) = .empty, // Count of transfers parked for CDP interception (request or auth phase). @@ -1424,9 +1425,26 @@ fn processTransfer(self: *Client, transfer: *Transfer) !void { transfer.state = .queued; } +// Until released, a sync tick delivers only `transfer_id` +pub fn blockOn(self: *Client, frame_id: u32, transfer_id: u32) !void { + try self.blocking_requests.putNoClobber(self.allocator, frame_id, transfer_id); + + // maybe the transfer was already gated + var node = self.gated_queue.first; + while (node) |n| : (node = n.next) { + const transfer: *Transfer = @fieldParentPtr("_queue_node", n); + if (transfer.id == transfer_id) { + transfer._gated = false; + self.gated_queue.remove(n); + self.dispatch_queue.append(n); + return; + } + } +} + // A blocking request is complete. Any completed transfer that was placed in the // gated_queue because of it can now be placed back in the dispatch queue. -fn releaseBlocking(self: *Client, frame_id: u32) void { +pub fn releaseBlocking(self: *Client, frame_id: u32) void { _ = self.blocking_requests.remove(frame_id); // items were added to the gate in order, so walking backwards restores that // order. (Order might not matter, but preserving it costs nothing) @@ -2496,7 +2514,7 @@ pub const Transfer = struct { req.shutdown_callback = SyncContext.shutdownCallback; const frame_id = req.frame_id; - client.blocking_requests.putNoClobber(client.allocator, frame_id, self.id) catch |err| { + client.blockOn(frame_id, self.id) catch |err| { self.deinit(); return err; };