Relax assertion on httpclient abort

It's ok to still have transfers, as long as whatever transfers still exists
are in an aborted state.

src/browser/HttpClient.zig

@@ -235,10 +235,6 @@ fn _abort(self: *Client, comptime abort_all: bool, frame_id: u32) void {
         }
     }
 
-    if (comptime IS_DEBUG and abort_all) {
-        std.debug.assert(self.active == 0);
-    }
-
     {
         var q = &self.queue;
         var n = q.first;
@@ -259,12 +255,16 @@ fn _abort(self: *Client, comptime abort_all: bool, frame_id: u32) void {
     }
 
     if (comptime IS_DEBUG and abort_all) {
-        std.debug.assert(self.in_use.first == null);
-
-        const running = self.handles.perform() catch |err| {
-            lp.assert(false, "multi perform in abort", .{ .err = err });
-        };
-        std.debug.assert(running == 0);
+        // Even after an abort_all, we could still have transfers, but, at the
+        // very least, they should all be flagged as aborted.
+        var it = self.in_use.first;
+        var leftover: usize = 0;
+        while (it) |node| : (it = node.next) {
+            const conn: *http.Connection = @fieldParentPtr("node", node);
+            std.debug.assert((Transfer.fromConnection(conn) catch unreachable).aborted);
+            leftover += 1;
+        }
+        std.debug.assert(self.active == leftover);
     }
 }
 

src/browser/Page.zig

@@ -351,30 +351,6 @@ pub fn deinit(self: *Page, abort_http: bool) void {
         session.releaseArena(qn.arena);
     }
 
-    {
-        // Release all objects we're referencing
-        {
-            var it = self._blob_urls.valueIterator();
-            while (it.next()) |blob| {
-                blob.*.releaseRef(session);
-            }
-        }
-
-        {
-            var it: ?*std.DoublyLinkedList.Node = self._mutation_observers.first;
-            while (it) |node| : (it = node.next) {
-                const observer: *MutationObserver = @fieldParentPtr("node", node);
-                observer.releaseRef(session);
-            }
-        }
-
-        for (self._intersection_observers.items) |observer| {
-            observer.releaseRef(session);
-        }
-
-        self.window._document._selection.releaseRef(session);
-    }
-
     session.browser.env.destroyContext(self.js);
 
     self._script_manager.shutdown = true;
