From 52634ddeb3d25d6cae4f8030bbbf33fa994a0a36 Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Tue, 20 May 2025 18:56:22 +0800 Subject: [PATCH 1/3] Allow webapis to register a destructor to do cleanup on scope (page) end Add destructor to XHR to abort any inflight requests. --- src/browser/xhr/xhr.zig | 13 +++++------ src/http/client.zig | 7 ++++++ src/runtime/js.zig | 49 ++++++++++++++++++++++++++++++++++++----- 3 files changed, 57 insertions(+), 12 deletions(-) diff --git a/src/browser/xhr/xhr.zig b/src/browser/xhr/xhr.zig index 4ea517cd..fc9a562a 100644 --- a/src/browser/xhr/xhr.zig +++ b/src/browser/xhr/xhr.zig @@ -257,6 +257,11 @@ pub const XMLHttpRequest = struct { }; } + pub fn destructor(self: *XMLHttpRequest, _: anytype) void { + const request = &(self.request orelse return); + request.abort(); + } + pub fn reset(self: *XMLHttpRequest) void { self.url = null; @@ -278,13 +283,6 @@ pub const XMLHttpRequest = struct { self.priv_state = .new; } - pub fn deinit(self: *XMLHttpRequest, alloc: Allocator) void { - if (self.response_obj) |v| { - v.deinit(); - } - self.proto.deinit(alloc); - } - pub fn get_readyState(self: *XMLHttpRequest) u16 { return @intFromEnum(self.state); } @@ -518,6 +516,7 @@ pub const XMLHttpRequest = struct { return; } + self.request = null; self.state = .done; self.send_flag = false; self.dispatchEvt("readystatechange"); diff --git a/src/http/client.zig b/src/http/client.zig index a6fa1615..6b0ae83b 100644 --- a/src/http/client.zig +++ b/src/http/client.zig @@ -261,6 +261,13 @@ pub const Request = struct { self._client.state_pool.release(self._state); } + pub fn abort(self: *Request) void { + const connection = self._connection orelse return; + self.destroyConnection(connection); + self._connection = null; + self.deinit(); + } + const DecomposedURL = struct { secure: bool, connect_port: u16, diff --git a/src/runtime/js.zig b/src/runtime/js.zig index 2de606a4..673fbfa3 100644 --- a/src/runtime/js.zig +++ b/src/runtime/js.zig @@ -502,14 +502,14 @@ pub fn Env(comptime State: type, comptime WebApis: type) type { // Callbacks are PesistendObjects. When the scope ends, we need // to free every callback we created. - callbacks: std.ArrayListUnmanaged(v8.Persistent(v8.Function)) = .{}, + callbacks: std.ArrayListUnmanaged(v8.Persistent(v8.Function)) = .empty, // Serves two purposes. Like `callbacks` above, this is used to free // every PeristentObjet we've created during the lifetime of the scope. // More importantly, it serves as an identity map - for a given Zig // instance, we map it to the same PersistentObject. // The key is the @intFromPtr of the Zig value - identity_map: std.AutoHashMapUnmanaged(usize, PersistentObject) = .{}, + identity_map: std.AutoHashMapUnmanaged(usize, PersistentObject) = .empty, // Similar to the identity map, but used much less frequently. Some // web APIs have to manage opaque values. Ideally, they use an @@ -517,7 +517,7 @@ pub fn Env(comptime State: type, comptime WebApis: type) type { // current call. They can call .persist() on their JsObject to get // a `*PersistentObject()`. We need to track these to free them. // The key is the @intFromPtr of the v8.Object.handle. - js_object_map: std.AutoHashMapUnmanaged(usize, PersistentObject) = .{}, + js_object_map: std.AutoHashMapUnmanaged(usize, PersistentObject) = .empty, // When we need to load a resource (i.e. an external script), we call // this function to get the source. This is always a reference to the @@ -525,8 +525,11 @@ pub fn Env(comptime State: type, comptime WebApis: type) type { // since this js module is decoupled from the browser implementation. module_loader: ModuleLoader, + // Some Zig types have code to execute to cleanup + destructor_callbacks: std.ArrayListUnmanaged(DestructorCallback) = .empty, + // Some Zig types have code to execute when the call scope ends - call_scope_end_callbacks: std.ArrayListUnmanaged(CallScopeEndCallback) = .{}, + call_scope_end_callbacks: std.ArrayListUnmanaged(CallScopeEndCallback) = .empty, const ModuleLoader = struct { ptr: *anyopaque, @@ -536,6 +539,10 @@ pub fn Env(comptime State: type, comptime WebApis: type) type { // no init, started with executor.startScope() fn deinit(self: *Scope) void { + for (self.destructor_callbacks.items) |cb| { + cb.destructor(self); + } + { var it = self.identity_map.valueIterator(); while (it.next()) |p| { @@ -694,6 +701,10 @@ pub fn Env(comptime State: type, comptime WebApis: type) type { return gop.value_ptr.*; } + if (comptime @hasDecl(ptr.child, "destructor")) { + try self.destructor_callbacks.append(scope_arena, DestructorCallback.init(value)); + } + if (comptime @hasDecl(ptr.child, "jsCallScopeEnd")) { try self.call_scope_end_callbacks.append(scope_arena, CallScopeEndCallback.init(value)); } @@ -1672,7 +1683,35 @@ pub fn Env(comptime State: type, comptime WebApis: type) type { } } - // An interface for types that want to their jsScopeEnd function to be + // An interface for types that want to have their jsDeinit function to be + // called when the call scope ends + const DestructorCallback = struct { + ptr: *anyopaque, + destructorFn: *const fn (ptr: *anyopaque, scope: *Scope) void, + + fn init(ptr: anytype) DestructorCallback { + const T = @TypeOf(ptr); + const ptr_info = @typeInfo(T); + + const gen = struct { + pub fn destructor(pointer: *anyopaque, scope: *Scope) void { + const self: T = @ptrCast(@alignCast(pointer)); + return ptr_info.pointer.child.destructor(self, scope); + } + }; + + return .{ + .ptr = ptr, + .destructorFn = gen.destructor, + }; + } + + pub fn destructor(self: DestructorCallback, scope: *Scope) void { + self.destructorFn(self.ptr, scope); + } + }; + + // An interface for types that want to have their jsScopeEnd function be // called when the call scope ends const CallScopeEndCallback = struct { ptr: *anyopaque, From f42bd02cfc26c89299dcf55c43c7d1a8bd8432d2 Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Tue, 20 May 2025 19:22:43 +0800 Subject: [PATCH 2/3] Don't crash on success Keep request around, as the http/client needs it for cleanup. Calling abort on an already deinit'd request is safe. --- src/browser/xhr/xhr.zig | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/browser/xhr/xhr.zig b/src/browser/xhr/xhr.zig index fc9a562a..be0fcfd6 100644 --- a/src/browser/xhr/xhr.zig +++ b/src/browser/xhr/xhr.zig @@ -258,8 +258,7 @@ pub const XMLHttpRequest = struct { } pub fn destructor(self: *XMLHttpRequest, _: anytype) void { - const request = &(self.request orelse return); - request.abort(); + self._abort(); } pub fn reset(self: *XMLHttpRequest) void { @@ -516,7 +515,6 @@ pub const XMLHttpRequest = struct { return; } - self.request = null; self.state = .done; self.send_flag = false; self.dispatchEvt("readystatechange"); @@ -541,6 +539,10 @@ pub const XMLHttpRequest = struct { } pub fn _abort(self: *XMLHttpRequest) void { + const request = &(self.request orelse return); + // safe to call even if the request is complete + request.abort(); + self.onErr(DOMError.Abort); } From ce832a8063733b4df916ad1b7602a73051531966 Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Wed, 21 May 2025 11:38:26 +0800 Subject: [PATCH 3/3] Rollback XHR/HTTP.client change This PR will be only for having the destructor hook. XHR/http.client changes to leverage this will be done in a subsequent PR. --- src/browser/dom/mutation_observer.zig | 2 +- src/browser/xhr/xhr.zig | 8 ------- src/http/client.zig | 7 ------ src/runtime/js.zig | 33 ++++++++++++++++----------- 4 files changed, 21 insertions(+), 29 deletions(-) diff --git a/src/browser/dom/mutation_observer.zig b/src/browser/dom/mutation_observer.zig index 203a05a0..71459e87 100644 --- a/src/browser/dom/mutation_observer.zig +++ b/src/browser/dom/mutation_observer.zig @@ -103,7 +103,7 @@ pub const MutationObserver = struct { } } - pub fn jsCallScopeEnd(self: *MutationObserver, _: anytype) void { + pub fn jsCallScopeEnd(self: *MutationObserver) void { const record = self.observed.items; if (record.len == 0) { return; diff --git a/src/browser/xhr/xhr.zig b/src/browser/xhr/xhr.zig index be0fcfd6..67dfbca1 100644 --- a/src/browser/xhr/xhr.zig +++ b/src/browser/xhr/xhr.zig @@ -257,10 +257,6 @@ pub const XMLHttpRequest = struct { }; } - pub fn destructor(self: *XMLHttpRequest, _: anytype) void { - self._abort(); - } - pub fn reset(self: *XMLHttpRequest) void { self.url = null; @@ -539,10 +535,6 @@ pub const XMLHttpRequest = struct { } pub fn _abort(self: *XMLHttpRequest) void { - const request = &(self.request orelse return); - // safe to call even if the request is complete - request.abort(); - self.onErr(DOMError.Abort); } diff --git a/src/http/client.zig b/src/http/client.zig index 6b0ae83b..a6fa1615 100644 --- a/src/http/client.zig +++ b/src/http/client.zig @@ -261,13 +261,6 @@ pub const Request = struct { self._client.state_pool.release(self._state); } - pub fn abort(self: *Request) void { - const connection = self._connection orelse return; - self.destroyConnection(connection); - self._connection = null; - self.deinit(); - } - const DecomposedURL = struct { secure: bool, connect_port: u16, diff --git a/src/runtime/js.zig b/src/runtime/js.zig index 673fbfa3..1a296ca8 100644 --- a/src/runtime/js.zig +++ b/src/runtime/js.zig @@ -539,8 +539,15 @@ pub fn Env(comptime State: type, comptime WebApis: type) type { // no init, started with executor.startScope() fn deinit(self: *Scope) void { - for (self.destructor_callbacks.items) |cb| { - cb.destructor(self); + { + // reverse order, as this has more chance of respecting any + // dependencies objects might have with each other. + const items = self.destructor_callbacks.items; + var i = items.len; + while (i > 0) { + i -= 1; + items[i].destructor(); + } } { @@ -1687,16 +1694,16 @@ pub fn Env(comptime State: type, comptime WebApis: type) type { // called when the call scope ends const DestructorCallback = struct { ptr: *anyopaque, - destructorFn: *const fn (ptr: *anyopaque, scope: *Scope) void, + destructorFn: *const fn (ptr: *anyopaque) void, fn init(ptr: anytype) DestructorCallback { const T = @TypeOf(ptr); const ptr_info = @typeInfo(T); const gen = struct { - pub fn destructor(pointer: *anyopaque, scope: *Scope) void { + pub fn destructor(pointer: *anyopaque) void { const self: T = @ptrCast(@alignCast(pointer)); - return ptr_info.pointer.child.destructor(self, scope); + return ptr_info.pointer.child.destructor(self); } }; @@ -1706,8 +1713,8 @@ pub fn Env(comptime State: type, comptime WebApis: type) type { }; } - pub fn destructor(self: DestructorCallback, scope: *Scope) void { - self.destructorFn(self.ptr, scope); + pub fn destructor(self: DestructorCallback) void { + self.destructorFn(self.ptr); } }; @@ -1715,16 +1722,16 @@ pub fn Env(comptime State: type, comptime WebApis: type) type { // called when the call scope ends const CallScopeEndCallback = struct { ptr: *anyopaque, - callScopeEndFn: *const fn (ptr: *anyopaque, scope: *Scope) void, + callScopeEndFn: *const fn (ptr: *anyopaque) void, fn init(ptr: anytype) CallScopeEndCallback { const T = @TypeOf(ptr); const ptr_info = @typeInfo(T); const gen = struct { - pub fn callScopeEnd(pointer: *anyopaque, scope: *Scope) void { + pub fn callScopeEnd(pointer: *anyopaque) void { const self: T = @ptrCast(@alignCast(pointer)); - return ptr_info.pointer.child.jsCallScopeEnd(self, scope); + return ptr_info.pointer.child.jsCallScopeEnd(self); } }; @@ -1734,8 +1741,8 @@ pub fn Env(comptime State: type, comptime WebApis: type) type { }; } - pub fn callScopeEnd(self: CallScopeEndCallback, scope: *Scope) void { - self.callScopeEndFn(self.ptr, scope); + pub fn callScopeEnd(self: CallScopeEndCallback) void { + self.callScopeEndFn(self.ptr); } }; }; @@ -1815,7 +1822,7 @@ fn Caller(comptime E: type, comptime State: type) type { // when a top-level (call_depth == 0) function ends. if (call_depth == 0) { for (scope.call_scope_end_callbacks.items) |cb| { - cb.callScopeEnd(scope); + cb.callScopeEnd(); } const arena: *ArenaAllocator = @alignCast(@ptrCast(scope.call_arena.ptr));