From a5d87ab948cb99a2cbb2d60ac77354752dc27234 Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Thu, 5 Jun 2025 23:38:30 +0800 Subject: [PATCH] Reduce duration of the main request We currently keep the main request open during loadHTMLDoc and processHTMLDoc. It _has_ to be open during loadHTMLDoc, since that streams the body. But it does not have to be open during processHTMLDoc, which can be log and itself could make use of that same connection if it was released. Reorganized the navigate flow to limit the scope of the request. Also, just like we track pending_write and pending_read, we now also track pending_connect and only shutdown when all are not pending. --- src/browser/html/window.zig | 4 +- src/browser/page.zig | 85 ++++++++++++++++++++----------------- src/browser/xhr/xhr.zig | 9 +++- src/http/client.zig | 23 ++++++++-- 4 files changed, 75 insertions(+), 46 deletions(-) diff --git a/src/browser/html/window.zig b/src/browser/html/window.zig index d65b47fa..02381971 100644 --- a/src/browser/html/window.zig +++ b/src/browser/html/window.zig @@ -164,7 +164,7 @@ pub const Window = struct { } pub fn _requestAnimationFrame(self: *Window, cbk: Function, page: *Page) !u32 { - return self.createTimeout(cbk, 5, page, .{.animation_frame = true}); + return self.createTimeout(cbk, 5, page, .{ .animation_frame = true }); } pub fn _cancelAnimationFrame(self: *Window, id: u32, page: *Page) !void { @@ -179,7 +179,7 @@ pub const Window = struct { // TODO handle callback arguments. pub fn _setInterval(self: *Window, cbk: Function, delay: ?u32, page: *Page) !u32 { - return self.createTimeout(cbk, delay, page, .{.repeat = true}); + return self.createTimeout(cbk, delay, page, .{ .repeat = true }); } pub fn _clearTimeout(self: *Window, id: u32, page: *Page) !void { diff --git a/src/browser/page.zig b/src/browser/page.zig index 9514ef0d..7044a4d9 100644 --- a/src/browser/page.zig +++ b/src/browser/page.zig @@ -207,58 +207,63 @@ pub const Page = struct { // redirect) self.url = request_url; - // load the data - var request = try self.newHTTPRequest(opts.method, &self.url, .{ .navigation = true }); - defer request.deinit(); - request.body = opts.body; - request.notification = notification; + { + // block exists to limit the lifetime of the request, which holds + // onto a connection + var request = try self.newHTTPRequest(opts.method, &self.url, .{ .navigation = true }); + defer request.deinit(); - notification.dispatch(.page_navigate, &.{ - .opts = opts, - .url = &self.url, - .timestamp = timestamp(), - }); + request.body = opts.body; + request.notification = notification; - var response = try request.sendSync(.{}); + notification.dispatch(.page_navigate, &.{ + .opts = opts, + .url = &self.url, + .timestamp = timestamp(), + }); - // would be different than self.url in the case of a redirect - self.url = try URL.fromURI(arena, request.request_uri); + var response = try request.sendSync(.{}); - const header = response.header; - try session.cookie_jar.populateFromResponse(&self.url.uri, &header); + // would be different than self.url in the case of a redirect + self.url = try URL.fromURI(arena, request.request_uri); - // TODO handle fragment in url. - try self.window.replaceLocation(.{ .url = try self.url.toWebApi(arena) }); + const header = response.header; + try session.cookie_jar.populateFromResponse(&self.url.uri, &header); - const content_type = header.get("content-type"); + // TODO handle fragment in url. + try self.window.replaceLocation(.{ .url = try self.url.toWebApi(arena) }); - const mime: Mime = blk: { - if (content_type) |ct| { - break :blk try Mime.parse(arena, ct); + const content_type = header.get("content-type"); + + const mime: Mime = blk: { + if (content_type) |ct| { + break :blk try Mime.parse(arena, ct); + } + break :blk Mime.sniff(try response.peek()); + } orelse .unknown; + + log.info(.http, "navigation", .{ + .status = header.status, + .content_type = content_type, + .charset = mime.charset, + .url = request_url, + }); + + if (!mime.isHTML()) { + var arr: std.ArrayListUnmanaged(u8) = .{}; + while (try response.next()) |data| { + try arr.appendSlice(arena, try arena.dupe(u8, data)); + } + // save the body into the page. + self.raw_data = arr.items; + return; } - break :blk Mime.sniff(try response.peek()); - } orelse .unknown; - log.info(.http, "navigation", .{ - .status = header.status, - .content_type = content_type, - .charset = mime.charset, - .url = request_url, - }); - - if (mime.isHTML()) { - self.raw_data = null; try self.loadHTMLDoc(&response, mime.charset orelse "utf-8"); - try self.processHTMLDoc(); - } else { - var arr: std.ArrayListUnmanaged(u8) = .{}; - while (try response.next()) |data| { - try arr.appendSlice(arena, try arena.dupe(u8, data)); - } - // save the body into the page. - self.raw_data = arr.items; } + try self.processHTMLDoc(); + notification.dispatch(.page_navigated, &.{ .url = &self.url, .timestamp = timestamp(), diff --git a/src/browser/xhr/xhr.zig b/src/browser/xhr/xhr.zig index 21b12ce7..7c1087a4 100644 --- a/src/browser/xhr/xhr.zig +++ b/src/browser/xhr/xhr.zig @@ -430,7 +430,14 @@ pub const XMLHttpRequest = struct { self.request_body = try self.arena.dupe(u8, b); } - try page.request_factory.initAsync(page.arena, self.method, &self.url.?.uri, self, onHttpRequestReady, self.loop,); + try page.request_factory.initAsync( + page.arena, + self.method, + &self.url.?.uri, + self, + onHttpRequestReady, + self.loop, + ); } fn onHttpRequestReady(ctx: *anyopaque, request: *http.Request) !void { diff --git a/src/http/client.zig b/src/http/client.zig index e336e49e..83536c4e 100644 --- a/src/http/client.zig +++ b/src/http/client.zig @@ -677,6 +677,7 @@ pub const Request = struct { if (self._connection_from_keepalive) { // we're already connected + async_handler.pending_connect = false; return async_handler.conn.connected(); } @@ -915,6 +916,7 @@ fn AsyncHandler(comptime H: type, comptime L: type) type { shutdown: bool = false, pending_write: bool = false, pending_receive: bool = false, + pending_connect: bool = true, const Self = @This(); const SendQueue = std.DoublyLinkedList([]const u8); @@ -939,10 +941,15 @@ fn AsyncHandler(comptime H: type, comptime L: type) type { fn abort(ctx: *anyopaque) void { var self: *Self = @alignCast(@ptrCast(ctx)); self.shutdown = true; + posix.shutdown(self.request._connection.?.socket, .both) catch {}; self.maybeShutdown(); } fn connected(self: *Self, _: *IO.Completion, result: IO.ConnectError!void) void { + self.pending_connect = false; + if (self.shutdown) { + return self.maybeShutdown(); + } result catch |err| return self.handleError("Connection failed", err); self.conn.connected() catch |err| { self.handleError("connected handler error", err); @@ -1109,7 +1116,7 @@ fn AsyncHandler(comptime H: type, comptime L: type) type { fn maybeShutdown(self: *Self) void { std.debug.assert(self.shutdown); - if (self.pending_write or self.pending_receive) { + if (self.pending_write or self.pending_receive or self.pending_connect) { return; } @@ -2538,12 +2545,14 @@ const StatePool = struct { pub fn release(self: *StatePool, state: *State) void { state.reset(); - self.mutex.lock(); var states = self.states; + + self.mutex.lock(); const available = self.available; states[available] = state; self.available = available + 1; self.mutex.unlock(); + self.cond.signal(); } }; @@ -2980,7 +2989,15 @@ test "HttpClient: async connect error" { }; const uri = try Uri.parse("HTTP://127.0.0.1:9920"); - try client.initAsync(testing.arena_allocator, .GET, &uri, &handler, Handler.requestReady, &loop, .{}, ); + try client.initAsync( + testing.arena_allocator, + .GET, + &uri, + &handler, + Handler.requestReady, + &loop, + .{}, + ); try loop.io.run_for_ns(std.time.ns_per_ms); try reset.timedWait(std.time.ns_per_s);