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.
This commit is contained in:
Karl Seguin
2026-03-16 20:56:18 +08:00
parent 18b635936c
commit 1ceaabe69f
2 changed files with 54 additions and 17 deletions

View File

@@ -37,6 +37,7 @@ pub fn registerTypes() []const type {
const IntersectionObserver = @This(); const IntersectionObserver = @This();
_rc: u8 = 0,
_arena: Allocator, _arena: Allocator,
_callback: js.Function.Temp, _callback: js.Function.Temp,
_observing: std.ArrayList(*Element) = .{}, _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 { pub fn deinit(self: *IntersectionObserver, shutdown: bool, session: *Session) void {
self._callback.release(); const rc = self._rc;
if ((comptime IS_DEBUG) and !shutdown) { if (comptime IS_DEBUG) {
std.debug.assert(self._observing.items.len == 0); 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 { 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 // Register with page if this is our first observation
if (self._observing.items.len == 0) { if (self._observing.items.len == 0) {
page.js.strongRef(self); self._rc += 1;
try page.registerIntersectionObserver(self); try page.registerIntersectionObserver(self);
} }
@@ -148,20 +161,26 @@ pub fn unobserve(self: *IntersectionObserver, target: *Element, page: *Page) voi
} }
if (self._observing.items.len == 0) { if (self._observing.items.len == 0) {
page.js.safeWeakRef(self); self.deinit(false, page._session);
} }
} }
pub fn disconnect(self: *IntersectionObserver, page: *Page) void { pub fn disconnect(self: *IntersectionObserver, page: *Page) void {
page.unregisterIntersectionObserver(self);
self._observing.clearRetainingCapacity();
self._previous_states.clearRetainingCapacity(); self._previous_states.clearRetainingCapacity();
for (self._pending_entries.items) |entry| { for (self._pending_entries.items) |entry| {
entry.deinit(false, page._session); entry.deinit(false, page._session);
} }
self._pending_entries.clearRetainingCapacity(); 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 { pub fn takeRecords(self: *IntersectionObserver, page: *Page) ![]*IntersectionObserverEntry {

View File

@@ -39,6 +39,7 @@ pub fn registerTypes() []const type {
const MutationObserver = @This(); const MutationObserver = @This();
_rc: u8 = 0,
_arena: Allocator, _arena: Allocator,
_callback: js.Function.Temp, _callback: js.Function.Temp,
_observing: std.ArrayList(Observing) = .{}, _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 { pub fn deinit(self: *MutationObserver, shutdown: bool, session: *Session) void {
self._callback.release(); const rc = self._rc;
if ((comptime IS_DEBUG) and !shutdown) { if (comptime IS_DEBUG) {
std.debug.assert(self._observing.items.len == 0); 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 { 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 // Register with page if this is our first observation
if (self._observing.items.len == 0) { if (self._observing.items.len == 0) {
page.js.strongRef(self); self._rc += 1;
try page.registerMutationObserver(self); 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 { pub fn disconnect(self: *MutationObserver, page: *Page) void {
page.unregisterMutationObserver(self);
self._observing.clearRetainingCapacity();
for (self._pending_records.items) |record| { for (self._pending_records.items) |record| {
record.deinit(false, page._session); record.deinit(false, page._session);
} }
self._pending_records.clearRetainingCapacity(); 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 { pub fn takeRecords(self: *MutationObserver, page: *Page) ![]*MutationRecord {