Skip to content

Commit 9041222

Browse files
committed
Fix race condition
1 parent 695093a commit 9041222

5 files changed

Lines changed: 142 additions & 101 deletions

File tree

src/browser/HttpClient.zig

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -747,7 +747,7 @@ fn processOneMessage(self: *Client, msg: Network.Handle.Completion, transfer: *T
747747
if (body.len > 0) {
748748
try transfer.req.data_callback(Response.fromTransfer(transfer), body);
749749

750-
if (transfer.aborted) {
750+
if (transfer.isAborted()) {
751751
transfer.requestFailed(error.Abort, true);
752752
return true;
753753
}
@@ -1029,7 +1029,7 @@ pub const Transfer = struct {
10291029
client: *Client,
10301030

10311031
start_time: u64,
1032-
aborted: bool = false,
1032+
aborted: std.atomic.Value(bool) = .init(false),
10331033

10341034
_notified_fail: bool = false,
10351035

@@ -1083,6 +1083,14 @@ pub const Transfer = struct {
10831083
arena_pool.release(arena);
10841084
}
10851085

1086+
pub fn isAborted(self: *const Transfer) bool {
1087+
return self.aborted.load(.acquire);
1088+
}
1089+
1090+
fn setAborted(self: *Transfer) void {
1091+
self.aborted.store(true, .release);
1092+
}
1093+
10861094
// Cancel this transfer with `err`. Fires error_callback once (latched
10871095
// via _notified_fail), then either deinits synchronously or, if we're
10881096
// mid-perform with a libcurl handle still in the multi, detaches and
@@ -1123,7 +1131,7 @@ pub const Transfer = struct {
11231131
// there is nothing left referencing this transfer and we can safely
11241132
// deinit inline even from inside a perform callback.
11251133
fn detachOrDeinit(self: *Transfer) void {
1126-
if (self.aborted) return;
1134+
if (self.isAborted()) return;
11271135
if (self._performing) {
11281136
self.detachInPerform();
11291137
} else if (self._conn) |conn| {
@@ -1147,7 +1155,7 @@ pub const Transfer = struct {
11471155
// `owner == null` and skips the list-remove that would otherwise
11481156
// UAF against a freed list.
11491157
fn detachInPerform(self: *Transfer) void {
1150-
self.aborted = true;
1158+
self.setAborted();
11511159
self.req.start_callback = null;
11521160
self.req.shutdown_callback = null;
11531161
self.req.header_callback = Noop.headerCallback;
@@ -1425,7 +1433,7 @@ pub const Transfer = struct {
14251433
return err;
14261434
};
14271435

1428-
return proceed and transfer.aborted == false;
1436+
return proceed and !transfer.isAborted();
14291437
}
14301438

14311439
fn dataCallback(buffer: [*]const u8, chunk_count: usize, chunk_len: usize, data: *anyopaque) usize {
@@ -1475,7 +1483,7 @@ pub const Transfer = struct {
14751483
return http.writefunc_error;
14761484
};
14771485

1478-
if (transfer.aborted) {
1486+
if (transfer.isAborted()) {
14791487
return http.writefunc_error;
14801488
}
14811489

src/browser/Runner.zig

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -212,18 +212,10 @@ fn _tick(self: *Runner, comptime is_cdp: bool, opts: TickOpts) !CDPTickResult {
212212
},
213213
}
214214

215-
if (http_active == 0 and http_client.handle.ws_active == 0 and http_client.queue.first == null and http_client.handle.ready_queue.first == null and (comptime is_cdp == false)) {
215+
if (http_active == 0 and http_client.handle.ws_active == 0 and http_client.queue.first == null and (comptime is_cdp == false)) {
216216
// we don't need to consider http_client.intercepted here
217217
// because is_cdp is false, and that can only be
218218
// the case when interception isn't possible.
219-
//
220-
// ready_queue is also part of the check: makeRequest now
221-
// wraps its handles.perform() in a performing=true window,
222-
// and any synchronous libcurl callback that ends up
223-
// calling trackConn during that window (e.g. JS creating
224-
// a WebSocket) will append to ready_queue. Without this
225-
// check we could observe it non-empty after
226-
// http_client.tick returns.
227219
if (comptime IS_DEBUG) {
228220
std.debug.assert(http_client.interception_layer.intercepted == 0);
229221
}

src/browser/ScriptManagerBase.zig

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -576,7 +576,7 @@ pub const Script = struct {
576576
.a9 = self.debug_transfer_easy_id,
577577
.b1 = transfer.id,
578578
.b2 = transfer._tries,
579-
.b3 = transfer.aborted,
579+
.b3 = transfer.isAborted(),
580580
.b4 = transfer.res.bytes_received,
581581
.b5 = transfer._notified_fail,
582582
.b8 = transfer._auth_challenge != null,
@@ -585,7 +585,7 @@ pub const Script = struct {
585585
self.header_callback_called = true;
586586
self.debug_transfer_id = transfer.id;
587587
self.debug_transfer_tries = transfer._tries;
588-
self.debug_transfer_aborted = transfer.aborted;
588+
self.debug_transfer_aborted = transfer.isAborted();
589589
self.debug_transfer_bytes_received = transfer.res.bytes_received;
590590
self.debug_transfer_notified_fail = transfer._notified_fail;
591591
self.debug_transfer_auth_challenge = transfer._auth_challenge != null;

0 commit comments

Comments
 (0)