From 11fb5f990e1e51407714439430d1a1c451ccd2f1 Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Tue, 10 Mar 2026 20:50:58 +0800 Subject: [PATCH] Call `resetContextGroup` on page removal Calling it here ensures that the inspector gets reset on internal page navigation. We were seeing intermittent segfaults on a problematic WPT tests (/encoding/legacy-mb-japanese/euc-jp/) which I believe this solves. (The tests are still broken. Because we don't support form targets, they cause the root page to reload in a tight cycle, causing a lot of context creation / destruction, which I thin was the issue. This commit doesn't fix the broken test but it hopefully fixes the crash). Also, clear out the Inspector's default_context when the default context is destroyed. (This was the first thing I did to try to fix the crash, it didn't work, but I believe it's correct). --- src/browser/js/Inspector.zig | 6 ++++++ src/cdp/cdp.zig | 7 ++++++- src/cdp/domains/page.zig | 4 ++++ 3 files changed, 16 insertions(+), 1 deletion(-) diff --git a/src/browser/js/Inspector.zig b/src/browser/js/Inspector.zig index a9032e33..aa35be7b 100644 --- a/src/browser/js/Inspector.zig +++ b/src/browser/js/Inspector.zig @@ -130,6 +130,12 @@ pub fn contextCreated( pub fn contextDestroyed(self: *Inspector, context: *const v8.Context) void { v8.v8_inspector__Inspector__ContextDestroyed(self.handle, context); + + if (self.default_context) |*dc| { + if (v8.v8__Global__IsEqual(dc, context)) { + self.default_context = null; + } + } } pub fn resetContextGroup(self: *const Inspector) void { diff --git a/src/cdp/cdp.zig b/src/cdp/cdp.zig index 8783e5a0..6d14c627 100644 --- a/src/cdp/cdp.zig +++ b/src/cdp/cdp.zig @@ -459,6 +459,12 @@ pub fn BrowserContext(comptime CDP_T: type) type { } self.isolated_worlds.clearRetainingCapacity(); + // do this before closeSession, since we don't want to process any + // new notification (Or maybe, instead of the deinit above, we just + // rely on those notifications to do our normal cleanup?) + + self.notification.unregisterAll(self); + // If the session has a page, we need to clear it first. The page // context is always nested inside of the isolated world context, // so we need to shutdown the page one first. @@ -466,7 +472,6 @@ pub fn BrowserContext(comptime CDP_T: type) type { self.node_registry.deinit(); self.node_search_list.deinit(); - self.notification.unregisterAll(self); self.notification.deinit(); if (self.http_proxy_changed) { diff --git a/src/cdp/domains/page.zig b/src/cdp/domains/page.zig index d4b7b301..c8b39d65 100644 --- a/src/cdp/domains/page.zig +++ b/src/cdp/domains/page.zig @@ -290,6 +290,10 @@ pub fn pageNavigate(bc: anytype, event: *const Notification.PageNavigate) !void } pub fn pageRemove(bc: anytype) !void { + // Clear all remote object mappings to prevent stale objectIds from being used + // after the context is destroy + bc.inspector_session.inspector.resetContextGroup(); + // The main page is going to be removed, we need to remove contexts from other worlds first. for (bc.isolated_worlds.items) |isolated_world| { try isolated_world.removeContext();