mirror of
https://github.com/lightpanda-io/browser.git
synced 2026-02-04 14:33:47 +00:00
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 63f1c85964. 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).
This commit is contained in:
@@ -204,10 +204,11 @@ pub fn deinit(self: *Page) void {
|
|||||||
// stats.print(&stream) catch unreachable;
|
// stats.print(&stream) catch unreachable;
|
||||||
}
|
}
|
||||||
|
|
||||||
// removeContext() will execute the destructor of any type that
|
|
||||||
// registered a destructor (e.g. XMLHttpRequest).
|
// some MicroTasks might be referencing the page, we need to drain it while
|
||||||
// Should be called before we deinit the page, because these objects
|
// the page still exists
|
||||||
// could be referencing it.
|
self.js.runMicrotasks();
|
||||||
|
|
||||||
const session = self._session;
|
const session = self._session;
|
||||||
session.executor.removeContext();
|
session.executor.removeContext();
|
||||||
|
|
||||||
|
|||||||
@@ -411,15 +411,16 @@ pub fn BrowserContext(comptime CDP_T: type) type {
|
|||||||
transfer.abort(error.ClientDisconnect);
|
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
|
// If the session has a page, we need to clear it first. The page
|
||||||
// context is always nested inside of the isolated world context,
|
// context is always nested inside of the isolated world context,
|
||||||
// so we need to shutdown the page one first.
|
// so we need to shutdown the page one first.
|
||||||
self.cdp.browser.closeSession();
|
self.cdp.browser.closeSession();
|
||||||
|
|
||||||
for (self.isolated_worlds.items) |*world| {
|
|
||||||
world.deinit();
|
|
||||||
}
|
|
||||||
self.isolated_worlds.clearRetainingCapacity();
|
|
||||||
self.node_registry.deinit();
|
self.node_registry.deinit();
|
||||||
self.node_search_list.deinit();
|
self.node_search_list.deinit();
|
||||||
self.cdp.browser.notification.unregisterAll(self);
|
self.cdp.browser.notification.unregisterAll(self);
|
||||||
|
|||||||
Reference in New Issue
Block a user