Bun.serve: hold a pipelined request until the response ahead of it completes instead of closing the connection by robobun · Pull Request #38128 · oven-sh/bun

robobun · 2026-08-13T09:04:17Z

Problem

Bun.serve closes the connection when it parses a request while the response ahead is still pending. That response is cut or lost: an awaited handler loses its answer, a streamed body stops after its first chunk, an 8 MiB body stops near 2.6 MB. The request behind never runs.
The cause: the HTTP_RESPONSE_PENDING branch of the request handler in HttpContext::onData (packages/bun-uws/src/HttpContext.h) closes the socket.

Fix

While a response is pending, the parser stops at the next request boundary and keeps the rest of the read in parkedRequestBytes. onData pauses reads. When the response completes and drains, onWritable parses the held bytes.
A request held behind a response that closes the connection is dropped.
Correct because a connection has one response slot, and nothing is dispatched while it is taken. Responses stay in request order. Synchronous pipelining is unchanged.
Verified: bun-serve-pipelining.test.ts (49 tests, 44 of the first 47 fail on main) and two cases in websocket-server-upgrade-early-frames.test.ts. Other suites: Notes.

Background

HTTP_RESPONSE_PENDING is set from dispatch until markDone(). A body that still drains counts as pending.
parkedRequestBytes is the parser-level hold. node:http already uses it for flood prevention.

Downsides

Reads are paused while a request is held. The server sees a client FIN only after the response in front, so req.signal does not fire for a graceful close.
A connection can hold one read (at most 512 KB) of unparsed requests.
A close that finds unread bytes behind held requests keeps the socket open until the peer's FIN, 8 MiB, or 4 to 8 seconds. A graceful stop() waits for that socket.

Notes

Measurements on f987e1b, 2026-09-22 (no code change). The price of the lingering close for a graceful stop(), on the debug build: a closing request held behind an awaited answer, 1 KiB unread behind it, then await server.stop(). A peer that ends on the FIN of the server: 8 ms. A peer that keeps its side open after the FIN: 7,813 ms, which is the bound of the linger. stop(true) closes at once. Sanitizer coverage of the close path: the debug build is an ASan build with assertions. A node client in its own process ran random pipelines (sync, awaited, streamed, 1 MiB, throwing, Connection: close on the request or the response, HTTP/1.0, POST bodies, Expect, a parse error, split writes, late writes behind held requests) over tcp, tls and unix, and ended each connection with a FIN, destroy(), a reset or an open half at 0 to 13 ms: 2 x 2,100 connections. A second run forced the lingering close on every connection (a closing request, a closing response or a parse error held behind an awaited or streamed answer, then one or two uploads of 1 KiB to 12 MB, the last one over the 8 MiB limit, ended at 0 to 40 ms): 1,500 connections. No sanitizer report, no assertion, and the server answered after each run. The x64-asan lane of Buildkite 119719 ran the test suite on this head and passed.

Twelfth update of 2026-09-22 (f987e1b), two later review findings. HTTP_LINGERING_CLOSE describes the connection, so it joins HTTP_CONNECTION_SCOPED: without it resetResponseState() clears the bit on a dispatch, which would drop the byte cap of the linger and the no-op guard of shutdownAndClose(). It has no test, and I state plainly that I could not reach that dispatch. A lingering socket is shut down and onData drops its reads. Over TLS the shutdown waits for spilled ciphertext, but a replay needs hasFullyDrained(), which counts the spill, so no spill of an earlier response survives into a replay. With the dispatch instrumented, the pipelining file starts 12 lingering closes and dispatches on none of them. An earlier probe of mine appeared to show the opposite. It was wrong: it derived a boolean from one hits array shared by 30 iterations, and the extra entry was a request on another connection. The second finding is in the test helper. The ended promise settled only on the end event, so a reset would make a test wait for its timeout instead of failing. It now settles on end, error and close. With a reset simulated, the closeIdleConnections() test fails in about 0.5 s and reports ECONNRESET. The file passes 49 of 49 in 3 of 3 runs. Build 119712 passed 181 of 181 jobs on the parent, e589bad.

Eleventh update of 2026-09-22 (e589bad), two review findings on the lingering close. shutdownAndClose() started a lingering close on every path. The parse-error path of onData is the one caller that reaches it with a response still pending: a held request is dispatched from the park, its body then fails to parse, and the client wrote more while it was held. The socket then stayed open for the linger, onClose did not run, and the handler did not see the abort. It now lingers only when no response is pending, so that path shuts down and closes at once again, as on main. A lingering socket also counted as idle when the closing response completed inside a replay, because markDone() sees no parked bytes there. closeIdle() then closed it in the middle of the linger. shutdownAndClose() now clears isIdle when the linger starts. The two depend on each other: since 29164bb setTimeout() no longer checks the linger flag, so a handler that still ran during a linger could replace its timeout through server.timeout() (from the source, not run). Two tests with a client that keeps its side open after the server's FIN. On 29164bb both fail in 3 of 3 runs (aborted: [], and a late client write that fails with EPIPE or EBADF). On e589bad both pass in 10 of 10 runs, and the file passes 49 of 49. I have not run the two new tests on main. Cost: a graceful stop() waits for a lingering socket. In a local probe it stayed pending for the 1000 ms sampled and resolved about 65 ms after the client's FIN.

Tenth update of 2026-09-22 (29164bb), the lingering close costs nothing per request. 8e68a68 tested HTTP_LINGERING_CLOSE in setTimeout(), in resetTimeout() and at the top of closeIfDoneAndMarked(): about four bit tests per keep-alive request for a state that only a close can enter. Now shutdownAndClose() sets idleTimeout to the linger's own bound when the linger starts. The resetTimeout() calls of the response teardown arm that bound again, also with idleTimeout: 0, so setTimeout() and resetTimeout() are identical to main again. The test in closeIfDoneAndMarked() was redundant: shutdownAndClose() has it, behind the branch that only a closing connection takes. What 8e68a68 and this commit add now sits only in the close branch, in the parse-error close and in the branch of onData for a socket that is shut down. Checked again: a silent peer that never closes is closed by the timeout with idleTimeout: 0 (8.0 s), a flood that ignores the FIN after 8.1 to 9.3 MB, 1 and 4 MiB behind an 8 MiB answer whole 21 of 21. 47 tests, 141 of 141 with --rerun-each 3. serve.test.ts (same two environment failures), request-smuggling, bun-server, websocket-server-upgrade-early-frames and node-http pass on the debug build. Buildkite 119691 on the merge 7bfa1ea: the one red test is spawn.test.ts, which is red on main too.

