Bun.serve: cancel ReadableStream body for HEAD requests - #33661
Conversation
A HEAD request to a handler returning new Response(readableStream) wrote the head and called end_without_body() without ever releasing the stream. The underlying source's cancel() never ran, so resources acquired in start() (SSE intervals, DB cursors, subscriptions) leaked for the life of the process, one full leak per HEAD probe. Cancel the stream after writing the head so the source's cancel() fires and the body is marked Used. Use cancel_with_reason since the stream was never read and has no reader (cancel() would no-op on the m_reader guard).
WalkthroughModifies HEAD response rendering to cancel and detach a locked ReadableStream body before ending the response, and adds a test that checks repeated HEAD requests trigger balanced start/cancel behavior and stable ReadableStream heap counts. ChangesHEAD Response Stream Cancellation Fix
Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
|
Updated 12:05 PM PT - Jul 7th, 2026
❌ @robobun, your commit 8bb203f has 1 failures in
🧪 To try this PR locally: bunx bun-pr 33661That installs a local version of the PR into your bun-33661 --bun |
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 `@src/runtime/server/RequestContext.rs`:
- Around line 2602-2612: The readable-stream detach/cancel/mark-used flow is
duplicated across this file, including in the current response-body handling
plus the existing handle_resolve_stream and handle_reject_stream paths. Extract
a small helper around the shared “get_body_readable_stream → EnsureStillAlive →
detach_readable_stream → stream action → mark body Used” sequence, parameterized
by the stream action (for example done() versus cancel_with_reason()), and
update all three call sites to use it so the logic stays consistent.
🪄 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: ffd0dd93-b83b-4167-a6d1-768767fc6890
📒 Files selected for processing (2)
src/runtime/server/RequestContext.rstest/js/bun/http/serve.test.ts
|
CI status: the diff is green on every lane that actually ran tests (284 passed). The remaining red is infra only:
|
… transmit (#41011) ### Problem - An async `fetch` handler that returns a streaming `Response` after the client disconnected never tells the stream's source to stop. hono's `streamSSE` loop then never exits and holds its closure until its next write parks. Measured with hono 4.12.28 on bun 1.4.1: +11.7 MB heap per 100 disconnects. - Cause: `on_abort` reclaims the promise cell and frees the `RequestContext`. When the `Promise<Response>` settles later, `on_resolve` (`src/runtime/server/RequestContext.rs:689`) finds no context and drops the `Response` with its body stream open. ### Fix - `cancel_unread_body`, split out of the HEAD path (#33661), detaches and cancels a body stream the server will not transmit (ReadableStreamCancel with reason `undefined`; `ReadableStream::cancel` is not used because it skips a stream with no reader). - `discard_response_body` applies it to a dropped handler result, a `Response` or a settled promise of one, through the new safe `response::from_js_ref`. `discard_handler_result` also subscribes a pending promise, while the context still holds its uWS response, so its value reaches `on_resolve` and is discarded there. - Called from `on_resolve` (reclaimed cell), `handle_resolve`, `on_response` when the request is already aborted, stopped, upgraded, or closed during dispatch, the `error()` early return, `do_render_stream`, and the 204/304 path. No new `unsafe`; the HEAD block's inline deref is gone. - Verified: `test/js/bun/http/serve-pending-promise-abort-leak.test.ts`, 8 new tests that fail on bun 1.4.1. Also `serve.test.ts`, `bun-server.test.ts`, `websocket-server.test.ts`, and the `serve-*` stream and abort files. ### Background - A `RequestContext` is the server's per-request state. While the handler's promise is pending, a GC-managed `NativePromiseContext` cell owns a ref on it. `on_resolve` takes the ref back from the cell. - `on_abort` runs when the connection closes. It reclaims the cell's claim itself and releases the context, so the later `take()` returns `None`. It also fires `request.signal`. - The server reads a stream body by attaching a sink in `do_render_stream`. Until then the stream has no reader. <details><summary>Notes</summary> Reproduction (hono 4.12.28, `streamSSE`, handler awaits 300 ms before returning; client aborts 100 ms after connecting): on bun 1.4.0 and 1.4.1 every round of 100 disconnects leaves 100 `streamJsonChanges` loops alive. hono's `responseReadable` pulls once at start, so the first SSE write goes through and sits in its queue; the loop then polls every second until the next write (a data change, or the heartbeat after 15 s) parks on the `TransformStream`'s backpressure. Only then is the closure unreachable. `heapUsed` grows about 11.7 MB per round while the load runs. The plain connect, read, disconnect pattern (the abort arrives after the stream is attached) does not leak: RSS and object count are flat over 8000 connections. With this fix the loops exit at their next `sleep()` and the heap returns to the idle baseline. `request.signal` is not touched here. Every path that drops a `Response` because the connection is gone already fires it through `on_abort`: the late-resolve case fired it at the abort, and the `server.stop(true)` and closed-during-dispatch cases reach `on_abort` through `set_abort_handler` in the same dispatch. The `stop(true)` tests assert both the abort event and the cancel. HEAD and 204/304 complete normally, so the signal must not fire there. `undefined` is the reason an abort after attachment already delivers to the source's `cancel()`. A `Response` fresh from the handler always holds an unlocked, undisturbed stream (the constructor rejects a locked or disturbed one), so `cancel_with_reason` never reaches the native-locked case that `ReadableStream::cancel`'s reader check guards against. The pending-promise subscription is gated on `resp` still being held. Then the socket under it is closed and `set_abort_handler` runs `on_abort`, which reclaims the claim at once (the "pending Promise<Response> after server.stop(true)" test sees `pendingRequests` at 0 before the promise settles). After an upgrade or a finished abort no teardown runs again, and a claim would pin the context until the promise settles, so those arms discard only a settled value. The `do_render_stream` change replaces a `cancel()` that was a no-op there (no reader yet). That arm needs the socket to close during a nested event loop run and has no portable test (see #40034). Known failures on this container that also fail on main: `serve.test.ts` root range port and `/bun:info` loopback, `bun-server.test.ts` IPv6 listen, and the four `websocket-server.test.ts` `send()` benchmark timeouts on the debug build (#39370). </details> <!-- robobun:evidence:begin --> --- **[review]** gate passed · iteration 0 · 3 files touched <details><summary>fails on main (without fix)</summary> ```console ASAN without fix: BUILD FAILED (no junit output) $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/js/bun/http/serve-pending-promise-abort-leak.test.ts ninja: Entering directory `/workspace/bun/build/debug' [1/165] gen ErrorCode+*.h [2/165] gen JSBuffer.lut.h Generating /workspace/bun/build/debug/codegen/JSBuffer.lut.h from /workspace/bun/src/jsc/bindings/JSBuffer.cpp [3/165] gen generated_host_exports.rs generated_host_exports.rs: 122 exports (host=5, lazy=10, generic=107, rust=0); 244 extern-C blocks audited [4/165] gen cpp.rs (cppbind) [4/165] cargo bun_runtime → libbun_runtime.a [162/165] cxx obj/unified/UnifiedSource-src_jsc_bindings_webcore-13.cpp.o FAILED: obj/unified/UnifiedSource-src_jsc_bindings_webcore-13.cpp.o /usr/bin/ccache /usr/lib/llvm-21/bin/clang++ -march=nehalem -O0 -glldb -g3 -gz=zstd -fno-standalone-debug -fsanitize=address -fno-exceptions -fno-c++-static-destructors -fno-rtti -fno-omit-frame-pointer -mno-omit-leaf-frame-pointer -fvisibility=hidden -fvisibility-inlines-hidden -fno-unwind-tables -fno-asynchronous-unwind-tables -Wno-c23-extensions -ffunction-sections -fdata-sections -faddrsig -fno-semantic ... (truncated) release without fix: 8 FAILED bun test v1.4.1-canary.1 (a6c4cc2) test/js/bun/http/serve-pending-promise-abort-leak.test.ts: (pass) RequestContext is freed when client aborts before Promise<Response> settles (http2: false) [64.57ms] (pass) RequestContext is freed when client aborts before Promise<Response> settles (http2: true) [40.93ms] (pass) Promise<Response> still works normally when not aborted [2.39ms] (pass) resolve() inside abort handler is handled safely [0.98ms] (pass) streaming 413 detaches the response so a late resolve/reject is a no-op [2325.31ms] (pass) chunked request body consumed as a ReadableStream is capped at maxRequestBodySize [8.40ms] (pass) client abort frees the context even while the resolve function stays reachable [1.29ms] (pass) client abort while a direct stream pull() is parked frees the context and rejects a pending req.text() read [1.30ms] (pass) client abort while a direct stream pull() is parked frees the context and rejects a pending for await (req.body) read [1.09ms] (pass) client abort while a direct stream pull() is parked frees the context and rejects a pending req.textStream() read [1.10ms] (pass) pendingRequests drops when the client aborts a parked di ... (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/bun/http/serve-pending-promise-abort-leak.test.ts bun test v1.4.1 (a6c4cc2) test/js/bun/http/serve-pending-promise-abort-leak.test.ts: (pass) RequestContext is freed when client aborts before Promise<Response> settles (http2: false) [2636.36ms] (pass) RequestContext is freed when client aborts before Promise<Response> settles (http2: true) [2084.17ms] (pass) Promise<Response> still works normally when not aborted [31.22ms] (pass) resolve() inside abort handler is handled safely [29.85ms] (pass) streaming 413 detaches the response so a late resolve/reject is a no-op [6837.80ms] (pass) chunked request body consumed as a ReadableStream is capped at maxRequestBodySize [453.02ms] (pass) client abort frees the context even while the resolve function stays reachable [34.19ms] (pass) client abort while a direct stream pull() is parked frees the context and rejects a pending req.text() read [47.07ms] (pass) client abort while a direct stream pull() is parked frees the context and rejects a pending for await (req.body) read [39.03ms] ... (truncated) release with fix: all passed $ bun scripts/build.ts --profile=release [configured] bun-profile → bun (stripped) in 586ms (unchanged) ninja: Entering directory `/workspace/bun/build/release' [1/125] gen ErrorCode+*.h [2/125] gen JSBuffer.lut.h Generating /workspace/bun/build/release/codegen/JSBuffer.lut.h from /workspace/bun/src/jsc/bindings/JSBuffer.cpp [3/125] gen generated_host_exports.rs generated_host_exports.rs: 122 exports (host=5, lazy=10, generic=107, rust=0); 244 extern-C blocks audited [4/125] gen cpp.rs (cppbind) [4/125] cargo bun_runtime → libbun_runtime.a �[1m�[92m Compiling�[0m bun_jsc v0.0.0 (/workspace/bun/src/jsc) �[1m�[92m Compiling�[0m bun_js_parser_jsc v0.0.0 (/workspace/bun/src/js_parser_jsc) �[1m�[92m Compiling�[0m bun_sys_jsc v0.0.0 (/workspace/bun/src/sys_jsc) �[1m�[92m Compiling�[0m bun_css_jsc v0.0.0 (/workspace/bun/src/css_jsc) �[1m�[92m Compiling�[0m bun_ast_jsc v0.0.0 (/workspace/bun/src/ast_jsc) �[1m�[92m Compiling�[0m bun_semver_jsc v0.0.0 (/workspace/bun/src/semver_jsc) �[1m�[92m Compiling�[0m bun_patch_jsc v0.0.0 (/workspace/bun/src/patch_jsc) �[1m�[92m Compiling�[0m bun_http_jsc v0.0.0 (/workspace/bun/src/http_jsc) �[1m�[92m Compiling�[0m ... (truncated) ``` </details> <details><summary>diff hotspot</summary> ``` src/runtime/server/RequestContext.rs | 94 +++++++-- src/runtime/webcore/Response.rs | 8 + .../http/serve-pending-promise-abort-leak.test.ts | 228 +++++++++++++++++++++ 3 files changed, 309 insertions(+), 21 deletions(-) ``` </details> **gate history** · 1 passed · 0 rejected · iteration 0 <details><summary>evidence per changed file</summary> ``` file reads edits tests src/runtime/server/RequestContext.rs 24 29 0 src/runtime/webcore/Response.rs 2 3 0 …st/js/bun/http/serve-pending-promise-abort-leak.test.ts 4 8 0 ``` </details> <!-- robobun:evidence:end -->
Problem
A
HEADrequest to a route that returnsnew Response(readableStream)writes the head and ends the response without ever releasing the stream. The underlying source'scancel()never runs, so anything acquired instart()(SSE intervals, DB cursors, subscriptions) leaks for the life of the process, and the abandoned controller's queue grows unbounded. Everycurl -Ior health-check probe against an SSE/streaming route leaks one full handler's worth of resources.After 8
HEAD /requests with every socket closed:{ starts: 8, cancels: 0, live: 8 }, 8 intervals still firing, 8ReadableStreamobjects retained. TheGET-then-disconnect path on the same route does firecancel()correctly.Cause
do_render_head_response()insrc/runtime/server/RequestContext.rshandlesBody::Value::Locked(_)by writingtransfer-encoding: chunkedand callingend_without_body(), but never cancels or detaches the locked stream. The other unsent-body paths (client abort, stream error) do release it.Fix
In the
Lockedarm, after writing the head, fetch the body'sReadableStream, detach it from theResponse, cancel it, and mark the bodyUsed. This usescancel_with_reasonrather thancancelbecause the stream was never read and has no reader attached;ReadableStream__cancelearly-returns whenm_readeris null. The wire output is unchanged (still200withtransfer-encoding: chunked, no body).Verification
Added tests in
test/js/bun/http/serve.test.tscovering both sync and async handlers; each sends 8 HEAD requests and assertscancel()fired for every one,live === 0, and noReadableStreamheap growth.