mirrors/bun.sh
1
0
Fork 0
mirror of https://github.com/oven-sh/bun synced 2026-02-12 11:59:00 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix fetch_error race conditions - protect all access with mutex

Browse Source 
CodeRabbit identified two more race conditions with fetch_error access:

1. onAbortCallback (lines 392-405): fetch_error.set() was called outside
   the mutex lock, racing with HTTP thread updates
2. writeEndRequest (lines 1841-1852): fetch_error operations without lock

Fix: Move ALL fetch_error and shared state access inside mutex-protected
scopes. This ensures:
- No off-thread jsc.Strong.Optional operations
- All threads serialize on same mutex
- No data races with fetch_error updates

Resolves CodeRabbit review comments #2488550465 and #2488550467

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
Claude Bot
2025-11-04 03:38:34 +00:00
parent 461f5bedd1
commit d21881905d
1 changed files with 26 additions and 15 deletions
Download Patch File Download Diff File Expand all files Collapse all files

41
src/bun.js/webcore/fetch/FetchTasklet.zig
View File

@@ -388,17 +388,18 @@ pub const FetchTasklet = struct {
log("AbortHandling.onAbortCallback", .{});
reason.ensureStillAlive();
// Store error in unified storage
fetch.fetch_error.set(.{ .abort_error = jsc.Strong.Optional.create(reason, fetch.main_thread.global_this) });
// Set atomic abort flag for HTTP thread fast-path
fetch.shared.signal_store.aborted.store(true, .monotonic);
// Transition to aborted state under lock
// All shared state modifications must be under lock
{
fetch.shared.mutex.lock();
defer fetch.shared.mutex.unlock();
// Store error in unified storage
fetch.fetch_error.set(.{ .abort_error = jsc.Strong.Optional.create(reason, fetch.main_thread.global_this) });
// Set atomic abort flag for HTTP thread fast-path
fetch.shared.signal_store.aborted.store(true, .monotonic);
// Transition to aborted state
if (!fetch.shared.lifecycle.isTerminal()) {
transitionLifecycle(fetch, fetch.shared.lifecycle, .aborted);
}
@@ -1838,15 +1839,25 @@ pub const FetchTasklet = struct {
log("writeEndRequest hasError? {}", .{err != null});
defer this.deref();
// Dual tracking: mark request stream as complete
this.shared.request_stream_state = .complete;
if (err) |jsError| {
if (this.shared.signal_store.aborted.load(.monotonic) or this.fetch_error.isAbort()) {
return;
}
if (!jsError.isUndefinedOrNull()) {
// Store error in unified storage
this.fetch_error.set(.{ .js_error = jsc.Strong.Optional.create(jsError, this.main_thread.global_this) });
// All shared state access must be under lock
{
this.shared.mutex.lock();
defer this.shared.mutex.unlock();
this.shared.request_stream_state = .complete;
if (err) |jsError| {
if (this.shared.signal_store.aborted.load(.monotonic) or this.fetch_error.isAbort()) {
return;
}
if (!jsError.isUndefinedOrNull()) {
// Store error in unified storage
this.fetch_error.set(.{ .js_error = jsc.Strong.Optional.create(jsError, this.main_thread.global_this) });
}
}
}
if (err) |_| {
this.abortTask();
} else {
if (!this.shared.upgraded_connection) {