Ninth update of 2026-09-22 (7bfa1ea), merge with main bf80d21. One conflict, in HttpResponseData.h: main (#43708) took state bit 19 for HTTP_NODE_PEER_ENDED, which this branch used for HTTP_LINGERING_CLOSE. Both stay, and HTTP_LINGERING_CLOSE moves to bit 20. The other changes of main under packages/bun-uws, packages/bun-usockets and src/runtime/server since the last merge are node:http only (#43708) or unrelated (#43675, #43678). Buildkite 119638 passed on 8e68a68 on every lane before the merge. After it, on the debug build: bun-serve-pipelining.test.ts 47 of 47, serve.test.ts (same two environment failures), request-smuggling, bun-server, websocket-server-upgrade-early-frames, node-http, node-http-backpressure and node-http-server-abort-events (the tests of #43708) pass.

Eighth update of 2026-09-22 (8e68a68), a close that lingers. 55af365 read what was unread once, right before the close. That covers what the server's receive buffer holds. A peer that queued more (a 1 MiB POST behind the parked requests) still has the rest in its own kernel behind the closed receive window. The one read opens the window, the rest arrives after close(), and the reset drops the unsent end of the response in front. Here, with a node client in its own process against the debug build, an 8 MiB answer arrived cut in 9 of 42 runs on 480dfe8, and in 0 of 42 with this change. A client on the server's own event loop does not show it. Now shutdownAndClose() lingers in that state: it sends the FIN and leaves the socket open with reads resumed. This is the close in stages that RFC 9112 9.6 describes for the TCP reset problem: the server closes its write side, reads on until the client closes, then closes fully. onData already ignores a socket that is shut down, so what the peer still sends is read and dropped, and the loop closes the socket on the peer's FIN. A peer that does not stop is bounded by 8 MiB and by a timeout of 4 to 8 seconds: a flood that ignores the FIN was closed after 8.1 to 8.9 MB, and a silent peer that never closes after 7.9 s with idleTimeout: 0. The linger starts only for Bun.serve, only when requests are parked or a replay is on the stack, and only when us_socket_queued_input() reports unread data. Every other close is the same shutdown() and close() as before. us_socket_discard_unread() is gone again. HTTP_LINGERING_CLOSE marks the socket: the gates skip it, and setTimeout()/resetTimeout() leave its timeout alone, because the runtime resets the timeout after the response ends. New test: the server runs in a child process, the client queues a 4 MiB upload behind the parked requests of three connections. It fails 8 of 8 runs on 480dfe8 (EPIPE or ECONNRESET, cut bodies in some) and passes 10 of 10 here. 47 tests in the file, 235 of 235 with --rerun-each 5, 44 fail on canary 367d939. serve.test.ts (same two environment failures), request-smuggling, bun-server, bun-serve-static, bun-serve-ssl, bun-serve-routes, websocket-server-upgrade-early-frames, node-http, node-http-backpressure, fetch-keepalive and proxy pass on the debug build. In bun-serve-file one FIFO backpressure test times out at 5 s on the loaded machine, with and without this change (4.8 to 5.0 s on 480dfe8 too). The visible part of this description now has a Downsides section. The sentences it replaced (held frames after server.upgrade(), graceful stop(), a peer that sent its FIN, us_socket_request_writable()) are covered by the updates below.

Seventh update of 2026-09-21 (480dfe8), review finding. HttpResponse::cork() had its own copy of the close gate, without the read of unread bytes that the other gates got in 55af365. A response without a body (HEAD, 304) completes while its socket is still corked, so its close runs from there. cork() now calls closeIfDoneAndMarked(). New test over a unix socket: an async HEAD handler that answers with Connection: close, one request held behind it and one more unread. It fails on 55af365 with ECONNRESET behind the response.

Sixth update of 2026-09-21 (55af365), a close over unread bytes. Reads are paused while requests are parked, so what the client writes after that stays unread in the kernel. The close gates then close over unread bytes. That resets the connection, and the kernel drops the part of the response that it has not sent yet. Two shapes reach that state on 4a66fc4. First: an awaited handler answers with a Connection: close response header, and the client writes two more requests in two later writes while the answer is pending. With an 8 MiB body the client gets 6,602,380 of 8,388,748 bytes and ECONNRESET over TCP, and the whole body and ECONNRESET over a unix socket (one later write: whole and clean, because that write is the parked one). Second: a Connection: close request that is itself parked, with a later write behind it: both responses arrive, then ECONNRESET on a unix socket. Main loses the whole response in both shapes. Now the three close gates and the parse-error close read and drop those bytes first (us_socket_discard_unread, at most 8 MiB, which a socket receive buffer bounds). A connection that closes dispatches none of them. The discard runs only for Bun.serve, and only when requests are parked or a replay is on the stack. Tests: two new cases over tcp, tls and unix. Without the change the first fails on all three transports (cut body on tcp and tls, ECONNRESET on all), and the second fails on unix. 45 tests in the file, 225 of 225 with --rerun-each 5, 41 or 42 fail on canary 367d939 (one case is a race there). serve.test.ts (same two environment failures), request-smuggling, bun-server, bun-serve-file, bun-serve-static, bun-serve-ssl, websocket-server-upgrade-early-frames, node-http and node-http-backpressure pass on the debug build. Not changed: a client that keeps writing after the close can still reset the connection, as on main.

Fifth update of 2026-09-21 (4a66fc4), a closing request ends cleanly again. The parser checked the hold before the sawConnectionClose latch. A request with Connection: close or HTTP/1.0 whose response was pending took the hold: bytes that arrived behind it were parked and reads were paused. Bytes that arrived after that stayed unread, and the close after the response reset the connection. A unix socket reports that to the client behind the complete response (ECONNRESET). Main reads and drops those bytes and ends the stream cleanly. For Bun.serve the latch is now checked first, as on main. node:http keeps its order. New test in the Connection: close block, over a unix socket: it passes on canary 367d939, fails on b9acb38 with ended: false, error: "ECONNRESET", and passes here. 39 tests in the file, 195 of 195 with --rerun-each 5. serve.test.ts (same two environment failures), request-smuggling, bun-server, bun-serve-file, bun-serve-static, bun-serve-routes, bun-serve-ssl, http-server-chunking, websocket-server-upgrade-early-frames, node-http and the upstream pipelining tests pass on the debug build. Not changed: a pending response that marks the connection close with its own Connection: close header. The bytes behind it are held, because nothing marks the connection until the response is rendered. Main cuts that response. This branch delivers it whole and then closes.

Fourth update of 2026-09-21 (b9acb38), review nits. Comments only: the added comment lines across the uWS and uSockets changes go from about 130 to 62, and each block keeps only its invariant or the reason for an ordering. The drain with nothing to drain after an async upgrade with held frames stays: it comes from us_socket_resume() arming writable for every caller, and #37099 changes that for all of them. A pre-existing node:http problem that the same review named (the flood-prevention replay task can run on a socket that was upgraded in the meantime) is outside this change and is reported separately.

Third update of 2026-09-21 (348b1c9), review finding. us_socket_resume() closes a socket whose poll the kernel does not take back (socket.c, since #33974), and the close destructs the HTTP state. The two resumes this PR adds went on regardless. With the failure injected (a temporary getenv next to the us_poll_change check, not committed) the replay site crashed in WTF::Vector::grow on the destructed parkedRequestBytes, and upgrade() sent the 101 on a closed socket and fired open with no close after it. Now the replay returns when the resume closed the socket, and upgrade() resumes after the adoption and after open, where a close is an ordinary WebSocket close (open, then close with 1006, held frames not dispatched). No test: epoll_ctl cannot be made to fail from one without a fault hook. The same injection shows a use-after-poison in RequestContext::on_start_buffering on main's own request-body resume. This PR does not touch that path, and it is reported separately.

Second update of 2026-09-21 (c5a70a7), after CI and review of a8613df.

Behaviour change to a test from main: websocket-server-upgrade-early-frames.test.ts (Bun.serve: parse WebSocket frames that arrive in the same read as the upgrade request #43153) pinned a 400 for a client that sends frames right behind its upgrade request when server.upgrade() runs in a later turn of the event loop. The 400 came from the HTTP parser reading the frame as the next request. This branch holds those bytes instead, and a8613df dropped them in upgrade(): 101, connection open, frames lost, which is the outcome Bun.serve: parse WebSocket frames that arrive in the same read as the upgrade request #43153 removed for an upgrade made during the dispatch. CI caught it on every lane.
Now upgrade() takes the held bytes before it destructs the HTTP state and dispatches them to the WebSocket after open (us_dispatch_data, in a buffer padded on both sides like the loop's receive buffer). The frames are handled before server.upgrade() returns, in the order open, early frames, then the return. open already runs inside upgrade(), so the re-entrancy is not new. A hand-over from a later loop dispatch would need a buffer in WebSocketData and a check at the top of every WebSocket read.
The Bun.serve: parse WebSocket frames that arrive in the same read as the upgrade request #43153 test now expects the frames, for frames in the read of the request and for frames in a read of their own before the upgrade. Both cases fail on main with the 400. An HTTP request held behind the handshake is never dispatched as HTTP. As frames it is not valid, so the WebSocket fails the connection, as it does after an upgrade made during the dispatch.
Review finding, quadratic copying: each replay moved the held bytes out, reallocated them for the parser's fence, and copied what was behind the dispatched request into a new vector. A 512 KiB read of minimal requests behind slow handlers came to about 10 GB of memcpy. A replay that parks again now gives its buffer back and moves only parkedRequestBytesStart. The buffer still belongs to the replay call while it is parsed, because a dispatch can close or upgrade the socket. The parser reaches it through replayedRequestBytes. A trace shows one copy of 66 bytes and then a give-back with start 33 in the three-requests test. sizeof(HttpParser) goes from 72 to 80 and sizeof(HttpResponseData<false>) from 224 to 232: the start fits in padding, the pointer does not. No before/after timing: there is no release build of this branch in this environment, and on the debug build a request costs about 1 ms of JavaScript, which hides the copying.
Review finding, test time: the file runs in about 6 s on the debug + ASAN build under load, and the slowest test takes 0.5 s. Not changed. The reasons are in the review thread.
Tests: 38 in bun-serve-pipelining.test.ts, 36 fail on canary 367d939. New: held frames delivered in order with a later frame, a frame held twice (behind an async handler, then behind an async upgrade; it fails when upgrade() ignores the start), and the HTTP request behind a handshake, each over tcp and tls.
Suites on the debug build after these two commits: serve.test.ts (same two environment failures as before), bun-server, bun-serve-file, request-smuggling, serve-http2, serve-pending-promise-abort-leak, ws.test.ts, websocket-server-upgrade-early-frames, node-http: all pass. Upstream test-http-pipeline-*, test-http-upgrade-server*, test-http-server-close-idle*, *-timeout-pipelining: all pass. websocket-server.test.ts: four concurrent tests time out next to the send() benchmark under ASAN, and pass alone.

Update of 2026-09-21 (merge with main a2b69f7).

Conflicts: four small hunks (HttpContext.h, HttpParser.h, libuwsockets.cpp, the allowlist). The allowlist hunk is gone: the merge takes main's file.
cannotDispatchAnotherRequest() now tests HTTP_RESPONSE_PENDING only. Main (Bun.serve: do not run the requests behind a response that closes the connection #42986) handles a connection that a complete response marked close with the sawConnectionClose latch and the close check at the top of the request handler. The hold leaves that case to them. A request held behind a pending response that closes the connection is still dropped: the close gate of onWritable runs before the replay.
New: the hold is also derived when the request handler returns. The parser reports no end of message for a head with Content-Length: 0 that it completed from its fallback buffer (a head split across reads). The request behind such a head reached the request handler with the response ahead pending: ASSERT_NOT_REACHED in a debug build, the close in a release build. A scratch build without the new derivation aborts in the new test at HttpContext.h(499).
New: shouldCloseConnection() waits for held bytes in its HTTP_NODE_RECEIVED_FIN branch (review finding). Over TLS the close_notify of the peer is decrypted in the same read as the requests, so onEnd runs while a request is held. A scratch build without the condition delivers /big whole and never dispatches /small, over TLS only. Over TCP and unix sockets the loop defers the FIN while reads are paused.
Review finding about idleTimeout: not changed, the premise does not hold. src/runtime/server/mod.rs arms idleTimeout for every request that reaches JavaScript, so a slow handler already runs under it with nothing pipelined. Numbers are in the review thread.

Reduced shapes (three GETs in one write, 30,000 byte body, script in the comments):

handler	canary `367d939`	1.4.2	this branch
returns a ready `Response`	3 answers, kept	same	same
awaits 5 ms	0 bytes, closed	same	3 answers, kept
streams the body in three steps	10,101 bytes, no last chunk, closed	0 bytes, closed	3 answers, kept
throws after an await	0 bytes, closed	not run	500, then 200, kept
first request has `Expect: 100-continue`	`100 Continue`, closed	not run	100, 200, 200, kept
first request is `HEAD`	0 bytes, closed	not run	200, 200, kept

The streamed case changed after 1.4.2 with #42762: the close in this branch became AsyncSocket::close(), which sends the cork buffer first.

Tests. 34 in bun-serve-pipelining.test.ts, 340 of 340 with --rerun-each 10 on the debug build. 32 fail on canary 367d939. The two that pass there are the Connection: close cases, which main fixed with #42986. New in this update: the three shapes above (Expect in two forms), the split head with Content-Length: 0, and a client that ends its side right behind the requests (tcp, tls, unix).

Suites, debug build. serve.test.ts (323 pass; root range port and /bun:info loopback fail, and they fail the same way on canary 367d939 here, where the tests run as root), bun-server, bun-serve-static, bun-serve-file, bun-serve-routes, bun-serve-ssl, bun-serve-headers, request-smuggling, http-server-chunking, serve-close-delimited-framing, serve-direct-readable-stream, serve-listen, proxy, fetch-keepalive, node-http, node-http-backpressure, node-http-server-socket-end-drain: all pass. Upstream test-http-pipeline-* (5), test-http-server-*-timeout-pipelining (2), test-http-server-keep-alive-timeout, test-http-upgrade-server* and test-http-upgrade-reconsume-stream: all pass. websocket-server.test.ts: the send() benchmark exceeds its 30 s budget under ASAN and takes eight concurrent tests with it. The same tests pass when run without the benchmark (50 of 50).

Description before this update

Problem

Bun.serve: a client that pipelines a second HTTP/1.1 request behind one whose response is still in flight gets the connection closed the moment the second head is parsed. The first response is cut off mid-body and the second request is never answered. Over a unix: listener this happens on every connection whose first response is bigger than the socket buffer (~220 KB of a 4 MiB body arrives, then EOF); over loopback TCP it depends on how much of the body the first write manages to hand to the kernel. An 8 MiB Bun.file() response (handler or file route) stops at ~2.6 MB the same way, which is the shape of Bun.file as response behaving weirdly #6961, Bun.file routes 404 #22174, Response(Bun.file) streams look like HTTP/0.9 over LAN #26406 and Response(Bun.file()) is slower than Response(await Bun.file().text()) #11228.
The close is the HTTP_RESPONSE_PENDING branch of the per-request callback in packages/bun-uws/src/HttpContext.h (us_socket_close, "denying async pipelining"). It fires for every way a response can still be pending when the next head is parsed: a buffered body whose tryEnd() tail is still draining through onWritable (the case above), an async handler that has not returned yet, a streaming body, a file response.
Found while testing this: a request pipelined behind a Connection: close (or HTTP/1.0) request was dispatched and answered, and the connection stayed open, because the next dispatch's resetResponseState() cleared the close mark before the close gate ran. RFC 9112 9.6 says nothing after such a request may be processed.

Fix

HttpParser: the request-boundary park that node:http compat already had for flood prevention is now shared. When parkAtNextBoundary is set (or bytes are already parked, which keeps wire order), the parse loop stops before getHeaders touches the next head and moves the rest of the read, verbatim, into parkedRequestBytes. Fields renamed from their nodeHttp* names since both servers use them; node:http's own pause/replay logic is unchanged (replayParkedRequestBytes replaces feedNodeHttpData, its only caller).
HttpContext::onData (Bun.serve instantiation only): derives the flag from cannotDispatchAnotherRequest() (HTTP_RESPONSE_PENDING | HTTP_CONNECTION_CLOSE) at entry and again after each request's body fin, which is the last point the handler can have completed the response at (synchronously, or from inside the body callback), so synchronous pipelining still takes the existing path and never parks. After the parse, if anything was parked, reads are paused (AsyncSocket::pause, so the pending response's own timeout stays armed; verified that a peer that pipelines and then stops reading still times out exactly as before) and, if the response is already complete, a writable dispatch is armed.
HttpResponseData::markDone(): if bytes are parked, arms one writable dispatch with us_socket_request_writable(). HttpContext::onWritable (Bun.serve) ends by replaying the parked bytes through onData when the response is complete and fully drained, resuming reads first. That is the one replay site. It is deliberately not done inside markDone(): every markDone() caller (internalEnd, the uws_res_end* wrappers, and the Rust RequestContext above them) keeps tearing the finished response down after it returns, so a dispatch from there would land in the middle of that teardown. The writable dispatch is the next point where the socket has nothing of the old response on the stack, which is also why this needs no changes on the Rust side. A tryEnd tail that finishes inside onWritable's own callback reaches the replay in the same dispatch.
Replayed bytes go through the ordinary parse path, so a held request gets its body, a request behind it is held again while its response is pending, and held bytes that are not a valid request get the parser's error response after the response ahead of them instead of replacing it.
Connection: close requests: the same flag parks whatever follows them; the existing close-after-drain gates then discard it with the socket. onWritable's close gate runs before the replay site on exactly the conditions the replay needs, so such bytes are never replayed.
HttpResponse::upgrade(): if the pending response turns out to be a WebSocket upgrade, the bytes held behind it are dropped with the HTTP state (as bytes trailing a synchronous upgrade in the same read always were; a client may not send anything before the 101, RFC 6455 4.1) and reads are resumed. Without this the paused flag survived us_socket_adopt, so the WebSocket sent its 101 and then never read a frame. The resume re-arms writable as well, so the WebSocket gets one drain callback with nothing to drain right after open (verified; a plain upgrade gets none); dropping the bytes before internalEnd() keeps markDone() from arming a second one for a replay that cannot happen.
HttpResponse::resume(): leaves the read side paused while bytes are parked. The pause belongs to the pipelining code (onWritable resumes right before replaying, upgrade() when dropping); the resumes that reach this function from the runtime release a request-body backpressure pause (RequestContext::detach_response, on_request_body_stream_drained, the response sink's end()) and can land after that body completed and the bytes behind it were parked. Reading on there only queues more bytes behind the parked ones, and on kqueue, which delivers the read and write filters separately, a peer FIN read that way reaches onEnd before the replay and closes the connection over the parked request. Not separately observable on epoll (the replaying writable dispatch runs before the read within the same event), so no dedicated test; node:http's flood prevention only calls resume() once its parked bytes are gone and is unaffected.
The old close in the per-request callback stays as the backstop (with ASSERT_NOT_REACHED() in debug); with the flag maintained at both points above it is no longer reachable for Bun.serve.
us_socket_request_writable() (socket.c) replaces two byte-identical helpers, us_socket_sendfile_needs_more and us_socket_mark_needs_more_not_ssl, whose job ("dispatch on_writable once, even if called from inside one") is what markDone() needs too; the sendfile callers now use it.
markDone() no longer marks a connection idle while requests are parked on it (review finding): a graceful server.stop() marks busy connections close-when-idle, and that mark used to fire at the completion of the response ahead, closing over a request that had been received in full. The connection now becomes idle at the markDone() of the last replayed request, mirroring how node:http's queued-response count is treated on the same line.
Hot-path cost: one flag store per read and per request, one predicted branch per request in the parse loop (the node:http instantiation already had it), one isEmpty() in markDone(), upgrade() and resume(). Per replayed request in a pipelined burst behind async handlers: the remainder of the recv is copied into a fresh vector when it is parked again, so a burst of N requests in one recv costs O(N x recv) bytes of copying spread over N dispatches, bounded by the one-recv park limit; requests behind synchronous handlers are unaffected. No new per-connection fields: the parser already carried the flag and vector for node:http.
Verification: test/js/bun/http/bun-serve-pipelining.test.ts, 26 tests, all fail on current main (in under a second, no timeouts) and pass with this change:
- a 16 MiB body as a buffered string, as a Bun.file() returned from the handler and as a Bun.file() route, each with a request pipelined behind it, over tcp, tls and unix (the tryEnd drain, sendfile and read+write file paths; the SSL instantiation; replay from inside onWritable);
- three requests behind async handlers in one read, over tcp, tls and unix: each is dispatched only after the previous response completes, and the replay parks the rest again;
- a request arriving in a later read while the handler is still running (the entry-time derivation);
- a request behind a streaming response that the app ends later, over tcp, tls and unix. Ending the stream also makes the response sink call resume() on the socket while the request behind it is parked, so this drives the HttpResponse::resume() guard (confirmed by instrumenting it locally: taken once per transport here); the held request is answered after the stream's terminating chunk;
- a request behind a request body the handler is still consuming (body delivered, request behind it held);
- a held POST with a body, with a Connection: close request behind it: the body reaches the replayed handler, the last request is answered, then the connection closes;
- held bytes that are a parse error: the response ahead of them arrives intact, then the 505;
- Connection: close with a sync handler and with an async handler: first response delivered in full, connection closed, second handler never invoked;
- graceful server.stop() while the response ahead of a held request is pending: the held request is answered, then the connection closes and stop() resolves (fails without the isIdle change: the held request is dropped);
- a WebSocket upgrade pipelined behind an async handler, over tcp and tls: replayed, 101 with the right accept key, frames flow (onWritable returning the adopted socket);
- an async upgrade with a request pipelined behind it, over tcp and tls: 101, the held request is dropped, frames flow (fails on this branch without the upgrade() change: the echo never arrives).
- Also ran locally with the debug build: serve, bun-server, bun-serve-file, bun-serve-static, bun-serve-routes, bun-serve-ssl, websocket-server, request-smuggling, http-server-chunking, node-http, and the upstream test-http-pipeline-* (node:http's renamed park/replay path), test-http-upgrade-* (the shared upgrade()) and *-timeout-pipelining tests. The only failures are the ones the released build also has in this environment (IPv6, privileged ports, egress proxy, and the websocket send benchmark exceeding its 30 s budget under ASAN).
- The test hunks of Bun.serve: defer pipelined HTTP/1.1 requests behind an async response instead of closing the socket #33664, Bun.serve: deliver the in-flight response when a pipelined request arrives while it is still pending #35036 and http: deliver the in-flight response when a pipelined request arrives behind an async handler #32868 were run against this branch as well: Bun.serve: defer pipelined HTTP/1.1 requests behind an async response instead of closing the socket #33664's four cases pass as written; Bun.serve: deliver the in-flight response when a pipelined request arrives while it is still pending #35036's and http: deliver the in-flight response when a pipelined request arrives behind an async handler #32868's cases deliver the in-flight response as they assert, and differ only where they pin the "then close the connection" behaviour those PRs implemented (this branch answers the pipelined request instead). The scenarios they covered that this file lacked are the file, POST-with-body, parse-error and upgrade-with-request-behind-it cases above.

Background

HTTP/1.1 pipelining: a client may send its next request on a keep-alive connection before reading the previous response; the server has to answer in request order (RFC 9112 9.3.2). uWS has one HttpResponseData per connection, so it can only work on one response at a time; HTTP_RESPONSE_PENDING is set when a request is dispatched and cleared by markDone() when the response has been fully handed to uWS.
tryEnd(): how Bun.serve sends buffered bodies. It writes as much as the kernel accepts without copying the rest into uWS's backpressure buffer; Bun keeps the bytes and writes more from the onWritable callback each time the socket drains. Until the last byte is written the response counts as pending, even though the application finished long ago. This is why the bug hit plain new Response(bigString). A Bun.file() body is pending the same way while the runtime's file pump (sendfile, or read+write chunks) moves it.
onWritable dispatch: uSockets calls a socket's on_writable when the kernel reports the socket writable and keeps that interest only while a write has recently come up short (last_write_failed). us_socket_request_writable() sets that flag and re-arms the poll, which is the existing idiom (from the sendfile path) for "call me back from the event loop for this socket".
upgrade(): writes the 101, destructs the connection's HttpResponseData and re-parents the socket into the WebSocket context with us_socket_adopt, which keeps the socket's flags (including paused) as they were.
node:http compat dispatches pipelined requests immediately and queues responses in JS; what it shares with this change is only the parser-level park of unparsed bytes, which it uses when it pauses a flooding connection.

Bun.serve: defer pipelined HTTP/1.1 requests behind an async response instead of closing the socket #33664 (park and replay, but synchronously from inside the uws_res_end* wrappers, which is why it also had to adjust the Rust teardown paths), Bun.serve: deliver the in-flight response when a pipelined request arrives while it is still pending #35036 and http: deliver the in-flight response when a pipelined request arrives behind an async handler #32868 (deliver the first response, then close) addressed the same close; their Bun.serve coverage is folded in here as described above, and they are being closed in favour of this PR. The node:http half of http: deliver the in-flight response when a pipelined request arrives behind an async handler #32868 is already on main since node:http/https/http2: raise Node v26.3.0 compat to ~94%, sync the upstream suites, and fix the Windows/macOS transport-layer teardown bugs they exposed #32488 (its node-http case passes there as written).
usockets: stop pause() from arming writable interest it never had #37099 adds us_socket_mark_writable_pending() for the same purpose as us_socket_request_writable() here; whichever lands second should reuse the other's helper. Nothing here depends on pause() arming writable interest, so the two compose either way.

[human-review] gate passed · iteration 6 · 13 files touched

fails on main (without fix)

ASAN without fix: 48 FAILED
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" test/js/bun/http/bun-serve-pipelining.test.ts test/js/bun/websocket/websocket-server-upgrade-early-frames.test.ts
bun test v1.4.3 (367d939d9)

test/js/bun/websocket/websocket-server-upgrade-early-frames.test.ts:
(pass) frames in the same read as the upgrade request > are delivered in order, and a ping gets its pong (tls: false) [726.24ms]
(pass) frames in the same read as the upgrade request > are delivered when server.upgrade() runs after `await Promise.resolve()` [362.00ms]
(pass) frames in the same read as the upgrade request > are delivered when server.upgrade() runs after `await req.text()` [356.44ms]
(pass) frames in the same read as the upgrade request > a frame that the read cuts short is completed by the next read [373.34ms]
(pass) frames in the same read as the upgrade request > are delivered in order, and a ping gets its pong (tls: true) [663.93ms]
(pass) frames in the same read as the upgrade request > a Content-Length body in the same read as the upgrade request is not parsed as frames [139.91ms]
(pass) frames in the same r
... (truncated)

release without fix: all passed
bun test v1.4.3-canary.1 (e589bad7f)

test/js/bun/websocket/websocket-server-upgrade-early-frames.test.ts:
(pass) frames in the same read as the upgrade request > are delivered in order, and a ping gets its pong (tls: false) [40.00ms]
(pass) frames in the same read as the upgrade request > are delivered when server.upgrade() runs after `await Promise.resolve()` [24.64ms]
(pass) frames in the same read as the upgrade request > are delivered when server.upgrade() runs after `await req.text()` [22.79ms]
(pass) frames in the same read as the upgrade request > a frame that the read cuts short is completed by the next read [21.93ms]
(pass) frames in the same read as the upgrade request > a Content-Length body in the same read as the upgrade request is not parsed as frames [16.92ms]
(pass) frames in the same read as the upgrade request > a chunked body in the same read as the upgrade request is not parsed as frames [15.85ms]
(pass) frames in the same read as the upgrade request > never reach the WebSocket of another connection [11.17ms]
(pass) frames in the same read as the upgrade request > are delivered when server.upgrade() runs in a later turn of the event loop (frames
... (truncated)

passes on PR (with fix)

ASAN with fix: all passed
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" test/js/bun/http/bun-serve-pipelining.test.ts test/js/bun/websocket/websocket-server-upgrade-early-frames.test.ts
bun test v1.4.3 (367d939d9)

test/js/bun/websocket/websocket-server-upgrade-early-frames.test.ts:
(pass) frames in the same read as the upgrade request > are delivered in order, and a ping gets its pong (tls: false) [987.24ms]
(pass) frames in the same read as the upgrade request > are delivered when server.upgrade() runs after `await Promise.resolve()` [441.02ms]
(pass) frames in the same read as the upgrade request > are delivered when server.upgrade() runs after `await req.text()` [449.64ms]
(pass) frames in the same read as the upgrade request > a frame that the read cuts short is completed by the next read [474.36ms]
(pass) frames in the same read as the upgrade request > are delivered in order, and a ping gets its pong (tls: true) [848.12ms]
(pass) frames in the same read as the upgrade request > a Content-Length body in the same read as the upgrade request is not parsed as frames [166.51ms]
(pass) frames in the same r
... (truncated)

release with fix: all passed
$ bun scripts/build.ts --profile=release
[configured] bun-profile → bun (stripped) in 1205ms (unchanged)
ninja: Entering directory `/workspace/bun/build/release'
[1/113] cxx obj/unified/UnifiedSource-packages_bun_usockets_src_crypto-0.cpp.o
[2/113] cxx obj/unified/UnifiedSource-src_runtime_webview-0.cpp.o
[3/113] cxx obj/src/jsc/bindings/ZigGlobalObject.cpp.o
[4/113] cc obj/packages/bun-usockets/src/eventing/epoll_kqueue.c.o
[5/113] cc obj/packages/bun-usockets/src/crypto/openssl.c.o
[6/113] cc obj/packages/bun-usockets/src/loop.c.o
[7/113] cc obj/packages/bun-usockets/src/quic.c.o
[8/113] cc obj/packages/bun-usockets/src/fault_inject.c.o
[9/113] cc obj/packages/bun-usockets/src/socket.c.o
[10/113] cc obj/packages/bun-usockets/src/eventing/libuv.c.o
[11/113] cc obj/packages/bun-usockets/src/node_quic_shim.c.o
[12/113] cc obj/packages/bun-usockets/src/bsd.c.o
[13/113] cc obj/packages/bun-usockets/src/udp.c.o
[14/113] cc obj/packages/bun-usockets/src/context.c.o
[15/113] gen cpp.rs (cppbind)
[16/112] build.rs build_script_build
[17/112] rustc bun_platform 
[18/112] rustc bun_core 
[19/112] rustc bun_safety 
[20/112] rustc bun_boringssl_sys 
[21/112] rustc bun_zlib_s
... (truncated)

diff hotspot

packages/bun-usockets/src/libusockets.h            |    4 +-
 packages/bun-usockets/src/socket.c                 |    8 +
 packages/bun-uws/src/HttpContext.h                 |  130 +-
 packages/bun-uws/src/HttpParser.h                  |   60 +-
 packages/bun-uws/src/HttpResponse.h                |   99 +-
 packages/bun-uws/src/HttpResponseData.h            |   23 +-
 src/jsc/bindings/node/JSNodeHTTPServerSocket.cpp   |   22 +-
 src/uws_sys/Response.rs                            |    4 +-
 src/uws_sys/libuwsockets.cpp                       |   22 -
 src/uws_sys/socket.rs                              |    2 +-
 src/uws_sys/us_socket_t.rs                         |    6 +-
 test/js/bun/http/bun-serve-pipelining.test.ts      | 1293 ++++++++++++++++++++
 .../websocket-server-upgrade-early-frames.test.ts  |   70 +-
 13 files changed, 1617 insertions(+), 126 deletions(-)

gate history · 13 passed · 0 rejected · iteration 6

evidence per changed file

file                                                      reads  edits  tests
packages/bun-usockets/src/libusockets.h                       2      1    116
packages/bun-usockets/src/socket.c                            2      1    117
packages/bun-uws/src/HttpContext.h                           20     21    119
packages/bun-uws/src/HttpParser.h                            10      6    117
packages/bun-uws/src/HttpResponse.h                          12     13    117
packages/bun-uws/src/HttpResponseData.h                       3      5    118
src/jsc/bindings/node/JSNodeHTTPServerSocket.cpp              2      3    116
src/uws_sys/Response.rs                                       1      3    117
src/uws_sys/libuwsockets.cpp                                  5      3    116
src/uws_sys/socket.rs                                         1      1    116
src/uws_sys/us_socket_t.rs                                    2      2    116
test/js/bun/http/bun-serve-pipelining.test.ts                 9     13    109
…websocket/websocket-server-upgrade-early-frames.test.ts      0      0     20

robobun · 2026-08-13T09:04:28Z

^{Updated 4:52 AM PT - Sep 22nd, 2026}

✅ @robobun, your commit f987e1b41bd8646e5f05b95952f4120e9eb56505 passed in Build #119719! 🎉

🧪 To try this PR locally:

bunx bun-pr 38128

That installs a local version of the PR into your bun-38128 executable, so you can run:

bun-38128 --bun

robobun · 2026-08-13T09:07:46Z

Status: reproduced on 1.4.0 with a raw client writing GET /big + GET /small in one segment against fetch: () => new Response("x".repeat(4 << 20)): over a unix listener 20/20 connections got 256 to 365 KB of the body and then EOF, over loopback TCP 20/20 got ~2.8 MB; the second request was never answered in either case. It still reproduces on canary 367d939 (main of 2026-09-18) with smaller shapes, for example three GETs in one write behind a handler that awaits 5 ms: 0 bytes, connection closed. With this branch every request is answered in order on every connection.

State on 2026-09-22 12:10 UTC: CI is green on the head, and all review threads are resolved. The head is f987e1b. It sits on e589bad and on 29164bb, with no history rewrite. All review threads are resolved (13 of 13).

e589bad fixed the two findings on the lingering close. A close no longer lingers while a response is pending, so a held request whose body fails to parse is aborted at once again. A lingering socket no longer counts as idle, so closeIdleConnections() and a graceful stop() leave it alone. Build 119712 passed on e589bad, 181 of 181 jobs.

f987e1b answers two later findings. HTTP_LINGERING_CLOSE joins HTTP_CONNECTION_SCOPED, because the header states that a connection bit must survive resetResponseState(). That one has no test: I could not reach a dispatch on a lingering socket, and with the dispatch instrumented the pipelining file starts 12 lingering closes and dispatches on none of them. The ended promise of the test helper now settles on every terminal event, so a reset fails the test in about 0.5 s instead of waiting for the timeout. Build 119719 passed on f987e1b, 181 of 181 jobs. The automated review of this head had nothing new to post.

I built e589bad and 29164bb myself (debug build, linux x64). Both new tests fail on 29164bb in 3 of 3 runs and pass on e589bad in 10 of 10. On e589bad the pipelining file (49 of 49), the early-frames file, request-smuggling, bun-server, three abort and close suites and two node:http suites pass. A graceful stop() now waits for such a socket: in a local probe it stayed pending for the 1000 ms I sampled and resolved about 65 ms after the client sent its FIN. I built e589bad and f987e1b myself and ran the pipelining file on both, 49 of 49. I did not run serve.test.ts, Windows or macOS.

An earlier build, 119691 on 7bfa1ea, had 180 jobs passed and one red job, test/js/bun/spawn/spawn.test.ts on the debian 13 x64-asan lane. That test also fails on main and is reported separately. The three builds after it, 119700, 119712 and 119719, passed on every lane.

Last head I built and ran myself: f987e1b. Since a8613df:

CI found a real gap on every lane: websocket-server-upgrade-early-frames.test.ts (Bun.serve: parse WebSocket frames that arrive in the same read as the upgrade request #43153, merged in with main) expects a 400 when a client sends frames right behind its upgrade request and server.upgrade() runs in a later turn. This branch held those bytes and then dropped them in upgrade(), so the 101 went out and the frames were lost. upgrade() now gives the held bytes to the WebSocket after open, as Bun.serve: parse WebSocket frames that arrive in the same read as the upgrade request #43153 does for an upgrade made during the dispatch. That test now expects the frames, in two arrival shapes, and both fail on main.
Review finding fixed: a replay that parks again no longer copies what is behind the dispatched request. It gives its buffer back and moves a start offset, so a read is copied once instead of once per request.
The other two findings of that review are answered in their threads (test time: measured, unchanged).
Review finding fixed: us_socket_resume() can close the socket (a poll the kernel does not take back), and the two resumes this PR adds went on with the destructed HTTP state. The replay now returns when that happens, and upgrade() resumes after the adoption and open, where it is an ordinary WebSocket close. Checked with the failure injected: the old code crashed at the replay site, the new code closes the one connection.
A behaviour of main restored: bytes that arrive behind a Connection: close (or HTTP/1.0) request are read and dropped again, instead of held. Holding them paused reads, and the close after the response then reset the connection (ECONNRESET on a unix socket behind the complete response).
A cut response that the hold itself could still cause is fixed: reads are paused while requests are held, so a close could land on bytes the client had written meanwhile, which resets the connection and drops the unsent end of the response. When such bytes are queued the close now lingers (8e68a68, Bun.serve only): the server sends its FIN, drops what still arrives, and closes on the client's FIN, after 8 MiB, or after 4 to 8 seconds.
Two review findings on that lingering close fixed (e589bad): it never starts while a response is pending, and a lingering socket is not idle.
Review finding fixed: the close gate inside HttpResponse::cork() was a copy without that read, so a HEAD or 304 response that closes the connection could still reset it. cork() now calls the shared gate.

49 tests in test/js/bun/http/bun-serve-pipelining.test.ts, all pass on e589bad here. The two newest fail on the commit before their fix. The count that fails on main is in the PR description.

CI for 480dfe8 (build 119405, finished): 180 jobs passed and one failed, test/bake/deinitialization.test.ts on alpine 3.23 aarch64 (Expected: 1, Received: 2). The same assertion fails on that lane in 13 of the 20 main builds before it, and the test does not go through this change. It is reported separately. The S3 upload timeouts that were red in the builds of the earlier heads are gone. The latest automated review of this head had nothing new to post.

Overlap with #37099: it adds the same arm-a-writable-dispatch primitive under the name us_socket_mark_writable_pending and keeps the two old wrappers as callers of it; this PR adds it as us_socket_request_writable and removes the wrappers. Whichever lands second needs a small rebase in socket.c / libusockets.h / libuwsockets.cpp.

coderabbitai · 2026-08-13T09:09:12Z

Understand this PR’s impact

Explore downstream dependencies and potential security impact with Blast Radius.

View blast radius →

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

@coderabbitai resume to resume automatic reviews.
@coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

▶️ Resume reviews
🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: oven-sh/bun/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Essentials

Run ID: 5504a2c3-6edd-46c5-a9b1-f28d2547fc92

📥 Commits

Reviewing files that changed from the base of the PR and between c5a70a7 and 348b1c9.

📒 Files selected for processing (2)

packages/bun-uws/src/HttpContext.h
packages/bun-uws/src/HttpResponse.h

Included review availability: Your plan provides up to 10 included reviews per hour; 2 remain after this review.

Walkthrough

Changes

The PR renames the writable notification socket API, adds shared HTTP request parking and replay for Bun.serve, preserves node:http backpressure handling, updates WebSocket upgrade behavior, and adds cross-transport HTTP/1.1 pipelining tests.

HTTP pipelining and writable notification

Layer / File(s)	Summary
Writable notification API `packages/bun-usockets/src/libusockets.h`, `packages/bun-usockets/src/socket.c`, `src/uws_sys/...`	The socket API now exposes `us_socket_request_writable`. Rust wrappers and sendfile continuation paths use `request_writable()`. Obsolete C++ helpers and bindings were removed.
Shared request parking and replay `packages/bun-uws/src/HttpParser.h`, `packages/bun-uws/src/HttpContext.h`, `packages/bun-uws/src/HttpResponse*.h`, `src/jsc/bindings/node/JSNodeHTTPServerSocket.cpp`	Parser state now stores shared parked request bytes. Bun.serve parks requests while responses are pending, resumes after output drains, and replays bytes through the parser. Node.js backpressure uses the shared state. WebSocket upgrades replay parked bytes after WebSocket initialization.
Pipelining and upgrade validation `test/js/bun/http/bun-serve-pipelining.test.ts`, `test/js/bun/websocket/websocket-server-upgrade-early-frames.test.ts`	Tests cover TCP, TLS, and Unix transports; ordered responses; request bodies; malformed requests; `Connection: close`; shutdown; and WebSocket upgrades with early frames.

Suggested reviewers: jarred-sumner, cirospaciari

Priority: ➖ Normal

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly summarizes the main change: Bun.serve now holds pipelined requests until the preceding response completes instead of closing the connection.
Description check	✅ Passed	The description explains the problem, implementation, behavior, trade-offs, related changes, and extensive verification results. It does not use the template headings exactly, but it provides the requ…

_{Comment @coderabbitai help to get the list of available commands.}

claude

I reviewed this PR and the automated bug hunter found no issues. Because it changes core HTTP/1.1 pipelining semantics across the uSockets/uWS/Rust boundary — including the parser's request-boundary park now shared between Bun.serve and node:http, and a new replay site inside onWritable — a human look is still warranted before merging.

What was reviewed:

The park/replay invariant (parked ⇒ reads paused; replay only when response complete, fully drained, and behind the close gate) holds across onData, markDone, and onWritable.
us_socket_request_writable is byte-identical to the two helpers it replaces; all callers (sendfile, mark_needs_more) route to it.
node:http's flood-prevention path is behavior-preserving after the field renames; replayParkedRequestBytes takes the vector before dispatch so re-parking during replay starts a fresh batch.
Connection: close parking cannot leak into a replayed dispatch: the only replay site sits after onWritable's close gate on the same predicate.

Extended reasoning...

Overview

This PR changes Bun.serve to hold a pipelined HTTP/1.1 request until the response ahead of it completes, instead of closing the connection mid-response. It touches 11 files across four layers: uSockets C (socket.c, libusockets.h), uWS C++ (HttpContext.h, HttpParser.h, HttpResponseData.h), the Rust FFI shims (Response.rs, socket.rs, us_socket_t.rs, libuwsockets.cpp), the node:http C++ binding (JSNodeHTTPServerSocket.cpp), and adds a 447-line test file with 11 tests covering tcp/tls/unix, sync/async handlers, request bodies, Connection: close, and a pipelined WebSocket upgrade.

The mechanism: the parser's existing parkAtNextBoundary (previously node:http-only for flood prevention) is now shared. For Bun.serve, onData derives it from HTTP_RESPONSE_PENDING | HTTP_CONNECTION_CLOSE at entry and after each request's body fin. Parked bytes are replayed from the tail of onWritable once the pending response is complete and fully drained; markDone() arms that writable dispatch. Two byte-identical C helpers are consolidated into us_socket_request_writable().

Security risks

HTTP pipelining is request-smuggling-adjacent. The change is careful here: parked bytes are replayed verbatim through the same onData parse path (no separate parser state), the Connection: close gate parks-and-discards rather than dispatching (fixing an RFC 9112 §9.6 violation the PR found), and reads are paused while anything is parked (bounding memory to one recv). The replay site sits after onWritable's close gate on the same HTTP_RESPONSE_PENDING | drained predicate, so parked bytes behind a closing response cannot be replayed. I did not spot a way for the parking to desynchronize framing between requests, but this is exactly the class of logic where a second pair of eyes is valuable.

Level of scrutiny

High. This is core HTTP server connection-state machinery shared by every Bun.serve connection, with a new re-entrancy point (onWritable → onData) and shared state now touched by both the Bun.serve and node:http instantiations. The PR description explicitly names three competing PRs that took different architectural approaches (synchronous replay from uws_res_end* vs. deliver-then-drop), which is a design decision a maintainer should ratify. The choice to replay from onWritable rather than markDone() (to avoid landing inside the caller's teardown) is well-argued but is the kind of subtle invariant that benefits from someone who knows the RequestContext teardown paths confirming it.

Other factors

Test coverage is thorough: the variant matrix (tcp/tls/unix × sync/async, request body, Connection: close, WebSocket upgrade) is covered, tests await observable conditions rather than sleeping, and the PR states all 11 fail on main.
The node:http rename (nodeHttpParkAtNextBoundary → parkAtNextBoundary, nodeHttpPausedSpill → parkedRequestBytes, feedNodeHttpData → replayParkedRequestBytes) is mechanical and the replay logic there is preserved; the extra writable event markDone() may arm on a node:http connection is stated to be tolerated by that instantiation's onWritable hook.
The ASSERT_NOT_REACHED() backstop keeps the old close behavior in release builds if the invariant is ever violated.
CI is still building; no results yet.

robobun · 2026-08-13T09:43:59Z

One detail from the review worth spelling out for whoever looks at this, since it is the only place the shared code changes what a node:http connection sees: markDone() arms a writable dispatch only when bytes are actually parked, which on a node:http connection means it is flood-paused (HTTP_NODE_READS_PAUSED set). That dispatch lands in onWritable<true>, whose only reaction to the paused state is the existing Bun__NodeHTTP__onReadsResumable call, and that hook is written to be re-entered by any writable event: it returns while outgoing backpressure remains, resumes directly when nothing is parked, and dedupes the replay task through nodeHttpSpillReplayScheduled. So the worst case is one redundant call into a guard that already runs on every drain event. The upstream test-http-pipeline-flood.js, the other test-http-pipeline-* / *-timeout-pipelining tests and test/js/node/http/node-http.test.ts pass locally on the debug build with this branch; they are the tests that drive that path.

robobun · 2026-08-13T20:03:48Z

Consolidating the open PRs for this close (#33664, #35036, #32868) onto this one. Pushed 7f9744e:

Ran the test hunks of the three older PRs against this branch. Bun.serve: defer pipelined HTTP/1.1 requests behind an async response instead of closing the socket #33664's four cases pass as written. Bun.serve: deliver the in-flight response when a pipelined request arrives while it is still pending #35036's and http: deliver the in-flight response when a pipelined request arrives behind an async handler #32868's cases deliver the in-flight response as they assert and differ only where they pin the "then close the connection" behaviour those PRs implemented; this branch answers the pipelined request instead. http: deliver the in-flight response when a pipelined request arrives behind an async handler #32868's node-http case passes on main as written (node:http/https/http2: raise Node v26.3.0 compat to ~94%, sync the upstream suites, and fix the Windows/macOS transport-layer teardown bugs they exposed #32488), so nothing of it is left to carry.
One gap found while doing that, from Bun.serve: deliver the in-flight response when a pipelined request arrives while it is still pending #35036's upgrade scenario: when the pending response is an async server.upgrade() and a request was held behind it, the WebSocket sent its 101 and then never read a frame. HttpContext::onData pauses reads while bytes are held, upgrade() destructs the HTTP state (and the held bytes with it) and us_socket_adopt keeps the paused flag. upgrade() now drops the held bytes and resumes before internalEnd(), which also keeps markDone() from arming a replay dispatch for them.
Folded in the scenarios the older PRs covered that this file did not: a 16 MiB Bun.file() body from the handler and as a file route over tcp/tls/unix (both truncate at one socket buffer on main), a held POST with a body followed by a Connection: close request, held bytes that are a parse error (first response intact, then the 505), and the upgrade case above. 20 tests in test/js/bun/http/bun-serve-pipelining.test.ts now; all 20 fail on main in under a second, all pass here, and the upgrade one also fails on this branch without the upgrade() change.
Description updated accordingly. The three older PRs are being closed with a pointer here.

claude

I reviewed this PR and didn't find any bugs. Because it reworks Bun.serve's per-connection request lifecycle across the uWS parse loop, onData/onWritable, markDone, and the WebSocket upgrade path — with shared state that node:http compat also touches — a human look would still be worthwhile.

What was reviewed:

The park → pause → writable-dispatch → replay invariant across onData, markDone(), and replayParkedRequestsIfResponseComplete, including the ordering of the close gate before replay so Connection: close bytes are never dispatched.
The upgrade() change: parked bytes cleared and reads resumed before internalEnd(), so the adopted WebSocket is not left paused and markDone() does not arm a replay for destructed HTTP state.
The us_socket_request_writable() consolidation against both removed helpers and its Rust callers; behavior is byte-identical (closed guard + last_write_failed + us_internal_rearm_writable).
node:http interaction: markDone()'s new writable arm on a flood-paused connection lands in onWritable<true>, whose Bun__NodeHTTP__onReadsResumable hook is idempotent and dedupes via nodeHttpSpillReplayScheduled.

Extended reasoning...

Overview

This PR changes how Bun.serve handles HTTP/1.1 pipelined requests: instead of closing the connection when a second request head is parsed while the first response is still pending, the parser parks the unparsed bytes and replays them from the next writable dispatch after the in-flight response completes. It touches the uWS HTTP parse loop (HttpParser.h), the socket handlers (HttpContext.h onData/onWritable), HttpResponseData::markDone(), HttpResponse::upgrade(), consolidates two duplicate uSockets helpers into us_socket_request_writable() (socket.c + Rust bindings), and adds a 20-test file. The park/replay fields were already present for node:http flood prevention and are renamed/shared rather than newly added.

Security risks

The change is on the request-smuggling-adjacent surface: it decides when a pipelined request behind a Connection: close (or HTTP/1.0) request is processed. The design parks such bytes and lets the existing close-after-drain gates discard them with the socket; the only replay site sits behind onWritable's close gate, which fires on the same conditions the replay needs, so parked bytes behind a closing response are never dispatched. That is tighter than main (which dispatched them and kept the connection open). The parked buffer is bounded to one recv by pausing reads while anything is parked. I did not find a path where parked bytes are replayed after the connection has been marked to close, but this is exactly the kind of invariant a maintainer should verify.

Level of scrutiny

High. This is core HTTP server behavior that every Bun.serve connection passes through, with state maintained across three callbacks (onData, markDone, onWritable) plus the upgrade path, and it shares the park vector with node:http compat. The replay-from-writable design is deliberately chosen over synchronous replay from markDone() to avoid re-entering the request handler while callers are still tearing down the previous response — that architectural choice, and the interaction with the Rust RequestContext teardown above it, deserves maintainer sign-off. The PR also changes user-visible behavior (pipelined requests are now answered instead of the connection being closed) and consolidates three older open PRs.

Other factors

Test coverage is thorough: 20 tests covering tcp/tls/unix, buffered/file/route bodies, async handlers, request bodies, parse errors, Connection: close, and both directions of WebSocket-upgrade-plus-pipelined-request; the description states all fail on main and pass here, and that the existing serve/websocket/node-http/request-smuggling suites pass locally. The ASSERT_NOT_REACHED() backstop keeps the old close as a debug tripwire. The uSockets helper consolidation is a straight dedupe (both removed bodies were byte-identical to the new one). No prior human review on the timeline.

robobun · 2026-08-13T23:36:50Z

7e720f4: the two upgrade cases now also run over tls, since upgrade() (and the resume added to it) is instantiated separately for the TLS flavour; 22 tests in the file, all still failing on main and passing here. The description's test list is updated to match. No further pushes planned from the consolidation side unless review turns something up.

claude

I reviewed this PR and the automated bug-hunting pass found no issues. Given the scope — a behavioral change to Bun.serve's core HTTP/1.1 dispatch path (from close-on-pipeline to park-and-replay), touching the parser boundary, onWritable replay, markDone(), WebSocket upgrade(), and the shared node:http park/spill machinery, and consolidating three prior PRs — a human look is still warranted.

What was reviewed:

The park/replay flow: parkAtNextBoundary derivation at entry and after body fin, replay only from onWritable tail after the close gate, and markDone() arming writable — checked that Connection: close bytes cannot reach the replay site.
us_socket_request_writable() vs the two removed helpers — byte-identical consolidation, all Rust/C callers updated.
upgrade() clearing parked bytes and resuming before internalEnd() so the adopted WebSocket reads and markDone() doesn't arm a stale replay.
node:http impact of the shared markDone() writable-arm — lands in onWritable<true>'s existing onReadsResumable hook, which is idempotent.

Extended reasoning...

Overview

This PR changes how Bun.serve handles HTTP/1.1 pipelined requests: instead of closing the connection when a second request head is parsed while the first response is still pending, it parks the unparsed bytes at the request boundary and replays them through onData from the tail of onWritable once the pending response is complete and drained. The change spans the uWS HTTP core (HttpContext.h, HttpParser.h, HttpResponse.h, HttpResponseData.h), a new usockets helper (us_socket_request_writable replacing two byte-identical helpers), Rust FFI wrappers, the node:http server socket bindings (rename-only), and a 584-line test file with 20 tests covering tcp/tls/unix × buffered/file/route bodies, async handlers, request bodies, parse errors, Connection: close, and WebSocket upgrade in both directions.

Security risks

HTTP/1.1 pipelining semantics are request-smuggling territory. The PR is careful here: bytes behind a Connection: close (or HTTP/1.0) request are parked by the same flag but never replayed because onWritable's close gate runs before the replay site on the exact conditions the replay checks (HTTP_RESPONSE_PENDING clear + fully drained), and the parked bytes are freed with the socket. Held bytes go through the ordinary parser on replay, so malformed input still gets the parser's error response. The parked buffer is bounded to one recv (reads are paused while anything is parked). I did not find a smuggling or unbounded-buffer vector, but this is exactly the class of change where a second pair of eyes on the state machine is valuable.

Level of scrutiny

High. This is production-critical hot-path code in the HTTP server that every Bun.serve request flows through, and it changes a long-standing behavior (close → hold-and-replay) that three other PRs have attempted. It also touches the shared node:http park/spill state (renames + one new markDone() writable-arm that reaches onWritable<true>) and the WebSocket upgrade path. The design decision itself — answer the pipelined request vs. deliver-then-close as #35036/#32868 did — is one a maintainer should sign off on.

Other factors

The PR is exceptionally well-documented: every non-obvious placement (why replay from onWritable and not markDone(), why AsyncSocket::pause not HttpResponse::pause, why drop parked bytes before internalEnd() in upgrade()) is justified in comments and the description. Test coverage is thorough (20 tests, all fail on main / pass here per the evidence block, across debug+ASAN and release). The old close is kept as an ASSERT_NOT_REACHED() backstop. The description also flags the interaction with #37099's overlapping helper. None of that removes the need for human review of a change this central.

robobun · 2026-08-14T01:48:13Z

Self-review of the park/replay state machine, tracing the code on this branch. Things that came out fine: the ASSERT_NOT_REACHED() backstop (every path that parses a head after a dispatched request passes the body-fin hook first, including the prelude and fallback paths, CL:0 and chunked; CONNECT never reaches a boundary; parked bytes imply an empty fallback and no body in progress, so a replay always starts at a boundary); parked bytes cannot get stuck (the only place HTTP_RESPONSE_PENDING is cleared arms the dispatch, last_write_failed keeps the interest on epoll, kqueue and libuv, TLS drains its spill first, both onWritable early returns get another event, the close gate sits ahead of the replay gate on the same drain conditions, and the pending response's timeout still bounds a client that goes silent); no double or reordered replay (the vector is exchanged out before re-feeding, anything read in the meantime is appended behind); memory is bounded by one recv per connection; a FIN arriving while parked is handled after the replay exactly as for an unpipelined request; server.stop() closes such a connection at markDone() ahead of the replay, which is the retriable outcome.

Two findings, both small, handled in 221a160 and in the description:

The runtime's own resume() calls (request-body backpressure release in RequestContext::detach_response / on_request_body_stream_drained, and the sink's end()) can land while bytes are parked and reopened reads that the pipelining code had closed. Harmless on epoll, where the replaying writable dispatch runs before the read in the same event, but on kqueue a peer FIN read that way reaches onEnd first and closes the connection over the parked request. HttpResponse::resume() now leaves the read side alone while bytes are parked; the two pipelining resume sites use AsyncSocket::resume() directly and node:http's flood prevention only resumes once its parked bytes are gone, so neither changes. Not observable on Linux, so no dedicated test; the body backpressure tests in serve.test.ts, the pipelining file (5 reruns), websocket-server, bun-serve-file, bun-server, node-http and the upstream pipeline/upgrade tests pass locally with it.
Re-parking copies the remainder of the recv once per replayed request, so a burst of N requests in one recv behind async handlers costs O(N x recv) of copying over the burst, bounded by the one-recv limit. Noted under the hot-path bullet rather than changed; a cursor would remove it if it ever matters.

Already documented in the code and description: the one drain callback a WebSocket gets after an upgrade that dropped parked bytes (the resume re-arms writable; #37099 removes that arming), and the isIdle window between markDone() and the replay.

robobun · 2026-08-14T02:15:43Z

ead253e adds one more scenario, over each transport: a request pipelined behind a streaming response that the app ends later. Besides covering the response-still-being-produced case, ending the stream is what makes the response sink call resume() on the socket while the request behind it is parked, so this exercises the HttpResponse::resume() guard from 221a160 (instrumented locally to confirm: taken once per transport; on epoll the outcome is the same without the guard, so it pins the scenario rather than distinguishing the guard). The raw-response reader in the test file learned chunked framing for it. 25 tests now; all fail on main in under a second, all pass here. Description updated.

For the record, 221a160 itself checked out locally as well: the serve.test.ts request-body and response backpressure tests and this file pass on it, and I could not find a Bun.serve path where the runtime's body-pause resume reaches the guard, since that pause is only ever taken on a non-final body chunk and nothing can be parked until the final one has been read; the streaming sink's unconditional resume at end of response is the reachable caller, which is what the new test uses.

claude

The isIdle finding from my earlier review is addressed in 7ba15c5 (with a test), and this pass found no further bugs. Given the scope — a new park/replay state machine in the core Bun.serve request dispatch path, touching onData/onWritable/markDone/upgrade() across both the Bun.serve and node:http instantiations — a human review is still warranted.

What was reviewed:

Traced the park → pause → markDone → us_socket_request_writable → onWritable replay loop for stuck/lost bytes and re-entrancy; every path that clears HTTP_RESPONSE_PENDING arms the replay, and the close gate in onWritable runs before it.
Checked that replayParkedRequestBytes swaps the vector out before re-feeding, so a re-park during replay cannot reorder or double-dispatch.
Verified the resume() guard and the upgrade() clear+resume against the node:http flood-prevention path (which only calls resume() once its parked bytes are gone).
Confirmed the two removed us_socket_*_needs_more helpers are byte-identical to the new us_socket_request_writable and every caller was migrated.

Extended reasoning...

Overview

This PR replaces the hard close that Bun.serve used to perform when a pipelined request head arrived while the previous response was still pending. Instead, the parser now parks the unparsed remainder of the recv buffer at the request boundary, pauses reads, and replays the parked bytes through onData from the tail of onWritable once the pending response is complete and drained. The park/replay primitive already existed for node:http flood prevention; this generalizes it (renaming nodeHttpParkAtNextBoundary/nodeHttpPausedSpill to parkAtNextBoundary/parkedRequestBytes) and wires it into the !IsNodeHttp instantiation. It also consolidates two duplicate "arm one writable dispatch" helpers into us_socket_request_writable(), guards HttpResponse::resume() and upgrade() against reopening reads over parked bytes, and fixes markDone()'s isIdle computation so a graceful server.stop() does not drop a held request (my earlier finding, now covered by a test). ~770 LOC across 12 files, including a new 700-line test file with 26 scenarios over tcp/tls/unix.

Security risks

The change sits on the request-smuggling-adjacent surface: it decides when a second request head is parsed vs. held, and it shares the park mechanism with node:http. The Connection: close handling is the sensitive bit — parking now also gates on HTTP_CONNECTION_CLOSE so bytes behind a close-marked request are held and then discarded by the existing close gate (never replayed), and the replay site sits behind that gate. Parked bytes are bounded to one recv (reads are paused while anything is parked), so there is no unbounded buffering. I did not find a way for parked bytes to be dispatched onto the wrong response slot or to bypass the HTTP_CONNECTION_CLOSE discard.

Level of scrutiny

High. This is the core Bun.serve HTTP/1.1 dispatch loop — every request on every connection passes through the code paths modified here, across both template instantiations (Bun.serve and node:http). The state machine has several interacting flags (parkAtNextBoundary, parkedRequestBytes, HTTP_RESPONSE_PENDING, HTTP_CONNECTION_CLOSE, HTTP_CLOSE_WHEN_IDLE, isIdle, socket pause state) maintained at four sites (onData entry, body-fin hook, markDone, onWritable tail), plus the upgrade() and resume() special cases. The PR description and code comments are unusually thorough about the invariants, and the test file exercises the matrix well, but the interaction surface is large enough that a maintainer who knows the uWS event-loop model should sign off.

Other factors

My one prior finding (isIdle not accounting for parked bytes → graceful stop drops a held request) was fixed in 7ba15c5 exactly as suggested, with a dedicated test that fails on the previous head.
The PR consolidates three overlapping open PRs (#33664, #35036, #32868) and folds their test coverage in.
There is a known small overlap with #37099 (same "arm writable" primitive under a different name); whichever lands second needs a trivial rebase.
CI reported green on all lanes with agents; the two never-scheduled darwin shards are a queue issue affecting all builds, not this PR.

claude

Re-reviewed after the isIdle fix in 7ba15c5 and the later commits (the HttpResponse::resume() guard, the streaming-response and tls-upgrade cases). This pass found nothing new. Given the scope — the Bun.serve request/response state machine, the shared parser park/replay path, and upgrade() — a maintainer sign-off is still worth having.

Checked in this pass:

markDone() now keeps isIdle false while parkedRequestBytes is non-empty; the graceful-stop test covers it.
replayParkedRequestBytes takes the vector before re-entering onData, so a re-park during replay cannot reorder or duplicate.
The resume() guard leaves node:http's flood-prevention resume alone (it only calls resume() once parked bytes are gone).
us_socket_request_writable is byte-equivalent to the two helpers it replaces; all Rust callers updated.

Extended reasoning...

Overview

This PR changes Bun.serve's handling of HTTP/1.1 pipelined requests: instead of closing the connection when a second request head arrives while the first response is still pending, the parser now parks the trailing bytes at the request boundary and replays them from onWritable once the response ahead completes and drains. The change spans the uWS HTTP state machine (HttpContext.h, HttpParser.h, HttpResponse.h, HttpResponseData.h), a consolidated uSockets primitive (us_socket_request_writable), the node:http compat replay path (JSNodeHTTPServerSocket.cpp, renamed fields only), and Rust FFI shims. A 700-line test file with 26 cases covers tcp/tls/unix, buffered/file/streaming bodies, request bodies, parse errors, Connection: close, graceful stop, and WebSocket upgrade in both directions.

Since my last review

My previous review flagged that markDone() set isIdle = true even with parked bytes, letting a graceful server.stop() drop a held request. That was fixed exactly as suggested and a test was added. Subsequent commits added the HttpResponse::resume() guard (so a request-body-backpressure resume from the runtime cannot reopen reads over parked bytes — the streaming-response test drives it), ran the upgrade cases over TLS, and refined comments. I traced each of these against the diff; the bug-hunting system found nothing.

Security risks

The parked bytes are re-fed through the same onData parse path fresh reads take, so all existing header/framing validation applies (the parse-error test confirms a held bad request still gets the 505 after the response ahead). Parking is bounded to one recv (reads pause while anything is parked). Connection: close and HTTP/1.0 requests park what follows and never replay it (RFC 9112 §9.6), covered by two tests. No new untrusted-input parsing was introduced.

Level of scrutiny

High. This is the per-connection request/response state machine for Bun.serve, and the park/replay flag and vector are shared with node:http's flood prevention. A mistake here could hang connections, reorder responses, or leak the parked buffer. The change is well-argued in the description and self-review comments, has 26 tests that fail on main and pass here, and CI has been green apart from a pre-existing Windows flake unrelated to this change — but it is not a mechanical change I would approve without a maintainer look.

Other factors

The PR consolidates three earlier attempts (#33664, #35036, #32868) and their test coverage. It notes a pending merge conflict with #37099's us_socket_mark_writable_pending (same primitive, different name) that whichever lands second will need to reconcile.

claude

I re-reviewed after the isIdle fix in markDone() (now also gated on parkedRequestBytes.isEmpty(), with the graceful-stop() test) and the bug hunting system found nothing further. Given the scope — a park/replay state machine threaded through onData/onWritable/markDone/upgrade()/resume() across both the Bun.serve and node:http instantiations, plus the shared us_socket_request_writable consolidation — a human look is still warranted.

What was reviewed

The two parkAtNextBoundary derivation points (entry and body-fin) vs. every path that reaches a request boundary, including the ConsumeMinimally fallback re-parse — the ASSERT_NOT_REACHED() backstop is unreachable for Bun.serve.
replayParkedRequestsIfResponseComplete ordering vs. shouldCloseConnection()'s three close conditions and onEnd's FIN-defer branches — parked bytes behind Connection: close are discarded, and the HTTP_CLOSE_WHEN_IDLE case is now covered by the isIdle fix.
node:http's shared park/replay path: markDone()'s extra writable dispatch on a flood-paused connection lands in onNodeHttpReadsResumable, which already dedupes via nodeHttpSpillReplayScheduled and holds under backpressure.
upgrade()'s drop-and-resume before endUpgradeHandshake(), and the HttpResponse::resume() guard against reopening reads over parked bytes.

Extended reasoning...

Overview

Twelve files, ~900 lines. The core is in packages/bun-uws/src/{HttpContext.h,HttpParser.h,HttpResponse.h,HttpResponseData.h}: the parser's node:http-only park-at-boundary flag and byte vector are generalised (parkAtNextBoundary / parkedRequestBytes), HttpContext::onData<false> derives the flag from cannotDispatchAnotherRequest() at entry and after each body fin, and onWritable<false> grows a tail (replayParkedRequestsIfResponseComplete) that re-feeds the parked bytes through onData once the response ahead has completed and drained. markDone() arms that dispatch via a new us_socket_request_writable() (socket.c), which also replaces the two byte-identical us_socket_sendfile_needs_more / us_socket_mark_needs_more_not_ssl helpers and their Rust callers. upgrade() drops parked bytes and resumes reads before adopting the WebSocket; HttpResponse::resume() leaves the read side paused while bytes are parked. JSNodeHTTPServerSocket.cpp is a rename-only refactor onto the shared replayParkedRequestBytes<true>. A 701-line test file exercises 26 scenarios over tcp/tls/unix.

Security risks

The change affects request-boundary handling on the server side, which is request-smuggling-adjacent. I checked that parked bytes are always re-fed through the ordinary onData → consumePostPadded path (so all existing header/framing validation applies), that bytes behind a Connection: close or HTTP/1.0 request are parked and then discarded with the socket rather than dispatched (RFC 9112 §9.6 — this is a fix; before, resetResponseState() cleared the close mark and the trailing request was answered), and that parked bytes are bounded to one recv per connection because reads are paused while anything is parked. No new parsing surface is introduced.

Level of scrutiny

High. This is production-critical: every Bun.serve HTTP/1.1 connection flows through the touched onData/onWritable handlers, and the state machine interacts with close gates, FIN handling, WebSocket upgrade, graceful shutdown, and the node:http flood-prevention path that shares the parser fields. The change is not mechanical — it introduces new invariants (reads paused ↔ bytes parked; replay only from onWritable after full drain) that must hold on every exit. That is why I am deferring rather than approving.

Other factors

My one prior finding on this PR (markDone() setting isIdle = true while bytes were parked, letting HTTP_CLOSE_WHEN_IDLE drop the held request during graceful stop()) was fixed exactly as suggested and covered by a new test; the thread is resolved.
The 26 tests are well-structured (one assertion object per scenario, until() awaits observable conditions with close short-circuiting, no sleeps), fail on main in <1s, and cover the tcp/tls/unix × buffered/file/route/stream/async matrix plus the upgrade, parse-error, POST-body, Connection: close, and graceful-stop edges.
The consolidation of us_socket_request_writable deletes two duplicate helpers whose bodies matched us_internal_rearm_writable; the Rust callers (mark_needs_more, mark_needs_more_for_sendfile) are updated in the same PR.
The PR description's self-review already traced the kqueue-vs-epoll ordering for the resume() guard and the O(N×recv) re-park copying bound; both match what I read in the code.

…mpletes A request head parsed while the connection's response was still pending (async handler, or a buffered body whose tryEnd tail was still draining through onWritable) made uWS close the connection: the in-flight response was cut off and the pipelined request was never answered. The parser now stops at the request boundary instead and parks the rest of the read verbatim (the mechanism node:http compat already used for flood prevention, shared and renamed). onData derives the park flag from the response state at entry and after each request's body fin, pauses reads while anything is parked, and markDone() arms one writable dispatch; HttpContext::onWritable replays the parked bytes through onData once the response is complete and drained, at which point nothing of that response is on the stack any more. A request behind one that will close the connection (Connection: close, HTTP/1.0) is parked too and goes down with the socket; previously it was dispatched and kept the connection open. us_socket_request_writable replaces the two identical copies of the arm-a-writable-dispatch helper (sendfile's us_socket_sendfile_needs_more and us_socket_mark_needs_more_not_ssl) and is what markDone() uses.

…d it HttpContext::onData pauses the socket while request bytes are parked behind a pending response. If that response turns out to be a WebSocket upgrade, upgrade() destructs the HttpResponseData (and the parked bytes with it, as for bytes trailing a synchronous upgrade) but us_socket_adopt carries the paused flag over, so the WebSocket sent its 101 and then never read a frame. Drop the parked bytes and resume before internalEnd(), so markDone() does not arm a replay dispatch for them either. Tests folded in from the earlier pipelining PRs (#33664, #35036, #32868) for the cases bun-serve-pipelining.test.ts did not cover yet: a Bun.file() body from the handler and a Bun.file() route over each transport, a held request that carries a body with a Connection: close request behind it, held bytes that are a parse error, and the upgrade case above.

The resume re-arms writable interest too, so the WebSocket gets one drain callback right after open; clearing the parked bytes before internalEnd() avoids a second one, it does not avoid that one.

…ests HttpContext pauses the socket while request bytes are parked behind a pending response and resumes it itself when it replays them (upgrade() when it drops them). The runtime's own resume() calls release a request-body backpressure pause and can arrive after that body completed and the bytes behind it were parked: RequestContext::detach_response, on_request_body_stream_drained, and the response sink's end(). Reopening reads there only queues more bytes behind the parked ones, and on kqueue, where the read and write filters are delivered separately, a peer FIN read that way reaches onEnd before the replay and closes the connection over the parked request. HttpResponse::resume() now leaves the read side alone while bytes are parked; the two pipelining resume sites call AsyncSocket::resume() directly and node:http's flood prevention only resumes once its parked bytes are gone, so neither is affected. Not separately observable on epoll, where the replaying writable dispatch runs before the read in the same event; the existing body backpressure and pipelining tests cover the paths involved.

Covers the response-still-being-produced case over each transport. Ending the stream is also what makes the response sink call resume() on the socket while the request behind it is parked, so this drives the HttpResponse:: resume() guard added in the previous commit (confirmed by instrumenting it locally: it is taken once per transport here). The reader learns chunked framing for it.

…e WebSocket A client that does not wait for the 101 gets its first frames into the read of the upgrade request. Since #43153 an upgrade made during the request's dispatch hands the rest of that read to the WebSocket, and one made in a later turn of the event loop failed with a 400, because by then the HTTP parser had read the frame as the next request. With requests held behind a pending response the parser no longer sees those bytes: upgrade() dropped them, so the 101 went out and the frames were lost without a sign, which is the outcome #43153 removed. upgrade() now takes the held bytes before it destructs the HTTP state and dispatches them to the WebSocket after open, as the loop would have for a read of their own. That covers frames in the read of the request and frames that arrive in a read of their own before the upgrade. An HTTP request held behind the handshake is not a valid frame, so the WebSocket fails the connection; it is never dispatched as HTTP. websocket-server-upgrade-early-frames.test.ts pinned the 400 for the later-turn upgrade; that test now expects the frames, in both arrival shapes.

us_socket_resume() closes a socket whose poll the kernel does not take back, and the close destructs the HTTP state. The replay then went on to take the parked bytes out of that destructed state (a crash when the failure is injected), and upgrade() went on with the handshake and the adoption of a closed socket, so open fired and close never did. The replay returns when the resume closed the socket. upgrade() resumes after the adoption and after open, where a close is an ordinary WebSocket close: open, then close with 1006, and the held frames are not dispatched.

claude

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Additional findings (outside the current diff — GitHub can't attach inline comments there):

🟣 src/jsc/bindings/node/JSNodeHTTPServerSocket.cpp — pre-existing: a node:http server can corrupt a WebSocket's memory when server.upgrade() runs while flood-prevention bytes are parked on that connection. The replay task posted at JSNodeHTTPServerSocket.cpp:498-509 only checks us_socket_is_closed(sock); after the upgrade, socket->socket is the adopted WebSocket and upgraded is true, so replayNodeHttpPausedSpill reinterprets WebSocketData as NodeHttpResponseData and writes to it. Fix: the task (and every replay entry point) must bail when self->upgraded is set, like the other socket methods at lines 274, 314, 578 and 603 do, or upgrade() must cancel the pending replay before adopting.

Extended reasoning...

A node:http compat connection has bytes parked by flood prevention (HTTP_NODE_READS_PAUSED with parkedRequestBytes non-empty, HttpContext.h:526-532 or 561-565, or pauseReads from _http_server.ts:2478) behind an Upgrade request, and JS calls server.upgrade(res) (ws.js:1566 -> NodeHTTPResponse.rs:545). NodeHTTPResponse.rs:570 calls resume_socket() first, which reaches onNodeHttpReadsResumable (JSNodeHTTPServerSocket.cpp:459): with no outgoing backpressure and parked bytes it posts the replay task at 496-509 holding a Strong ref to the cell (or one was already posted by the drain's onWritable at HttpContext.h:953). HttpResponse::upgrade then takes the parked bytes (HttpResponse.h:403), destructs the NodeHttpResponseData (436) and adopts the socket (453); NodeHTTP.cpp:261-266 sets socket->socket to the WebSocket and upgraded = true. When the task runs, sock is that live WebSocket, us_socket_is_closed is false, and replayNodeHttpPausedSpill (428-456) reinterpret_casts its ext as NodeHttpResponseData at 430, writes nodeHttpSpillReplayScheduled and parkAtNextBoundary into…

Verification: pre-existing (the base has the same replay-after-upgrade type confusion via the same route; the diff rewrites replayNodeHttpPausedSpill but keeps it) — triggered when node:http flood prevention has parked bytes that contain an Upgrade request and the replay of those bytes dispatches it as an 'upgrade' that is completed synchronously (e.g. ws attached to the http server). Mechanism verified.…

Review asked for it: about 130 added comment lines across the uWS and uSockets changes, many of them narrating the neighbouring code. Each block now keeps only its invariant or the reason for an ordering. No code change.

claude

Code review found no issues

No high-confidence issues detected in this change.

… can park it The parser checked the hold before the sawConnectionClose latch. A request with Connection: close (or HTTP/1.0) whose response is pending took the hold: bytes that arrived behind it were parked and reads were paused. Bytes that arrived after that stayed unread, and the close that follows the response then resets the connection. A unix socket reports that to the client behind the complete response (ECONNRESET), where main ends the stream cleanly. For Bun.serve the latch is now checked first, as on main: the connection keeps reading and drops the bytes, because nothing behind a closing request is ever dispatched (RFC 9112 9.6). node:http keeps its order, where the flood-prevention park comes before llhttp's closed state.

robobun · 2026-09-21T17:30:59Z

4a66fc4 restores one behaviour of main that this branch had changed.

A first request with Connection: close (or HTTP/1.0), a handler that takes 30 ms, and a client that keeps writing meanwhile. On main the client gets the complete response and a clean end of stream. On b9acb38, over a unix socket, it gets the complete response and then ECONNRESET. Over TCP the reset does not reach the client in this timing.

The cause: the parser checked the hold before the sawConnectionClose latch. The response was pending, so the bytes behind the closing request were parked and reads were paused. What arrived after that stayed unread, and a close over unread bytes resets the connection. Main keeps reading and drops those bytes, because nothing behind a closing request is ever dispatched (RFC 9112 9.6).

For Bun.serve the latch is now checked first, as on main. node:http keeps its order (the flood-prevention park comes before llhttp's closed state). One new test in the Connection: close block of bun-serve-pipelining.test.ts, over a unix socket: it passes on canary 367d939, fails on b9acb38 with ended: false, error: "ECONNRESET", and passes here.

Script (argument: unix | tcp)

import net from "node:net";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { rmSync } from "node:fs";
const transport = process.argv[2] || "unix";
const sleep = ms => new Promise(r => setTimeout(r, ms));
const sock = join(tmpdir(), `closing-${process.pid}.sock`);
const server = Bun.serve({
  ...(transport === "unix" ? { unix: sock } : { port: 0, hostname: "127.0.0.1" }),
  async fetch(req) {
    await sleep(30);
    return new Response("body of " + new URL(req.url).pathname);
  },
});
const second = "GET /b HTTP/1.1\r\nHost: x\r\n\r\n";
const c = transport === "unix" ? net.connect({ path: sock }) : net.connect(server.port, "127.0.0.1");
let got = "", ended = false, error = null;
c.on("data", d => (got += d.toString("latin1")));
c.on("end", () => (ended = true));
c.on("error", e => (error = e.code));
const closed = new Promise(r => c.on("close", r));
c.on("connect", async () => {
  c.write("GET /a HTTP/1.1\r\nHost: x\r\nConnection: close\r\n\r\n");
  await sleep(5);
  c.write(second);
  await sleep(10);
  c.write(second);
});
await Promise.race([closed, sleep(3000)]);
console.log(JSON.stringify({ transport, answers: [...got.matchAll(/body of (\/\w)/g)].map(m => m[1]), cleanEnd: ended, error }));
server.stop(true);
c.destroy();
try { rmSync(sock); } catch {}

canary 367d939d9  {"transport":"unix","answers":["/a"],"cleanEnd":true,"error":null}
b9acb38dba        {"transport":"unix","answers":["/a"],"cleanEnd":false,"error":"ECONNRESET"}
4a66fc42cf        {"transport":"unix","answers":["/a"],"cleanEnd":true,"error":null}

Not changed: a pending response that marks the connection close with its own Connection: close header. Nothing marks the connection until that response is rendered, so the bytes behind it are held. Main cuts that response. This branch delivers it whole and then closes.

claude

Code review found no issues

No high-confidence issues detected in this change.

Reads are paused while requests are parked, so what the peer writes after that stays unread in the kernel. The close gates then close over unread bytes. That resets the connection, and the kernel drops the part of the response that it has not sent yet. Two shapes reach that state. An awaited handler answers with a Connection: close response header while the client writes more requests in two later writes: over TCP and TLS the client gets 6.6 MB of an 8 MiB body and ECONNRESET, over a unix socket the whole body and ECONNRESET. A Connection: close request that is itself parked, with a later write behind it: both responses arrive, then ECONNRESET on a unix socket. Main loses the whole response in both shapes. The three close gates and the parse-error close now read and drop those bytes first (us_socket_discard_unread, at most 8 MiB, which a socket receive buffer bounds). A connection that closes dispatches none of them. Only for Bun.serve, and only when requests are parked or a replay is on the stack.

robobun · 2026-09-21T21:27:23Z

55af365 fixes a cut response that the hold itself could still cause.

Reads are paused while requests are parked, so what the client writes after that stays unread in the kernel. When the connection then closes, the close is over unread bytes. That resets the connection, and the kernel drops the part of the response that it has not sent yet.

shape, on `4a66fc4`	tcp	unix
awaited handler answers 8 MiB with a `Connection: close` response header, the client sends two more requests in two later writes meanwhile	6,602,380 of 8,388,748 bytes, `ECONNRESET`	whole body, `ECONNRESET`
the same with one later write (that write is the parked one)	whole, clean	whole, clean
`[awaited] [GET, Connection: close] [third]` in one write, then one later write	two responses, clean	two responses, `ECONNRESET`

Main loses the whole response in each row, so this is not a regression. It is the failure this PR is about: what is queued behind a response takes that response down.

The three close gates and the parse-error close now read and drop those bytes first: us_socket_discard_unread(), at most 8 MiB, which a socket receive buffer bounds. A connection that closes dispatches none of them. The discard runs only for Bun.serve, and only when requests are parked or a replay is on the stack. With it every row above is whole and clean, also with five later writes.

Two new tests over tcp, tls and unix. Without the change the first fails on all three transports (cut body on tcp and tls, ECONNRESET on all) and the second fails on unix. A client that keeps writing after the close can still reset the connection, as on main.

claude

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Beyond the inline finding, I checked the other close gates that fire while requests are parked (closeIfDoneAndMarked in HttpResponse.h, the parser-loop and onWritable gates in HttpContext.h at the FIN/close-when-idle and Connection: close paths) — each of those calls discardBytesUnreadBehindParkedRequests before shutdown, so the cork() gate is the only remaining site that closes over unread bytes.

Extended reasoning...

This push (55af365) extends the pipelining change with us_socket_discard_unread and a discard call at the close gates so a Connection: close response behind parked bytes ends cleanly instead of resetting. The one confirmed finding is the cork() close path that skips that discard; the other four gates were traced and do call it. No auth, crypto, or injection surface is touched, but the change spans the uWS parser, context, and usockets hot paths, so the inline finding plus a human look remain warranted.

Additional findings (outside the current diff — GitHub can't attach inline comments there):

🟡 packages/bun-uws/src/HttpResponse.h — A client that pipelined behind an async HEAD or 304 handler can still get an RST instead of a clean end when that response says Connection: close. The close gate inside HttpResponse::cork() at HttpResponse.h:943-951 shuts down and closes without calling discardBytesUnreadBehindParkedRequests, so the bytes left unread while reads were paused reset the connection. Fix: every shutdown+close gate that can run with bytes parked or a replay on the stack must drain unread input first; replace the duplicated block in cork() with closeIfDoneAndMarked(httpResponseData), which already does, so the corked end_without_body completions (HEAD, 304) match the other gates.

Why this was flagged

Client sends HEAD /a (async handler) then GET /b in one read, then writes more. onData parks /b and pauses reads (HttpContext.h:776-777), so the later bytes stay unread in the kernel. The handler resolves with a Response carrying Connection: close; NodeHTTP.cpp:455-456 sets HTTP_CONNECTION_CLOSE when the header is written. RequestContext.rs:2816 renders HEAD under run_corked_with_type -> HttpResponse::cork(); do_render_head_response ends via end_without_body (RequestContext.rs:1316-1331). uws_res_end_without_body (libuwsockets.cpp:1154-1213) only calls markDone; uws_res_close_if_done_and_marked (libuwsockets.cpp:1110-1134) skips closeIfDoneAndMarked because the socket is still corked. Control returns to HttpResponse::cork(), which uncorks and runs its own gate at HttpResponse.h:943-951: shouldCloseConnection() is true, HTTP_RESPONSE_PENDING is clear, hasFullyDrained() is true, so it calls shutdown() and close() with no discardBytesUnreadBehindParkedRequests. close() over unread receive data sends RST and the kernel drops any unsent tail; over a unix socket the client sees…

Verification: normal (bounded: the response headers are already sent; the client sees an RST/ECONNRESET instead of a clean FIN, which is exactly the outcome the PR's fifth update declares a failure and tests for with seen: { ended: true } at test/js/bun/http/bun-serve-pipelining.test.ts:529-553). Trigger: an async handler for a HEAD request (or a 304, or any response completed through end_without_body)…

… too HttpResponse::cork() had its own copy of the close gate, without the read that the other gates got in 55af365. A response without a body (HEAD, 304) completes while its socket is still corked, so its close runs from there: with a request held behind it and more bytes unread, the close reset the connection (ECONNRESET on a unix socket behind the response). cork() now calls closeIfDoneAndMarked(), which is the same gate with the read.

robobun · 2026-09-21T21:55:42Z

The cork() close gate from the last review is fixed in 480dfe8. HttpResponse::cork() had its own copy of the gate without the read that the other gates got in 55af365. It now calls closeIfDoneAndMarked(), which is the same gate with the read, so the copy is gone.

New test: an async HEAD handler whose response says Connection: close, with one request held behind it and one more left unread. It runs over a unix socket only, because that is where the reset is visible for a response this small. Before the fix the client saw ECONNRESET behind the response. Now the stream ends. serve.test.ts, bun-server, request-smuggling, serve-http2, bun-serve-ssl and node-http pass with the change.

claude

Code review completed

Nothing new to post: everything this review found is already covered by existing comments on this pull request or didn't merit a separate one.

…rked requests 55af365 read what was unread once, right before the close. That covers what the server's receive buffer holds. A peer that queued more than that (a 1 MiB POST behind the parked requests) still has the rest in its own kernel, behind the closed receive window. The one read opens the window, the rest arrives after close(), and the reset drops the unsent end of the response in front: an 8 MiB answer arrived cut in 9 of 42 runs here (debug build, node client in its own process), and the client's own write failed with EPIPE in the others. The close gates now linger in that state instead. shutdownAndClose() sends the FIN and leaves the socket open with reads resumed. onData already ignores a socket that is shut down, so what the peer still sends is read and dropped. The peer's FIN closes the socket (the loop does that for a socket that has sent its own FIN). A timeout of 4 to 8 seconds and a limit of 8 MiB bound a peer that does not stop. The linger starts only for Bun.serve, only when requests are parked or a replay is on the stack, and only when us_socket_queued_input() reports unread data. Every other close is the same shutdown() and close() as before. us_socket_discard_unread() is gone again: the linger reads through the normal path, so usockets needs no new function. HTTP_LINGERING_CLOSE marks the socket. The gates skip a socket that has it, and setTimeout()/resetTimeout() leave its timeout alone: the runtime resets the timeout after the response ends, and with idleTimeout: 0 that would remove the bound.

robobun · 2026-09-22T07:15:48Z

8e68a68 replaces the one read before the close (55af365) with a close that lingers. One case was still open.

The one read covers what the server's receive buffer holds. A peer that queued more than that behind the parked requests, for example a 1 MiB POST, still has the rest in its own kernel behind the closed receive window. The read opens the window, the rest arrives after close(), and the reset drops the unsent end of the response in front. A client on the server's own event loop does not show it, so I measured with a node client in its own process against the debug build: [GET /slow, 8 MiB after 30 ms] [GET /held, Connection: close], then a POST with 1 or 4 MiB 5 ms later.

	whole 8 MiB answer	cut, `ECONNRESET`
`480dfe8`	33 of 42	9 of 42
`8e68a68`	42 of 42	0 of 42

What changed:

HttpResponse::shutdownAndClose() is the end of every close gate and of the parse-error close. When requests are parked (or a replay is on the stack) and us_socket_queued_input() reports unread data, it sends the FIN, resumes reads and does not close. onData already ignores a socket that is shut down, so what the peer still sends is read and dropped. The loop closes a socket that has sent its FIN when the peer's FIN arrives.
Bounds: 8 MiB of dropped bytes (counted in onData) and a timeout of 4 to 8 seconds. A flood that ignores the FIN was closed after 8.1 to 8.9 MB. A silent peer that never closes was closed after 7.9 s, with idleTimeout: 0.
HTTP_LINGERING_CLOSE marks the socket. The gates skip it. setTimeout() and resetTimeout() leave its timeout alone, because the runtime resets the timeout after the response ends, and idleTimeout: 0 would remove the bound.
us_socket_discard_unread() is gone again. usockets gets no new function from this PR for the close.
Every other close is the same shutdown() and close() as before. node:http is not touched.

New test, with the server in a child process: the client queues a 4 MiB upload behind the parked requests. On 480dfe8 it failed 8 of 8 runs (EPIPE or ECONNRESET, some with cut bodies; those runs used four connections, the committed test uses three). It passes 10 of 10 here. The file has 47 tests, 235 of 235 with --rerun-each 5.

The description now has a Downsides section, and its visible part is back under 250 words.

robobun · 2026-09-22T07:16:42Z

A note for whoever resolves the conflict with main. I have not touched the branch.

8e68a68 made this PR conflict with main. The previous head, 480dfe8, still merges cleanly with the same main (1016a7a). The conflict is in packages/bun-uws/src/HttpResponseData.h, and it is not a "keep both lines" conflict. Both sides added a state flag with the same value:

this branch (8e68a68): HTTP_LINGERING_CLOSE = 1 << 19
main (node:http: emit 'end' and 'error' on req.socket when the client closes the connection #43708): HTTP_NODE_PEER_ENDED = 1 << 19

Two enumerators with one value compile without a warning, and then the flags alias each other. HTTP_LINGERING_CLOSE is read in setTimeout(), resetTimeout(), shutdownAndClose() and closeIfDoneAndMarked() (HttpResponse.h) and at the top of onData (HttpContext.h). Only the line that sets it is limited to Bun.serve. The reads are not. So a node:http connection whose peer sent its FIN first would read as a lingering close: its timeout is no longer armed, and its close gates return without closing. HTTP_NODE_PEER_ENDED is also in HTTP_CONNECTION_SCOPED on main, so the alias would stay set for the life of the connection.

Suggested resolution: keep HTTP_NODE_PEER_ENDED = 1 << 19 and main's HTTP_CONNECTION_SCOPED as they are, and move the new flag to the next free bit, HTTP_LINGERING_CLOSE = 1 << 20. Main uses bits 8 to 19. The new flag does not need to be connection-scoped, because nothing is dispatched on a socket that has sent its FIN.

What I checked: the two dry-run merges above, the flag values and the mask on main, and the read sites of the new flag on this branch. What I did not do: I have not built or run 8e68a68, and I read only its HttpResponse.h and HttpResponseData.h changes, not the onData part or the new tests. My last verified head is 480dfe8.

claude

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

…-response Conflict: HttpResponseData.h. Main (#43708) took state bit 19 for HTTP_NODE_PEER_ENDED, which this branch used for HTTP_LINGERING_CLOSE. Keep both: HTTP_LINGERING_CLOSE moves to bit 20.

robobun · 2026-09-22T09:29:22Z

7bfa1ea merges main bf80d21. The PR had a conflict again: main (#43708) took state bit 19 of HttpResponseData for HTTP_NODE_PEER_ENDED, the bit that 8e68a68 used for HTTP_LINGERING_CLOSE. Both stay, and HTTP_LINGERING_CLOSE moves to bit 20. Buildkite 119638 passed on 8e68a68 on every lane before the merge. After it, the pipelining file (47 of 47), serve.test.ts, request-smuggling, bun-server, the early-frames file, node-http, node-http-backpressure and the tests of #43708 (node-http-server-abort-events) pass on the debug build.

claude

Code review found no issues

No high-confidence issues detected in this change.

Still open from earlier reviews (2):

Unresolved: 2 minor or pre-existing.

8e68a68 tested HTTP_LINGERING_CLOSE in setTimeout(), resetTimeout() and at the top of closeIfDoneAndMarked(): about four tests per keep-alive request for a state that only a close can enter. shutdownAndClose() now sets idleTimeout to the linger's own bound when it starts. The resetTimeout() calls of the response teardown then arm that bound again, also on a server with idleTimeout: 0, so setTimeout() and resetTimeout() are the same as on main again. The test in closeIfDoneAndMarked() was redundant: shutdownAndClose() has it, behind the branch that only a closing connection takes. A silent peer that never closes is still closed by the timeout with idleTimeout: 0 (8.0 s here).

robobun · 2026-09-22T10:14:58Z

29164bb takes the lingering close of 8e68a68 off the request path. That commit tested HTTP_LINGERING_CLOSE in setTimeout(), in resetTimeout() and at the top of closeIfDoneAndMarked(), about four bit tests per keep-alive request. Now shutdownAndClose() sets idleTimeout to the linger's own bound when the linger starts, so the resetTimeout() calls of the response teardown arm that bound again (also with idleTimeout: 0), and setTimeout() and resetTimeout() are identical to main again. The test in closeIfDoneAndMarked() was redundant with the one in shutdownAndClose(). No behaviour change: a silent peer that never closes is still closed by the timeout with idleTimeout: 0 (8.0 s here), a flood that ignores the FIN after 8.1 to 9.3 MB, and the backlog case is whole 21 of 21. Buildkite 119691 on the merge 7bfa1ea had one red test, spawn.test.ts, which is red on main too.

Two review findings on the lingering close of 8e68a68. shutdownAndClose() started a lingering close on every path, also from the parse-error path of onData, which is the one caller that can reach it with a response still pending. That happens when a request that was held fails to parse its body after its handler was dispatched, and the client wrote more while it was held. The socket then stayed open for the 4 to 8 seconds of the linger, so onClose did not run and the handler did not see the abort. shutdownAndClose() now lingers only when no response is pending. Every other caller checks that first, so only the parse-error path changes: it shuts down and closes at once again, as on main. A lingering close also counted as idle when the response that closed the connection completed inside a replay: markDone() then sees no parked bytes, because the replay has moved them out, and sets isIdle. closeIdle(), from closeIdleConnections() or a graceful stop(), closed that socket in the middle of the linger. shutdownAndClose() now clears isIdle when the linger starts. No request is dispatched on a socket that has sent its FIN, so nothing sets it again. The two changes depend on each other. Since 29164bb setTimeout() no longer looks at the linger flag, so a handler that still ran during a linger could replace its timeout through server.timeout(). With no response pending there is no such handler. Two tests in bun-serve-pipelining.test.ts, with a client that keeps its side open after the FIN of the server. Both fail without the change.

claude

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Beyond the inline findings, I also checked whether an async handler calling server.upgrade() after an await could land on a socket left half-closed by the new lingering close — it cannot, because shutdownAndClose now enters the linger only with no response pending, so the parse-error path with a handler in flight still does a plain shutdown+close. The isIdle = false on linger entry and the !responsePending gate added in the latest commits address the two nits from the previous review.

Extended reasoning...

The change rewires HTTP/1.1 pipelining in the uWS layer (HttpContext/HttpParser/HttpResponse/HttpResponseData), adds a lingering-close state, a new usockets writable-request primitive, and about 1,300 lines of tests; it touches connection lifecycle and parser buffers, which is memory-safety-sensitive native code, not auth or crypto. Two nit-level findings are posted inline and one candidate (upgrade after await on a lingering socket) was ruled out from the responsePending gate in shutdownAndClose, so this note only records that ruled-out check and the addressed prior nits.

… test fast Two optional review findings. HTTP_LINGERING_CLOSE describes the connection, not the response in flight, so it belongs in HTTP_CONNECTION_SCOPED. Without it resetResponseState() clears the bit on a dispatch, which would drop the byte cap of the linger in onData and the no-op guard in shutdownAndClose(). I could not reach that dispatch. A lingering socket is shut down, and onData drops its reads. Over TLS the shutdown waits for spilled ciphertext, but the replay runs only after hasFullyDrained(), which counts the spill. With the dispatch instrumented, the pipelining file starts 12 lingering closes and dispatches nothing on any of them. So this is a correctness rule of the header, not a fix for a reachable defect, and it has no test. The `ended` promise of connectNodeSocket settled only on 'end'. A connection that is reset never ends, so a test that awaits it would reach its timeout instead of failing on what `seen` recorded. It now settles on 'end', 'error' and 'close'. With a reset simulated, the closeIdleConnections() test fails in about 0.5 s and reports ECONNRESET, where it used to hang.

claude

Code review completed

Nothing new to post: everything this review found is already covered by existing comments on this pull request or didn't merit a separate one.

robobun assigned Jarred-Sumner Aug 13, 2026

github-actions Bot added the claude label Aug 13, 2026

claude Bot reviewed Aug 13, 2026

View reviewed changes

This was referenced Aug 13, 2026

usockets: keep the writable poll armed after a short us_socket_write2 from inside drain() #38177

Open

Bun.serve: queue the parse-error response behind the buffered response instead of truncating it #38181

Open

claude Bot reviewed Aug 13, 2026

View reviewed changes

claude Bot reviewed Aug 14, 2026

View reviewed changes

Comment thread packages/bun-uws/src/HttpResponseData.h Outdated

claude Bot reviewed Aug 14, 2026

View reviewed changes

robobun force-pushed the farm/3aa1ef0f/serve-pipelined-behind-pending-response branch from 61b884a to 20ca6a0 Compare August 23, 2026 07:05

claude Bot reviewed Aug 23, 2026

View reviewed changes

robobun force-pushed the farm/3aa1ef0f/serve-pipelined-behind-pending-response branch from 20ca6a0 to 5dd931c Compare August 25, 2026 03:06

claude Bot reviewed Aug 25, 2026

View reviewed changes

robobun added 6 commits August 27, 2026 20:06

uws: say what the resume in upgrade() costs the adopted WebSocket

1958e9a

The resume re-arms writable interest too, so the WebSocket gets one drain callback right after open; clearing the parked bytes before internalEnd() avoids a second one, it does not avoid that one.

test: run the upgrade pipelining cases over tls as well

e809be2

claude Bot reviewed Sep 21, 2026

View reviewed changes

Comment thread packages/bun-uws/src/HttpContext.h

claude Bot reviewed Sep 21, 2026

View reviewed changes

Comment thread packages/bun-uws/src/HttpResponse.h

Comment thread packages/bun-uws/src/HttpParser.h Outdated

uws: cut the pipelining comments down to what the code cannot say

b9acb38

Review asked for it: about 130 added comment lines across the uWS and uSockets changes, many of them narrating the neighbouring code. Each block now keeps only its invariant or the reason for an ordering. No code change.

claude Bot reviewed Sep 21, 2026

View reviewed changes

robobun mentioned this pull request Sep 22, 2026

usockets: close a socket whose resume the kernel refuses from the loop, not from the caller #43738

Open

claude Bot reviewed Sep 22, 2026

View reviewed changes

Comment thread packages/bun-uws/src/HttpContext.h

Comment thread packages/bun-uws/src/HttpResponse.h

Merge branch 'main' into farm/3aa1ef0f/serve-pipelined-behind-pending…

7bfa1ea

…-response Conflict: HttpResponseData.h. Main (#43708) took state bit 19 for HTTP_NODE_PEER_ENDED, which this branch used for HTTP_LINGERING_CLOSE. Keep both: HTTP_LINGERING_CLOSE moves to bit 20.

claude Bot reviewed Sep 22, 2026

View reviewed changes

Comment thread packages/bun-uws/src/HttpResponseData.h

Comment thread test/js/bun/http/bun-serve-pipelining.test.ts Outdated

claude Bot reviewed Sep 22, 2026

View reviewed changes

robobun mentioned this pull request Sep 23, 2026

Bun.serve: send Connection: keep-alive and Keep-Alive: timeout=N by default #43850

Open

Conversation

robobun commented Aug 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Fix

Background

Downsides

Problem

Fix

Background

Related

Uh oh!

robobun commented Aug 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

robobun commented Aug 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented Aug 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews paused

Walkthrough

Changes

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Overview

Security risks

Level of scrutiny

Other factors

Uh oh!

robobun commented Aug 13, 2026

Uh oh!

robobun commented Aug 13, 2026

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Overview

Security risks

Level of scrutiny

Other factors

Uh oh!

robobun commented Aug 13, 2026

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Overview

Security risks

Level of scrutiny

Other factors

Uh oh!

robobun commented Aug 14, 2026

Uh oh!

robobun commented Aug 14, 2026

Uh oh!

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Overview

Security risks

Level of scrutiny

Other factors

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Overview

Since my last review

Security risks

Level of scrutiny

Other factors

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Overview

Security risks

Level of scrutiny

Other factors

Uh oh!

Uh oh!

claude Bot left a comment

robobun commented Aug 13, 2026 •

edited

Loading

robobun commented Aug 13, 2026 •

edited

Loading

robobun commented Aug 13, 2026 •

edited

Loading

coderabbitai Bot commented Aug 13, 2026 •

edited

Loading