diff --git a/src/browser/ScriptManager.zig b/src/browser/ScriptManager.zig index 6f55d12a..344d6232 100644 --- a/src/browser/ScriptManager.zig +++ b/src/browser/ScriptManager.zig @@ -684,10 +684,6 @@ pub const Script = struct { }); } - // If this isn't true, then we'll likely leak memory. If you don't - // set `CURLOPT_SUPPRESS_CONNECT_HEADERS` and CONNECT to a proxy, this - // will fail. This assertion exists to catch incorrect assumptions about - // how libcurl works, or about how we've configured it. lp.assert(self.source.remote.capacity == 0, "ScriptManager.HeaderCallback", .{ .capacity = self.source.remote.capacity }); var buffer = self.manager.buffer_pool.get(); if (transfer.getContentLength()) |cl| { diff --git a/src/http/Client.zig b/src/http/Client.zig index f5f33993..cc61b681 100644 --- a/src/http/Client.zig +++ b/src/http/Client.zig @@ -17,6 +17,8 @@ // along with this program. If not, see . const std = @import("std"); +const lp = @import("lightpanda"); + const log = @import("../log.zig"); const builtin = @import("builtin"); @@ -235,7 +237,7 @@ pub fn request(self: *Client, req: Request) !void { if (req.blocking == false) { // The request was interecepted, but it isn't a blocking request, so we // dont' need to block this call. The request will be unblocked - // asynchronously via eitehr continueTransfer or abortTransfer + // asynchronously via either continueTransfer or abortTransfer return; } @@ -458,7 +460,7 @@ pub fn disableTlsVerify(self: *const Client) !void { } } -fn makeRequest(self: *Client, handle: *Handle, transfer: *Transfer) !void { +fn makeRequest(self: *Client, handle: *Handle, transfer: *Transfer) anyerror!void { const conn = handle.conn; const easy = conn.easy; const req = &transfer.req; @@ -563,7 +565,7 @@ fn processMessages(self: *Client) !bool { // In case of auth challenge // TODO give a way to configure the number of auth retries. - if (transfer._auth_challenge != null and transfer._tries < 10) { + if (transfer._auth_challenge != null and transfer._tries < 10) { var wait_for_interception = false; transfer.req.notification.dispatch(.http_request_auth_required, &.{ .transfer = transfer, .wait_for_interception = &wait_for_interception }); if (wait_for_interception) { @@ -572,16 +574,34 @@ fn processMessages(self: *Client) !bool { log.debug(.http, "wait for auth interception", .{ .intercepted = self.intercepted }); } transfer._intercept_state = .pending; + + // Wether or not this is a blocking request, we're not going + // to process it now. We can end the transfer, which will + // release the easy handle back into the pool. The transfer + // is still valid/alive (just has no handle). + self.endTransfer(transfer); if (!transfer.req.blocking) { - // the request is put on hold to be intercepted. - // In this case we ignore callbacks for now. - // Note: we don't deinit transfer on purpose: we want to keep - // using it for the following request - self.endTransfer(transfer); + // In the case of an async request, we can just "forget" + // about this transfer until it gets updated asynchronously + // from some CDP command. continue; } - _ = try self.waitForInterceptedResponse(transfer); + // In the case of a sync request, we need to block until we + // get the CDP command for handling this case. + if (try self.waitForInterceptedResponse(transfer)) { + // we've been asked to continue with the request + // we can't process it here, since we're already inside + // a process, so we need to queue it and wait for the + // next tick (this is why it was safe to endTransfer + // above, because even in the "blocking" path, we still + // only process it on the next tick). + self.queue.append(&transfer._node); + } else { + // aborted, already cleaned up + } + + continue; } } @@ -887,10 +907,19 @@ pub const Transfer = struct { }; pub fn reset(self: *Transfer) void { + // There's an assertion in ScriptManager that's failing. Seemingly because + // the headerCallback is being called multiple times. This shouldn't be + // possible (hence the assertion). Previously, this `reset` would set + // _header_done_called = false. That could have been how headerCallback + // was called multuple times (because _header_done_called is the guard + // against that, so resetting it would allow a 2nd call to headerCallback). + // But it should also be impossible for this to be true. So, I've added + // this assertion to try to narrow down what's going on. + lp.assert(self._header_done_called == false, "Transert.reset header_done_called", .{}); + self._redirecting = false; self._auth_challenge = null; self._notified_fail = false; - self._header_done_called = false; self.response_header = null; self.bytes_received = 0;