fetch: don't double-release the request-stream ref when a native body sink ends inline

[robobun]

Crash

Sentry BUN-3BZF (2,975 events since 2026-05-25, macOS-dominant): Panic: called Option::unwrap() on a None value at FetchTasklet::callback's task_ref.http.as_mut().unwrap(), reached from the HTTP thread's result dispatch (us_internal_ssl_on_data -> HTTPClient::fail -> dispatch_result_and_reset -> AsyncHTTP::on_async_http_callback_raw -> FetchTasklet::callback). http is set once at creation and cleared only at deinit, so the panic means the callback ran against a freed FetchTasklet.

Cause

start_request_stream takes a +1 on the tasklet that must be released exactly once by write_end_request. For a native ByteStream request body (an upstream response body piped into fetch()), wire_native_sink installs the sink's source handle before any of its EndedInline returns (ReadableStream.rs:328 vs :337/:352/:359), so a stream that picked up an error or its last chunk between fetch() and the can_stream tick comes back EndedInline with a native source attached.

The EndedInline arm released the +1 (via write_end_request) but left self.sink installed with ended == false. Every terminal path then runs cancel_request_body_sink, which saw a "live" native sink and took its native arm: abort_task() plus a second write_end_request — releasing the same +1 again.

The double release collapses the refcount while the other owners (the JS-side initial ref and the HTTP thread's in-flight ref) still use the tasklet. Under ASAN the deterministic form is the trace below (deinit runs inside cancel_request_body_sink, then on_progress_update keeps using self). In release builds the same imbalance frees the tasklet while it is still in use (or double-frees, handing a live tasklet's block back to the allocator), which surfaces as downstream crashes in the fetch completion path — the BUN-3BZF unwrap is the tasklet's http field read from freed/recycled memory.

READ of size 8 ... core::mem::replace::<bun_jsc::js_promise::Strong>
  #2 FetchTasklet::on_progress_update        FetchTasklet.rs:1158
freed by thread T0 here:
  #12 FetchTasklet::deinit                   FetchTasklet.rs:509
  #16 FetchTasklet::write_end_request        FetchTasklet.rs:2281
  #17 FetchTasklet::cancel_request_body_sink FetchTasklet.rs:2368
  #18 FetchTasklet::on_progress_update       FetchTasklet.rs:1143

Fix

Leave the sink in the same state end_from_stream (the normal native termination) leaves it: ended = true, source and task detached. The terminal cancel_request_body_sink then hits its existing if sink.ended { return } guard and cannot release the ref a second time (it also no longer spuriously aborts a request whose body simply ended inline).

Verification

• New fixture fetch-stream-body-ended-inline-fixture.ts drives the window: an upstream server that advertises a larger content-length than it sends and closes a few ms later, piped as the body of a TLS fetch() (the handshake keeps the wire-attempt window open), 100 iterations.
• Unfixed debug+ASAN build: heap-use-after-free with the trace above, 8/8 runs.
• Fixed build: bun bd test test/js/web/fetch/fetch-abort-stream-body.test.ts passes (5 pass, 1 pre-existing skip), including the new test.
• test/js/web/fetch/body-stream.test.ts: 9086 pass / 0 fail. fetch.test.ts and fetch.stream.test.ts: identical pass/fail counts to an unfixed baseline in the same container (the failures are pre-existing network/timeout issues).
• The test is skipIf(!isASAN): the release build corrupts silently, so only sanitizer lanes can observe the failure.

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 1 minute

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: fbeecdc6-450b-4b03-8009-906e1f62723a

📥 Commits

Reviewing files that changed from the base of the PR and between d601782 and 5c18dc7.

📒 Files selected for processing (3)

• src/runtime/webcore/fetch/FetchTasklet.rs
• test/js/web/fetch/fetch-abort-stream-body.test.ts
• test/js/web/fetch/fetch-stream-body-ended-inline-fixture.ts

---

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

[claude]

No issues found, but this is a refcount-balance fix in the FetchTasklet lifecycle (memory-safety in a production-critical path), so it warrants a human look.

What was reviewed:

• Traced wire_native_sink (ReadableStream.rs:328/337/352/359): ByteStream path calls set_source before every EndedInline return, so sink.source is left populated — confirming the double-release via cancel_request_body_sink's is_native arm.
• Confirmed the new state (ended=true, source.clear(), task=None) matches FetchRequestBodySink::end_from_stream and hits the existing if sink.ended { return } guard in cancel_request_body_sink.
• Checked the sibling synchronous Fulfilled/Rejected arms and the FileReader EndedInline path — neither leaves a native sink.source installed, so they were already safe from this bug.

Extended reasoning...

Overview

Two-line state fix in FetchTasklet::start_request_stream's EndedInline arm (sink.ended = true; sink.source.clear();), plus an ASAN-gated regression test with a subprocess fixture. Addresses a Sentry crash cluster (BUN-3BZF, ~3k events) where a native ByteStream request body that finishes inside wire_native_sink caused the request-stream +1 to be released twice — once by the EndedInline arm's write_end_request, then again by cancel_request_body_sink seeing a still-"live" native sink.

Verification of the mechanism

I traced the claimed flow against the current source:

• start_request_stream takes the +1 at FetchTasklet.rs:636 and installs self.sink at :662 before calling wire_native_sink.
• wire_native_sink (ReadableStream.rs) invokes the set_source closure for the ByteStream branch before any of the three EndedInline returns (:337/:352/:359), so on EndedInline the sink's source is ByteStream(_). (The FileReader branch calls set_source after its EndedInline returns, so it was never affected.)
• cancel_request_body_sink (FetchTasklet.rs:2347) early-returns only when sink.ended; otherwise it computes is_native from sink.source and, when native, calls write_end_request(Some(reason)) — the second release.
• write_end_request unconditionally derefs the tasklet on every path.

The fix leaves the sink in exactly the state FetchRequestBodySink::end_from_stream (:211-232) leaves it — ended=true, source cleared, task taken — so the terminal cleanup hits the existing if sink.ended { return } guard. I also checked that the synchronous JS-pump Fulfilled/Rejected arms (which similarly only clear task) don't share the bug: they never set a native source, so is_native is false in cancel_request_body_sink.

Security risks

None identified. This is a lifetime/refcount correctness fix; no new input parsing, trust boundaries, or auth surfaces.

Level of scrutiny

High. Per the repo's review guidance, native memory safety and refcount balance on terminal paths is the most-blocked category. The change is tiny and the reasoning checks out end-to-end, but a wrong assumption here is a UAF in every fetch with a piped body — a human should confirm the ref accounting.

Other factors

• Test follows house patterns (subprocess fixture, skipIf(!isASAN), concurrent, drains stdout/stderr/exited together, asserts stderr for AddressSanitizer and exact stdout before exit code). The fixture uses randomized 1-8ms delays and 100 iterations to hit the race window; PR reports 8/8 ASAN repros pre-fix.
• The fixture's Bun.listen upstream is not closed via using, but upstream.stop(true) is called at the end and the whole thing runs in a subprocess that exits, so no CI-runner leakage.
• No prior human review comments; CodeRabbit was rate-limited.