From b63d93e3255274a3cc4b9fe9e553f1e5cf20a53b Mon Sep 17 00:00:00 2001
From: Karl Seguin <k@openmymind.io>
Date: Mon, 19 Jan 2026 19:09:20 +0800
Subject: [PATCH] Add XHR finalizer and ArenaPool

Any object we return from Zig to V8 becomes a v8::Global that we track in our
`ctx.identity_map`. V8 will not free such objects. On the flip side, on its own,
our Zig code never knows if the underlying v8::Object of a global can still be
used from JS. Imagine an XHR request where we fire the last readyStateChange
event..we might think we no longer need that XHR instance, but nothing stops
the JavaScript code from holding a reference to it and calling a property on it,
e.g. `xhr.status`.

What we can do is tell v8 that we're done with the global and register a callback.
We make our reference to the global weak. When v8 determines that this object
cannot be reached from JavaScript, it _may_ call our registered callback. We can
then clean things up on our side and free the global (we actually _have_ to
free the global).

v8 makes no guarantee that our callback will ever be called, so we need to track
these finalizable objects and free them ourselves on context shutdown. Furthermore
there appears to be some possible timing issues, especially during context shutdown,
so we need to be defensive and make sure we don't double-free (we can use the
existing identity_map for this).

An type like XMLHttpRequest can be re-used. After a request succeeds or fails,
it can be re-opened and a new request sent. So we also need a way to revert a
"weak" reference back into a "strong" reference. These are simple v8 calls on
the v8::Global, but it highlights how sensitive all this is. We need to mark
it as weak when we're 100% sure we're done with it, and we need to switch it to
strong under any circumstances where we might need it again on our side.

Finally, none of this makes sense if there isn't something to free. Of course,
the finalizer lets us release the v8::Global, and we can free the memory for the
object itself (i.e. the `*XMLHttpRequest`). This PR also adds an ArenaPool. This
allows the XMLHTTPRequest to be self-contained and not need the `page.arena`.
On init, the `XMLHTTPRequest` acquires an arena from the pool. On finalization
it releases it back to the pool. So we now have:

- page.call_arena: short, guaranteed for 1 v8 -> zig -> v8 flow
- page.arena long: lives for the duration of the entire page
- page.arena_pool: ideally lives for as long as needed by its instance (but no
guarantees from v8 about this, or the script might leak a lot of global, so worst
case, same as page.arena)
---
 src/browser/js/Local.zig                  | 5 ++++-
 src/browser/webapi/net/XMLHttpRequest.zig | 9 +++++++--
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/src/browser/js/Local.zig b/src/browser/js/Local.zig
index 4affc114..312d7127 100644
--- a/src/browser/js/Local.zig
+++ b/src/browser/js/Local.zig
@@ -207,7 +207,10 @@ pub fn mapZigInstanceToJs(self: *const Local, js_obj_handle: ?*const v8.Object,
                 }
 
                 try ctx.finalizer_callbacks.put(ctx.arena, @intFromPtr(resolved.ptr), .init(value));
-                if (@hasDecl(JsApi.Meta, "finalizer")) {
+                if (@hasDecl(JsApi.Meta, "weak")) {
+                    if (comptime IS_DEBUG) {
+                        std.debug.assert(JsApi.Meta.weak == true);
+                    }
                     v8.v8__Global__SetWeakFinalizer(gop.value_ptr, resolved.ptr, JsApi.Meta.finalizer.from_v8, v8.kParameter);
                 }
             }
diff --git a/src/browser/webapi/net/XMLHttpRequest.zig b/src/browser/webapi/net/XMLHttpRequest.zig
index 85b961c5..38ef0564 100644
--- a/src/browser/webapi/net/XMLHttpRequest.zig
+++ b/src/browser/webapi/net/XMLHttpRequest.zig
@@ -81,8 +81,7 @@ const ResponseType = enum {
 pub fn init(page: *Page) !*XMLHttpRequest {
     const arena = try page.getArena(.{.debug = "XMLHttpRequest"});
     errdefer page.releaseArena(arena);
-
-    return try page._factory.xhrEventTarget(XMLHttpRequest{
+    return page._factory.xhrEventTarget(XMLHttpRequest{
         ._page = page,
         ._arena = arena,
         ._proto = undefined,
@@ -157,6 +156,7 @@ pub fn send(self: *XMLHttpRequest, body_: ?[]const u8) !void {
     if (self._ready_state != .opened) {
         return error.InvalidStateError;
     }
+    self._page.js.strongRef(self);
 
     if (body_) |b| {
         if (self._method != .GET and self._method != .HEAD) {
@@ -394,6 +394,8 @@ fn httpDoneCallback(ctx: *anyopaque) !void {
         .total = loaded,
         .loaded = loaded,
     }, local, page);
+
+    page.js.weakRef(self);
 }
 
 fn httpErrorCallback(ctx: *anyopaque, err: anyerror) void {
@@ -401,6 +403,7 @@ fn httpErrorCallback(ctx: *anyopaque, err: anyerror) void {
     // http client will close it after an error, it isn't safe to keep around
     self._transfer = null;
     self.handleError(err);
+    self._page.js.weakRef(self);
 }
 
 pub fn abort(self: *XMLHttpRequest) void {
@@ -409,6 +412,7 @@ pub fn abort(self: *XMLHttpRequest) void {
         transfer.abort(error.Abort);
         self._transfer = null;
     }
+    self._page.js.weakRef(self);
 }
 
 fn handleError(self: *XMLHttpRequest, err: anyerror) void {
@@ -486,6 +490,7 @@ pub const JsApi = struct {
         pub const name = "XMLHttpRequest";
         pub const prototype_chain = bridge.prototypeChain();
         pub var class_id: bridge.ClassId = undefined;
+        pub const weak = true;
         pub const finalizer = bridge.finalizer(XMLHttpRequest.deinit);
     };