@@ -1362,24 +1338,20 @@ pub fn schedulePerformanceObserverDelivery(self: *Page) !void {
 }
 
 pub fn registerMutationObserver(self: *Page, observer: *MutationObserver) !void {
-    observer.acquireRef();
     self._mutation_observers.append(&observer.node);
 }
 
 pub fn unregisterMutationObserver(self: *Page, observer: *MutationObserver) void {
-    observer.releaseRef(self._session);
     self._mutation_observers.remove(&observer.node);
 }
 
 pub fn registerIntersectionObserver(self: *Page, observer: *IntersectionObserver) !void {
-    observer.acquireRef();
     try self._intersection_observers.append(self.arena, observer);
 }
 
 pub fn unregisterIntersectionObserver(self: *Page, observer: *IntersectionObserver) void {
     for (self._intersection_observers.items, 0..) |obs, i| {
         if (obs == observer) {
-            observer.releaseRef(self._session);
             _ = self._intersection_observers.swapRemove(i);
             return;
         }

src/browser/Session.zig

@@ -501,11 +501,7 @@ pub const FinalizerCallback = struct {
     session: *Session,
     resolved_ptr_id: usize,
     finalizer_ptr_id: usize,
-    release_ref: *const fn (ptr_id: usize, session: *Session) void,
-
-    // Track how many identities (JS worlds) reference this FC.
-    // Only cleanup when all identities have finalized.
-    identity_count: u8 = 0,
+    _deinit: *const fn (ptr_id: usize, session: *Session) void,
 
     // For every FinalizerCallback we'll have 1+ FinalizerCallback.Identity: one
     // for every identity that gets the instance. In most cases, that'l be 1.
@@ -514,9 +510,8 @@ pub const FinalizerCallback = struct {
         fc: *Session.FinalizerCallback,
     };
 
-    // Called during page reset to force cleanup regardless of identity_count.
     fn deinit(self: *FinalizerCallback, session: *Session) void {
-        self.release_ref(self.finalizer_ptr_id, session);
+        self._deinit(self.finalizer_ptr_id, session);
         session.releaseArena(self.arena);
     }
 };

src/browser/js/Local.zig

@@ -266,6 +266,7 @@ pub fn mapZigInstanceToJs(self: *const Local, js_obj_handle: ?*const v8.Object,
             v8.v8__Global__New(isolate.handle, js_obj.handle, gop.value_ptr);
             if (resolved.finalizer) |finalizer| {
                 const finalizer_ptr_id = finalizer.ptr_id;
+                finalizer.acquireRef(finalizer_ptr_id);
 
                 const session = ctx.session;
                 const finalizer_gop = try session.finalizer_callbacks.getOrPut(session.page_arena, finalizer_ptr_id);
@@ -274,8 +275,7 @@ pub fn mapZigInstanceToJs(self: *const Local, js_obj_handle: ?*const v8.Object,
                     // see this Zig instance. We need to create the FinalizerCallback
                     // so that we can cleanup on page reset if v8 doesn't finalize.
                     errdefer _ = session.finalizer_callbacks.remove(finalizer_ptr_id);
-                    finalizer.acquire_ref(finalizer_ptr_id);
-                    finalizer_gop.value_ptr.* = try self.createFinalizerCallback(resolved_ptr_id, finalizer_ptr_id, finalizer.release_ref_from_zig);
+                    finalizer_gop.value_ptr.* = try self.createFinalizerCallback(resolved_ptr_id, finalizer_ptr_id, finalizer.deinit);
                 }
                 const fc = finalizer_gop.value_ptr.*;
                 const identity_finalizer = try fc.arena.create(Session.FinalizerCallback.Identity);
@@ -283,9 +283,8 @@ pub fn mapZigInstanceToJs(self: *const Local, js_obj_handle: ?*const v8.Object,
                     .fc = fc,
                     .identity = ctx.identity,
                 };
-                fc.identity_count += 1;
 
-                v8.v8__Global__SetWeakFinalizer(gop.value_ptr, identity_finalizer, finalizer.release_ref, v8.kParameter);
+                v8.v8__Global__SetWeakFinalizer(gop.value_ptr, identity_finalizer, finalizer.release, v8.kParameter);
             }
             return js_obj;
         },
@@ -1129,9 +1128,9 @@ const Resolved = struct {
         // Resolved.ptr is the most specific value in a chain (e.g. IFrame, not EventTarget, Node,  ...)
         // Finalizer.ptr_id is the most specific value in a chain that defines an acquireRef
         ptr_id: usize,
-        acquire_ref: *const fn (ptr_id: usize) void,
-        release_ref: *const fn (handle: ?*const v8.WeakCallbackInfo) callconv(.c) void,
-        release_ref_from_zig: *const fn (ptr_id: usize, session: *Session) void,
+        deinit: *const fn (ptr_id: usize, session: *Session) void,
+        acquireRef: *const fn (ptr_id: usize) void,
+        release: *const fn (handle: ?*const v8.WeakCallbackInfo) callconv(.c) void,
     };
 };
 pub fn resolveValue(value: anytype) Resolved {
@@ -1171,49 +1170,32 @@ fn resolveT(comptime T: type, value: *T) Resolved {
             const finalizer_ptr = getFinalizerPtr(value);
 
             const Wrap = struct {
+                fn deinit(ptr_id: usize, session: *Session) void {
+                    FT.deinit(@ptrFromInt(ptr_id), session);
+                }
+
                 fn acquireRef(ptr_id: usize) void {
                     FT.acquireRef(@ptrFromInt(ptr_id));
                 }
 
-                fn releaseRef(handle: ?*const v8.WeakCallbackInfo) callconv(.c) void {
+                fn release(handle: ?*const v8.WeakCallbackInfo) callconv(.c) void {
                     const ptr = v8.v8__WeakCallbackInfo__GetParameter(handle.?).?;
                     const identity_finalizer: *Session.FinalizerCallback.Identity = @ptrCast(@alignCast(ptr));
 
                     const fc = identity_finalizer.fc;
-                    const session = fc.session;
-                    const finalizer_ptr_id = fc.finalizer_ptr_id;
-
-                    // Remove from this identity's map
                     if (identity_finalizer.identity.identity_map.fetchRemove(fc.resolved_ptr_id)) |kv| {
                         var global = kv.value;
                         v8.v8__Global__Reset(&global);
                     }
 
-                    const identity_count = fc.identity_count;
-                    if (identity_count == 1) {
-                        // All IsolatedWorlds that reference this object have
-                        // released it. Release the instance ref, remove the
-                        // FinalizerCallback and free it.
-                        FT.releaseRef(@ptrFromInt(finalizer_ptr_id), session);
-                        const removed = session.finalizer_callbacks.remove(finalizer_ptr_id);
-                        if (comptime IS_DEBUG) {
-                            std.debug.assert(removed);
-                        }
-                        session.releaseArena(fc.arena);
-                    } else {
-                        fc.identity_count = identity_count - 1;
-                    }
-                }
-
-                fn releaseRefFromZig(ptr_id: usize, session: *Session) void {
-                    FT.releaseRef(@ptrFromInt(ptr_id), session);
+                    FT.releaseRef(@ptrFromInt(fc.finalizer_ptr_id), fc.session);
                 }
             };
             break :blk .{
                 .ptr_id = @intFromPtr(finalizer_ptr),
-                .acquire_ref = Wrap.acquireRef,
-                .release_ref = Wrap.releaseRef,
-                .release_ref_from_zig = Wrap.releaseRefFromZig,
+                .deinit = Wrap.deinit,
+                .acquireRef = Wrap.acquireRef,
+                .release = Wrap.release,
             };
         },
     };
@@ -1472,7 +1454,7 @@ fn createFinalizerCallback(
     // The most specific value where finalizers are defined
     // What actually gets acquired / released / deinit
     finalizer_ptr_id: usize,
-    release_ref: *const fn (ptr_id: usize, session: *Session) void,
+    deinit: *const fn (ptr_id: usize, session: *Session) void,
 ) !*Session.FinalizerCallback {
     const session = self.ctx.session;
 
@@ -1483,7 +1465,7 @@ fn createFinalizerCallback(
     fc.* = .{
         .arena = arena,
         .session = session,
-        .release_ref = release_ref,
+        ._deinit = deinit,
         .resolved_ptr_id = resolved_ptr_id,
         .finalizer_ptr_id = finalizer_ptr_id,
     };

src/browser/tests/node/child_nodes.html

@@ -4,7 +4,7 @@
 <div id=empty></div>
 <div id=one><p id=p10></p></div>
 
-<!--<script id=childNodes>
+<script id=childNodes>
   const div = $('#d1');
   const children = div.childNodes;
   testing.expectEqual(true, children instanceof NodeList);
@@ -65,24 +65,24 @@
   testing.expectEqual([], Array.from(empty.values()));
   testing.expectEqual([], Array.from(empty.entries()));
   testing.expectEqual([], Array.from(empty));
-</script> -->
+</script>
 
 <script id=one>
   const one = $('#one').childNodes;
-  // const p10 = $('#p10');
-  // testing.expectEqual(1, one.length);
-  // testing.expectEqual(p10, one[0]);
-  // testing.expectEqual([0], Array.from(one.keys()));
-  // testing.expectEqual([p10], Array.from(one.values()));
-  // testing.expectEqual([[0, p10]], Array.from(one.entries()));
+  const p10 = $('#p10');
+  testing.expectEqual(1, one.length);
+  testing.expectEqual(p10, one[0]);
+  testing.expectEqual([0], Array.from(one.keys()));
+  testing.expectEqual([p10], Array.from(one.values()));
+  testing.expectEqual([[0, p10]], Array.from(one.entries()));
 
-  // testing.expectEqual([p10], Array.from(one));
+  testing.expectEqual([p10], Array.from(one));
   let foreach = [];
   one.forEach((p) => foreach.push(p));
   testing.expectEqual([p10], foreach);
 </script>
 
-<!-- <script id=contains>
+<script id=contains>
   testing.expectEqual(true, document.contains(document));
   testing.expectEqual(true, $('#d1').contains($('#d1')));
   testing.expectEqual(true, document.contains($('#d1')));
@@ -94,4 +94,3 @@
   testing.expectEqual(false, $('#d1').contains($('#empty')));
   testing.expectEqual(false, $('#d1').contains($('#p10')));
 </script>
- -->

src/browser/webapi/IntersectionObserver.zig

@@ -114,9 +114,7 @@ pub fn init(callback: js.Function.Temp, options: ?ObserverInit, page: *Page) !*I
 pub fn deinit(self: *IntersectionObserver, session: *Session) void {
     self._callback.release();
     for (self._pending_entries.items) |entry| {
-        // These were never handed to v8, they do not have a corresponding
-        // FinalizerCallback. We 100% own them.
-        entry.deinit(session);
+        entry.deinitIfUnused(session);
     }
     session.releaseArena(self._arena);
 }
@@ -137,11 +135,14 @@ pub fn observe(self: *IntersectionObserver, target: *Element, page: *Page) !void
         }
     }
 
-    try self._observing.append(self._arena, target);
-    if (self._observing.items.len == 1) {
+    // Register with page if this is our first observation
+    if (self._observing.items.len == 0) {
+        self._rc._refs += 1;
         try page.registerIntersectionObserver(self);
     }
 
+    try self._observing.append(self._arena, target);
+
     // Don't initialize previous state yet - let checkIntersection do it
     // This ensures we get an entry on first observation
 
@@ -165,7 +166,7 @@ pub fn unobserve(self: *IntersectionObserver, target: *Element, page: *Page) voi
             while (j < self._pending_entries.items.len) {
                 if (self._pending_entries.items[j]._target == target) {
                     const entry = self._pending_entries.swapRemove(j);
-                    entry.deinit(page._session);
+                    entry.deinitIfUnused(page._session);
                 } else {
                     j += 1;
                 }
@@ -175,21 +176,25 @@ pub fn unobserve(self: *IntersectionObserver, target: *Element, page: *Page) voi
     }
 
     if (original_length > 0 and self._observing.items.len == 0) {
-        page.unregisterIntersectionObserver(self);
+        self._rc._refs -= 1;
     }
 }
 
 pub fn disconnect(self: *IntersectionObserver, page: *Page) void {
     for (self._pending_entries.items) |entry| {
-        entry.deinit(page._session);
+        entry.deinitIfUnused(page._session);
     }
     self._pending_entries.clearRetainingCapacity();
     self._previous_states.clearRetainingCapacity();
 
-    if (self._observing.items.len > 0) {
-        page.unregisterIntersectionObserver(self);
-    }
+    const observing_count = self._observing.items.len;
     self._observing.clearRetainingCapacity();
+
+    page.unregisterIntersectionObserver(self);
+
+    if (observing_count > 0) {
+        _ = self.releaseRef(page._session);
+    }
 }
 
 pub fn takeRecords(self: *IntersectionObserver, page: *Page) ![]*IntersectionObserverEntry {
@@ -335,6 +340,13 @@ pub const IntersectionObserverEntry = struct {
         session.releaseArena(self._arena);
     }
 
+    fn deinitIfUnused(self: *IntersectionObserverEntry, session: *Session) void {
+        if (self._rc._refs == 0) {
+            // hasn't been handed to JS yet.
+            self.deinit(session);
+        }
+    }
+
     pub fn releaseRef(self: *IntersectionObserverEntry, session: *Session) void {
         self._rc.release(self, session);
     }

src/browser/webapi/MutationObserver.zig

@@ -87,12 +87,8 @@ pub fn init(callback: js.Function.Temp, page: *Page) !*MutationObserver {
     return self;
 }
 
+/// Force cleanup on Session shutdown.
 pub fn deinit(self: *MutationObserver, session: *Session) void {
-    for (self._pending_records.items) |record| {
-        // These were never handed to v8, they do not have a corresponding
-        // FinalizerCallback. We 100% own them.
-        record.deinit(session);
-    }
     self._callback.release();
     session.releaseArena(self._arena);
 }
@@ -167,14 +163,16 @@ pub fn observe(self: *MutationObserver, target: *Node, options: ObserveOptions,
         }
     }
 
+    // Register with page if this is our first observation
+    if (self._observing.items.len == 0) {
+        self._rc._refs += 1;
+        try page.registerMutationObserver(self);
+    }
+
     try self._observing.append(arena, .{
         .target = target,
         .options = store_options,
     });
-
-    if (self._observing.items.len == 1) {
-        try page.registerMutationObserver(self);
-    }
 }
 
 pub fn disconnect(self: *MutationObserver, page: *Page) void {
@@ -182,11 +180,13 @@ pub fn disconnect(self: *MutationObserver, page: *Page) void {
         _ = record.releaseRef(page._session);
     }
     self._pending_records.clearRetainingCapacity();
-
-    if (self._observing.items.len > 0) {
-        page.unregisterMutationObserver(self);
-    }
+    const observing_count = self._observing.items.len;
     self._observing.clearRetainingCapacity();
+
+    if (observing_count > 0) {
+        _ = self.releaseRef(page._session);
+    }
+    page.unregisterMutationObserver(self);
 }
 
 pub fn takeRecords(self: *MutationObserver, page: *Page) ![]*MutationRecord {

src/browser/webapi/collections/NodeList.zig

@@ -42,8 +42,8 @@ _rc: lp.RC(u32) = .{},
 
 pub fn deinit(self: *NodeList, session: *Session) void {
     switch (self._data) {
-        .child_nodes => |cn| cn.deinit(session),
         .selector_list => |list| list.deinit(session),
+        .child_nodes => |cn| cn.deinit(session),
         else => {},
     }
 }
@@ -92,12 +92,7 @@ pub fn entries(self: *NodeList, page: *Page) !*EntryIterator {
 
 pub fn forEach(self: *NodeList, cb: js.Function, page: *Page) !void {
     var i: i32 = 0;
-
     var it = try self.values(page);
-
-    // the iterator takes a reference against our list
-    defer self.releaseRef(page._session);
-
     while (true) : (i += 1) {
         const next = try it.next(page);
         if (next.done) {

src/browser/webapi/collections/iterator.zig

@@ -26,8 +26,7 @@ pub fn Entry(comptime Inner: type, comptime field: ?[]const u8) type {
     const R = reflect(Inner, field);
 
     return struct {
-        _inner: Inner,
-        _rc: lp.RC(u8) = .{},
+        inner: Inner,
 
         const Self = @This();
 
@@ -39,31 +38,29 @@ pub fn Entry(comptime Inner: type, comptime field: ?[]const u8) type {
         };
 
         pub fn init(inner: Inner, page: *Page) !*Self {
-            const self = try page._factory.create(Self{ ._inner = inner });
-
-            if (@hasDecl(Inner, "acquireRef")) {
-                self._inner.acquireRef();
-            }
-            return self;
+            return page._factory.create(Self{ .inner = inner });
         }
 
         pub fn deinit(self: *Self, session: *Session) void {
-            if (@hasDecl(Inner, "releaseRef")) {
-                self._inner.releaseRef(session);
-            }
-            session.factory.destroy(self);
+            _ = self;
+            _ = session;
         }
 
         pub fn releaseRef(self: *Self, session: *Session) void {
-            self._rc.release(self, session);
+            // Release the reference to the inner type that we acquired
+            if (@hasDecl(Inner, "releaseRef")) {
+                self.inner.releaseRef(session);
+            }
         }
 
         pub fn acquireRef(self: *Self) void {
-            self._rc.acquire();
+            if (@hasDecl(Inner, "acquireRef")) {
+                self.inner.acquireRef();
+            }
         }
 
         pub fn next(self: *Self, page: *Page) if (R.has_error_return) anyerror!Result else Result {
-            const entry = (if (comptime R.has_error_return) try self._inner.next(page) else self._inner.next(page)) orelse {
+            const entry = (if (comptime R.has_error_return) try self.inner.next(page) else self.inner.next(page)) orelse {
                 return .{ .done = true, .value = null };
             };
 

src/browser/webapi/net/XMLHttpRequest.zig

@@ -90,13 +90,13 @@ const ResponseType = enum {
 pub fn init(page: *Page) !*XMLHttpRequest {
     const arena = try page.getArena(.{ .debug = "XMLHttpRequest" });
     errdefer page.releaseArena(arena);
-    const self = try page._factory.xhrEventTarget(arena, XMLHttpRequest{
+    const xhr = try page._factory.xhrEventTarget(arena, XMLHttpRequest{
         ._page = page,
         ._arena = arena,
         ._proto = undefined,
         ._request_headers = try Headers.init(null, page),
     });
-    return self;
+    return xhr;
 }
 
 pub fn deinit(self: *XMLHttpRequest, session: *Session) void {
@@ -243,10 +243,7 @@ pub fn send(self: *XMLHttpRequest, body_: ?[]const u8) !void {
         try page.headersForRequest(&headers);
     }
 
-    self.acquireRef();
-    self._active_request = true;
-
-    http_client.request(.{
+    try http_client.request(.{
         .ctx = self,
         .url = self._url,
         .method = self._method,
@@ -263,10 +260,9 @@ pub fn send(self: *XMLHttpRequest, body_: ?[]const u8) !void {
         .done_callback = httpDoneCallback,
         .error_callback = httpErrorCallback,
         .shutdown_callback = httpShutdownCallback,
-    }) catch |err| {
-        self.releaseSelfRef();
-        return err;
-    };
+    });
+    self.acquireRef();
+    self._active_request = true;
 }
 
 fn handleBlobUrl(self: *XMLHttpRequest, page: *Page) !void {
@@ -522,7 +518,6 @@ fn httpErrorCallback(ctx: *anyopaque, err: anyerror) void {
 fn httpShutdownCallback(ctx: *anyopaque) void {
     const self: *XMLHttpRequest = @ptrCast(@alignCast(ctx));
     self._transfer = null;
-    self.releaseSelfRef();
 }
 
 pub fn abort(self: *XMLHttpRequest) void {

src/lightpanda.zig

@@ -259,6 +259,9 @@ pub fn RC(comptime T: type) type {
                 return;
             }
             value.deinit(session);
+            if (session.finalizer_callbacks.fetchRemove(@intFromPtr(value))) |kv| {
+                session.releaseArena(kv.value.arena);
+            }
         }
 
         pub fn format(self: @This(), writer: *std.Io.Writer) !void {