From 223a6170d587479f55fcf4d8789fb16e6b73ad90 Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Wed, 14 Jan 2026 09:37:10 +0800 Subject: [PATCH] Fix use-after free On CDP.BrowserContext.deinit, clear the isolated world ExecutionContext before terminating the session. This is important as the isolated_world list is allocated from the session.arena. Also, semi-revert 63f1c859645c18e5032c7b8855f9381debaf940e. Before all this we were running microtasks on ExecutionWorld.removeContext. That didn't seem right (and I thought it was the original source of the bug). But, for the "real" Page context, this is critical, since Microtasks can reference the Page object. Since microTasks are isolation-level, it's possible for a microtasks for Page1 to execute after Page1 goes away (if we create a new page, Page2). This re-adds the microtask "draining", but only for the Page (i.e. in Page.deinit). --- src/browser/Page.zig | 9 +++++---- src/cdp/cdp.zig | 9 +++++---- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/browser/Page.zig b/src/browser/Page.zig index 86374179..8e4e6806 100644 --- a/src/browser/Page.zig +++ b/src/browser/Page.zig @@ -204,10 +204,11 @@ pub fn deinit(self: *Page) void { // stats.print(&stream) catch unreachable; } - // removeContext() will execute the destructor of any type that - // registered a destructor (e.g. XMLHttpRequest). - // Should be called before we deinit the page, because these objects - // could be referencing it. + + // some MicroTasks might be referencing the page, we need to drain it while + // the page still exists + self.js.runMicrotasks(); + const session = self._session; session.executor.removeContext(); diff --git a/src/cdp/cdp.zig b/src/cdp/cdp.zig index 3aef758a..b58ff9b2 100644 --- a/src/cdp/cdp.zig +++ b/src/cdp/cdp.zig @@ -411,15 +411,16 @@ pub fn BrowserContext(comptime CDP_T: type) type { transfer.abort(error.ClientDisconnect); } + for (self.isolated_worlds.items) |*world| { + world.deinit(); + } + self.isolated_worlds.clearRetainingCapacity(); + // 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. self.cdp.browser.closeSession(); - for (self.isolated_worlds.items) |*world| { - world.deinit(); - } - self.isolated_worlds.clearRetainingCapacity(); self.node_registry.deinit(); self.node_search_list.deinit(); self.cdp.browser.notification.unregisterAll(self);