Stop Response.clone() from retaining ~2x the body for an unread clone - #33130
Conversation
structuredCloneForStream, which Request/Response.clone()'s tee uses to copy each chunk for the second branch, cloned the chunk's entire backing ArrayBuffer (buffer->slice(0)) instead of the range the view covers. Chunks from Bun's native body streams are narrow subarray views into a shared 256KB-2MB receive buffer, so every queued clone dragged along a full copy of that buffer: reading a fetched Response while holding an unread clone retained about 2x the body size instead of 1x. Slice only [byteOffset, byteOffset + byteLength) and create the cloned view at offset 0.
|
Updated 2:20 AM PT - Jun 30th, 2026
❌ @robobun, your commit 1e87cb0 has 3 failures in
🧪 To try this PR locally: bunx bun-pr 33130That installs a local version of the PR into your bun-33130 --bun |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
Walkthrough
ArrayBufferView slice fix and streaming clone tests
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/js/web/fetch/body-clone.test.ts`:
- Around line 523-527: Shorten the explanatory comment near the
Request/Response.clone() typed array handling to fit the repository’s 3-line
comment limit. Keep the key point about tee/structured-clone copying only the
viewed bytes and the memory impact of cloning the full backing ArrayBuffer, but
remove extra detail so the comment stays concise.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 34ad2072-17d5-4c80-8521-0f4739a73b3c
📒 Files selected for processing (2)
src/jsc/bindings/webcore/StructuredClone.cpptest/js/web/fetch/body-clone.test.ts
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
I didn't find any issues — the slice-only-the-view's-bytes transformation looks correct for all typed-array element sizes and DataView — but this is a native WebCore/JSC ArrayBuffer-handling change with an observable behavior difference (cloned chunks now have byteOffset === 0 and a tight backing buffer), so it's worth a quick human look, especially with CI currently red.
Extended reasoning...
Overview
The PR changes structuredCloneForStream in src/jsc/bindings/webcore/StructuredClone.cpp so that when teeing a body for Request.clone() / Response.clone(), each JSArrayBufferView chunk's backing buffer is cloned as buffer->slice(byteOffset, byteOffset + byteLength) instead of buffer->slice(0), and the recreated view uses offset 0 instead of the original byteOffset. Three new tests in test/js/web/fetch/body-clone.test.ts verify the cloned chunk's data is unchanged while its backing buffer is exactly byteLength long, including an end-to-end fetch().clone() case.
I verified that structuredCloneForStream is only reachable via $readableStreamTee(stream, true), which is only invoked from the two C++ tee paths used by body cloning (ReadableStream::tee() and ReadableStream__tee); the public ReadableStream.prototype.tee() passes false and never hits this code. So the blast radius is exactly Request/Response clone, as the description claims.
Security risks
None apparent. ArrayBuffer::slice(begin, end) clamps its arguments and allocates a fresh buffer; the new view's (offset=0, length=bufferView->length()) fits exactly because the slice is sized to byteLength == length * elementSize (and for JSDataView, length() is the byte length). I checked the non-Uint8 element-size case (e.g. a Float64Array subarray) and the arithmetic holds.
Level of scrutiny
Medium. The diff is ~6 logical lines, the transformation is mechanical, and the tests are solid. But it lives in native JSC/WebCore bindings on a hot path (every body clone), and it intentionally changes observable behavior — cloned chunks now report byteOffset === 0 and buffer.byteLength === byteLength instead of mirroring the original view's offset/backing. That aligns with the byte-stream tee's CloneAsUint8Array semantics and Node's behavior, but it is a user-visible change someone should consciously sign off on.
Other factors
CodeRabbit's only comment (a 3-line comment-length nit) was addressed in 1679280. The robobun status shows widespread build-rust / build-cpp failures in scripts/build/ci.ts across every platform, which look like infrastructure rather than anything this 2-file change could cause, but I'd want a green build before merging. No CODEOWNERS cover this path.
There was a problem hiding this comment.
Looks correct to me — the slice/offset math checks out for all typed array element sizes and DataView, and I verified structuredCloneForStream is only reachable via the shouldClone=true tee path from Request/Response.clone(). Deferring only because it's a small observable behavior change (cloned chunk's .byteOffset/.buffer.byteLength) in WebCore bindings that's worth a quick maintainer glance.
Extended reasoning...
Overview
This PR changes structuredCloneForStream in src/jsc/bindings/webcore/StructuredClone.cpp to slice only [byteOffset, byteOffset + byteLength) of a chunk's backing ArrayBuffer (instead of slice(0) of the whole thing) and construct the cloned typed array / DataView at offset 0. Three new tests in test/js/web/fetch/body-clone.test.ts cover Request.clone(), Response.clone(), and a real fetch() body, asserting that cloned chunks own exactly their bytes.
Correctness
I verified the math: buffer->slice(begin, end) produces a buffer of exactly byteLength bytes; for multi-byte typed arrays byteLength = length() * BYTES_PER_ELEMENT so the view at offset 0 fits exactly, and offset 0 is always aligned regardless of the original byteOffset. For DataView, bufferView->length() is already the byte length and was unchanged from the previous code. I also confirmed scope: $structuredCloneForStream is called only from readableStreamTeePullFunction when shouldClone is true, and the only callers passing true are ReadableStream::tee() and ReadableStream__tee in ReadableStream.cpp (the body-clone path) — public ReadableStream.prototype.tee() passes false.
Security risks
None. This is a memory-efficiency fix with no auth, crypto, permission, or untrusted-input parsing implications. The only data handled is already-allocated ArrayBuffer bytes owned by the same realm.
Level of scrutiny
Moderate. The logic change is ~5 lines and mechanically verifiable, but it lives in WebCore C++ bindings and intentionally changes observable behavior: a cloned chunk's .byteOffset becomes 0 and .buffer.byteLength becomes the view's byte length rather than the original backing buffer's. The PR description argues (and I agree) that this matches the streams spec's CloneAsUint8Array for byte-stream tee and what Node/browsers do for fetch bodies, but a maintainer should confirm they're comfortable with that observable shift.
Other factors
No bugs were found by the multi-agent review. CodeRabbit's only comment (a 3-line comment-length nit) was addressed in 1679280. No CODEOWNERS cover this path. CI build #67163 is running. The PR description is thorough with before/after RSS measurements.
|
CI assessment for the latest run (build 67163, commit 1e87cb0): 281 jobs passed, 5 failed, none related to this change.
The tests added here ( |
… branch ReadableStream__tee (the extern called from Rust for Request/Response body clone) passed cloneForBranch2 = true, which StructuredClone-copies every chunk into the second tee branch. An N-deep clone chain therefore retained N independent copies of every body chunk: a 10 MB streaming body cloned 100 times held ~1 GB of duplicated buffers. Node (undici), Chrome, Firefox, and Deno all share the chunk reference between tee branches when cloning a body. The fetch spec text asks for cloneForBranch2 = true but no engine ships that; see whatwg/streams#1156. This flips the extern to cloneForBranch2 = false. The cloned branch now receives the same Uint8Array the original branch does, so a clone chain retains one copy of the body regardless of depth. The #33130 tests that asserted exact-size copies are updated to assert reference identity, which is the stronger invariant (zero copies rather than right-sized copies).
… branch (#35843) ### What `Response.clone()` / `Request.clone()` on a stream body deep-copied every chunk into the second tee branch. An N-deep clone chain therefore retained N independent copies of every body chunk. ```js // {"depth":100,"leafReadBytes":10485760,"rssDeltaMB":1042} (node: ~55) const MB = 1 << 20, N = 100, SZ = 10 * MB; let pulled = 0; const big = () => new ReadableStream({ pull(c) { if (pulled >= SZ) { c.close(); return; } c.enqueue(new Uint8Array(65536)); pulled += 65536; } }); const base = process.memoryUsage().rss; let cur = new Response(big()); const chain = [cur]; for (let i = 0; i < N; i++) { cur = cur.clone(); chain.push(cur); } const n = (await chain.at(-1).arrayBuffer()).byteLength; console.log({ depth: N, leafReadBytes: n, rssDeltaMB: ((process.memoryUsage().rss - base) / MB).toFixed(1) }); ``` ### Memory (RSS delta, 10 MB streaming body, only the leaf clone is read) | depth | bun before | **bun (this PR)** | node v26.3.0 | deno 2.9.4 | |------:|-----------:|------------------:|-------------:|-----------:| | 10 | 130 MB | **15 MB** | 43 MB | 123 MB | | 30 | 335 MB | **15 MB** | 49 MB | 329 MB | | 100 | 1043 MB | **16 MB** | 56 MB | 1035 MB | bun is now flat across depth and below node (node still allocates one `Uint8Array` wrapper per tee level; bun enqueues the same `JSValue`). ### Cause `ReadableStream__tee` (the extern called from Rust `Body` clone) passed `cloneForBranch2 = true` to `readableStreamTee`, which routed each chunk through `structuredCloneForStream` (`ArrayBuffer::slice` memcpy) before enqueuing into branch2. In a clone chain each level copies again, so a single source chunk ends up duplicated once per tee level. The fetch spec's "clone a body" step does say `ReadableStreamTee(stream, true)`, but Node (undici), Chrome, and Firefox all share the chunk reference between branches instead. Deno follows the spec and exhibits the same O(depth × bytes) growth. See whatwg/streams#1156 for the spec discussion. ### Fix Pass `cloneForBranch2 = false` in `ReadableStream__tee`. The cloned branch now receives the exact same `Uint8Array` object the original branch does, so a clone chain retains one copy of the body regardless of depth. This supersedes #33130's right-sized-copy optimisation: instead of copying only the view's bytes, we don't copy at all. Branch1 always received the original reference, so branch2 sharing that reference never retains more than branch1 already did. The now-unreachable `cloneForBranch2 = true` path (`structuredCloneChunk`, `m_shouldClone`, the `$structuredCloneForStream` private global and host function, and `CloneMode::Full`) is deleted; the `cloneForBranch2` parameter is kept on `readableStreamDefaultTee` to mirror the spec signature and asserted false. `ReadableStream.prototype.tee()` was already passing `false`; only the body-clone path changes. Byte-controller tee (`type: "bytes"`) is unchanged and still copies per spec. ### Verification `test/js/web/fetch/body-clone.test.ts` gains a 50-deep clone chain RSS check (fails at ~219 MB on main, passes at <50 MB with this change) and the existing per-chunk tests now assert `clonedChunk === originalChunk`. All 63 tests in the file pass. <!-- robobun:evidence:begin --> --- **[review]** gate passed · iteration 5 · 11 files touched <details><summary>fails on main (without fix)</summary> ```console ASAN without fix: 4 FAILED $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/js/web/fetch/body-clone.test.ts bun test v1.4.0 (0edb264) test/js/web/fetch/body-clone.test.ts: (pass) Request with streaming body can be cloned [17.11ms] (pass) Response with streaming body can be cloned [9.26ms] (pass) Request with large streaming body can be cloned [17.40ms] (pass) Request with large streaming body can be cloned (pull) [29.66ms] (pass) Response with chunked streaming body can be cloned [50.22ms] (pass) Request with streaming body can be cloned multiple times [11.05ms] (pass) Request with string body can be cloned [5.62ms] (pass) Response with string body can be cloned [5.08ms] (pass) Request with ArrayBuffer body can be cloned [8.25ms] (pass) Response with ArrayBuffer body can be cloned [6.60ms] (pass) Request with Uint8Array body can be cloned [6.58ms] (pass) Response with Uint8Array body can be cloned [7.54ms] (pass) Request with mixed body types can be cloned [16.51ms] (pass) Response with mixed body types can be cloned [14.65ms] (pass) Request with non-ASCII string body can be cloned [5.97ms] (pass) Resp ... (truncated) release without fix: 12 FAILED bun test v1.4.0-canary.1 (1498d7b) test/js/web/fetch/body-clone.test.ts: (pass) Request with streaming body can be cloned [2.85ms] (pass) Response with streaming body can be cloned [0.45ms] (pass) Request with large streaming body can be cloned [4.24ms] (pass) Request with large streaming body can be cloned (pull) [4.45ms] (pass) Response with chunked streaming body can be cloned [33.70ms] (pass) Request with streaming body can be cloned multiple times [0.42ms] (pass) Request with string body can be cloned [0.10ms] (pass) Response with string body can be cloned [0.07ms] (pass) Request with ArrayBuffer body can be cloned [0.18ms] (pass) Response with ArrayBuffer body can be cloned [0.11ms] (pass) Request with Uint8Array body can be cloned [0.09ms] (pass) Response with Uint8Array body can be cloned [0.07ms] (pass) Request with mixed body types can be cloned [0.72ms] (pass) Response with mixed body types can be cloned [0.30ms] (pass) Request with non-ASCII string body can be cloned [0.10ms] (pass) Response with non-ASCII string body can be cloned [0.07ms] (pass) Request with streaming non-ASCII body can be cloned [0.24ms] (pass) Response with streaming non-ASCII bod ... (truncated) ``` </details> <details><summary>passes on PR (with fix)</summary> ```console ASAN with fix: all passed $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/js/web/fetch/body-clone.test.ts bun test v1.4.0 (0edb264) test/js/web/fetch/body-clone.test.ts: (pass) Request with streaming body can be cloned [13.55ms] (pass) Response with streaming body can be cloned [13.22ms] (pass) Request with large streaming body can be cloned [16.48ms] (pass) Request with large streaming body can be cloned (pull) [28.65ms] (pass) Response with chunked streaming body can be cloned [50.55ms] (pass) Request with streaming body can be cloned multiple times [10.94ms] (pass) Request with string body can be cloned [5.51ms] (pass) Response with string body can be cloned [4.90ms] (pass) Request with ArrayBuffer body can be cloned [8.64ms] (pass) Response with ArrayBuffer body can be cloned [6.89ms] (pass) Request with Uint8Array body can be cloned [7.01ms] (pass) Response with Uint8Array body can be cloned [7.23ms] (pass) Request with mixed body types can be cloned [17.22ms] (pass) Response with mixed body types can be cloned [15.54ms] (pass) Request with non-ASCII string body can be cloned [6.37ms] (pass) Res ... (truncated) release with fix: all passed $ bun scripts/build.ts --profile=release [configured] bun-profile → bun (stripped) in 691ms (unchanged) ninja: Entering directory `/workspace/bun/build/release' [1/459] cxx obj/vendor/boringssl/crypto/trust_token/trust_token.cc.o [2/459] cxx obj/vendor/boringssl/crypto/trust_token/voprf.cc.o [3/459] cxx obj/vendor/boringssl/crypto/x509/a_sign.cc.o [4/459] cxx obj/vendor/boringssl/crypto/x509/a_verify.cc.o [5/459] cxx obj/vendor/boringssl/crypto/x509/i2d_pr.cc.o [6/459] cxx obj/vendor/boringssl/crypto/trust_token/pmbtoken.cc.o [7/459] gen cpp.rs (cppbind) [8/459] cxx obj/vendor/boringssl/crypto/x509/t_x509a.cc.o [9/459] cxx obj/vendor/boringssl/crypto/x509/algorithm.cc.o [10/459] cxx obj/vendor/boringssl/crypto/x509/v3_akeya.cc.o [11/459] cxx obj/vendor/boringssl/crypto/x509/v3_alt.cc.o [12/459] cxx obj/vendor/boringssl/crypto/x509/t_req.cc.o [13/459] cxx obj/vendor/boringssl/crypto/x509/v3_akey.cc.o [14/459] cxx obj/vendor/boringssl/crypto/x509/t_x509.cc.o [15/459] cxx obj/vendor/boringssl/crypto/x509/t_crl.cc.o [16/459] cxx obj/vendor/boringssl/crypto/x509/rsa_pss.cc.o [17/459] cxx obj/vendor/boringssl/crypto/x509/asn1_gen.cc.o [18/459] cxx obj/vendor/boringssl/c ... (truncated) ``` </details> <details><summary>diff hotspot</summary> ``` src/js/builtins.d.ts | 2 - src/js/builtins/BunBuiltinNames.h | 2 - src/jsc/bindings/ZigGlobalObject.cpp | 2 - src/jsc/bindings/webcore/StructuredClone.cpp | 84 ---------------------- src/jsc/bindings/webcore/StructuredClone.h | 2 - .../bindings/webcore/streams/JSReadableStream.cpp | 2 +- .../bindings/webcore/streams/JSStreamTeeState.h | 3 - .../webcore/streams/ReadableStreamOperations.cpp | 58 ++------------- .../bindings/webcore/streams/WebStreamsExports.cpp | 5 +- .../bindings/webcore/streams/WebStreamsInternals.h | 11 ++- test/js/web/fetch/body-clone.test.ts | 84 ++++++++++++++++------ 11 files changed, 81 insertions(+), 174 deletions(-) ``` </details> **gate history** · 5 passed · 0 rejected · iteration 5 <details><summary>evidence per changed file</summary> ``` file reads edits tests src/js/builtins.d.ts 2 3 0 src/js/builtins/BunBuiltinNames.h 2 2 0 src/jsc/bindings/ZigGlobalObject.cpp 2 3 0 src/jsc/bindings/webcore/StructuredClone.cpp 3 3 0 src/jsc/bindings/webcore/StructuredClone.h 2 3 0 src/jsc/bindings/webcore/streams/JSReadableStream.cpp 1 1 0 src/jsc/bindings/webcore/streams/JSStreamTeeState.h 1 1 0 …c/bindings/webcore/streams/ReadableStreamOperations.cpp 6 2 0 src/jsc/bindings/webcore/streams/WebStreamsExports.cpp 2 4 0 src/jsc/bindings/webcore/streams/WebStreamsInternals.h 2 3 0 test/js/web/fetch/body-clone.test.ts 3 6 0 ``` </details> <!-- robobun:evidence:end -->
### Problem - GitHub closes only the first reference after a keyword, so "Fixes #1, #2" leaves #2 open. "Supersedes #3" links nothing, and no reference closes a pull request. - The last 1000 merged PRs name 274 such references. PR #32292 is open although merged #36135 says "Supersedes #32292". ### Fix - `.github/workflows/close-linked-issues.yml` runs on `pull_request_target` `closed` (a merge into the default branch of `oven-sh/bun`) and on `workflow_dispatch` with a PR number and `dry_run`. Everything is inline in one `actions/github-script` step, with no checkout. - Each open target is closed as `completed` with the comment "Closed as completed by #N." or "Superseded by #N.". Closed or missing targets, the PR itself and other repositories are skipped. - The parser has no regex. A closing keyword (close, fix, resolve, supersede, replace, any tense) must lead the reference, alone or in a list. A negated, hedged or noun keyword, or one whose subject is another reference, does not count ("may fix", "the rm fix #1", "#100 supersedes #1"). - Verified: `test/internal/close-linked-issues.test.ts` (333 cases) runs the YAML's script against fake `github`, `context` and `core`. Also the 1000-PR parse (Notes). ### Background - GitHub's own keywords are close, fix and resolve (-s, -ed). Each links one reference, and only a merge into the default branch closes it. - `pull_request_target` runs in the base repository with a write token, also for fork PRs. That is safe only when no PR-controlled code runs. Here the description is the only PR input, parsed as text. <details><summary>Notes</summary> A close through the API does not create the "closed this in #N" timeline link that GitHub makes for its own closes. The comment carries the PR number instead. How the parser was calibrated. I pulled the descriptions of the last 1000 merged PRs and listed every line with a keyword next to a reference. The keyword families, list shapes and reference forms in the script are the ones that appear there. A reference is `#1`, `owner/repo#1`, an issue or pull URL (bare or in `<>`), or a markdown link. Four lines would have been wrong with a plain keyword-then-reference rule, and each led to a rule: - "the open `rm` fix #37521" (#38379): "fix" as a noun. Base forms (fix, close, resolve, supersede, replace) count only at the start of a sentence or line, or after will, should, does, and, and a few similar words. "to" is not one of them ("unable to fix #1", "how to fix #1"). - "May also fix #12318 / #10046, untested" (#38242): hedged. may, might, could, would, partially and the negations disqualify the keyword, looking past adverbs such as "also". - "Supersedes the closed #26040" (#36289) and "a comment on closed #35351" (#35365): "closed" as an adjective. A determiner or preposition before the keyword disqualifies it. - "supersedes #33130's optimisation" (#35843): a number that continues into a word is not a reference. Review added: a reference before the keyword is the subject ("#100 supersedes #1"), also through "which" or "that" ("reverts #100, which fixed #1") and across a removed span ("#100 ~~also~~ fixes #1"). A hedge two words before the keyword disqualifies it ("hopefully this fixes #1", "could this fix #1?"). A clause that starts with if, when, once, until or unless is not a statement. The tokenizer keeps a line break as a token so that "Fixes #1" on one line and "Fixes #2" on the next stay two statements. Code spans, fences, indented code, blockquotes, HTML comments and strikethrough are skipped. The block stripping follows CommonMark for fences (also inside a blockquote), indented code, blockquotes with lazy continuation, setext underlines and HTML comments, and GFM for `~~` flanking. Result over the 1000 descriptions: 274 distinct references in 135 PRs. I checked the current state of all of them through GraphQL. All but one are closed (202 issues completed, 5 duplicates, 66 pull requests). The one open target is PR #32292, superseded by merged #36135. No open target is a false positive. Every review change kept this result. Patterns that are deliberately not handled: a bulleted list under "Closes:" on its own line (not seen in the sample), references separated by whitespace only ("#1 #2"), "fix for #1", and GH-1 style references. A `?` after the list is not treated as a question. The block parser tracks no list containers, so a second paragraph of a list item indented by four spaces is read as an indented code block and skipped. A removed span or inline comment reads as one word, so "Fixes <!-- n --> #1" finds nothing. The test suite covers: the phrases above, stopping at the right place in real sentences, CRLF descriptions, URLs with fragments or a `/files` suffix, case-insensitive `Owner/Repo#1`, the fake API where a lookup, an update or a comment fails, the `dry_run` input, an invalid `pr_number` input, an unmerged PR, a PR merged into a non-default branch, the merge event body against a later edit, and a description with no closing statement. The first revision of this PR checked out the repository and ran `scripts/close-linked-issues.ts`. Jarred asked for no checkout and no script file, so the script moved inline into the workflow and the test now reads it out of the YAML. </details> <!-- robobun:evidence:begin --> --- **[stamp-90s]** gate passed · iteration 9 · 2 files touched <details><summary>passes on PR (with fix)</summary> ```console Test-only change. Debug/ASAN (expected pass): $ bun bd test 'test/internal/close-linked-issues.test.ts' $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test test/internal/close-linked-issues.test.ts bun test v1.4.1 (4448a2e) test/internal/close-linked-issues.test.ts: (pass) finds "Fixes #39852" [176.21ms] (pass) finds "Closes #31772. Fixes #31771." [22.28ms] (pass) finds "- Fixes #39930" [12.28ms] (pass) finds "Fixes: #30429" [10.46ms] (pass) finds "FIXES #1" [7.86ms] (pass) finds "(Fixes #1)" [8.97ms] (pass) finds "**Fixes #1**" [10.20ms] (pass) finds "__Fixes #1__" [9.83ms] (pass) finds "_Fixes #1_" [11.25ms] (pass) finds "Fixes **#1**" [9.72ms] (pass) finds "**Fixes** #1" [7.13ms] (pass) finds "**Fixes:** #1" [8.11ms] (pass) finds "Fixes #1 and **#2**" [11.47ms] (pass) finds "Fixes **#1**, **#2**" [9.13ms] (pass) finds "## Why (fixes #13771, closes #30543)" [16.08ms] (pass) finds "Closes #11418" [19.46ms] (pass) finds "Resolves #1. Resolved #2. Resolve #3." [12.09ms] (pass) finds "Fixes #34055, #30327, #24394, #20816, #32403, #11898, #10056." [17.11ms] (pass) finds "Fixes #18192 and #31675 as a consequence" [10.45ms] (pass) finds "Fixes #1, #2, and #3" [10.96ms] (pass) finds "Fixes #1 & #2" [7.63ms] (pass) finds "Closes #33280, Closes #32864 and Closes #29696 (the timer in #32949 is orthogonal)" [20.29ms] (pass) finds "Closes #33182 and #32947 on top of current main (which already has #36304 for catalogs)." [16.12ms] (pass) finds "Fixes #1,\n#2" [7.76ms] (pass) finds "Fixes #1, #2,\nand #3" [9.27ms] (pass) finds "Fixes #1\nand #2" [8.57ms] (pass) finds "Fixes #1\n& #2" [6.80ms] (pass) finds "Fixes #1 and\n#2" [7.31ms] (pass) finds "Supersedes #39908 (same change, moved from a fork branch)" [13.21ms] (pass) finds "Supersedes #38778 and #38391. Carries the entry point arm of #35053." [14.43ms] (pass) finds "Supersedes #39193 and keeps its three tests." [11.48ms] (pass) finds "This supersedes #33306 and #32803. Their tests are kept here." [13.73ms] (pass) finds "- This replaces #33793. Its ... (truncated) Exit: 0 ``` </details> <details><summary>diff hotspot</summary> ``` .github/workflows/close-linked-issues.yml | 950 ++++++++++++++++++++++++++++++ test/internal/close-linked-issues.test.ts | 598 +++++++++++++++++++ 2 files changed, 1548 insertions(+) ``` </details> **gate history** · 29 passed · 0 rejected · iteration 9 <details><summary>evidence per changed file</summary> ``` file reads edits tests .github/workflows/close-linked-issues.yml 6 12 0 test/internal/close-linked-issues.test.ts 3 11 0 ``` </details> <!-- robobun:evidence:end -->
2 participants
What
Response.clone()/Request.clone()tee the body stream and structured-clone every chunk for the second branch. The cloner,structuredCloneForStream, copied each chunk's entire backing ArrayBuffer (buffer->slice(0)) rather than the range the view covers.Chunks produced by Bun's native body streams are subarray views into a shared 256KB-2MB receive buffer (several chunks per buffer, see
#handleNumberResultinReadableStreamInternals.ts), so every clone queued for the second branch dragged along a full copy of that shared buffer. The standard "cache a copy" middleware pattern paid for the body about twice:Measured on 1.4.0 with a 256 MB body: 608 MB RSS without the clone, 1068 MB with it, and the unread clone's queue held 2.2x the body in ArrayBuffers. Node holds ~1x for the same code. Smallest repro of the underlying bug:
Fix
structuredCloneForStreamnow slices[byteOffset, byteOffset + byteLength)and creates the cloned view at offset 0, so a cloned chunk costs exactly its own bytes. This is what the streams spec's byte stream tee does (CloneAsUint8Array), which is the path other engines take for fetch bodies because their body streams are byte streams.The function is only reachable from
Request.clone()/Response.clone()(ReadableStream.prototype.tee()passesshouldClone = false), so nothing else observes the change. The producer side (the shared receive buffer) is intentional and stays as is; normal reads of a body always cost 1x, only the clone path over-copied.Verification
Three tests added to
test/js/web/fetch/body-clone.test.ts, all failing on the released 1.4.0 and passing with this change:Request.clone()/Response.clone()chunk clones do not retain the chunk's whole backing buffer (before: a cloned 64 byte chunk held a 1 MiB buffer)fetch().clone(): chunks buffered for the unread clone own exactly their bytes (before: 16.6 MB of ArrayBuffers retained for 8.4 MB of data)With the fix, the 32 MB fetch scenario retains exactly 32 MB in the unread clone's queue (was 71 MB).