Stop treating a locked body stream as a consumed one

RO
3mo ago
+357-8

Problem

  • body.getReader() followed by releaseLock(), with nothing read, sets bodyUsed and makes the body permanently unreadable: text() rejects with TypeError: Body already used. Node returns the content. tee() does the same. Affects every body with a native source (string, Blob, Bun.file(), network), not user ReadableStream bodies.
  • materializeNativeSource() (BunStreamSource.cpp) sets m_disturbed when it installs the controller, and getReader()/tee() both run it. The flag also told several fast paths that the native handle no longer holds the whole body.

Fix

  • materializeNativeSource() sets a new bit, m_nativeSourceMaterialized. [[disturbed]] is now set only by reads and cancels.
  • Fast paths that read the handle check nativeSourceConsumed(), the OR of both bits, so each stays as conservative as before: the C++ buffered consumers, to_any_blob, body extraction, RequestContext's renderer, wire_native_sink (fetch() uploads, FileSink, HTMLRewriter), and the S3 uploader. Without a gate, a re-wrapped materialized body is served or uploaded truncated (about 870 KB of 1 MB).
  • The body getters recorded a read before the stream rejected it as locked, which burned the body too. They now reject a locked stream first.
  • Verified: test/js/web/fetch/body.test.ts (41 new cases fail on main), test/js/bun/s3/s3-materialized-stream-upload.test.ts, test/regression/issue/07001.test.ts (one assertion encoded the bug). The other suites run are listed in Notes.

Background

  • A native-source body's ReadableStream gets its controller lazily, from materializeNativeSource(). That step calls the handle's start()/drain(), which moves the handle's buffered bytes into the controller's queue.
  • From then on the handle alone is incomplete. The fast paths read it directly to skip the JS stream machinery, so they would deliver only the bytes that arrive later.
  • Spec [[disturbed]] means a read or cancel happened, and bodyUsed reports it. Locking (getReader(), tee()) is a separate state.
Notes

Differential against node v26.3.0, 12 cases covering getReader+releaseLock, tee, clone, re-wrapping the released stream, and the cases that must still consume the body (a real read(), cancel(), a reader held across the consume). Bun diverged on 6 before and matches on all 12 after.

Truncation without the gates, 1 MB body, three runs each:

Bun.serve() re-wrapped fetch() body      873024 / 672585 / 607102
fetch() upload of a re-wrapped body      872000 / 738068 / 541619
S3 upload of a re-wrapped body           934464
HTMLRewriter on a re-wrapped body        872000

A short body hides this: it is served off the socket before anything buffers, so the tests use 1 MB.

Other suites run: body-stream.test.ts (9086 cases), body-clone, body-mixin-errors (#36499's tests), 09555, the four html-rewriter suites, bun-write, serve, and fetch.test.ts as a differential against main (the container fails about 40 network tests either way; the failure sets with and without this diff are identical).

History. The PR originally also fixed fetch() not rejecting a locked Request body, a ValueBufferer arm that hit unreachable!(), and ResumableSink. After rebasing onto current main: #36499 had fixed the fetch() case independently (its tests pass unchanged here), #38656 removed ValueBufferer, and #36087 replaced ResumableSink with wire_native_sink, which carried the same bug, so the gate moved there. The S3 uploader has an inlined copy of that fast path and gets its own gate and test.

Issues suggested by the find-issues bot (#19939, #24766, #28952) were each reproduced and are not fixed by this change; see the comment below.

The S3 test spawns its fixture with the proxy variables cleared, following s3-connection-close.test.ts, because the S3 client does not honor NO_PROXY.

While verifying, found that Bun.write(path, new Response(stream)) hangs and Bun.write(path, stream) stringifies the stream, on main and on the release build alike. Unrelated; reported separately.

no test proof · iteration 7 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/web/fetch/body.test.ts