Bun.serve: apply TCP backpressure to a request body the handler reads slowly - #36006
Conversation
… slowly When a Bun.serve handler consumes req.body (via for-await, getReader, or pipeTo) slower than the client sends, the uWS onData callback kept pushing every chunk into the ByteStream's internal buffer without ever pausing the socket. A client on loopback could therefore finish writing a multi-GB PUT in a few hundred milliseconds while the server's RSS grew to ~body-size and the next read() returned one body-sized mega-chunk. The node:http server on the same uWS socket already paused reads via NodeHTTPResponse; the Bun.serve req.body path simply never wired the equivalent. on_buffered_body_chunk now pauses the socket once the ByteStream's unconsumed buffer (or the pre-stream request_body_buf, for a handler that has not touched req.body yet) crosses 1 MiB. The existing Body/PendingValue on_stream_drained hook is wired so the ByteStream's signal_drained (fired when a read() empties the buffer) resumes the socket. on_start_buffering resumes and opts out for .text()/.json()/... which need the whole body. The drain handler is detached from the stream source before the request context is released so the source cannot call back into a freed context, and detach_response resumes a paused socket so keep-alive connections return to a readable state after an early response. Fixes #4970
|
Updated 8:26 PM PT - Jul 26th, 2026
✅ @robobun, your commit 2f9c163312a44a4b8cd00d8b08fb5110691e2127 passed in 🧪 To try this PR locally: bunx bun-pr 36006That installs a local version of the PR into your bun-36006 --bun |
|
Found 1 issue this PR may fix:
🤖 Generated with Claude Code |
|
Re: the find-issues bot, this does not fix #35283. That issue is about draining (or connection-closing for) the unread tail of a request body when the handler responds early on a keep-alive connection; the leftover bytes misframe the next request. The |
|
Warning Review limit reached
Next review available in: 8 minutes 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 (1)
WalkthroughRequest-body handling now applies high-water backpressure, stream-drain resumption, buffering overrides, teardown cleanup, and regression coverage for streamed and fully buffered request bodies. ChangesRequest-body backpressure
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/bun/http/serve.test.ts`:
- Line 3271: Update the issue reference comment adjacent to the request-body
backpressure test: remove the stale `#4970` URL or replace it with the issue that
actually tracks this behavior. Do not change the test logic.
- Around line 3321-3349: The duplicated write-pump and plateau-poll logic should
be consolidated into a shared helper. Extract the block around writeMore and the
sent/stable polling loop into a helper that accepts the socket, BLOCK, and TOTAL
inputs, centralizes the threshold constants, and returns { sent, drainWaiters };
update all three tests to use this helper, preserving the existing backpressure
behavior.
- Around line 3314-3319: Update all three raw socket test setups at
test/js/bun/http/serve.test.ts lines 3314-3319, 3396-3400, and 3461-3465 to
attach a persistent no-op error handler after the connect promise resolves. Wrap
each corresponding test body so sock.destroy() at lines 3355, 3432, and 3497
runs in a finally block, including when an await or assertion fails.
- Around line 3387-3390: Update the test’s pre-stream buffered request-body loop
around the async iteration over req.body to accumulate or compare the received
chunks’ content, not only their lengths. Mirror Test 1’s existing contentOk
assertion so corrupted leading bytes fail this test while preserving the current
byte-count and maxChunk checks.
🪄 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: a0471428-7f8c-4e88-8f86-656fe3542d4f
📒 Files selected for processing (4)
src/runtime/server/RequestContext.rssrc/runtime/server/mod.rssrc/runtime/server/server_body.rstest/js/bun/http/serve.test.ts
…yBuffer body past loopback socket buffers
…ready-responded-stream teardown path A handler that materialises req.body and then calls .text()/.arrayBuffer() routes through ByteStream::to_buffered_value (buffer_action mode), which never drives on_pull, so the HWM pause was never released. Skip the pause (and release any earlier one) when buffer_action or a native pipe is set. end_already_responded_stream() is the one HTTP/1 teardown path that bypasses detach_response; mirror the resume there so a keep-alive socket whose body was paused is readable for the next request. on_request_body_stream_drained_callback now dispatches via *mut Self (borrow = ptr): ByteStream::on_data re-enters it from inside on_buffered_body_chunk while that frame already holds &mut Self. Tests: add error() handlers that reject serverDone, and a variant that touches req.body before .arrayBuffer() to cover the buffer_action path.
There was a problem hiding this comment.
Additional findings (outside current diff — PR may have been updated during review):
-
🟡
src/runtime/server/RequestContext.rs:4479— The comment at line 4474 still says "sobitflags!over u16 works", but this PR widenedFlagsBitstou32to fitREQUEST_BODY_PAUSED(1<<16) andREQUEST_BODY_BUFFER_ALL(1<<17). Update the comment to say u32, or drop the width mention entirely since it's now self-evident from the type on the next line.Extended reasoning...
What
The PR widens
FlagsBitsfromu16tou32at RequestContext.rs:4479 so the two new flag bits —REQUEST_BODY_PAUSED = 1 << 16andREQUEST_BODY_BUFFER_ALL = 1 << 17— fit. The comment two lines above (RequestContext.rs:4474) was not updated and still reads:"All flags are bool (with two debug-conditional ones), so
bitflags!over u16 works."That is now factually wrong:
bitflags!overu16does not work anymore, which is precisely why the PR changed the type.Where
// All flags are bool (with two debug-conditional ones), so `bitflags!` over u16 // works. We keep all bits in every build and just gate the // `is_web_browser_navigation` / `has_finalized` accessors on the const params. bitflags::bitflags! { #[derive(Default, Clone, Copy)] pub struct FlagsBits: u32 { // <- widened by this PR
The diff shows
-pub struct FlagsBits: u16/+pub struct FlagsBits: u32without touching the comment immediately above it.Why it matters
REVIEW.md's "Comments carry only durable non-obvious content" and "one source of truth" rules apply: a comment that contradicts the line it annotates is worse than no comment — the next reader has to figure out which one is lying. This one is easy to miss because the comment and the type are on separate lines and the diff only touched the type line.
Impact
None at runtime — this is documentation only. The compiler already enforces the real constraint (bit 17 wouldn't fit in a
u16), so nothing can silently go wrong. It's purely reader-facing drift.Step-by-step proof
- Before this PR: 16 flags (bits 0..=15),
FlagsBits: u16, comment says "over u16 works" — comment and code agree. - This PR adds
REQUEST_BODY_PAUSED = 1 << 16andREQUEST_BODY_BUFFER_ALL = 1 << 17. Bit 17 requires ≥ 18 bits of storage. - The PR changes line 4479 to
FlagsBits: u32to accommodate this. - Line 4474 still says "over u16 works" — now false, since 18 flags do not fit in 16 bits.
Fix
Either change "u16" → "u32" on line 4474, or (arguably better per REVIEW.md's guidance against comments that restate what the code shows) drop the width mention entirely — the
: u32on the next line already states it, so the comment could reduce to just the debug-conditional / accessor-gating note. - Before this PR: 16 flags (bits 0..=15),
Main fails to build on all lanes since bfd1e92 (Buildkite main #83211, #83215). ## Cause #36067 renamed `AnyResponse::resume_` to `AnyResponse::resume` in `src/uws_sys/Response.rs`. #36006, merged just before it, added six new callers of the old name. Both passed CI on their own branches; combined on main: ``` error[E0599]: no method named `resume_` found for enum `AnyResponse` in the current scope --> src/runtime/server/RequestContext.rs:2382:22 --> src/runtime/server/RequestContext.rs:4126:18 --> src/runtime/server/RequestContext.rs:4182:22 --> src/runtime/webcore/streams.rs:1408:25 --> src/runtime/webcore/streams.rs:1785:17 --> src/runtime/webcore/streams.rs:1873:21 ``` ## Fix Rename the six call sites to `resume()`. ## Verification - `cargo check -p bun_runtime` on main: 6× E0599; with this change: clean. - `bun bd` builds. - `bun bd test test/js/bun/http/serve.test.ts -t 'request body backpressure'`: 5/5 pass (the tests #36006 added that exercise these call sites). <!-- 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/bun/http/serve.test.ts <!-- robobun:evidence:end -->
… slowly (#36006) ## Repro A `Bun.serve` handler that reads `req.body` slower than the client sends gets no TCP backpressure: the client is never throttled and the whole remaining body is buffered in server memory. ```js const s = Bun.serve({ port: 0, maxRequestBodySize: 2 ** 33, async fetch(req) { let n = 0, mx = 0; for await (const c of req.body) { n += c.length; mx = Math.max(mx, c.length); await Bun.sleep(n / 5e6 * 1000 - performance.now()); // ~5 MB/s sink } return Response.json({ bytes: n, maxChunkMB: Math.round(mx / 1e6) }); }, }); // raw-socket client PUTs 60 MB at loopback speed ``` Before: `{"clientWriteMs":116}` / server `{"bytes":60000000,"maxChunkMB":55,"peakRssMB":204}` (client dumps 60 MB in 116 ms; one 55 MB chunk). After: `{"clientWriteMs":10586}` / server `{"maxChunkMB":1}` (client throttled to the sink rate; every chunk ≤ ~1.4 MB). The same shape held for `pipeTo(slowWritable)`, for `getReader()` + one `read()` then idle, and for a handler that did not touch `req.body` at all while the body arrived. Bun's own `node:http` server already applied backpressure on the same uWS socket (`NodeHTTPResponse::pause_socket`), so the primitive was in place; the `Bun.serve` `req.body` path just never used it. ## Cause `RequestContext::on_buffered_body_chunk` forwards every uWS `onData` chunk into `ByteStream::on_data`. When no JS reader is waiting, `on_data` appends to the ByteStream's internal `buffer: Vec<u8>` and returns; nothing observed that buffer's size and nothing called `resp.pause()`. For a body that has not been touched yet the chunk is appended to `request_body_buf` with the same unbounded growth. ## Fix **Pause.** `on_buffered_body_chunk` pauses the socket once the ByteStream's unconsumed buffer (or the pre-stream `request_body_buf`) crosses a 1 MiB high-water mark. The ByteStream's existing `signal_drained` (fired from `on_pull` when the buffer empties) is wired to resume via `on_stream_drained` on the request's `PendingValue`. **Whole-body consumers.** A consumer that wants the whole body never drives `on_pull`, so pausing would wedge it: - `.text()`/`.json()`/`.arrayBuffer()` on an untouched body fire `on_start_buffering`, which resumes and sets `REQUEST_BODY_BUFFER_ALL` to suppress further pre-stream pausing. - `.text()` after `req.body` has been touched goes through the ByteStream's `buffer_action`; `on_buffered_body_chunk` skips the pause when `buffer_action` or a native `pipe` is set. - `Bun.write(file, req)` registered `on_receive_value` without calling `on_start_buffering`; it now does, mirroring `BodyValueBufferer`. **Stale-`resp` window.** Once a streaming-response sink calls `res.end()`, uWS `markDone()` drops `onAborted` and (for `Connection: close`) the socket may be freed on the next loop tick while the `RequestContext` still holds `resp`. The sink resumes the socket at both points it sets `ended_response = true` (same frame as `res.end()`, so the socket is at worst closed, never freed). Every Rust-side resume path consults `resp_may_be_freed()` (i.e. `sink.ended_response`) and clears the flag without dereferencing `resp` once the sink has ended. `handle_resolve_stream`/`handle_reject_stream` clear `REQUEST_BODY_PAUSED` and the ByteStream `drain_handler` immediately after reading `ended_response`, before `detach()`/`run_error_handler` can re-enter JS. `end_already_responded_stream` and `detach_response` clear the flag without dereferencing. uWS itself is unchanged, so node:http's own pause owners (`pausePipelineReads`, `IncomingMessage` pause, the C++ pipeline-flood guard) are unaffected. Two `FlagsBits` are added (widening the set to `u32`): `REQUEST_BODY_PAUSED` and `REQUEST_BODY_BUFFER_ALL`. ## Verification `bun bd test test/js/bun/http/serve.test.ts -t "request body backpressure"` Five tests next to the existing response-side backpressure tests: a stalled streaming reader, a handler that defers touching `req.body`, `Bun.write(file, req)` after the pre-stream pause, and `.arrayBuffer()` with and without `req.body` touched first. Each writes a 32 MiB body from a raw socket, polls the client's `sent` counter until it plateaus, and asserts the plateau is well short of the total and the largest chunk the handler sees is under 4 MiB. All five fail on `main` and pass with this change; bytes are delivered intact. The stale-`resp` paths (reader.read and req.text inside a direct-stream `pull()` after `c.end()` across a loop tick) were verified under ASAN. `node-http.test.ts` "pipelined responses buffered past the high water mark pause reads on the connection" passes. ## Binary size The +~530 KB flagged by the size check is measured against main build 79916, the last passing canary. Twelve commits landed on main between that baseline and this PR's parent, including `node:quic` (#32602), the full `node:repl` (#31827), and `node:inspector` (#31823), none of which has produced a passing canary yet. <!-- robobun:evidence:begin --> --- **no test proof** · iteration 2 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/bun/http/serve.test.ts <!-- robobun:evidence:end -->
Main fails to build on all lanes since bfd1e92 (Buildkite main #83211, #83215). ## Cause #36067 renamed `AnyResponse::resume_` to `AnyResponse::resume` in `src/uws_sys/Response.rs`. #36006, merged just before it, added six new callers of the old name. Both passed CI on their own branches; combined on main: ``` error[E0599]: no method named `resume_` found for enum `AnyResponse` in the current scope --> src/runtime/server/RequestContext.rs:2382:22 --> src/runtime/server/RequestContext.rs:4126:18 --> src/runtime/server/RequestContext.rs:4182:22 --> src/runtime/webcore/streams.rs:1408:25 --> src/runtime/webcore/streams.rs:1785:17 --> src/runtime/webcore/streams.rs:1873:21 ``` ## Fix Rename the six call sites to `resume()`. ## Verification - `cargo check -p bun_runtime` on main: 6× E0599; with this change: clean. - `bun bd` builds. - `bun bd test test/js/bun/http/serve.test.ts -t 'request body backpressure'`: 5/5 pass (the tests #36006 added that exercise these call sites). <!-- 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/bun/http/serve.test.ts <!-- robobun:evidence:end -->
…kpressure test (#36088) `serve.test.ts` has been red on the darwin lanes since #36072 landed (main #83238, also the PR's own build #83229 as a retry-flake). The subtest is `request body backpressure > releases a paused request body when the handler responds without reading it`, which #36072 added to cover the `detach_response()` resume at `RequestContext.rs:2382`. ``` EPIPE: broken pipe, write syscall: "write", errno: -32, code: "EPIPE" ✗ request body backpressure > releases a paused request body when the handler responds without reading it ``` ## Cause The test sends a 32 MiB `PUT` over a raw `net.Socket` with `Connection: close`, lets the server's pre-stream pause stall the upload, then has the handler respond without touching `req.body` and waits for the client to receive the response. Once the handler returns, `render_bytes()` writes the response via `try_end()` and uWS does `shutdown(SHUT_WR)` immediately followed by `close()` (`HttpResponse.h:860-871`). Because the socket was paused, the unread body bytes are still sitting in the kernel recv buffer, and `close()` with unread recv data sends **RST**. On macOS that RST races two things: 1. the upload pump's `sock.once('drain', writeMore)`, which is still armed from `pumpUploadUntilPlateau`; when it fires and writes into the reset peer the socket emits EPIPE, which the test's `sock.once('error', reject)` turns into a failure, and 2. delivery of the already-written response bytes, which the RST can drop before the client's `'data'` handler sees them. On Linux the `'data'` event reliably wins the race, so the test passed there; on darwin it loses ~70-80% of the time (verified on `darwin-test-arm64-3` and `darwin-test-x64-2` with the main #83238 binary). `detach_response()`'s `resp.resume()` is also a no-op on this path: for a simple text body `try_end()` has already closed the socket by the time it runs (`us_socket_resume` early-returns on a closed socket). So with `Connection: close` the test was not actually observing the code it was written to cover. ## Fix Switch this one test to `Connection: keep-alive`. The server does not close after responding, so there is no RST, the response is delivered deterministically, and `detach_response()`'s resume runs on a live socket. The test now additionally asserts that the client's upload progresses past the plateau after the response, which is the direct observable of the resume (without it the keep-alive socket would stay paused and `getSent()` would never move). Also: resolve the response promise on the body content rather than the header terminator, and drop the `sock.once('error', reject)`; neither race applies on a keep-alive connection but both were latent flake on the old path. The `pumpUploadUntilPlateau` helper gains a `connection` parameter (defaulting to `"close"`, so the other five callers are untouched) and returns a `getSent()` accessor. ## Verification With the main #83238 `bun` binary: | lane | before | after | | --- | --- | --- | | darwin 26 aarch64 (`darwin-test-arm64-3`) | 7/10 fail (EPIPE) | 50/50 pass | | darwin 14 x64 (`darwin-test-x64-2`) | 8/10 fail (EPIPE) | 20/20 pass | | linux x64 debug+ASAN | 10/10 pass | 10/10 pass | `USE_SYSTEM_BUN=1 bun test test/js/bun/http/serve.test.ts -t 'releases a paused'` still fails on `toBeLessThan(TOTAL)` (no #36006 backpressure), so the test still exercises the same `src/` change. This is a test-only diff; fail-before is against a Bun without #36006, not against this PR's own `src/`. <!-- robobun:evidence:begin --> --- **no test proof** · iteration 0 · Platform-specific test-only change; deferring to CI. <!-- robobun:evidence:end -->
…aused (#37977) ### Problem - On Windows, a `node:http` server whose handler stops reading the request body (`req.pause()`, or simply not consuming `req`) keeps accepting the upload at full speed; the bytes pile up in native memory until the request is resumed. Same scenario as #26332, which was filed from Windows: #34740 bounded the JS-side `IncomingMessage` buffer on every platform, but on Windows that only moved the growth into the native pause buffer. - Repro below, 2.5 s after `req.pause()` (loopback upload of 256 KiB chunks): Windows x64 canary `9a543cc18` has pulled 8195 chunks (2 GB) and RSS is 2.1 GB and climbing; Linux stays at 12 chunks and 37 MB; Node v26 on Windows stays at 15 chunks. - A client that finishes its upload and half-closes while the request is paused also gets its request aborted on Windows (the body was parked natively, so `'end'` never fired before the FIN arrived); Node and Bun on Linux deliver the body and the response. - Cause: `NodeHTTPResponse::do_pause` (`src/runtime/server/NodeHTTPResponse.rs`) re-arms uWS `onData` with `on_buffer_paused_shim`, which appends every chunk to `buffered_request_body_data_during_pause` with no bound, but the `self.pause_socket()` call that stops the kernel reads was under `#[cfg(not(windows))]` (`// TODO: figure out why windows is not emitting EOF with UV_DISCONNECT`). Every pause path (`req.pause()`, the `push() === false` -> `readStop(socket)` path from #34740, `req.socket.pause()`) ends in `do_pause`, so none of them reached TCP on Windows. ### Fix - Remove the cfg guard: `do_pause` calls `pause_socket()` on every platform (and `pause_socket` loses the `#[allow(dead_code)]` that existed only because it was dead on Windows). No other code changes. - Why the guard is obsolete: it was added in #18599 (March 2025), which taught the epoll and kqueue backends to still see a peer FIN/RST on a socket that is polling for nothing (`EPOLLRDHUP|EPOLLHUP|EPOLLERR`, a kept `EVFILT_WRITE`) but had no equivalent for the libuv backend. #32488 added that equivalent: `us_poll_start`/`us_poll_change` in `packages/bun-usockets/src/eventing/libuv.c` always arm `UV_DISCONNECT`, `poll_cb` probes a paused socket with `MSG_PEEK` to tell a graceful FIN (deferred until resume) from a reset (closed immediately), and the shared dispatch in `loop.c` defers EOF for a paused socket until it resumes. The symptom the TODO names is exactly what #32488 fixed. - Why a paused `node:http` socket always gets resumed: `do_resume` calls `resume_socket()` before any flag checks, and `end()`, `writeHeadAndEnd` and `abort()` resume the socket first as well, so a response ending with an unread body (`req._dump()` after `res.end()`) or a teardown re-arms the poll and any deferred FIN is delivered. This is the behavior Linux and macOS have had all along; this change gives Windows the same one. - Same primitive, already live on Windows: `Bun.serve` request-body backpressure (#36006, `RequestContext::pause_request_body_socket`) and the node:http pipelining flood guard (`pause_socket_reads`) call the same `uws_res_pause` -> `us_socket_pause` on every platform. - Verification: `test/js/node/http/node-http-backpressure.test.ts`, new `request body` group. Each stall test uploads a 32 MiB body into a request that is paused (explicitly, or implicitly by never being read) and requires the client's upload to stall short of the total, then resumes and requires all 32 MiB plus a 200 response; run over both http and https. A fifth test sends a small body plus FIN while the request is paused and requires them to be delivered on resume. - Windows x64, debug build without the fix: the 4 stall tests fail (`Expected: < 33554432, Received: 33554432`); with the fix the whole file passes (19/19). The FIN test passes on both and is coverage for the newly enabled deferred-EOF path, not the fail-before proof. - Linux, debug build: the file passes before and after (the compiled code is unchanged there), so the fail-before half of this proof exists only on Windows. - The standalone version of the stall scenario, same script under Node v26.3.0 and Bun on Linux: stalls at 2.75 MB; Bun on Windows with the fix: 3.25 MB (http), 3 MB (https); without the fix: all 32 MB sent, no stall. - Windows x64, all 498 upstream `test-http-*` / `test-https-*` files from `test/js/node/test/parallel` with the debug build: 497 pass both before and after. The one failure (`test-http-set-timeout-server.js`, a 1 ms `server.setTimeout` firing twice) is identical before and after and passes on the release canary. - Windows x64, `test/js/node/http` directory with the fix: 756 pass, 23 skip, 4 todo, 0 fail. ### Background - `us_socket_pause` drops the socket's readable interest in the event backend (epoll/kqueue on POSIX, libuv `uv_poll` on Windows); the kernel receive buffer then fills, the peer's send window closes, and its writes block. That is how read-side backpressure reaches a TCP peer. `us_socket_resume` re-adds the interest. - The pause contract in usockets: a FIN that arrives while a socket is paused is not acted on; it is re-discovered and delivered as `on_end` after the socket resumes. A reset closes the socket right away. The libuv backend needs extra machinery for this because Windows AFD only reports a FIN to a poll without read interest through the one-shot `UV_DISCONNECT` event; that machinery is what #32488 added. - `on_buffer_paused_shim` / `buffered_request_body_data_during_pause`: while a node:http request is paused, body chunks that uWS has already read are parked in this `Vec` and handed to JS as one `Buffer` on resume. With the socket actually paused it holds at most what was already in flight (one recv buffer); without the pause it held the rest of the upload. - Adjacent: #34761 (pipelined POST bodies) has context lines in this hunk but keeps the guard; it is a different bug. <details> <summary>Repro script and measurements</summary> ```js import http from "node:http"; import { once } from "node:events"; const got = Promise.withResolvers(); const server = http.createServer(req => { req.on("data", () => {}); req.pause(); got.resolve(req); }); await once(server.listen(0, "127.0.0.1"), "listening"); let pulls = 0; const CHUNK = 256 * 1024; const body = new ReadableStream({ pull(c) { pulls++; c.enqueue(new Uint8Array(CHUNK)); } }, { highWaterMark: 1 }); const ac = new AbortController(); fetch(`http://127.0.0.1:${server.address().port}/`, { method: "POST", body, duplex: "half", signal: ac.signal }).catch(() => {}); const req = await got.promise; for (let t = 500; t <= 2500; t += 500) { await new Promise(r => setTimeout(r, 500)); console.log({ ms: t, pulls, readableLength: req.readableLength, rssMB: Math.round(process.memoryUsage().rss / 1048576) }); } ac.abort(); req.destroy(); server.closeAllConnections(); server.close(); process.exit(0); ``` Windows x64, release canary `1.4.0-canary.1+9a543cc18` (unfixed): ``` {"ms":500,"pulls":1242,"readableLength":262144,"rssMB":368} {"ms":1000,"pulls":2527,"readableLength":262144,"rssMB":679} {"ms":1500,"pulls":4099,"readableLength":262144,"rssMB":2098} {"ms":2000,"pulls":6553,"readableLength":262144,"rssMB":1694} {"ms":2500,"pulls":8195,"readableLength":262144,"rssMB":2104} ``` Windows x64, debug build of this branch's parent (unfixed): ``` {"ms":500,"pulls":131,"readableLength":262144,"rssMB":215} {"ms":2500,"pulls":2051,"readableLength":262144,"rssMB":1652} ``` Windows x64, debug build with this change: ``` {"ms":500,"pulls":14,"readableLength":262144,"rssMB":101} {"ms":2500,"pulls":15,"readableLength":262144,"rssMB":101} ``` Linux, canary `da3851e57` (unchanged by this PR): `pulls` stays at 12, RSS 37 MB. </details>
- The sink ref is a FileSinkRef (made pub(crate), given Deref and as_ptr) held by pipe_readable_stream_to_blob and by FileStreamWrapper, whose own Drop, raw pointer and unsafe accessor are gone. Byte counts are read through the guard. - received_bytes is opt-in: Option<u64> that only the Bun.write pipe enables, and count_received takes the length as a closure, so every other FileSink (writer(), stdout, subprocess stdin, stream pumps) no longer pays a simdutf pass per string chunk. - body_dispatch calls to_blob_if_possible() first, like the other native Body consumers, so blob- and file-backed streams take the copy engines instead of the JS pump. The Bun.file().stream() test now covers that. - The on_start_buffering half of the fetch fix already landed in #36006; the dispatch keeps the merge-base ordering and the loop/continue re-dispatch is removed. - The streaming-fetch test, which passed on main, is replaced by one that drops the Response wrapper while the write is parked and forces GC, which pins the on_response_finalize change (times out without it). - New s3-source-to-file tests assert the byte count, replacement of a longer destination and createPath against a Bun.serve endpoint, and the sync-close test now asserts the destination state.
Repro
A
Bun.servehandler that readsreq.bodyslower than the client sends gets no TCP backpressure: the client is never throttled and the whole remaining body is buffered in server memory.Before:
{"clientWriteMs":116}/ server{"bytes":60000000,"maxChunkMB":55,"peakRssMB":204}(client dumps 60 MB in 116 ms; one 55 MB chunk).After:
{"clientWriteMs":10586}/ server{"maxChunkMB":1}(client throttled to the sink rate; every chunk ≤ ~1.4 MB).The same shape held for
pipeTo(slowWritable), forgetReader()+ oneread()then idle, and for a handler that did not touchreq.bodyat all while the body arrived. Bun's ownnode:httpserver already applied backpressure on the same uWS socket (NodeHTTPResponse::pause_socket), so the primitive was in place; theBun.servereq.bodypath just never used it.Cause
RequestContext::on_buffered_body_chunkforwards every uWSonDatachunk intoByteStream::on_data. When no JS reader is waiting,on_dataappends to the ByteStream's internalbuffer: Vec<u8>and returns; nothing observed that buffer's size and nothing calledresp.pause(). For a body that has not been touched yet the chunk is appended torequest_body_bufwith the same unbounded growth.Fix
Pause.
on_buffered_body_chunkpauses the socket once the ByteStream's unconsumed buffer (or the pre-streamrequest_body_buf) crosses a 1 MiB high-water mark. The ByteStream's existingsignal_drained(fired fromon_pullwhen the buffer empties) is wired to resume viaon_stream_drainedon the request'sPendingValue.Whole-body consumers. A consumer that wants the whole body never drives
on_pull, so pausing would wedge it:.text()/.json()/.arrayBuffer()on an untouched body fireon_start_buffering, which resumes and setsREQUEST_BODY_BUFFER_ALLto suppress further pre-stream pausing..text()afterreq.bodyhas been touched goes through the ByteStream'sbuffer_action;on_buffered_body_chunkskips the pause whenbuffer_actionor a nativepipeis set.Bun.write(file, req)registeredon_receive_valuewithout callingon_start_buffering; it now does, mirroringBodyValueBufferer.Stale-
respwindow. Once a streaming-response sink callsres.end(), uWSmarkDone()dropsonAbortedand (forConnection: close) the socket may be freed on the next loop tick while theRequestContextstill holdsresp. The sink resumes the socket at both points it setsended_response = true(same frame asres.end(), so the socket is at worst closed, never freed). Every Rust-side resume path consultsresp_may_be_freed()(i.e.sink.ended_response) and clears the flag without dereferencingresponce the sink has ended.handle_resolve_stream/handle_reject_streamclearREQUEST_BODY_PAUSEDand the ByteStreamdrain_handlerimmediately after readingended_response, beforedetach()/run_error_handlercan re-enter JS.end_already_responded_streamanddetach_responseclear the flag without dereferencing. uWS itself is unchanged, so node:http's own pause owners (pausePipelineReads,IncomingMessagepause, the C++ pipeline-flood guard) are unaffected.Two
FlagsBitsare added (widening the set tou32):REQUEST_BODY_PAUSEDandREQUEST_BODY_BUFFER_ALL.Verification
bun bd test test/js/bun/http/serve.test.ts -t "request body backpressure"Five tests next to the existing response-side backpressure tests: a stalled streaming reader, a handler that defers touching
req.body,Bun.write(file, req)after the pre-stream pause, and.arrayBuffer()with and withoutreq.bodytouched first. Each writes a 32 MiB body from a raw socket, polls the client'ssentcounter until it plateaus, and asserts the plateau is well short of the total and the largest chunk the handler sees is under 4 MiB. All five fail onmainand pass with this change; bytes are delivered intact.The stale-
resppaths (reader.read and req.text inside a direct-streampull()afterc.end()across a loop tick) were verified under ASAN.node-http.test.ts"pipelined responses buffered past the high water mark pause reads on the connection" passes.Binary size
The +~530 KB flagged by the size check is measured against main build 79916, the last passing canary. Twelve commits landed on main between that baseline and this PR's parent, including
node:quic(#32602), the fullnode:repl(#31827), andnode:inspector(#31823), none of which has produced a passing canary yet.no test proof · iteration 2 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/bun/http/serve.test.ts