From f921869fb6a1186a8e199fac47e6a07a2aa19be7 Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Thu, 30 Apr 2026 18:04:56 +0800 Subject: [PATCH] Defer window.close() Cannot Frame.deinit in window.close() as that's happening inside JS runtime. Instead, defer on Page.deinit. This is MUCH later than necessary, but I'd like to address the timing separately. This commit, as-is, prevents real crashes. --- src/browser/Page.zig | 11 +++++++++ src/browser/tests/window/open.html | 24 +++++++++++++++++++ .../window/support/popup_self_close.html | 12 ++++++++++ src/browser/webapi/Window.zig | 9 ++++++- 4 files changed, 55 insertions(+), 1 deletion(-) create mode 100644 src/browser/tests/window/support/popup_self_close.html diff --git a/src/browser/Page.zig b/src/browser/Page.zig index 929cd07c..927d2f05 100644 --- a/src/browser/Page.zig +++ b/src/browser/Page.zig @@ -96,6 +96,12 @@ frame: Frame, // to the original page like this. popups: std.ArrayList(*Frame) = .empty, +// Popups that have called window.close() but whose teardown is deferred to +// Page.deinit. We can't deinit synchronously from window.close() because +// that's invoked from JS still running on top of the Frame's V8 context (or +// from a script eval whose parser still holds the Frame). +queued_close: std.ArrayList(*Frame) = .empty, + // Initialize a Page and its root Frame. pub fn init(self: *Page, session: *Session, frame_id: u32) !void { const frame_arena = try session.arena_pool.acquire(.large, "Page.frame_arena"); @@ -115,6 +121,11 @@ pub fn init(self: *Page, session: *Session, frame_id: u32) !void { // Tear down the Page and its root Frame. Equivalent to the old // Session.removePage + Session.resetFrameResources. pub fn deinit(self: *Page, abort_http: bool) void { + for (self.queued_close.items) |popup| { + popup.deinit(abort_http); + } + self.queued_close = .empty; + for (self.popups.items) |popup| { popup.deinit(abort_http); } diff --git a/src/browser/tests/window/open.html b/src/browser/tests/window/open.html index 4bc0b72d..864339ac 100644 --- a/src/browser/tests/window/open.html +++ b/src/browser/tests/window/open.html @@ -107,3 +107,27 @@ testing.expectEqual(null, window.opener); } + + diff --git a/src/browser/tests/window/support/popup_self_close.html b/src/browser/tests/window/support/popup_self_close.html new file mode 100644 index 00000000..6b39da4a --- /dev/null +++ b/src/browser/tests/window/support/popup_self_close.html @@ -0,0 +1,12 @@ + + diff --git a/src/browser/webapi/Window.zig b/src/browser/webapi/Window.zig index 0945dc7f..570c5a3e 100644 --- a/src/browser/webapi/Window.zig +++ b/src/browser/webapi/Window.zig @@ -568,7 +568,14 @@ pub fn close(self: *Window) void { } } - frame.deinit(true); + // We can't tear the Frame down here — close() is invoked from JS still + // running on top of this Frame's V8 context, often deep inside a script + // eval whose parser is still holding the Frame. Destroying the context + // now leaves dangling pointers in the unwinding script eval (load event + // dispatch, runMacrotasks, etc.). Defer to Page.deinit instead. + page.queued_close.append(page.frame_arena, frame) catch |err| { + log.err(.frame, "queue popup close", .{ .err = err }); + }; } pub fn postMessage(self: *Window, message: js.Value.Temp, target_origin: ?[]const u8, frame: *Frame) !void {