Body: downgrade the .body getter's Strong<ReadableStream> once the wrapper's traced slot owns it - #36624
Conversation
… from get_body Reading .body on a Response/Request whose body is a Blob/InternalBlob/string materializes a ReadableStream and stores a Strong handle to it in PendingValue.readable. check_body_stream_ref() moves that Strong into the JS wrapper's traced m_stream WriteBarrier slot so the stream stops being a GC root, but it was only called from construct/to_js/clone, not from the .body getter. The stream snapshots the async context at creation; under Next.js App Router SSR that context (via React's request-scoped dedupe cache) holds the Response, so the un-migrated Strong closed an uncollectable cycle and every request's AsyncLocalStorage store was retained forever. to_error_instance read PendingValue.readable to decide was_disturbed and to error the stream; callers that own the wrapper now pass the stream from get_body_readable_stream() via the new _with_readable variant so clearing PendingValue.readable does not change that behaviour. Fixes #29267
|
Warning Review limit reached
Next review available in: 11 seconds Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the 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 configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
Comment |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
Found 5 issues this PR may fix:
🤖 Generated with Claude Code |
|
Independently arrived at the same Next.js 15.5.6 standalone (
The #36628 branch also has three subprocess regression tests at |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Additional findings (outside current diff — PR may have been updated during review):
-
🔴
src/runtime/webcore/Body.rs:1820-1829— Callingcheck_body_stream_ref()fromget_bodyemptiesLocked.readable, butBufferOutputSinkinsrc/runtime/api/html_rewriter.rsstill reads that field to complete the output Response's.bodystream — afterout = rewriter.transform(asyncResponse); out.body.getReader(), the pendingreader.read()now hangs forever on both the success path (done()→Value::resolveat :943, which seeslocked.readable.get() == Noneand never callsreadable.done()) and the error paths (l.readable.has()at :833 normalizes toEmptysoto_error_instanceat :852 skips the ByteStream; likewise the plainto_error_instancecalls at :898/:917). These sites have theresponsepointer in scope and should callget_body_readable_stream()and pass it through (viato_error_instance_with_readableand an equivalent forresolve), mirroring the three call sites this PR did update.Extended reasoning...
What changed
BodyMixin::get_bodynow callsself.check_body_stream_ref(global_this)(Body.rs:1828) afterto_readable_stream(). When the body isLockedandto_readable_stream→locked_to_native_streamhas just stored aStrong<ReadableStream>inlocked.readable(Body.rs:910),check_body_stream_refmoves that handle into the wrapper's tracedm_streamslot and doesmem::take(&mut locked.readable)(Body.rs:~1715), leavinglocked.readableempty. The PR addedto_error_instance_with_readableand updated three call sites that own the wrapper (FetchTasklet::on_body_received,BodyAbortListener::on_abort,RequestContext::end_request_streaming) to pass the stream in explicitly; the description states "the remaining callers operate on bodies the getter never touched."Why HTMLRewriter is a counterexample
BufferOutputSink::init(html_rewriter.rs:609–621) creates the outputResponsewithValue::Locked(PendingValue { task: sink, on_start_streaming: None, on_receive_value: None, on_readable_stream_available: None, promise: None, readable: empty }), callsto_json it (:715), and returns it to user JS while the async input body is still buffering. User code can then callout.body.getReader()before the input settles.get_bodyseesLocked,get_body_readable_streamreturnsNone(m_stream empty, locked.readable empty), so it falls through tolocked_to_native_stream, creates a ByteStream, setslocked.readable = Strong(stream)— and then, new in this PR,check_body_stream_refmigrates it out again.locked.readableis now empty for the remainder of the sink's lifetime.The four un-migrated sites (all in html_rewriter.rs)
(a) Success path —
done()at :931–945. When the async input finishes,run_output_sink→rewriter.end()emits the final empty chunk →SinkRef::handle_chunk([])(:964) →sink.done().done()doesmem::replace(body_value, InternalBlob(bytes))thenValue::resolve(&mut prev_value, body_value, &global, None).Value::resolve(Body.rs:1082–1086) readslocked.readable.get(global)— nowNone— so it never callsreadable.done().on_receive_valueandpromiseare bothNone, soresolvereturns without touching the ByteStream.(b) Input-body error path —
on_finished_bufferingat :824–863. Line 833 testsl.readable.has(), which is nowfalse. Withl.task == sink && l.promise.is_none()also true, line 842 sets*sink_body_value = Value::Empty.to_error_instanceat :852 onValue::Emptytakes the non-Lockedfallthrough (just*self = Value::Error(err)) and never callsbytes.on_data(Err(...)). The comment at :829–831 explicitly documents the invariant this PR breaks: "If a .body readable is already attached, stay Locked so to_error_instance delivers the error to its ByteStream; clearing to Empty here would strand any pending reader.read() forever." Even without theEmptynormalization, plainto_error_instancewould still findlocked.readableempty andowned_readable = None.(c)/(d) lol-html write/end error paths —
run_output_sinkat :898 and :917. Both call(*response).get_body_value().to_error_instance(err, &global)directly. Withlocked.readableempty and noowned_readable,to_error_instance_with_readable(err, global, None)never reaches the ByteStream branch.Step-by-step proof (success path)
const out = new HTMLRewriter().on("*", {}).transform(await fetch(url)); // input body is async const reader = out.body.getReader(); await reader.read(); // ← never settles
transform()returnsoutwithbody = Locked{task: sink, promise: None, readable: empty}; async input buffering starts.out.body→get_body→locked_to_native_streamsetslocked.readable = Strong(stream)→ newcheck_body_stream_refmoves it tom_streamand emptieslocked.readable.getReader()locks the JS stream.- Input finishes →
on_finished_buffering(bytes, None, true)→run_output_sink→rewriter.end()→done(). done()swaps inInternalBlob, callsValue::resolve(&mut prev_locked, ...).prev_locked.readable.get()isNone;on_receive_value/promiseareNone;resolvedoes nothing to the stream.- The ByteStream never receives data, an error, or a close signal.
reader.read()hangs forever; the stream is kept alive viam_stream.
Before this PR, step 2 did not empty
locked.readable, so step 4 found the stream and calledreadable.done()(resolving the read with{done: true}), and the error paths deliveredbytes.on_data(Err(...)). The success path was already lossy (rewritten bytes land in the replacedInternalBlob, not the ByteStream), but it completed; this PR turns it into an unsettled-promise hang, which per REVIEW.md "pins objects and hangs callers forever."Fix
All four sites have the
responsepointer in scope ((*sink).response/(*self.response)), so they can recover the stream via(*response).get_body_readable_stream(&global), exactly as the three updated call sites do.on_finished_bufferingshould testget_body_readable_stream().is_some()instead ofl.readable.has()and pass the result intoto_error_instance_with_readable; :898/:917 should switch toto_error_instance_with_readable; anddone()should either close the recovered stream around theresolvecall orValue::resolveshould grow anowned_readableparameter matchingto_error_instance_with_readable. This is the "fix the whole class in the same PR" case from REVIEW.md — same pattern, sibling site.
…s released The previous commit cleared Locked.readable entirely after migrating into m_stream, which broke every Value/PendingValue reader that doesn't have the wrapper in scope (to_any_blob_allow_promise, size_hint, Value::resolve, to_blob_if_possible callers, HTMLRewriter done()). Instead, give readable_stream::Strong a non-rooting weak slot: check_body_stream_ref now drops the GC root but keeps the JSValue readable so all existing locked.readable consumers behave exactly as before.
robobun
changed the title
There was a problem hiding this comment.
ec5bd99's downgrade() resolves the earlier locked.readable-readers-see-None concern — to_any_blob_allow_promise, size_hint, Value::resolve, etc. now read the stream via weak. Beyond the new inline finding, Strong::tee() in the downgraded state (storing the fresh teed stream in weak) was also examined and ruled out. The release-mode-passes-without-fix test comment on 29267.test.ts is still open.
Extended reasoning...
The latest commit switched check_body_stream_ref from mem::take to downgrade(), which addresses my previous inline comment on Body.rs:1806 — every locked.readable.get() reader now sees the stream through the new weak slot. This run's finder/verifier pass surfaced one new concern (the ValueBufferer ownership-move at Body.rs:2481 now moves a non-rooting handle where the SAFETY comment at :2260 requires a root), which is posted inline. Two candidate issues around Strong::tee() writing a fresh unrooted stream into weak were examined and refuted. The prior test-validity comment (fixture passes on release main) remains unaddressed — the test file is unchanged. Given the open GC-lifetime finding and the unaddressed test concern, this still needs human review.
| stream.value.ensure_still_alive(); | ||
| Self::stream_set_cached(js_value, global_object, stream.value); | ||
| let _ = core::mem::take(&mut locked.readable); | ||
| locked.readable.downgrade(); |
There was a problem hiding this comment.
🔴 Switching check_body_stream_ref to downgrade() means locked.readable.get() now returns Some via weak after the root is released, so ValueBufferer::buffer_locked_body_value at Body.rs:2481 fires mem::take(&mut locked.readable) and moves a non-rooting handle into self.readable_stream_ref — skipping the Strong::init fallback at :2486. That invalidates the invariant at Body.rs:2260/2514 that readable_stream_ref keeps the raw byte_stream pointer alive across ValueBufferer::Drop's deref. One-line fix: change :2481 to webcore::readable_stream::Strong::init(stream, self.global) to match the fallback branch.
Extended reasoning...
What changed and why it breaks
Before this PR, check_body_stream_ref did mem::take(&mut locked.readable), so afterwards locked.readable.get() returned None. ValueBufferer::buffer_locked_body_value (Body.rs:2476-2488) was written against that contract: when get() is None it falls through to line 2484-2487 and does self.readable_stream_ref = readable_stream::Strong::init(stream, self.global) — a real GC root. When get() is Some, line 2481 does self.readable_stream_ref = mem::take(&mut locked.readable), which was correct because a populated locked.readable always held a live bun_jsc::Strong.
After ec5bd99, check_body_stream_ref calls downgrade(), which empties held and stores the JSValue in weak. Strong::get() now consults weak via the new value() helper (ReadableStream.rs:47-55, 90-96), so locked.readable.get() returns Some after downgrade. Line 2476 fires, line 2481 moves {held: empty, weak: stream} into self.readable_stream_ref, and the rooting fallback at :2484 is skipped. The moved struct's weak field is a bare JSValue (not a JSC Weak handle) sitting in malloc'd memory — it provides zero GC protection.
The invalidated invariant
ValueBufferer records self.byte_stream = NonNull::new(byte_stream_ptr) at Body.rs:2561, and its Drop (Body.rs:2259-2264) dereferences that pointer under the explicit SAFETY comment at :2260: "Kept alive by readable_stream_ref while set — satisfies the BackRef outlives-holder invariant." Three more comments state the same contract (:2249 "readable stream strong ref to keep byte stream alive", :2478-2480 "moving the existing GC root preserves the refcount balance", :2514 "kept alive via self.readable_stream_ref above"). All four are now factually wrong whenever the moved handle came from a downgraded locked.readable.
downgrade()'s own doc-comment (ReadableStream.rs:75-77) states its precondition: "Caller must keep the stream alive elsewhere (the owning Request/Response wrapper's m_stream WriteBarrier slot)." But mem::take at :2481 moves the handle out of the wrapper's ownership tree into a heap-allocated ValueBufferer owned by BufferOutputSink, which Strong-refs only the output Response (html_rewriter.rs:715-717 stores (*sink).response = result), not the input. Nothing in the bufferer ties the stream's lifetime back to the input wrapper's m_stream.
Step-by-step
let res = await fetch(url)— body still streaming (Locked,has_received_last_chunkfalse).void res.body—get_body→to_readable_stream→locked_to_native_streamcreates a ByteStream and setslocked.readable = Strong::init(stream, ...); then the newcheck_body_stream_refcall at Body.rs:1805 storesstreamin the wrapper'sm_streamand downgradeslocked.readableto{held: empty, weak: stream}.const out = new HTMLRewriter().on(...).transform(res)—begin_transform→bufferer.run(value, owned_readable_stream).to_blob_if_possible()fails (last chunk not received) →buffer_locked_body_value. Line 2476:locked.readable.get()returnsSomeviaweak. Line 2481:mem::takemoves the weak-only handle intoself.readable_stream_ref. Line 2493:*value = Used. Line 2561:self.byte_stream = NonNull::new(byte_stream_ptr).res = null— the inputResponsewrapper is now unreferenced from JS.FetchTasklet.responseis ajsc::Weak, so it doesn't root the wrapper. The stream is reachable only via the wrapper'sm_stream(dies with the wrapper) andFetchTasklet.readable_stream_ref(a real Strong while the fetch runs).- Fetch completes;
FetchTaskletdeinits itsreadable_stream_ref. GC now sees the input wrapper unreachable → itsm_streamedge is gone → theReadableStreamis unmarked →NewSource<ByteStream>is finalized and freed. BufferOutputSinkrefcount hits zero →ValueBufferer::Dropruns →BackRef::from(byte_stream).unpipe_without_deref()on freed memory.
On the normal completion path, ValueBufferer::Drop runs synchronously inside the ByteStream's sink dispatch (via on_finished_buffering), so the ByteStream is on the call stack and the FetchTasklet Strong is still held — the exact interleaving above may not fire on the happy path. But the code now works by incidental call ordering, not by the documented invariant; any change to when BufferOutputSink drops (an extra ref, deferred deref, or an abort path that clears the FetchTasklet ref without invoking the sink) turns this into a live UAF. Per REVIEW.md's memory-safety section — "Never let a pointer or slice outlive the memory it points into", "never raw JSValues in malloc'd memory", "be able to answer 'who frees this, when, on which paths' in one sentence" — a raw byte_stream pointer whose documented liveness guarantee has been silently invalidated is exactly the class this section blocks on.
Not a duplicate
The previous inline comment on Body.rs:1806 flagged the earlier commit's mem::take-in-get_body approach breaking readers of locked.readable; ec5bd99 switched to downgrade() to fix that. This finding is the converse: downgrade() keeps the readers working but breaks the one ownership transfer (line 2481) that depended on the old "populated ⇒ rooted" invariant. The PR description's own claim — "Every existing locked.readable reader continues to see the stream; only the GC-root behaviour changes" — is precisely the problem: line 2481 is not a reader, it's an ownership move.
Fix
One line, zero downside — change Body.rs:2481 from
self.readable_stream_ref = core::mem::take(&mut locked.readable);to
self.readable_stream_ref = webcore::readable_stream::Strong::init(stream, self.global);matching the fallback branch at :2486-2487. ValueBufferer needs a real root regardless of the source handle's state; *value = Used two lines later drops locked.readable anyway, so the extra Strong doesn't reintroduce the leak this PR fixes (that leak is about Locked.readable living inside a Response reachable from an ALS store, not about a transient bufferer). The now-stale comment at :2478-2480 can then be dropped.
…n't deref a freed NewSource box (#36799) ## What `fetch(url, { signal })` + `resp.body.getReader()` + `reader.cancel()`, drop the Response, then abort the signal: heap-use-after-free on the response body stream's native `Box<NewSource<_>>`. Reproduces 5/5 under ASAN on `main`, clean on `5b7c3cacbe63` (before #36624). ``` READ of size 1 in NewSource<ByteBlobLoader>::cancel (ReadableStream.rs:936) <- ReadableStream::done <- ReadableStream::error <- BodyAbortListener::on_abort (Response.rs:142) <- AbortSignal::runAbortSteps <- Timeout::run <- __bun_fire_timer freed by: JSDestructibleObjectDestroyFunc <- PreciseAllocation::sweep <- sweepPreciseAllocations <- Heap::sweepInFinalize allocated by: Box::new(NewSource<..>) <- Value::to_readable_stream <- Response.body getter ``` Minimal repro (crashes at iteration 0 on debug and release ASAN): ```js const server = Bun.serve({ port: 0, fetch: () => new Response(Buffer.alloc(20000, "x")) }); async function one(signal) { const resp = await fetch(server.url, { signal }); const rd = resp.body.getReader(); await rd.read(); await rd.cancel(); } for (;;) { const ac = new AbortController(); await Promise.all([one(ac.signal), one(ac.signal), one(ac.signal)]); Bun.gc(false); // must NOT be gc(true) await Bun.sleep(5); ac.abort(); await Bun.sleep(5); } ``` ## Cause #36624 changed `readable_stream::Strong` so that `check_body_stream_ref` downgrades `Body.Locked.readable` from a `bun_jsc::Strong` to a raw `weak: JSValue` once the Response wrapper's traced `m_stream` WriteBarrier slot owns the stream. That raw JSValue is never cleared when the stream is collected. After `reader.cancel()` (ByteStream path) or once the body is fully buffered (ByteBlobLoader path), nothing but the Response wrapper's `m_stream` roots the stream. When the user drops the Response, one eden GC: 1. reaps weak handles (so a real `JSC::Weak` would already read `None`), 2. runs `sweepInFinalize` -> `sweepPreciseAllocations`, which sweeps the `JS{Bytes,Blob}InternalReadableStreamSource` cell **synchronously**; its destructor runs `NewSource::finalize` -> `decrement_count` -> `drop(Box<NewSource<_>>)`, 3. leaves the `JSResponse` wrapper's MarkedBlock cell for lazy sweep. `Response::finalize` (the wrapper cell destructor) is what sets `js_ref` to `Finalized` and drops `abort_listener`, so in that window the native `Response` is still alive, `Locked.readable` still holds the stream's raw JSValue, and `BodyAbortListener` is still registered. `AbortSignal.timeout` / `ac.abort()` then fires: * `on_abort` -> `get_body_readable_stream` reaches the dead stream through either `js_ref()` (raw JSValue to the dead-but-unswept wrapper) or `Locked.readable`'s raw JSValue -> `ReadableStreamTag__tagged` -> `m_nativePtr` -> the destructed source cell's `m_ctx` -> the freed `Box<NewSource<_>>`, and * `Value::to_error_instance` reads the same handle via `strong_readable.get()` -> `bytes.on_data()`. `Bun.gc(true)` does not reproduce: a full synchronous sweep runs the wrapper destructor in the same pass, which drops the listener before abort can fire. ## Fix * `readable_stream::Strong::weak` is now `bun_jsc::Weak<()>` instead of a raw `JSValue`. `downgrade()` creates it with the new `WeakRefType::None` (no finalize owner; `JSC::Weak` accepts a null owner). `get()` / `has()` / `is_disturbed()` / `tee()` all see `None` once the stream is reaped, so `Value::to_error_instance` and every other `Locked.readable` reader skip the freed `NewSource` deref. * `BodyAbortListener::on_abort` reads the stream via `Locked.readable` directly instead of `get_body_readable_stream`, whose `js_ref()` path would still read a raw `JsRef::Weak(JSValue)` to the dead-but-unswept wrapper. `Locked.readable.get()` returns the live stream when a reader roots it without the wrapper, and `None` exactly when the stream (and its `NewSource` box) is collected. * `bun_jsc::Weak::create_passive` and a `WeakRefType::None` arm in `Bun__WeakRef__new` support an ownerless `JSC::Weak`. No user-visible behaviour changes: when the stream is collected `readable.error()` was already a no-op (`webStreamControllerError` early-returns on a non-Readable stream; `NewSource::cancel` early-returns on `cancelled`); when the stream is alive `readable.error()` runs exactly as before. ## Verification * `test/js/web/fetch/fetch-abort-after-cancel-gc-fixture.ts` + test: crashes at iteration 0 on `main` under debug+ASAN (`bun bd test`), passes with this PR. * release+ASAN: 5/5 clean on both the fixture (100 iters) and the original fuzzer repro (300 iters); `main` crashes 5/5 and 4/4 respectively. * `test/js/bun/http/serve-http3.test.ts` "client abort during streaming response" passes 3/3 on release+ASAN (regressed on the first commit of this PR; fixed in c5da841). * #36624's leak regression test `test/regression/issue/29267` still passes (the downgrade is preserved; only the storage is now a real Weak). * `test/js/web/fetch/{fetch-abort-stream-body,body,body-stream,body-clone,fetch-abort-queued,fetch-stream-cancel-leak}.test.ts` and `test/js/web/streams/readable-stream-terminal-barrier-release.test.ts` unchanged vs `main`. <!-- robobun:evidence:begin --> --- **no test proof** · iteration 1 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/web/fetch/fetch-abort-stream-body.test.ts <!-- robobun:evidence:end --> --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
…ry connections Bun 1.3.x leaks one request's worth of memory per Next.js SSR render (oven-sh/bun#29267): Next's fetch dedupe reads `.body` on a buffered Response, Bun roots the resulting ReadableStream in a Strong handle, and its async context pins the whole per-request store. Pods climbed to their 1Gi limit and were OOMKilled about every 30 hours, burning 3+ cores in GC for hours beforehand. Fixed in Bun 1.4.0 (oven-sh/bun#36624); the image pinned oven/bun:1.3, which never got it. Locally with a 1Gi limit, v1.13.0 (bun 1.3.14) was OOMKilled after ~450 home page requests; bun 1.4.2 held ~240MB flat over 3200. The /api/v1/query fetches are the only POSTs, so they are the only ones a stale keep-alive socket surfaces on (GETs are retried) and they threw ECONNRESET into the section error boundary. They now fall back to the card's existing failure state. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
2 participants
What
bun --bun next starton a Next.js 16 App Router app grew the JS heap unboundedly under SSR load (RSS ~3 GB and OOM after ~12,000 requests in the reporter's 3 GB container) while Node stayed flat.A heap snapshot after 2,000 requests showed one
ReadableStreamper request rooted viaStrongRootBlock, and each one reaching the entire per-requestAsyncLocalStoragestore throughm_asyncContext:Cause
Reading
.bodyon aResponse/Requestwhose body is aBlob/InternalBlob/string materializes aReadableStreamand caches areadable_stream::Stronghandle to it inPendingValue.readable.check_body_stream_ref()exists to move that handle into the wrapper's GC-tracedm_streamWriteBarrierslot (so the stream is owned by the wrapper, not rooted independently), but it was only called fromconstruct/to_js/clone, not from the.bodygetter.JSReadableStreamsnapshots the current async context inm_asyncContextat creation. Next's per-request ALS store (viadedupe-fetch'sReact.cache((url) => [])andcloneResponse, which reads.bodyon aResponsebuilt from a bufferedArrayBuffer) holds theResponsethat owns thatStrong. TheStrongrooted the stream, the stream'sm_asyncContextreached the store, the store reached theResponse, and theResponsewrapper never finalized, so theStrongnever dropped.Minimal reproduction (no Next.js, no network):
500 iterations leave 500
ReadableStreaminprotectedObjectTypeCountsand ~30 MB of heap retained after full GC.Fix
PendingValue.readableserves two roles: it roots the stream for GC, and it letsValue/PendingValueconsumers (to_any_blob_allow_promise,size_hint,resolve,to_error_instance,to_blob_if_possible, ...) find the stream without the wrapper in scope. Clearing it from the getter broke those consumers; this PR instead keeps the JSValue readable while releasing the GC root:readable_stream::Stronggains aweak: JSValueslot and adowngrade()that drops thebun_jsc::Strongroot and moves the value intoweak.has()/get()/is_disturbed()/tee()consultweakwhen the root is empty.check_body_stream_ref()nowdowngrade()s instead ofmem::take()ing.BodyMixin::get_bodycallscheck_body_stream_ref()afterto_readable_stream()so the wrapper's tracedm_streamslot owns the stream from the getter too.Every existing
locked.readablereader continues to see the stream; only the GC-root behaviour changes.Verification
Reporter's Next.js 16.2.3 repro, 4,000 unique-URL requests, release build from
main:heapStats().heapSizeprotectedObjectTypeCounts.ReadableStreamtest/regression/issue/29267runs the minimal cycle in a subprocess and assertsprotectedObjectTypeCounts.ReadableStreamand the liveResponse/ReadableStreamcounts stay bounded after 500 iterations + full GC.Fail-before on release
df49a6e1c:Related suites pass on release and debug+ASAN:
test/js/web/fetch/body.test.ts,body-clone.test.ts,body-mixin-errors.test.ts,body-stream.test.ts,fetch.stream.test.ts,readable-stream-blob-consumed.test.ts,test/js/workerd/html-rewriter.test.js,test/js/bun/http/serve-body-leak.test.ts;serve.test.tsfailures are identical tomain.Fixes #29267
Fixes #16339
[review] gate passed · iteration 0 · 4 files touched
fails on main (without fix)
passes on PR (with fix)
diff hotspot
gate history · 2 passed · 0 rejected · iteration 0
evidence per changed file