mirror of
https://github.com/oven-sh/bun
synced 2026-02-14 04:49:06 +00:00
Phase 2, Step 2.2: Fix Response Finalization Race
Fix critical race condition in Bun__FetchResponse_finalize where the function accessed shared state without acquiring the mutex, racing with the HTTP thread's callback() which does lock the mutex. Changes: - Add mutex acquisition at function entry with RAII defer unlock - Inline ignoreRemainingResponseBody logic under lock protection - Set abort flag atomically with .release ordering - Clear buffers safely under lock (prevents concurrent modification) - Invert promise check logic for clearer control flow The dual ownership pattern (response_weak + native_response) is intentional and remains unchanged for finalization tracking. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -1274,10 +1274,20 @@ pub const FetchTasklet = struct {
|
||||
this.ignore_data = true;
|
||||
}
|
||||
|
||||
/// Called when Response JS object is garbage collected.
|
||||
/// This is our signal to stop processing body data.
|
||||
export fn Bun__FetchResponse_finalize(this: *FetchTasklet) callconv(.C) void {
|
||||
log("onResponseFinalize", .{});
|
||||
|
||||
// === ACQUIRE LOCK - Fix race condition ===
|
||||
// The HTTP thread accesses shared state in callback(), so we must lock
|
||||
this.mutex.lock();
|
||||
defer this.mutex.unlock();
|
||||
|
||||
// Check if we have a native response to work with
|
||||
if (this.native_response) |response| {
|
||||
const body = response.getBodyValue();
|
||||
|
||||
// Three scenarios:
|
||||
//
|
||||
// 1. We are streaming, in which case we should not ignore the body.
|
||||
@@ -1293,13 +1303,26 @@ pub const FetchTasklet = struct {
|
||||
}
|
||||
|
||||
if (body.Locked.promise) |promise| {
|
||||
if (promise.isEmptyOrUndefinedOrNull()) {
|
||||
// Scenario 2b.
|
||||
this.ignoreRemainingResponseBody();
|
||||
if (!promise.isEmptyOrUndefinedOrNull()) {
|
||||
// Scenario 2b - promise exists, keep loading
|
||||
return;
|
||||
}
|
||||
} else {
|
||||
// Scenario 3.
|
||||
this.ignoreRemainingResponseBody();
|
||||
}
|
||||
|
||||
// Scenario 2a or 3 - ignore remaining body
|
||||
// Signal abort to HTTP thread (under lock)
|
||||
this.signal_store.aborted.store(true, .release);
|
||||
|
||||
// Set ignore_data flag to stop buffering
|
||||
this.ignore_data = true;
|
||||
|
||||
// Clear accumulated buffers since we're ignoring the rest
|
||||
this.response_buffer.list.clearRetainingCapacity();
|
||||
this.scheduled_response_buffer.list.clearRetainingCapacity();
|
||||
|
||||
// Enable streaming to drain remaining data without buffering
|
||||
if (this.http) |http_| {
|
||||
http_.enableResponseBodyStreaming();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user