From d78ab7ff05fb208be4fe17c05bc28dd8384001d6 Mon Sep 17 00:00:00 2001 From: Claude Bot Date: Mon, 3 Nov 2025 13:55:49 +0000 Subject: [PATCH] Phase 2, Step 2.2: Fix Response Finalization Race MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- src/bun.js/webcore/fetch/FetchTasklet.zig | 35 +++++++++++++++++++---- 1 file changed, 29 insertions(+), 6 deletions(-) diff --git a/src/bun.js/webcore/fetch/FetchTasklet.zig b/src/bun.js/webcore/fetch/FetchTasklet.zig index d63b36bf76..77685be2c0 100644 --- a/src/bun.js/webcore/fetch/FetchTasklet.zig +++ b/src/bun.js/webcore/fetch/FetchTasklet.zig @@ -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(); } } }