From 1ceaabe69f2a864343805fc7acbc36628b34bf28 Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Mon, 16 Mar 2026 20:56:18 +0800 Subject: [PATCH] Switch to reference counting for Mutation Observer and Intersection Observer This may be a stopgap. Our identity model assumes that v8 won't allow cross-origin access. It turns out that with CDP and Inspector, this isn't true. Inspectors can break / violate cross-origin restrictions. The result is that 2 origins can see the same zig instance, which causes 2 v8::Objects to reference the same Zig instance. This likely causes some consistency issue. Like, if you take mo in 1 context, and write an arbitrary property, mo.hack = true, you won't observe that in the 2nd context (because it's a different v8::Object). But, it _is_ the same Zig instance, so if you set a known/real property, it will be updated. That's probably a pretty minor issue. The bigger issue is that it can result in a use-after-free when using explicit strong/weak ref: 1 - Mutation observer is created in Origin1 2 - It's automatically set to weak 3 - Something is observed, the reference is made strong 4 - The MO is accessed from Origin2 5 - Creates a new v8::Object 6 - Sets it to weak 7 - Object goes out of scope in Origin2 8 - Finalizer is called <- free 9 - MO is manipulated in Origin 1 <- use after free Maybe the right option is to have a single shared identity map. I need to think about it. As a stopgap, switching to reference counting (which we already support) shold prevent the use-after free. While we'll still create 2 v8::Objects, they'll each acquireRef (_rc = 2) and thus it won't be freed until they both release i Maybe the right option is to have a single shared identity map. I need to think about it. As a stopgap, switching to reference counting (which we already support) shold prevent the use-after free. While we'll still create 2 v8::Objects, they'll each acquireRef (_rc = 2) and thus it won't be freed until they both release it. --- src/browser/webapi/IntersectionObserver.zig | 37 ++++++++++++++++----- src/browser/webapi/MutationObserver.zig | 34 ++++++++++++++----- 2 files changed, 54 insertions(+), 17 deletions(-) diff --git a/src/browser/webapi/IntersectionObserver.zig b/src/browser/webapi/IntersectionObserver.zig index 74a5d79e..b4c07e77 100644 --- a/src/browser/webapi/IntersectionObserver.zig +++ b/src/browser/webapi/IntersectionObserver.zig @@ -37,6 +37,7 @@ pub fn registerTypes() []const type { const IntersectionObserver = @This(); +_rc: u8 = 0, _arena: Allocator, _callback: js.Function.Temp, _observing: std.ArrayList(*Element) = .{}, @@ -93,12 +94,24 @@ pub fn init(callback: js.Function.Temp, options: ?ObserverInit, page: *Page) !*I } pub fn deinit(self: *IntersectionObserver, shutdown: bool, session: *Session) void { - self._callback.release(); - if ((comptime IS_DEBUG) and !shutdown) { - std.debug.assert(self._observing.items.len == 0); + const rc = self._rc; + if (comptime IS_DEBUG) { + std.debug.assert(rc != 0); } - session.releaseArena(self._arena); + if (rc == 1 or shutdown) { + self._callback.release(); + if ((comptime IS_DEBUG) and !shutdown) { + std.debug.assert(self._observing.items.len == 0); + } + session.releaseArena(self._arena); + } else { + self._rc = rc - 1; + } +} + +pub fn acquireRef(self: *IntersectionObserver) void { + self._rc += 1; } pub fn observe(self: *IntersectionObserver, target: *Element, page: *Page) !void { @@ -111,7 +124,7 @@ pub fn observe(self: *IntersectionObserver, target: *Element, page: *Page) !void // Register with page if this is our first observation if (self._observing.items.len == 0) { - page.js.strongRef(self); + self._rc += 1; try page.registerIntersectionObserver(self); } @@ -148,20 +161,26 @@ pub fn unobserve(self: *IntersectionObserver, target: *Element, page: *Page) voi } if (self._observing.items.len == 0) { - page.js.safeWeakRef(self); + self.deinit(false, page._session); } } pub fn disconnect(self: *IntersectionObserver, page: *Page) void { - page.unregisterIntersectionObserver(self); - self._observing.clearRetainingCapacity(); self._previous_states.clearRetainingCapacity(); for (self._pending_entries.items) |entry| { entry.deinit(false, page._session); } self._pending_entries.clearRetainingCapacity(); - page.js.safeWeakRef(self); + + const observing_count = self._observing.items.len; + self._observing.clearRetainingCapacity(); + + if (observing_count > 0) { + self.deinit(false, page._session); + } + + page.unregisterIntersectionObserver(self); } pub fn takeRecords(self: *IntersectionObserver, page: *Page) ![]*IntersectionObserverEntry { diff --git a/src/browser/webapi/MutationObserver.zig b/src/browser/webapi/MutationObserver.zig index b8608381..8b625fa8 100644 --- a/src/browser/webapi/MutationObserver.zig +++ b/src/browser/webapi/MutationObserver.zig @@ -39,6 +39,7 @@ pub fn registerTypes() []const type { const MutationObserver = @This(); +_rc: u8 = 0, _arena: Allocator, _callback: js.Function.Temp, _observing: std.ArrayList(Observing) = .{}, @@ -86,12 +87,24 @@ pub fn init(callback: js.Function.Temp, page: *Page) !*MutationObserver { } pub fn deinit(self: *MutationObserver, shutdown: bool, session: *Session) void { - self._callback.release(); - if ((comptime IS_DEBUG) and !shutdown) { - std.debug.assert(self._observing.items.len == 0); + const rc = self._rc; + if (comptime IS_DEBUG) { + std.debug.assert(rc != 0); } - session.releaseArena(self._arena); + if (rc == 1 or shutdown) { + self._callback.release(); + if ((comptime IS_DEBUG) and !shutdown) { + std.debug.assert(self._observing.items.len == 0); + } + session.releaseArena(self._arena); + } else { + self._rc = rc - 1; + } +} + +pub fn acquireRef(self: *MutationObserver) void { + self._rc += 1; } pub fn observe(self: *MutationObserver, target: *Node, options: ObserveOptions, page: *Page) !void { @@ -158,7 +171,7 @@ pub fn observe(self: *MutationObserver, target: *Node, options: ObserveOptions, // Register with page if this is our first observation if (self._observing.items.len == 0) { - page.js.strongRef(self); + self._rc += 1; try page.registerMutationObserver(self); } @@ -169,13 +182,18 @@ pub fn observe(self: *MutationObserver, target: *Node, options: ObserveOptions, } pub fn disconnect(self: *MutationObserver, page: *Page) void { - page.unregisterMutationObserver(self); - self._observing.clearRetainingCapacity(); for (self._pending_records.items) |record| { record.deinit(false, page._session); } self._pending_records.clearRetainingCapacity(); - page.js.safeWeakRef(self); + + const observing_count = self._observing.items.len; + self._observing.clearRetainingCapacity(); + + if (observing_count > 0) { + self.deinit(false, page._session); + } + page.unregisterMutationObserver(self); } pub fn takeRecords(self: *MutationObserver, page: *Page) ![]*MutationRecord {