Bun.write(path, fetch()): opt into BufferAll so a streaming body completes

[robobun]

Fixes #35854.

Repro

const srv = Bun.serve({ port: 0, fetch: () => new Response("x".repeat(500_000)) });
const res = await fetch(`http://127.0.0.1:${srv.port}/`);
await Bun.write("/tmp/out.bin", res);   // never settles on 1.4.0; 1.3.14 => 500000

On 1.4.0 (and main at df6c7eed6), await Bun.write(path, await fetch(url)) never resolves once the response body is large enough to arrive as a stream (observed at roughly 128 KB and above; bodies that fit in one buffer still work). No file is created, no error. 1.3.14 returns the byte count.

The abort escape hatch is dead for the same reason: fetch(url, { signal }) followed by Bun.write(path, res) hangs after the signal fires once the Response wrapper has been collected, because the finalizer drops the body before the abort can reach it.

Cause

#29831 introduced BodyReceiveMode::AutoPause: after the first body chunk, FetchTasklet::callback CASes AutoPause to Paused and the transport stops reading until a consumer opts into BufferAll via on_start_buffering (used by .text()/.arrayBuffer()/... and ValueBufferer) or a ReadableStream drains it.

write_file_internal's file-destination BodyValue::Locked arm only registers on_receive_value and then overwrites locked.task with the WriteFileWaitFromLockedValueTask*, never calling on_start_buffering. The transport parks after roughly one socket buffer, has_more stays true, BodyValue::resolve never runs, and the task's promise is never settled.

Separately, on_response_finalize only recognises locked.promise as a live consumer. Bun.write does not set locked.promise, so when the Response JS wrapper is collected while the body is still arriving the finalizer calls ignore_remaining_response_body, the tasklet is torn down, and a later signal.abort() has no listener to deliver to.

Fix

Introduce PendingValue::take_over_as_buffering_consumer(), which performs the on_start_buffering handshake with the producer's own task pointer (the same handshake set_promise and ValueBufferer already do) and clears the remaining producer hooks so they cannot be invoked against the consumer's task after it has been repurposed. Call it from both the write_file_internal Locked arm and the WriteFileWaitFromLockedValueTask::then() Locked re-register arm before overwriting locked.task.

In FetchTasklet::on_response_finalize, treat locked.on_receive_value as a live consumer alongside locked.promise, so collecting the Response wrapper does not drop a body that Bun.write is still waiting on.

