Add finalizer to Response and use an pooled arena

Unlike XHR, Response is a bit more complicated as it can exist in Zig code
without ever being given to v8. So we need to track this handoff to know who is
responsible for freeing it (zig code, on error/shutdown) or v8 code after
promise resolution.

This also cleansup a bad merge for the XHR finalizer and adds cleaning up the
`XMLHttpRequestEventTarget` callbacks.
This commit is contained in:
Karl Seguin
2026-01-26 19:18:32 +08:00
parent 1f14eb62d4
commit a11ae912b4
8 changed files with 147 additions and 57 deletions

View File

@@ -37,6 +37,8 @@ const XMLHttpRequestEventTarget = @import("webapi/net/XMLHttpRequestEventTarget.
const Blob = @import("webapi/Blob.zig");
const AbstractRange = @import("webapi/AbstractRange.zig");
const Allocator = std.mem.Allocator;
const IS_DEBUG = builtin.mode == .Debug;
const assert = std.debug.assert;
@@ -344,9 +346,7 @@ pub fn svgElement(self: *Factory, tag_name: []const u8, child: anytype) !*@TypeO
return chain.get(4);
}
pub fn xhrEventTarget(self: *Factory, child: anytype) !*@TypeOf(child) {
const allocator = self._slab.allocator();
pub fn xhrEventTarget(_: *const Factory, allocator: Allocator, child: anytype) !*@TypeOf(child) {
return try AutoPrototypeChain(
&.{ EventTarget, XMLHttpRequestEventTarget, @TypeOf(child) },
).create(allocator, child);

View File

@@ -754,7 +754,7 @@ pub const Script = struct {
return;
}
switch (self.mode) {
switch (self.mode) {
.import_async => |ia| ia.callback(ia.data, error.FailedToLoad),
.import => {
const entry = manager.imported_modules.getPtr(self.url).?;

View File

@@ -92,7 +92,7 @@ pub fn Builder(comptime T: type) type {
return entries;
}
pub fn finalizer(comptime func: *const fn (self: *T, comptime shutdown: bool) void) Finalizer {
pub fn finalizer(comptime func: *const fn (self: *T, shutdown: bool) void) Finalizer {
return .{
.from_zig = struct {
fn wrap(ptr: *anyopaque) void {

View File

@@ -37,22 +37,26 @@ _url: []const u8,
_buf: std.ArrayList(u8),
_response: *Response,
_resolver: js.PromiseResolver.Global,
_owns_response: bool,
pub const Input = Request.Input;
pub const InitOpts = Request.InitOpts;
pub fn init(input: Input, options: ?InitOpts, page: *Page) !js.Promise {
const request = try Request.init(input, options, page);
const response = try Response.init(null, .{ .status = 0 }, page);
errdefer response.deinit(true);
const resolver = page.js.local.?.createPromiseResolver();
const fetch = try page.arena.create(Fetch);
const fetch = try response._arena.create(Fetch);
fetch.* = .{
._page = page,
._buf = .empty,
._url = try page.arena.dupe(u8, request._url),
._url = try response._arena.dupe(u8, request._url),
._resolver = try resolver.persist(),
._response = try Response.init(null, .{ .status = 0 }, page),
._response = response,
._owns_response = true,
};
const http_client = page._session.browser.http_client;
@@ -74,26 +78,30 @@ pub fn init(input: Input, options: ?InitOpts, page: *Page) !js.Promise {
.headers = headers,
.resource_type = .fetch,
.cookie_jar = &page._session.cookie_jar,
.start_callback = httpStartCallback,
.header_callback = httpHeaderDoneCallback,
.data_callback = httpDataCallback,
.done_callback = httpDoneCallback,
.error_callback = httpErrorCallback,
.shutdown_callback = httpShutdownCallback,
});
return resolver.promise();
}
pub fn deinit(self: *Fetch) void {
if (self.transfer) |transfer| {
transfer.abort();
self.transfer = null;
fn httpStartCallback(transfer: *Http.Transfer) !void {
const self: *Fetch = @ptrCast(@alignCast(transfer.ctx));
if (comptime IS_DEBUG) {
log.debug(.http, "request start", .{ .url = self._url, .source = "fetch" });
}
self._response._transfer = transfer;
}
fn httpHeaderDoneCallback(transfer: *Http.Transfer) !bool {
const self: *Fetch = @ptrCast(@alignCast(transfer.ctx));
const arena = self._response._arena;
if (transfer.getContentLength()) |cl| {
try self._buf.ensureTotalCapacity(self._page.arena, cl);
try self._buf.ensureTotalCapacity(arena, cl);
}
const res = self._response;
@@ -108,12 +116,12 @@ fn httpHeaderDoneCallback(transfer: *Http.Transfer) !bool {
}
res._status = header.status;
res._url = try self._page.arena.dupeZ(u8, std.mem.span(header.url));
res._url = try arena.dupeZ(u8, std.mem.span(header.url));
res._is_redirected = header.redirect_count > 0;
// Determine response type based on origin comparison
const page_origin = URL.getOrigin(self._page.call_arena, self._page.url) catch null;
const response_origin = URL.getOrigin(self._page.call_arena, res._url) catch null;
const page_origin = URL.getOrigin(arena, self._page.url) catch null;
const response_origin = URL.getOrigin(arena, res._url) catch null;
if (page_origin) |po| {
if (response_origin) |ro| {
@@ -139,17 +147,19 @@ fn httpHeaderDoneCallback(transfer: *Http.Transfer) !bool {
fn httpDataCallback(transfer: *Http.Transfer, data: []const u8) !void {
const self: *Fetch = @ptrCast(@alignCast(transfer.ctx));
try self._buf.appendSlice(self._page.arena, data);
try self._buf.appendSlice(self._response._arena, data);
}
fn httpDoneCallback(ctx: *anyopaque) !void {
const self: *Fetch = @ptrCast(@alignCast(ctx));
self._response._body = self._buf.items;
var response = self._response;
response._transfer = null;
response._body = self._buf.items;
log.info(.http, "request complete", .{
.source = "fetch",
.url = self._url,
.status = self._response._status,
.status = response._status,
.len = self._buf.items.len,
});
@@ -157,12 +167,23 @@ fn httpDoneCallback(ctx: *anyopaque) !void {
self._page.js.localScope(&ls);
defer ls.deinit();
return ls.toLocal(self._resolver).resolve("fetch done", self._response);
const js_val = try ls.local.zigValueToJs(self._response, .{});
self._owns_response = false;
return ls.toLocal(self._resolver).resolve("fetch done", js_val);
}
fn httpErrorCallback(ctx: *anyopaque, err: anyerror) void {
const self: *Fetch = @ptrCast(@alignCast(ctx));
self._response._type = .@"error"; // Set type to error for network failures
var response = self._response;
response._transfer = null;
// the response is only passed on v8 on success, if we're here, it's safe to
// clear this. (defer since `self is in the response's arena).
defer if (self._owns_response) {
response.deinit(err == error.Abort);
self._owns_response = false;
};
var ls: js.Local.Scope = undefined;
self._page.js.localScope(&ls);
@@ -171,6 +192,21 @@ fn httpErrorCallback(ctx: *anyopaque, err: anyerror) void {
ls.toLocal(self._resolver).reject("fetch error", @errorName(err));
}
fn httpShutdownCallback(ctx: *anyopaque) void {
const self: *Fetch = @ptrCast(@alignCast(ctx));
if (comptime IS_DEBUG) {
// should always be true
std.debug.assert(self._owns_response);
}
if (self._owns_response) {
var response = self._response;
response._transfer = null;
response.deinit(true);
self._owns_response = false;
}
}
const testing = @import("../../../testing.zig");
test "WebApi: fetch" {
try testing.htmlRunner("net/fetch.html", .{});

View File

@@ -18,10 +18,12 @@
const std = @import("std");
const js = @import("../../js/js.zig");
const Http = @import("../../../http/Http.zig");
const Page = @import("../../Page.zig");
const Headers = @import("Headers.zig");
const ReadableStream = @import("../streams/ReadableStream.zig");
const Allocator = std.mem.Allocator;
const Response = @This();
@@ -34,6 +36,7 @@ pub const Type = enum {
opaqueredirect,
};
_page: *Page,
_status: u16,
_arena: Allocator,
_headers: *Headers,
@@ -42,6 +45,7 @@ _type: Type,
_status_text: []const u8,
_url: [:0]const u8,
_is_redirected: bool,
_transfer: ?*Http.Transfer = null,
const InitOpts = struct {
status: u16 = 200,
@@ -50,14 +54,19 @@ const InitOpts = struct {
};
pub fn init(body_: ?[]const u8, opts_: ?InitOpts, page: *Page) !*Response {
const arena = try page.getArena(.{ .debug = "Response" });
errdefer page.releaseArena(arena);
const opts = opts_ orelse InitOpts{};
// Store empty string as empty string, not null
const body = if (body_) |b| try page.arena.dupe(u8, b) else null;
const status_text = if (opts.statusText) |st| try page.dupeString(st) else "";
const body = if (body_) |b| try arena.dupe(u8, b) else null;
const status_text = if (opts.statusText) |st| try arena.dupe(u8, st) else "";
return page._factory.create(Response{
._arena = page.arena,
const self = try arena.create(Response);
self.* = .{
._page = page,
._arena = arena,
._status = opts.status,
._status_text = status_text,
._url = "",
@@ -65,7 +74,20 @@ pub fn init(body_: ?[]const u8, opts_: ?InitOpts, page: *Page) !*Response {
._type = .basic,
._is_redirected = false,
._headers = try Headers.init(opts.headers, page),
});
};
return self;
}
pub fn deinit(self: *Response, shutdown: bool) void {
if (self._transfer) |transfer| {
if (shutdown) {
transfer.terminate();
} else {
transfer.abort(error.Abort);
}
self._transfer = null;
}
self._page.releaseArena(self._arena);
}
pub fn getStatus(self: *const Response) u16 {
@@ -134,6 +156,8 @@ pub const JsApi = struct {
pub const name = "Response";
pub const prototype_chain = bridge.prototypeChain();
pub var class_id: bridge.ClassId = undefined;
pub const weak = true;
pub const finalizer = bridge.finalizer(Response.deinit);
};
pub const constructor = bridge.constructor(Response.init, .{});

View File

@@ -81,7 +81,7 @@ const ResponseType = enum {
pub fn init(page: *Page) !*XMLHttpRequest {
const arena = try page.getArena(.{ .debug = "XMLHttpRequest" });
errdefer page.releaseArena(arena);
return page._factory.xhrEventTarget(XMLHttpRequest{
return page._factory.xhrEventTarget(arena, XMLHttpRequest{
._page = page,
._arena = arena,
._proto = undefined,
@@ -89,7 +89,7 @@ pub fn init(page: *Page) !*XMLHttpRequest {
});
}
pub fn deinit(self: *XMLHttpRequest, comptime shutdown: bool) void {
pub fn deinit(self: *XMLHttpRequest, shutdown: bool) void {
if (self._transfer) |transfer| {
if (shutdown) {
transfer.terminate();
@@ -103,8 +103,33 @@ pub fn deinit(self: *XMLHttpRequest, comptime shutdown: bool) void {
if (self._on_ready_state_change) |func| {
page.js.release(func);
}
{
const proto = self._proto;
if (proto._on_abort) |func| {
page.js.release(func);
}
if (proto._on_error) |func| {
page.js.release(func);
}
if (proto._on_load) |func| {
page.js.release(func);
}
if (proto._on_load_end) |func| {
page.js.release(func);
}
if (proto._on_load_start) |func| {
page.js.release(func);
}
if (proto._on_progress) |func| {
page.js.release(func);
}
if (proto._on_timeout) |func| {
page.js.release(func);
}
}
page.releaseArena(self._arena);
page._factory.destroy(self);
}
fn asEventTarget(self: *XMLHttpRequest) *EventTarget {
@@ -161,7 +186,6 @@ 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) {
@@ -319,7 +343,11 @@ fn httpHeaderDoneCallback(transfer: *Http.Transfer) !bool {
if (header.contentType()) |ct| {
self._response_mime = Mime.parse(ct) catch |e| {
log.info(.http, "invalid content type", .{.content_Type = ct, .err = e, .url = self._url,});
log.info(.http, "invalid content type", .{
.content_Type = ct,
.err = e,
.url = self._url,
});
return false;
};
}
@@ -399,8 +427,6 @@ fn httpDoneCallback(ctx: *anyopaque) !void {
.total = loaded,
.loaded = loaded,
}, local, page);
page.js.weakRef(self);
}
fn httpErrorCallback(ctx: *anyopaque, err: anyerror) void {
@@ -408,7 +434,6 @@ 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 {
@@ -417,7 +442,6 @@ 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 {

View File

@@ -26,13 +26,13 @@ const XMLHttpRequestEventTarget = @This();
_type: Type,
_proto: *EventTarget,
_on_abort: ?js.Function.Global = null,
_on_error: ?js.Function.Global = null,
_on_load: ?js.Function.Global = null,
_on_load_end: ?js.Function.Global = null,
_on_load_start: ?js.Function.Global = null,
_on_progress: ?js.Function.Global = null,
_on_timeout: ?js.Function.Global = null,
_on_abort: ?js.Function.Temp = null,
_on_error: ?js.Function.Temp = null,
_on_load: ?js.Function.Temp = null,
_on_load_end: ?js.Function.Temp = null,
_on_load_start: ?js.Function.Temp = null,
_on_progress: ?js.Function.Temp = null,
_on_timeout: ?js.Function.Temp = null,
pub const Type = union(enum) {
request: *@import("XMLHttpRequest.zig"),
@@ -71,85 +71,85 @@ pub fn dispatch(self: *XMLHttpRequestEventTarget, comptime event_type: DispatchT
);
}
pub fn getOnAbort(self: *const XMLHttpRequestEventTarget) ?js.Function.Global {
pub fn getOnAbort(self: *const XMLHttpRequestEventTarget) ?js.Function.Temp {
return self._on_abort;
}
pub fn setOnAbort(self: *XMLHttpRequestEventTarget, cb_: ?js.Function) !void {
if (cb_) |cb| {
self._on_abort = try cb.persistWithThis(self);
self._on_abort = try cb.tempWithThis(self);
} else {
self._on_abort = null;
}
}
pub fn getOnError(self: *const XMLHttpRequestEventTarget) ?js.Function.Global {
pub fn getOnError(self: *const XMLHttpRequestEventTarget) ?js.Function.Temp {
return self._on_error;
}
pub fn setOnError(self: *XMLHttpRequestEventTarget, cb_: ?js.Function) !void {
if (cb_) |cb| {
self._on_error = try cb.persistWithThis(self);
self._on_error = try cb.tempWithThis(self);
} else {
self._on_error = null;
}
}
pub fn getOnLoad(self: *const XMLHttpRequestEventTarget) ?js.Function.Global {
pub fn getOnLoad(self: *const XMLHttpRequestEventTarget) ?js.Function.Temp {
return self._on_load;
}
pub fn setOnLoad(self: *XMLHttpRequestEventTarget, cb_: ?js.Function) !void {
if (cb_) |cb| {
self._on_load = try cb.persistWithThis(self);
self._on_load = try cb.tempWithThis(self);
} else {
self._on_load = null;
}
}
pub fn getOnLoadEnd(self: *const XMLHttpRequestEventTarget) ?js.Function.Global {
pub fn getOnLoadEnd(self: *const XMLHttpRequestEventTarget) ?js.Function.Temp {
return self._on_load_end;
}
pub fn setOnLoadEnd(self: *XMLHttpRequestEventTarget, cb_: ?js.Function) !void {
if (cb_) |cb| {
self._on_load_end = try cb.persistWithThis(self);
self._on_load_end = try cb.tempWithThis(self);
} else {
self._on_load_end = null;
}
}
pub fn getOnLoadStart(self: *const XMLHttpRequestEventTarget) ?js.Function.Global {
pub fn getOnLoadStart(self: *const XMLHttpRequestEventTarget) ?js.Function.Temp {
return self._on_load_start;
}
pub fn setOnLoadStart(self: *XMLHttpRequestEventTarget, cb_: ?js.Function) !void {
if (cb_) |cb| {
self._on_load_start = try cb.persistWithThis(self);
self._on_load_start = try cb.tempWithThis(self);
} else {
self._on_load_start = null;
}
}
pub fn getOnProgress(self: *const XMLHttpRequestEventTarget) ?js.Function.Global {
pub fn getOnProgress(self: *const XMLHttpRequestEventTarget) ?js.Function.Temp {
return self._on_progress;
}
pub fn setOnProgress(self: *XMLHttpRequestEventTarget, cb_: ?js.Function) !void {
if (cb_) |cb| {
self._on_progress = try cb.persistWithThis(self);
self._on_progress = try cb.tempWithThis(self);
} else {
self._on_progress = null;
}
}
pub fn getOnTimeout(self: *const XMLHttpRequestEventTarget) ?js.Function.Global {
pub fn getOnTimeout(self: *const XMLHttpRequestEventTarget) ?js.Function.Temp {
return self._on_timeout;
}
pub fn setOnTimeout(self: *XMLHttpRequestEventTarget, cb_: ?js.Function) !void {
if (cb_) |cb| {
self._on_timeout = try cb.persistWithThis(self);
self._on_timeout = try cb.tempWithThis(self);
} else {
self._on_timeout = null;
}

View File

@@ -186,7 +186,7 @@ pub fn abort(self: *Client) void {
while (n) |node| {
n = node.next;
const transfer: *Transfer = @fieldParentPtr("_node", node);
self.transfer_pool.destroy(transfer);
transfer.kill();
}
self.queue = .{};
@@ -392,6 +392,8 @@ fn requestFailed(self: *Client, transfer: *Transfer, err: anyerror, comptime exe
if (execute_callback) {
transfer.req.error_callback(transfer.ctx, err);
} else if (transfer.req.shutdown_callback) |cb| {
cb(transfer.ctx);
}
}
@@ -781,6 +783,7 @@ pub const Request = struct {
data_callback: *const fn (transfer: *Transfer, data: []const u8) anyerror!void,
done_callback: *const fn (ctx: *anyopaque) anyerror!void,
error_callback: *const fn (ctx: *anyopaque, err: anyerror) void,
shutdown_callback: ?*const fn (ctx: *anyopaque) void = null,
const ResourceType = enum {
document,
@@ -995,6 +998,9 @@ pub const Transfer = struct {
if (self._handle != null) {
self.client.endTransfer(self);
}
if (self.req.shutdown_callback) |cb| {
cb(self.ctx);
}
self.deinit();
}