From ed802c0404a1a6c5e44b3204a95abae7041f4311 Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Thu, 12 Feb 2026 11:29:47 +0800 Subject: [PATCH 1/2] Remove potential recursive abort call in curl Curl doesn't like recursive calls. For example, you can't call curl_multi_remove_handle from within a dataCallback. This specifically means that, as-is, transfer.abort() calls aren't safe to be called during a libcurl callback. Consider this code: ``` req.open('GET', 'http://127.0.0.1:9582/xhr'); req.onreadystatechange = (e) => { req.abort(); } req.send(); ``` onreadystatechange is triggered by network events, i.e. it executes in libcurl callback. Thus, the above code fails to truly "abort" the request with `curl_multi_remove_handle` error, saying it's a recursive call. To solve this, transfer.abort() now sets an `aborted = true` flag. Callbacks can now use this flag to signal to libcurl to stop the transfer. A test was added which reproduced this issue, but this comes from: https://github.com/lightpanda-io/browser/issues/1527 which I wasn't able to reliably reproduce. I did see it happen regularly, just not always. It seems like this commit fixes that issue. --- src/browser/tests/net/xhr.html | 31 +++++++++++++++++++++++++++++++ src/http/Client.zig | 22 ++++++++++++++++++---- 2 files changed, 49 insertions(+), 4 deletions(-) diff --git a/src/browser/tests/net/xhr.html b/src/browser/tests/net/xhr.html index b7132f6e..61cfb1ee 100644 --- a/src/browser/tests/net/xhr.html +++ b/src/browser/tests/net/xhr.html @@ -222,3 +222,34 @@ testing.expectEqual(XMLHttpRequest.UNSENT, req.readyState); }); + + + --> diff --git a/src/http/Client.zig b/src/http/Client.zig index 3cce2ddb..4072b548 100644 --- a/src/http/Client.zig +++ b/src/http/Client.zig @@ -1092,6 +1092,8 @@ pub const Transfer = struct { // the headers, and the [encoded] body. bytes_received: usize = 0, + aborted: bool = false, + max_response_size: ?usize = null, // We'll store the response header here @@ -1224,10 +1226,18 @@ pub const Transfer = struct { pub fn abort(self: *Transfer, err: anyerror) void { requestFailed(self, err, true); - if (self._handle != null) { - self.client.endTransfer(self); + if (self._handle == null) { + self.deinit(); + return; } - self.deinit(); + + // abort can be called from a libcurl callback, e.g. we get data, we + // do the header done callback, the client aborts. + // libcurl doesn't support re-entrant calls. We can't remove the + // handle from the multi during a callback. Instead, we flag this as + // aborted, which will signal the callbacks to stop processing the + // transfer + self.aborted = true; } pub fn terminate(self: *Transfer) void { @@ -1359,7 +1369,7 @@ pub const Transfer = struct { .transfer = transfer, }); - return proceed; + return proceed and transfer.aborted == false; } // headerCallback is called by curl on each request's header line read. @@ -1519,6 +1529,10 @@ pub const Transfer = struct { .transfer = transfer, }); + if (transfer.aborted) { + return -1; + } + return @intCast(chunk_len); } From 0c89dca2610d2c968a1709da7367000950db55f2 Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Thu, 12 Feb 2026 18:29:35 +0800 Subject: [PATCH 2/2] When _not_ in a libcurl callback, deinit the transfer normally --- src/browser/tests/net/xhr.html | 1 - src/http/Client.zig | 34 ++++++++++++++++++++++++++-------- 2 files changed, 26 insertions(+), 9 deletions(-) diff --git a/src/browser/tests/net/xhr.html b/src/browser/tests/net/xhr.html index 61cfb1ee..7dc89a23 100644 --- a/src/browser/tests/net/xhr.html +++ b/src/browser/tests/net/xhr.html @@ -252,4 +252,3 @@ testing.expectEqual(XMLHttpRequest.UNSENT, req.readyState); }); - --> diff --git a/src/http/Client.zig b/src/http/Client.zig index 4072b548..cda2adeb 100644 --- a/src/http/Client.zig +++ b/src/http/Client.zig @@ -111,6 +111,10 @@ config: *const Config, cdp_client: ?CDPClient = null, +// keep track of when curl_multi_perform is happening so that we can avoid +// recursive calls into curl (which it will fail) +performing: bool = false, + // libcurl can monitor arbitrary sockets, this lets us use libcurl to poll // both HTTP data as well as messages from an CDP connection. // Furthermore, we have some tension between blocking scripts and request @@ -735,7 +739,12 @@ pub const PerformStatus = enum { fn perform(self: *Client, timeout_ms: c_int) !PerformStatus { const multi = self.multi; var running: c_int = undefined; - try errorMCheck(c.curl_multi_perform(multi, &running)); + + { + self.performing = true; + defer self.performing = false; + try errorMCheck(c.curl_multi_perform(multi, &running)); + } // We're potentially going to block for a while until we get data. Process // whatever messages we have waiting ahead of time. @@ -1231,13 +1240,22 @@ pub const Transfer = struct { return; } - // abort can be called from a libcurl callback, e.g. we get data, we - // do the header done callback, the client aborts. - // libcurl doesn't support re-entrant calls. We can't remove the - // handle from the multi during a callback. Instead, we flag this as - // aborted, which will signal the callbacks to stop processing the - // transfer - self.aborted = true; + const client = self.client; + if (client.performing) { + // We're currently in a curl_multi_perform. We cannot call endTransfer + // as that calls curl_multi_remove_handle, and you can't do that + // from a curl callback. Instead, we flag this transfer and all of + // our callbacks will check for this flag and abort the transfer for + // us + self.aborted = true; + return; + } + + if (self._handle != null) { + client.endTransfer(self); + } + self.deinit(); + } pub fn terminate(self: *Transfer) void {