Covers every non-S3 file destination (path, Bun.file, fd blobs, Bun.stdout). The S3 destination pulls the stream itself and is unaffected. Bun.write(path, new Response(res.body)) (a Locked value carrying a JS ReadableStream, #13237) is a separate arm and is not touched here.

Verification

test/js/bun/io/bun-write.test.js gains a five-case acceptance suite under Bun.write(path, fetch()) with a streaming body:

• Content-Length body (500 KB) settles with the byte count
• chunked body settles
• abort mid-transfer rejects with AbortError
• abort mid-transfer with a TimeoutError reason rejects with TimeoutError
• abort after the body has been fully received still settles

The abort cases scope the Response to a helper and force GC before aborting so the on_response_finalize path is exercised deterministically. All five time out on main and pass here. An ASAN-gated test also checks that touching resp.body / resp.clone() after Bun.write(path, resp) does not crash.

Related: #31739 reworks this path to stream to disk instead of buffering; #32906 covers the JS-constructed ReadableStream body case. This PR is the minimal regression fix.

---

no test proof · iteration 4 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/bun/io/bun-write.test.js

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 4 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 63db0fc5-ee5c-4749-a4f2-a0290c82525c

📥 Commits

Reviewing files that changed from the base of the PR and between 916492f and cb1d02d.

📒 Files selected for processing (5)

• src/runtime/webcore/Blob.rs
• src/runtime/webcore/Body.rs
• src/runtime/webcore/blob/write_file.rs
• src/runtime/webcore/fetch/FetchTasklet.rs
• test/js/bun/io/bun-write.test.js

---

_{Comment @coderabbitai help to get the list of available commands.}

[robobun]

^{Updated 11:06 PM PT - Jul 25th, 2026}

❌ @robobun, your commit cb1d02d has 1 failures in Build #81729 (All Failures):

• step failed outside runner - exit 1 on 🐧 x64-asan - build-bun
• step failed outside runner - exit 1 on 🐧 aarch64-android - build-bun

---

🧪   To try this PR locally:

bunx bun-pr 35531

That installs a local version of the PR into your bun-35531 executable, so you can run:

bun-35531 --bun

[github-actions]

Found 2 issues this PR may fix:

1. Bun.write with new Response(req.body) hangs #13237 - Bun.write with new Response(req.body) hangs indefinitely — same root cause (missing on_start_buffering in BodyValue::Locked arm of write_file_internal)
2. Bun.write writing a Response from a fetch leaks memory #10686 - Bun.write writing a Response from fetch leaks memory — the parked transport from missing on_start_buffering likely leaves allocated buffers and FetchTasklet unreachable but not freed

If this is helpful, copy the block below into the PR description to auto-close these issues on merge.

Fixes #13237
Fixes #10686

🤖 Generated with Claude Code

[robobun]

Verified the suggested issues against this build:

• Bun.write with new Response(req.body) hangs #13237 still hangs: new Response(req.body) wraps a materialized ReadableStream in a fresh Response whose Locked body has no producer on_start_buffering hook, so there is nothing for this change to call. That case is what Bun.write: fix hang when source is a Response/Request with a ReadableStream body #32906 addresses.
• Bun.write writing a Response from a fetch leaks memory #10686 predates the backpressure change and describes a leak on the 1.3.14-era path this PR restores; not claiming it without a separate measurement.

Leaving both out of the description.

[github-actions]

This PR may be a duplicate of:

1. Bun.write: fix hang when source is a Response/Request with a ReadableStream body #32906 - Fixes the same Bun.write hang for fetch Response with streaming body by calling on_start_buffering in the BodyValue::Locked arm (identical fix, broader scope covering JS ReadableStream bodies too)
2. Stream network bodies to disk in Bun.write instead of buffering them #31739 - Fixes the same Bun.write deadlock for network-streamed fetch bodies by streaming to disk via pipe_readable_stream_to_blob, patching the same locked-body code path in Blob.rs

🤖 Generated with Claude Code

[robobun]

On the duplicate flags:

#32906 does include this same on_start_buffering call (plus handling for the hook resolving synchronously), alongside the broader JS-ReadableStream pipe work, truncation/createPath/mode fixes, and byte-count fix. It has a test covering this exact fetch case. This PR is a strict subset: only the 7-line on_start_buffering call that unblocks the regression on the canary.

#31739 takes a different approach (pipe the network body through a FileSink instead of buffering) and does not touch on_start_buffering.

Happy to close this in favor of #32906 if that one is landing soon; leaving it open as the minimal regression patch otherwise.

[claude]

LGTM — minimal regression fix that mirrors the existing ValueBufferer pattern.

What was reviewed:

• Verified the new on_start_buffering(producer_task) call passes the original FetchTasklet ctx (not the new task) before locked.task is overwritten, matching Body.rs:2497.
• Confirmed FetchTasklet is the only producer that sets on_start_buffering, and it always sets task alongside it — the if let (Some, Some) guard is safe and the .take() side-effect when task is None is harmless.
• Test follows repo conventions (tempDir, port: 0, await using, Buffer.alloc over .repeat, local server) and asserts exact byte count + content.

Extended reasoning...

Overview

This PR fixes a hang regression in Bun.write(path, await fetch(url)) introduced by #29831's BodyReceiveMode::AutoPause. The fix is a 6-line addition in src/runtime/webcore/Blob.rs (write_file_internal's BodyValue::Locked arm) that calls locked.on_start_buffering(producer_task) before overwriting locked.task with the WriteFileWaitFromLockedValueTask pointer. A new test in test/js/bun/io/bun-write.test.js fetches a 1 MB body from a local Bun.serve and writes it to disk, which times out on main and passes with the fix.

Correctness

The fix is a byte-for-byte copy of the pattern already used at Body.rs:2497-2501 (ValueBufferer). The critical detail — that on_start_buffering must be called with the original producer task (the FetchTasklet*) before locked.task is repurposed — is handled correctly: the tuple destructure reads locked.task before line 5252 overwrites it. on_start_buffering_callback in FetchTasklet.rs:1669 casts ctx back to FetchTasklet, sets BodyReceiveMode::BufferAll, re-refs the poll_ref, and schedules a receive resume — exactly what's needed to un-park the transport so resolve() eventually fires on_receive_value → then_wrap.

I checked whether .take() on on_start_buffering when locked.task is None could drop a needed callback: FetchTasklet::to_body_value is the only site that sets on_start_buffering, and it always sets task on the line before, so the guard is defensive rather than load-bearing. The other stale callbacks left on locked (on_start_streaming, on_stream_cancelled, etc.) that still expect a FetchTasklet* after task is overwritten are pre-existing and won't fire on this path once BufferAll is set — no new hazard introduced.

Security risks

None. No untrusted input parsing, no new allocation, no lifetime changes. The c_void pointer passed is the same one the producer registered and expects.

Level of scrutiny

Medium — this touches the fetch/body-locking machinery which has memory-safety implications (raw *mut c_void ctx pointers, task ownership), but the change is purely additive, follows an established in-tree pattern verbatim, and the mechanism is well-explained in the PR description with reference to the causing PR (#29831) and adjacent work (#32906, #31739).

Other factors

The test is well-constructed per REVIEW.md: hermetic local server, port: 0, using/await using for cleanup, Buffer.alloc(n, fill) instead of "x".repeat(n), asserts exact written byte count and file content (not just length). It sits alongside the existing "Bun.file -> Response" test which covers the small-body (single-buffer) case, so both variants are now covered.

[claude]

Additional findings (outside current diff — PR may have been updated during review):

• 🟣 src/runtime/webcore/Blob.rs:5254-5260 — Pre-existing (not introduced here, don't block on it): after locked.task is overwritten with the WriteFileWaitFromLockedValueTask pointer at line 5261, on_start_streaming / on_readable_stream_available / on_stream_cancelled / on_stream_drained still hold FetchTasklet callbacks — a subsequent resp.body or resp.clone() reaches to_readable_stream / tee() and calls them with the wrong pointer, casting *mut WriteFileWaitFromLockedValueTask to *mut FetchTasklet (type confusion). Since this PR already does partial hook cleanup here, consider also .take()-ing / None-ing those four hooks alongside on_start_buffering so a later resp.body degrades to the known #13237 hang instead of UB — or leave it for #32906.

  Extended reasoning...

  What

  In the block this PR edits, write_file_internal overwrites locked.task with a pointer to its own WriteFileWaitFromLockedValueTask (Blob.rs:5261, unchanged context) and installs on_receive_value = WriteFileWaitFromLockedValueTask::then_wrap (Blob.rs:5262). The PR now additionally .take()s on_start_buffering and calls it — but deliberately leaves on_start_streaming in place (only checks .is_some()), and neither the PR nor the surrounding code touches on_readable_stream_available, on_stream_cancelled, or on_stream_drained. Those four hooks were installed by the producer at FetchTasklet.rs:1723-1726 and each does FetchTasklet::from_ctx(ctx) — an unchecked cast of ctx to *mut FetchTasklet.

  If user code touches resp.body or resp.clone() after Bun.write(path, resp) has returned its promise, those stale hooks are invoked with locked.task, which now points at a WriteFileWaitFromLockedValueTask. That's a type-confused raw-pointer cast — the callee reads/writes FetchTasklet fields (signal_store.aborted, response.body, poll_ref, …) at offsets that are actually WriteFileWaitFromLockedValueTask fields → crash / heap corruption.

  Trigger path (step-by-step)

  const resp = await fetch(url);           // FetchTasklet installs on_start_streaming/on_readable_stream_available/
                                            // on_stream_cancelled/on_stream_drained; locked.task = *mut FetchTasklet
  const p = Bun.write("/tmp/out", resp);   // Blob.rs:5254-5262: .take()s on_start_buffering, then
                                            // locked.task = *mut WriteFileWaitFromLockedValueTask
  resp.body;                                // or resp.clone()

  1. resp.body → get_body (Body.rs:1746). get_body_readable_stream returns None (no JS-side cache, locked.readable empty), so it falls through to to_readable_stream.
  2. Value::to_readable_stream, Locked arm (Body.rs:776): locked.readable.get() is None (:777); locked.promise is None and locked.action is None (:780) — write_file_internal set neither — so the used-body early-return does not fire.
  3. Line 785-786: if let Some(drain) = locked.on_start_streaming.take() { drain_result = drain(locked.task.unwrap()); }. Here drain is FetchTasklet::on_start_streaming_http_response_body_callback, but locked.task.unwrap() is the *mut WriteFileWaitFromLockedValueTask installed at Blob.rs:5261. from_ctx casts it to *mut FetchTasklet and immediately reads this.signal_store.aborted etc. from garbage → type confusion.
  4. Lines 806-814 wire the same wrong locked.task into the ByteStream's cancel_ctx / drain_ctx, and lines 843-848 call on_readable_stream_available(locked.task.unwrap(), …) with the wrong pointer as well. resp.clone() hits the same shape via tee() at Body.rs:1434 / 1476 / 1496.

  Why nothing guards it

  bodyUsed is false in this state: body_stream_check (Body.rs:1763) sees Locked with action == None, no cached JS stream, and locked.readable empty, so throw_if_body_unusable doesn't intervene and get_body / clone are reachable. Contrast ValueBufferer::buffer_locked_body_value (Body.rs:2494-2516), which handles the same situation correctly by only overwriting locked.task when it was already None, otherwise routing through to_readable_stream while task still points at the real producer.

  Why this is pre-existing

  The locked.task overwrite at line 5261 and the failure to clear the sibling producer hooks both predate this PR — they appear as unchanged context in the diff. The trigger (const p = Bun.write(out, resp); resp.body;) was equally reachable before this PR (Bun.write returned synchronously with a promise then too; if anything the pre-PR window was longer because the transport parked forever). The PR neither introduces nor widens the surface — to_readable_stream / tee never call on_start_buffering, which is the only hook the PR removes. It's surfaced here only because the PR edits the exact block and does partial hook cleanup, making the inconsistency visible.

  Suggested hardening (optional, non-blocking)

  At the point locked.task is repurposed, also None-out the remaining producer hooks:

  locked.on_start_streaming = None;
  locked.on_readable_stream_available = None;
  locked.on_stream_cancelled = None;
  locked.on_stream_drained = None;
  locked.task = Some(task.cast::<c_void>());
  locked.on_receive_value = Some(WriteFileWaitFromLockedValueTask::then_wrap);

  With those cleared, a subsequent resp.body after Bun.write(path, resp) degrades to the known #13237 hang (empty ByteStream, never fed) rather than a type-confused pointer deref. That's still wrong, but it's the pre-existing wrong that #32906 is already tracked to fix — not UB. Fine to defer to #32906 if you'd rather keep this PR strictly minimal.

[robobun]

Applied the hook-clearing hardening in f5dbc18: verified the resp.body-after-Bun.write sequence was a real ASAN heap-buffer-overflow on main (read at offset 744 into the 400-byte WriteFileWaitFromLockedValueTask allocation via on_start_streaming_http_response_body_callback). With the four hooks cleared it now returns an inert ReadableStream instead. Added an isASAN-gated test alongside the regression test.

[claude]

Beyond the inline nit: checked whether on_start_buffering(producer_task) can synchronously re-enter and invalidate the locked borrow before line 5256 — for FetchTasklet it only does poll_ref.ref_() + an atomic receive-mode CAS + an http-thread post (FetchTasklet.rs:1669-1678), so no re-entry into body_value. The four-hook clearing from my earlier comment is in place at 5252-5255.

Extended reasoning...

The prior review concern (four producer hooks left pointing at FetchTasklet after locked.task is overwritten) was addressed in f5dbc18 and the thread is resolved. This run's remaining finding is the inline nit about on_start_buffering not being cleared unconditionally — a narrow, pre-existing edge case. Separately verified that the new on_start_buffering(producer_task) call cannot synchronously resolve or mutate *body_value while the &mut *body_value borrow at 5242 is live: FetchTasklet's callback (the only producer that installs both on_start_streaming and on_start_buffering on a fetch Response reaching this arm) only refs the poll, flips body_receive_mode atomically, and posts a resume to the HTTP thread. Not approving because this touches type-erased raw-pointer state in a memory-safety-adjacent path and overlaps with #32906/#31739, which is a maintainer call.

[claude]

All three earlier inline findings are addressed (hook clearing in f5dbc18, unconditional on_start_buffering = None in 46c7689, subprocess ASAN repro in 080a299). I didn't find anything new, but this touches fetch-body lifetime and the Response GC finalizer, and there's an open which-PR-lands question vs #32906/#31739 — worth a maintainer look.

Checked: the on_start_buffering(producer_task) call mirrors the existing ValueBufferer pattern at Body.rs:2497-2500; all five producer hooks are now cleared before locked.task is repurposed; the new on_receive_value early-return in on_response_finalize — verified the other setters (ValueBufferer, write_file.rs:1423) also represent live consumers, so no new over-retention.

Extended reasoning...

Overview

Three files: Blob.rs adds an on_start_buffering call plus clears all five producer hooks before overwriting locked.task in the Bun.write(path, Response) locked-body path (12 lines); FetchTasklet.rs makes on_response_finalize treat locked.on_receive_value as a live consumer so the body isn't dropped when the Response wrapper is GC'd mid-download (4 lines); two tests added to bun-write.test.js.

Security risks

None user-facing. The hook-clearing is a hardening that downgrades a pre-existing type-confusion (FetchTasklet callbacks invoked with a WriteFileWaitFromLockedValueTask*) from ASAN heap-buffer-overflow to the tracked #13237 hang. No new untrusted-input parsing.

Level of scrutiny

High. This is squarely in the memory-safety category REVIEW.md flags as most-blocked: it repurposes locked.task (a type-erased *mut c_void dispatched through function-pointer hooks), and it changes behavior inside a JSC Weak finalizer (on_response_finalize) that runs during sweep. The change itself is small and the mechanism is well-argued — the on_start_buffering call is copied from ValueBufferer at Body.rs:2497-2500, and I checked that every other on_receive_value setter (Body.rs:2515, write_file.rs:1423, RequestContext.rs:3261) also represents a consumer that should keep the body flowing — but finalizer-path edits and callback-context repurposing are exactly where a maintainer's eyes are worth having.

Other factors

• All three of my prior inline findings were addressed in follow-up commits; the last (subprocess ASAN repro) landed in 080a299 and matches the pattern used elsewhere in the file.
• The author explicitly flagged this as a strict subset of #32906 and an alternative to #31739; deciding which lands is a maintainer call, not something I should preempt with an approval.
• Bug hunter found nothing this run.
• No CI results visible for the latest commit yet (Build #80314 was building at last timeline update).

[robobun]

CI status: bun-write.test.js passes on every lane that has run it across builds #80414 and #81729. Remaining reds are unrelated to this diff:

• build #81729: :linux: aarch64 - build-bun timed out (build lane, so the 41 dependent aarch64 test jobs never started); three unrelated flaky tests (test-https-server-connections-checking-leak.js, terminal-platform-gaps.test.ts, deinitialization.test.ts) each passed on retry.
• build #80414 (pre-refactor, same fix): green on every lane including aarch64; remaining reds were android/freebsd build-bun infra and unrelated retried flakes.
• build #81484: buildkite pipeline broke at the trigger step and nothing ran.

Ready for review.

[claude]

No issues found this round — all three earlier inline concerns (stale-hook type confusion, unconditional on_start_buffering clearing, subprocess-spawned ASAN repro) are now applied. Deferring to a maintainer for the final look given this touches fetch body lifetime + the Response GC finalizer, and overlaps two open PRs (#32906, #31739) reworking the same Locked arm.

Checked: the on_start_buffering(producer_task) call matches the existing ValueBufferer/set_promise pattern in Body.rs and cannot synchronously replace *body_value under the caller's borrow. The on_response_finalize widening only affects the three on_receive_value setters (WriteFileWaitFromLockedValueTask ×2, ValueBufferer), all of which hold their own promise/task independent of the Response wrapper, so returning early there does not strand the transport.

Extended reasoning...

Overview

Two-file runtime change plus tests. Blob.rs (+12 lines in write_file_internal's BodyValue::Locked arm): call on_start_buffering(producer_task) while on_start_streaming is still set as the producer-liveness witness, then unconditionally null all five producer hooks before overwriting locked.task with the WriteFileWaitFromLockedValueTask*. FetchTasklet.rs (+4 lines in on_response_finalize): treat locked.on_receive_value.is_some() as a live consumer and return early instead of falling through to ignore_remaining_response_body. Three new tests in bun-write.test.js: the 300 KB streaming-body regression, an abort-propagation test, and an ASAN-gated subprocess repro for the resp.body-after-Bun.write type confusion.

Security risks

None identified. No user-controlled data reaches new parsing or allocation paths; the change tightens a pointer-type-confusion hazard by clearing stale callbacks rather than introducing one.

Level of scrutiny

High. This is memory-safety-adjacent: locked.task is a type-erased *mut c_void that different callbacks cast to different concrete types, and on_response_finalize runs inside a GC finalizer. The on_start_buffering invocation pattern is copied verbatim from Body.rs:2497-2500 (ValueBufferer) and Body.rs:421-424 (set_promise), and a verifier confirmed the call cannot synchronously mutate *body_value out from under the re-borrowed locked. The finalizer change was checked against every on_receive_value = Some(...) site in the tree — all three own an independent strong promise/task that outlives the Response wrapper, so keeping the transport alive is correct and does not leak (BufferAll was already opted into, so the body will resolve and then_wrap consumes the task).

Other factors

All three of my earlier inline comments were applied and the threads are resolved. CI on the latest push has bun-write.test.js passing on every lane; remaining reds are unrelated infra. The author explicitly flags this as the minimal regression patch subset of #32906, with #31739 taking a different (stream-to-disk) approach — a maintainer should decide sequencing between the three. Given the finalizer-path change and the overlapping-PR coordination question, deferring rather than approving.

[claude]

All prior inline concerns are addressed and this run found nothing new, but the change reworks the locked.task ownership handshake and adds a live-consumer branch to the Response GC finalizer — worth a human pass, and someone should decide whether this lands independently of #32906.

What was reviewed:

• take_over_as_buffering_consumer mirrors the set_promise/ValueBufferer handshake and clears every producer hook keyed on the old task — the earlier type-confusion path (resp.body after Bun.write) is closed and covered by the subprocess ASAN test.
• on_response_finalize's new on_receive_value guard: the only other setter (ValueBufferer, Body.rs:2527) also sets locked.promise, so its behavior is unchanged.
• The dead on_start_buffering = None store flagged last run is gone (969afa8).

Extended reasoning...

Overview

Fixes a 1.4.0 regression where Bun.write(path, await fetch(url)) hangs on bodies large enough to stream, caused by #29831's AutoPause backpressure never being released because write_file_internal's Locked arm registered on_receive_value without calling on_start_buffering. The fix extracts a shared PendingValue::take_over_as_buffering_consumer() (Body.rs) that performs the buffering opt-in with the producer's task pointer and then clears the five producer hooks, and calls it from both the initial registration (Blob.rs:5247) and the re-register arm (write_file.rs:1423) before overwriting locked.task with the consumer pointer. FetchTasklet::on_response_finalize gains an on_receive_value.is_some() early-return so GC of the Response wrapper doesn't tear down a body Bun.write is still waiting on. Five acceptance tests plus an ASAN-gated subprocess crash repro are added to bun-write.test.js.

Security risks

None identified. No untrusted-input parsing; the change is internal callback lifecycle.

Level of scrutiny

High. This is native code at the intersection of GC finalization, event-loop backpressure, and a repurposed type-erased *mut c_void context pointer — exactly the class REVIEW.md calls out as most-blocked. The prior revision had a real ASAN heap-buffer-overflow (FetchTasklet::from_ctx(WriteFileWaitFromLockedValueTask*)) that the hook-clearing now closes; the finalizer edit changes when a paused transport is torn down. It has been through several rounds of inline review here (type-confusion via stale hooks, unconditional on_start_buffering clear, subprocess isolation for the ASAN repro, dead-store cleanup) and each was addressed in a follow-up commit; the current diff looks correct to me.

Other factors

• The author notes this is a strict subset of #32906 and orthogonal to #31739; a maintainer should decide sequencing.
• I checked the other on_receive_value setter (ValueBufferer at Body.rs:2527) — it also sets locked.promise, so the new finalizer guard changes behavior only for the Bun.write path.
• Tests follow harness conventions (tempDir, port: 0, drained pipes, subprocess for the crash fixture); the abort tests use Bun.gc(true) to force the finalizer path deterministically. Build #80414 was green on every lane that ran the file.

[claude]

No issues found, but deferring: this touches GC-finalizer logic (on_response_finalize runs inside sweep) and repurposes the type-erased locked.task pointer in the fetch body state machine — worth a maintainer's eyes, and there's a landing decision to make against the overlapping #32906 / #31739.

What was reviewed:

• take_over_as_buffering_consumer mirrors the existing set_promise/ValueBufferer handshake; the re-register arm in then() can't type-confuse because on_start_buffering was already .take()n on the first pass.
• The new on_receive_value.is_some() early-return in on_response_finalize — checked the other setters (ValueBufferer, server RequestContext); keeping the tasklet alive is correct for all of them.
• Prior inline findings (producer-hook clearing, unconditional on_start_buffering clear, subprocess ASAN fixture, dead store after .take()) are all applied in the current diff.

Extended reasoning...

Overview

Fixes a 1.4.0 regression where await Bun.write(path, await fetch(url)) never settles for bodies large enough to stream (~128 KB+). Root cause: #29831's AutoPause backpressure requires consumers to call on_start_buffering, and write_file_internal's Locked arm never did. The fix introduces PendingValue::take_over_as_buffering_consumer() (Body.rs) which performs the same handshake set_promise/ValueBufferer already do and clears the four remaining producer hooks so they can't be invoked with the repurposed locked.task. Called from Blob.rs:5247 and the then() re-register arm in write_file.rs:1423. on_response_finalize (FetchTasklet.rs) now treats on_receive_value as a live consumer alongside promise. Six new tests in bun-write.test.js cover Content-Length, chunked, mid-transfer abort (two error variants) with forced GC, post-completion abort, and a subprocess ASAN repro for the resp.body-after-Bun.write type confusion.

Security risks

None identified. The producer-hook clearing is a strict safety improvement — it closes a pre-existing type-confusion path (FetchTasklet::from_ctx(WriteFileWaitFromLockedValueTask*)) reachable from ordinary JS via resp.body after Bun.write(path, resp).

Level of scrutiny

High. The change is small and mechanically straightforward, but it sits at the intersection of three delicate subsystems: (1) on_response_finalize runs inside a JSC Weak finalizer during MutatorState::Sweeping, where the set of safe operations is narrow; (2) locked.task is a type-erased *mut c_void whose interpretation depends on which callback set is installed — clearing hooks and reassigning it must be atomic from the producer's point of view; (3) the fetch body Locked state machine has multiple consumers (.text(), ReadableStream, ValueBufferer, server RequestContext, Bun.write) that share these fields.

Other factors

• All four of my prior inline findings on this PR have been addressed in the current diff; the last one (dead = None store after .take()) landed in 969afa8.
• I traced the re-register arm in then(): on second entry on_start_buffering is already None (cleared by .take() on first pass), so the tuple pattern fails and no callback is invoked with the wrong task; if then() is handed a different Locked body, that body's own producer task is used, which is correct.
• I checked every other setter of on_receive_value (ValueBufferer at Body.rs:2527, server RequestContext at RequestContext.rs:3261) against the new finalizer early-return — for each, keeping the fetch tasklet alive when the JS Response wrapper is collected is the desired behavior.
• The scenario-comment relabeling in on_response_finalize (2b→2a) matches the doc block above it; the unlabeled fall-through when promise is non-empty still does nothing, preserving the pre-PR behavior.
• Two open PRs (#32906, #31739) cover the same regression with broader scope / a different mechanism. Choosing which to land is a maintainer call.
• CI on build #80414 was green for bun-write.test.js on every lane per the author; I did not independently verify.

[robobun]

Issue #35854 reports this same hang (Bun.write of an in-flight fetch Response never settles, regression from 1.3). Verified its repro against the analysis here: a chunked body still streaming when Bun.write is called hangs forever on main. Added "Fixes #35854" to the body so the issue closes on merge.

[robobun]

Heads up: as written, take_over_as_buffering_consumer() still calls on_start_buffering(producer_task) unconditionally. If the caller has already touched resp.body (materialising the body as a ByteStream) and let the transfer finish, producer_task points at a freed FetchTasklet and that call is a heap-use-after-free in KeepAlive::ref_. #36809 clears the producer hooks on the PendingValue at the stream-materialisation point so they're None by the time this arm runs.

[robobun]

Closing: the bug this PR was opened for is fixed on main, and the one piece of it that is still needed is carried by #32906.

What landed on main since this was opened:

• Bun.serve: apply TCP backpressure to a request body the handler reads slowly #36006 added the on_start_buffering call to the Locked arm of write_file_internal. That is the part of this PR that fixed Writing Response to disk hangs indefinitely (1.4 regression) #35854 (Bun.write(path, await fetch(url)) on a body large enough to pause the transport), and Writing Response to disk hangs indefinitely (1.4 regression) #35854 has since been closed as fixed.
• HTMLRewriter: stream through SinkHandle/SourceHandle; suspend async handlers instead of wait_for_promise #36733 makes .body / .clone() return a used stream once Bun.write has registered on_receive_value, and body: clear producer hooks on PendingValue once the body is realised as a ByteStream #36809 clears the producer hooks when the body is realised as a stream. Between them they cover what take_over_as_buffering_consumer() was clearing here.

Verified on main at 165dc9f (debug build) with this PR's test hunk applied on its own: the Content-Length, chunked, late-abort and both mid-transfer abort tests pass when run one at a time, and the ASAN then resp.body does not crash test passes. Run together under describe.concurrent, the abort tests' Bun.gc(true) collects the other tests' Response objects and four of them time out, which is the one case below that is still open.

Still broken on main: on_response_finalize. If the Response wrapper is collected while the body is still arriving (for example Bun.write(path, await fetch(url)) on a download long enough for a GC to run), the finalizer still drops the body and the write never settles. This hangs 10 of 10 runs on main:

const server = Bun.serve({ port: 0, fetch: () => new Response(new ReadableStream({ async start(c) {
  c.enqueue(Buffer.alloc(65536)); await gate; for (let i = 0; i < 5; i++) { c.enqueue(Buffer.alloc(65536)); await Bun.sleep(10); } c.close();
} })) });
let release; const gate = new Promise(r => (release = r));
async function start() { const res = await fetch(server.url); return Bun.write("/tmp/out.bin", res); }
const p = start();
await Bun.sleep(20); Bun.gc(true); await Bun.sleep(20); Bun.gc(true);
release();
console.log(await p); // never prints

This PR's on_response_finalize change fixes that, but #32906 contains the same change, is rebased on a main that already has #36006 / #36733 / #36809, and also fixes the new Response(readableStream) / Request / .body-touched hangs from #13237, which still reproduce on main. This branch conflicts with main and the rest of it is now redundant, so #32906 is the one to keep.