Bun.serve: close idle connections on graceful stop(), declare closeIdleConnections() - #37074
Conversation
A graceful stop(false) closed only the listener, so idle keep-alive connections lingered until idleTimeout (or forever with idleTimeout: 0) and the drain promise waited on them. Graceful stop now sweeps the HTTP socket group: idle connections close immediately, and busy ones are marked with a connection-scoped HTTP_CLOSE_WHEN_IDLE flag that the shouldCloseConnection() gates act on once their in-flight work completes. In-flight responses still reach the client in full, open WebSockets and CONNECT/Upgrade tunnels are untouched, and a connection mid-request is spared until it completes or the client closes. Two gate fixes back this up. The completion gates never ran for a response ending corked outside the parser (the common async-handler case): internalEnd's own uncork releases the cork slot, so cork()'s post-uncork close check was skipped; internalEnd and the end-without-body shim now run the gate when outside the parser. markDone also marked a connection idle while node:http pipelined responses were still queued, so a closeIdleConnections() in that window could kill the connection mid-pipeline; isIdle now accounts for the queue and is cleared when a response (re)starts. node:http servers are exempt from the stop() sweep: Node's close() closes idle connections exactly once, and a connection whose response completes after close() stays keep-alive until its timeout reaps it (verified against Node v26). The public closeIdleConnections() keeps one-shot Node semantics on Bun.serve too, and is now declared in bun-types and documented.
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Walkthrough
ChangesHTTP shutdown and connection draining
Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/runtime/http/server.mdx (1)
303-312: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd
closeIdleConnections()to the Reference interface.The narrative documents this public API, but the
Serverinterface under Reference omits it. AddcloseIdleConnections(): numbernearstop()so the reference matchespackages/bun-types/serve.d.ts.Proposed documentation update
stop(closeActiveConnections?: boolean): Promise<void>; + /** + * Close idle keep-alive connections without stopping the server. + */ + closeIdleConnections(): number; + /** * Update handlers without restarting the server.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/runtime/http/server.mdx` around lines 303 - 312, Update the Server Reference interface to declare closeIdleConnections(): number alongside stop(), matching the public API and the signature in packages/bun-types/serve.d.ts.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@docs/runtime/http/server.mdx`:
- Around line 303-312: Update the Server Reference interface to declare
closeIdleConnections(): number alongside stop(), matching the public API and the
signature in packages/bun-types/serve.d.ts.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: ad560ff8-39b7-4562-bb81-1200701d5718
📒 Files selected for processing (8)
docs/runtime/http/server.mdxpackages/bun-types/serve.d.tspackages/bun-uws/src/App.hsrc/runtime/server/mod.rssrc/runtime/server/server_body.rssrc/uws_sys/App.rssrc/uws_sys/libuwsockets.cpptest/js/bun/http/bun-server.test.ts
|
Added closeIdleConnections() to the Reference interface in docs/runtime/http/server.mdx (f680d20), matching the declaration in packages/bun-types/serve.d.ts. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
…xture's close uws_res_end_sendfile bypasses internalEnd and the surrounding FileResponseStream frame returns false to uWS's onWritable, so none of the shouldCloseConnection() gates ran when a sendfile response completed: a connection marked close-when-idle by a graceful stop() (or carrying Connection: close) stayed open until idleTimeout and the stop() promise hung on it. Run the gate explicitly after the stream's on_complete callback, which must still see a live socket. Also assert the previously unread sawClose in the TLS drain fixture.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/runtime/server/FileResponseStream.rs`:
- Around line 482-488: Move the resp.close_if_done_and_marked() call out of the
path after on_complete and into the callback-owned completion path before the
terminal callback returns, while the response is still valid. Ensure finish()
executes afterward without accessing or reusing resp after on_complete.
In `@src/uws_sys/Response.rs`:
- Around line 246-251: Update the FFI declaration and the
`close_if_done_and_marked` call to pass a raw `*mut uws_res` pointer instead of
a reference, invoking it within an `unsafe` block. Ensure the method performs no
access to the response after the FFI call returns because `on_close` may
synchronously destroy its `HttpResponseData`.
In `@test/js/bun/http/bun-server.test.ts`:
- Around line 756-780: Update the response validation around received and
gotWholeBody to parse the HTTP response framing instead of counting raw socket
bytes. Extract and assert the response body length equals the declared
Content-Length and SIZE, while preserving the existing stop/drain timing checks
around server.stop(false).
- Around line 996-1000: Replace the fixed 2-second polling loop around the
socket close assertion with a close-event promise resolved by the existing close
handler. Await that promise after initiating shutdown, while preserving the
sawClose tracking and relying on the test runner timeout as the stall bound.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6744737e-905d-45a1-a53e-ab44a4fde679
📒 Files selected for processing (7)
src/runtime/bake/bake_body.rssrc/runtime/server/FileResponseStream.rssrc/runtime/server/server_body.rssrc/uws_sys/Response.rssrc/uws_sys/h3.rssrc/uws_sys/libuwsockets.cpptest/js/bun/http/bun-server.test.ts
…ket guard Parse the response framing in the sendfile drain fixture and assert body bytes against Content-Length instead of counting raw socket bytes; await the TLS fixture's server-initiated close instead of polling with a fixed deadline; and make the sendfile completion gate bail when the socket was already closed by an upstream callback, so it can never touch a destructed ext block.
…wide bit isParsingHttp is per-context, so a response completing on socket B inside socket A's onData window (a microtask drained during A's dispatch) skipped B's post-uncork close gate, and no later gate runs for B: a connection marked close-when-idle lingered until idleTimeout and the stop() promise hung on it. Track which socket the parser is on (save/restore for node:http's nested read replay) and defer to the post-parse gate only for that socket.
…omplete The gate inside uws_res_end_without_body could close the socket (and destruct its ext) before FileResponseStream::finish()'s completion callback touched the response, and it silently broke the documented invariant that end_without_body alone cannot close the socket, which the DevServer and HTMLBundle error paths rely on. Corked callers are covered by the cork() wrapper's post-uncork gate; the one uncorked Bun.serve completion path (finish()'s no-body fallback) now runs the gate itself after on_complete, mirroring end_sendfile. Adds an in-flight HEAD drain test to pin the corked no-body path.
|
Data point from an independent investigation of test/regression/issue/29181.test.ts, which this PR fixes: On current main (898b169) the "graceful stop resolves after 304 / HEAD" case in that file times out deterministically at Verified at b3955e9: the unmodified test passes again in ~100ms with a debug build (times out on main built the same way), and standalone repros (GET, 5x 304, 5x HEAD on a static file route, then graceful stop) all resolve immediately. Might be worth adding that file to this PR's verification list. |
…on closed sockets RequestContext::end_without_body can run uncorked (render_production_error from a rejection microtask), where no cork or parser gate evaluates the close mark: a stop()-marked connection answering a HEAD request whose handler rejected lingered until idleTimeout. Run the completion gate there, after the end. The third FileResponseStream completion sibling (on_read_chunk's EOF end) can close the socket inside resp.end() through internalEnd's uncorked gate, after which the on_complete bookkeeping callbacks cleared callbacks on the destructed ext block. Registering or clearing a callback on a closed socket is now a no-op in the shims, which also covers the pre-existing force_close variants of the same pattern.
There was a problem hiding this comment.
Additional findings (outside current diff — PR may have been updated during review):
-
🟡
test/js/bun/http/bun-server.test.ts:1442-1445— Two mid-fixture comments still describe the pre-PR "pipelined request dispatches → 200" behavior that this PR inverted: (1)runLateKeepAlive's body at L1307-1310 ("the pipelined request dispatches against it → 200") now only holds for the node:http caller — the Bun.serve caller passessecondOutcome: "closed"and the pipelined request is dropped; (2) the WebSocket-upgrade fixture at L1423 ("// Wait for both responses.") — the test now asserts exactly one status andout.upgraded === undefined, so the loop exits viasockClosedand "both responses" is stale. The outer/function-level doc comments were updated; these two inner ones were missed.Extended reasoning...
What the issue is
This PR flips the Bun.serve late-keep-alive tests from "the pipelined request dispatches → 200" to "the connection closes right after the held response, and the pipelined request is dropped". The function-level doc comment above
runLateKeepAlive(L1223-1240), the assertion comment (L1335-1337), the test titles, and the WebSocket test's outer doc comment were all rewritten to describe the new split behavior. But two comments inside the fixture bodies — surrounded above and below by the new code — were left describing the old behavior.The two stale comments
(1)
test/js/bun/http/bun-server.test.ts:1307-1310, inside the-escript body of the sharedrunLateKeepAliverig:// First request completes; the connection is still open so the // wrapper stays Strong, and the pipelined request dispatches against // it → 200. Previously: panic (or 503 when the gate checked // Strong-only).
This fixture is now shared by two callers with divergent expectations, threaded through the new
secondOutcomeparameter. The Bun.serve caller passessecondOutcome: "closed"(L1370), and the assertion at L1338 expectsstdoutto be""— meaningnextResponse()returned viasockClosedwith no second status line. The pipelined request does not dispatch on that path; the connection closes right after the held response. The comment now describes only the node:http caller (secondOutcome: "200", L1472) and directly contradicts what the Bun.serve caller asserts.(2)
test/js/bun/http/bun-server.test.ts:1423, inside the WebSocket-upgrade fixture body:// Wait for both responses. while (!sockClosed && (received.match(/\r\n\r\n/g) || []).length < 2) {
The new assertions this PR added at L1442-1445 expect
out.upgradedto beundefined(fetch is never called for the pipelined upgrade) andout.statusesto equal exactly[expect.stringMatching(/^HTTP\/1\.1 200\b/)]— one entry, not two. In the expected path the loop exits viasockClosed = trueafter the server-initiated close, and the< 2condition is dead. "Wait for both responses" is the pre-PR expectation (200 then 101).Step-by-step proof (Bun.serve path through comment 1)
runLateKeepAlive("/r", <Bun.serve snippet>, "closed")is called from the test at L1344.- The fixture holds the first request across
stop();stop_listeningrunscloseIdle(true), marking the busy connectionHTTP_CLOSE_WHEN_IDLE. release.resolve()(L1311) lets the held handler complete;internalEnd's post-uncork gate findsshouldCloseConnection()true (isIdle && CLOSE_WHEN_IDLE) and closes the socket.- The client's
closehandler setssockClosed = true;nextResponse()(called at L1314 forsecond) returns"". console.log(second)prints an empty line; the outer assertion at L1338 matchesstdout: ""forsecondOutcome === "closed".
At no point did "the pipelined request dispatch against it → 200" — the comment at L1307-1310 describes something the test now asserts does not happen for this caller.
Why this matters / how to fix
REVIEW.md: "Only comment what the code cannot say." A comment inside a shared fixture that describes only one caller's outcome — and directly contradicts the other's assertion — actively misleads the next reader about what the fixture proves. The fix is a two-line reword each: (1) note that the second outcome depends on the caller (Bun.serve closes; node:http delivers the queued response), or simply delete the sentence about the pipelined request since the outer doc comment already covers both cases; (2) change L1423 to something like "// Wait for the held response and the server-initiated close."
Impact / severity
No behavioral bug — the tests are correct and pass; only the inline prose is stale. This is a nit: worth tidying since the PR already updated every surrounding comment, but not merge-blocking.
… comments detach_response can resume a socket that the completion gate already closed (resume's resetTimeout reads the destructed ext), so pause and resume join the callback shims in no-oping on closed sockets. The two late-keep-alive fixture comments still described the pre-drain pipelined behavior; reworded for the split outcomes.
|
The two stale fixture comments from the additional-findings note are reworded in 8b98bd3: the runLateKeepAlive body comment now describes both caller outcomes (Bun.serve drops the pipelined request at the drain close, node:http delivers the queued response), and the WebSocket fixture's wait comment reflects the held-response-then-close flow. |
…ture's stderr The corked-arm close gate can close the socket inside internalEnd, after which tryEnd's hasResponded() read and uws_res_try_end's inline clearOnWritableAndAborted() touched the destructed ext (a direct method, so the guarded shims did not cover it). tryEnd now reports responded without the read when the socket closed (the gate only fires on a completed response), and the shim skips the redundant clear, which markDone already performed. Also drain stderr in the HEAD-rejection fixture like its siblings.
| // `on_complete`, which must see a live socket — so Connection: close | ||
| // and the graceful-stop close-when-idle mark actually close. Only | ||
| // `finish()` runs after this, and it never touches `resp`. | ||
| resp.close_if_done_and_marked(); |
There was a problem hiding this comment.
What gurantees resp is still alive and correct here?
There was a problem hiding this comment.
Three separate guarantees, one per failure mode:
Alive (memory): usockets never frees a socket synchronously. us_socket_close only unlinks it and pushes it onto loop->data.closed_head (packages/bun-usockets/src/socket.c:209, 342, 389); the allocation is freed exclusively by us_internal_free_closed_sockets, which the loop runs at the end of the iteration (packages/bun-usockets/src/loop.c:362, called at :449). This whole sequence is one synchronous frame on the loop thread, so the us_socket_t behind resp outlives it regardless of what on_complete did. This is the same invariant end_sendfile already leaned on to pass resp to on_complete after end_send_file, and the one detach_resp's comment documents (uWS may free it on the NEXT loop tick).
Correct (still this HTTP socket): the only thing that re-types a socket mid-tick is us_socket_adopt from a WebSocket upgrade, which is only reachable from a live in-flight request dispatch; this request just completed (end_send_file ran markDone), and do_upgrade rejects a responded request. None of the on_complete callees (RequestContext::on_file_stream_complete, FileRoute/DirectoryRoute::on_response_complete) dispatch an upgrade; they do bookkeeping, and server deinit from on_request_complete is deferred via schedule_deinit, never a synchronous app.close().
Closed-but-not-freed (state): if anything in the frame did close the socket (the end itself, or JS reached through a drained microtask calling stop(true)), HttpContext::onClose has destructed the ext block, and the shim handles exactly that: uws_res_close_if_done_and_marked checks us_socket_is_closed first and returns before touching HttpResponseData (src/uws_sys/libuwsockets.cpp).
Pushed 7913af6 putting this argument in the comment at the call site so it does not live only in the PR.
| * any) has completed and all buffered outgoing data has been flushed. */ | ||
| bool shouldCloseConnection() const { | ||
| return (state & HTTP_CONNECTION_CLOSE) | ||
| || ((state & HTTP_NODE_RECEIVED_FIN) && nodeHttpQueuedPipelinedCount == 0); | ||
| || ((state & HTTP_NODE_RECEIVED_FIN) && nodeHttpQueuedPipelinedCount == 0) | ||
| || ((state & HTTP_CLOSE_WHEN_IDLE) && this->isIdle); |
There was a problem hiding this comment.
🟡 One layer up from the shims fb828c2/8b98bd35/a0337dc3 guarded: after callOnWritable returns, its own if (onWritable) restore (HttpResponseData.h:78-83) and HttpContext::onWritable's tail (HttpContext.h:743-746, 777-800) read httpResponseData->state/offset, hasFullyDrained() (unguarded — reads the destructed BackPressure buffer), shouldCloseConnection(), and resetTimeout() on an ext that ~HttpResponseData() already ran on when the Rust onWritable callback (on_writable_bytes, uncorked) called try_end and internalEnd's uncorked closeIfDoneAndMarked gate closed the socket synchronously. Pre-existing for HTTP_CONNECTION_CLOSE; this line adds the HTTP_CLOSE_WHEN_IDLE && isIdle trigger, widening it to every busy connection during stop(false). Same practically-benign class (ext inline in closed-list us_socket_t swept at tick end; POD reads on mapped memory; flush() at :761 IS guarded so that specific write is safe), so filing under REVIEW.md "fix the whole class — grep for every sibling site": simplest fix is if (us_socket_is_closed((us_socket_t *) s)) return s; right after callOnWritable returns, mirroring the parser's post-dispatch check.
Extended reasoning...
What was left out
Three fix commits (fb828c2, 8b98bd3, a0337dc) hardened callers against closeIfDoneAndMarked destructing the HttpResponseData ext mid-frame — guarding the shims (uws_res_on_aborted, clear_on_writable, on_timeout, on_data, pause, resume), tryEnd's hasResponded(), and uws_res_try_end's inline clearOnWritableAndAborted(). But one layer higher was not touched: HttpResponseData::callOnWritable (HttpResponseData.h:68-85) and HttpContext<SSL>::onWritable (HttpContext.h:737-800) both read httpResponseData fields after the borrowed onWritable callback returns, and that callback can synchronously close the socket via internalEnd's uncorked close gate — which now has this hunk's new HTTP_CLOSE_WHEN_IDLE && isIdle trigger.
The code path
HttpContext::onWritable (:737, does not cork) → callOnWritable → Rust on_writable_bytes / on_writable_complete_response_buffer (RequestContext.rs:1648/1304, uncorked) → resp.try_end(...) → uws_res_try_end → tryEnd → internalEnd, non-chunked arm, offset == totalSize, markDone() sets isIdle = true, then !Super::isCorked() → closeIfDoneAndMarked(httpResponseData). With HTTP_CONNECTION_CLOSE (pre-existing) or the PR's new (HTTP_CLOSE_WHEN_IDLE && isIdle) (this line), and hasFullyDrained() true (the tail fit in the kernel buffer), shutdown()+close() runs → us_socket_close → HttpContext::onClose synchronously runs ~HttpResponseData() on the ext (HttpContext.h:286-289). try_end returns true (a0337dc's guard makes tryEnd and uws_res_try_end themselves safe); the Rust callback calls detach_response() (guarded shims — safe) and returns true.
Then: callOnWritable reads if (onWritable) on the destructed ext (was nulled by markDone before the dtor, so happens to read null). Back in HttpContext::onWritable:
- :743-746 (the
!IsNodeHttpblock) readshttpResponseData->state,->offset, andasyncSocket->hasFullyDrained()— which readsgetAsyncSocketData()->buffer.length()on the destructedBackPressure(AsyncSocket.h:197-198, unguarded). - :761
asyncSocket->flush()— this one is safe: AsyncSocket.h:247 checksus_socket_is_closedfirst and returns 0. - :777-791 read
shouldCloseConnection(),state, andhasFullyDrained()again on the destructed ext, thenshutdown()+close()a second time on the already-closed socket (both no-op on the is_closed check). - :800
resetTimeout()readsidleTimeoutfrom the destructed ext.
Why nothing else prevents it
None of the shims fb828c2/8b98bd35/a0337dc3 guarded sit between internalEnd and HttpContext::onWritable's tail; there is no us_socket_is_closed check after callOnWritable returns; and on_writable_bytes returns true unconditionally, so the if (!success) return s short-circuit at :752 is skipped.
Step-by-step proof
- A Bun.serve response's first
try_endhits backpressure (>~kernel-buffer body).on_writable_bytesis armed with the remaining tail. server.stop(false)sweeps: the connection is busy, socloseIdle(true)setsHTTP_CLOSE_WHEN_IDLEon it (App.h:414). (Or, pre-existing: the client sentConnection: close.)- The socket becomes writable →
HttpContext::onWritable→callOnWritable→on_writable_bytescallsresp.try_end(tail, total, ...), uncorked. - The tail fits:
internalEndnon-chunked arm,offset == totalSize,markDone()setsisIdle = true,!isCorked()→closeIfDoneAndMarked:shouldCloseConnection()is true via(HTTP_CLOSE_WHEN_IDLE && isIdle),HTTP_RESPONSE_PENDINGcleared,hasFullyDrained()true →shutdown()+close()→onClose→~HttpResponseData(). try_endreturnstrue(a0337dc's is_closed guard);on_writable_bytescallsdetach_response()(fb828c2/8b98bd35's guarded shims no-op), returnstrue.callOnWritablereads/writesonWritableon the destructed ext;HttpContext::onWritable:743-800 readstate/offset/buffer.length()/idleTimeouton the destructed ext.
Impact
Same practically-benign class as the three prior accepted-and-fixed findings on this PR: the ext block is inline in a closed-list us_socket_t swept at loop-tick end, so the memory is still mapped; state/offset/onWritable/idleTimeout are POD and read as-left; BackPressure is a custom struct whose .length() is tail - head (POD size_t fields), not a std::string; flush() at :761 is guarded and no-ops; the second shutdown()/close() no-op on the is_closed check. Formally UB (object lifetime ended), no observable failure.
Pre-existing for Connection: close, but this hunk adds the HTTP_CLOSE_WHEN_IDLE trigger and the PR explicitly undertook this class across three fix commits at 6+ sibling sites, so filing as a nit under REVIEW.md "fix the whole class — grep for every sibling site".
Fix
bool success = httpResponseData->callOnWritable(...);
/* The onWritable callback may have completed the response and closed the
* socket via a shouldCloseConnection() gate; the ext is destructed. */
if (us_socket_is_closed((us_socket_t *) s)) {
return s;
}right after :737, mirroring the parser's post-dispatch check. This also makes the if (onWritable) restore in callOnWritable unreachable-after-close moot in practice (the field was nulled by markDone before the close), or that read can be guarded the same way inside callOnWritable.
|
The parse-window drain test added here was timing out on darwin CI (90s, build 90105): the partial-head staging could race the accept on a loaded host. Deterministic restaging in #37141, same gate coverage. |
`test/js/bun/http/bun-server.test.ts` has been flaking on darwin lanes since the graceful `stop()` drain work landed (#35130 on Aug 5, #37074 on Aug 6): about 30 failing builds in the 36 hours after the second merge, plus the 90s-timeout reported red in build [90105](https://buildkite.com/bun/bun/builds/90105). Three distinct failure modes, each with a verified cause. This PR is test-only; the code paths the tests pin are unchanged and still covered (see Verification). ## 1. GC churn test answered by a foreign server `request on a connection surviving graceful stop() never reaches a collected handler` failed with ``` FAIL fetch round 11: bad initial response [{"status":200,"body":"\n <!DOCTYPE html>\...//x/-/static/main.55cdd4aea43adabcb109.js\"></script>..."}] ``` That body is a verdaccio web UI page; the `routes` variant got verdaccio's 404 `{"error":"no such package available"}`. A freshly dialed connection to the fixture's own freshly bound port was answered by an npm test registry. Cause, reproduced on a macOS 14 CI host: the ephemeral port allocator on macOS honors only exact-address conflicts. A wildcard `Bun.serve({ port: 0 })` bind gets handed a port that another process already holds at `127.0.0.1` (one collision per ~16,384 binds, i.e. once per wrap of the ephemeral range; a loopback-bound holder under both a real node and a bun process, same result). Connects to `127.0.0.1:port` then reach the more specific foreign listener. The churn fixture performs hundreds of wildcard binds per run, so it finds such a port regularly, and leaked verdaccio processes supply the listeners: `VerdaccioRegistry.stop()` calls `kill(0)`, which is a liveness probe rather than a kill (regressed in #16540; fix open in #36352). Fix: bind the fixture's servers (and decoys) to `127.0.0.1`, the address the parks dial. Verified on the same host: 36,000 loopback-bound port-0 binds against a live loopback holder, zero collisions. This protects the test regardless of what else leaks on the machine; #36352 independently removes the main leak source. ## 2. Parse-window test 90s timeout (the build 90105 red) ``` ✗ server.stop() drain promise counts open connections > a response completing inside another socket's parse window still closes its drained connection [90001.99ms] ^ this test timed out after 90000ms. ``` The fixture wrote a partial request head on connection A and waited 20 `setImmediate` ticks before `stop(false)`. A partial head gives the server nothing observable, and on a loaded darwin host those ticks can elapse before the server has even accepted the socket. Verified on a macOS 14 CI host: when `stop(false)` runs while a handshake-completed connection is still waiting in the accept queue, macOS strands it - never accepted, never counted, never closed, and the client side stays silently open. (Linux keeps the pending accept, which is why this never fired there.) In the fixture that means `/poke` never dispatches, `releaseB` never resolves, B stays parked, and `while (!a.closed || !b.closed)` spins until the test timeout with no other output - exactly the observed signature. Fix: A's poke is now a `POST` with a held body. The handler's dispatch is awaited before `stop()` (so the sweep provably sees a busy connection), and writing the 2-byte body afterwards completes the request. The body's fin chunk is delivered inside A's parse window, so B's completion still runs in that window's microtask drain - the per-socket close-gate property the test exists to pin is exercised exactly as before. ## 3. Mid-request sparing test resolving early Build [89946](https://buildkite.com/bun/bun/builds/89946) hit `error: stop() resolved while a mid-request connection was open` in `a connection mid-request survives stop() until the client closes`. Same root cause: the connection's partial head had not reached the server when `stop()` ran, so the server (correctly) had nothing to count and the drain promise resolved. Fix: the mid-request state is staged as a partial second head on a keep-alive connection that already completed a full request, so the server demonstrably owns the socket. If the sweep still closes it, the head had not arrived and the connection was legitimately idle - that round proves nothing about sparing, so it is voided and retried on a fresh server (bounded, fails loudly if every round races). A resolution while the socket is left open is still reported as the bug it would be. Confirmed-spared rounds run in two variants, and the test requires one success of each: - destroy: the client hangs up and that resolves the drain (the original assertion). - complete: the client finishes the head after `stop()`. The request must still dispatch and be answered, which requires the close-when-idle mark to survive the dispatch's response-state reset (`HTTP_CONNECTION_SCOPED` in uWS `HttpResponseData.h`), and the mark must then close the served connection. This preserves the dispatch-after-stop coverage the old parse-window staging provided incidentally (its partial head completed after `stop()`), which the redesign in section 2 otherwise moves ahead of the stop. ## Verification - `bun bd test test/js/bun/http/bun-server.test.ts`: 74 pass / 3 fail, the 3 failures (`parse source map and fetch small stream`, `rejected promise handled by error method`, `abrubtly close a upload request`) fail identically on an unmodified checkout in this environment. - Drain block run 5x, churn test 2x locally: all pass. - Both redesigned fixtures extracted and run 30x on a macOS 14 CI host under the exact canary from build 90105 (`89d30ad11`): 60/60 pass, plus 40x under a 14-way CPU-spin load: all pass. - Coverage check for the parse-window test: patching the `internalEnd` close gates back to the context-wide `isParsingHttp` bit (the bug #37074 guards against) makes the redesigned test fail by timeout, and restoring the per-socket gate makes it pass. - Coverage check for the mid-request test's complete variant: dropping `HTTP_CLOSE_WHEN_IDLE` from `HTTP_CONNECTION_SCOPED` (so the mark is wiped when the post-stop request dispatches) makes it fail by timeout; restored, it passes. The updated fixture also runs 30/30 on the macOS 14 CI host under the build 90105 canary, and hangs as expected under a pre-#37074 build. The remaining hazard - any long-running wildcard port-0 listener on darwin CI can have its loopback traffic stolen by a later explicit `127.0.0.1` bind such as `VerdaccioRegistry`'s `randomPort()` (range 1024-65535 overlaps the kernel's ephemeral range) - is worth addressing in the harness separately; noted on #36352. <!-- robobun:evidence:begin --> --- **no test proof** · iteration 0 · Platform-specific test-only change; deferring to CI. <!-- robobun:evidence:end -->
…37141) `test/js/bun/http/bun-server.test.ts` has been flaking on darwin lanes since the graceful `stop()` drain work landed (oven-sh#35130 on Aug 5, oven-sh#37074 on Aug 6): about 30 failing builds in the 36 hours after the second merge, plus the 90s-timeout reported red in build [90105](https://buildkite.com/bun/bun/builds/90105). Three distinct failure modes, each with a verified cause. This PR is test-only; the code paths the tests pin are unchanged and still covered (see Verification). ## 1. GC churn test answered by a foreign server `request on a connection surviving graceful stop() never reaches a collected handler` failed with ``` FAIL fetch round 11: bad initial response [{"status":200,"body":"\n <!DOCTYPE html>\...//x/-/static/main.55cdd4aea43adabcb109.js\"></script>..."}] ``` That body is a verdaccio web UI page; the `routes` variant got verdaccio's 404 `{"error":"no such package available"}`. A freshly dialed connection to the fixture's own freshly bound port was answered by an npm test registry. Cause, reproduced on a macOS 14 CI host: the ephemeral port allocator on macOS honors only exact-address conflicts. A wildcard `Bun.serve({ port: 0 })` bind gets handed a port that another process already holds at `127.0.0.1` (one collision per ~16,384 binds, i.e. once per wrap of the ephemeral range; a loopback-bound holder under both a real node and a bun process, same result). Connects to `127.0.0.1:port` then reach the more specific foreign listener. The churn fixture performs hundreds of wildcard binds per run, so it finds such a port regularly, and leaked verdaccio processes supply the listeners: `VerdaccioRegistry.stop()` calls `kill(0)`, which is a liveness probe rather than a kill (regressed in oven-sh#16540; fix open in oven-sh#36352). Fix: bind the fixture's servers (and decoys) to `127.0.0.1`, the address the parks dial. Verified on the same host: 36,000 loopback-bound port-0 binds against a live loopback holder, zero collisions. This protects the test regardless of what else leaks on the machine; oven-sh#36352 independently removes the main leak source. ## 2. Parse-window test 90s timeout (the build 90105 red) ``` ✗ server.stop() drain promise counts open connections > a response completing inside another socket's parse window still closes its drained connection [90001.99ms] ^ this test timed out after 90000ms. ``` The fixture wrote a partial request head on connection A and waited 20 `setImmediate` ticks before `stop(false)`. A partial head gives the server nothing observable, and on a loaded darwin host those ticks can elapse before the server has even accepted the socket. Verified on a macOS 14 CI host: when `stop(false)` runs while a handshake-completed connection is still waiting in the accept queue, macOS strands it - never accepted, never counted, never closed, and the client side stays silently open. (Linux keeps the pending accept, which is why this never fired there.) In the fixture that means `/poke` never dispatches, `releaseB` never resolves, B stays parked, and `while (!a.closed || !b.closed)` spins until the test timeout with no other output - exactly the observed signature. Fix: A's poke is now a `POST` with a held body. The handler's dispatch is awaited before `stop()` (so the sweep provably sees a busy connection), and writing the 2-byte body afterwards completes the request. The body's fin chunk is delivered inside A's parse window, so B's completion still runs in that window's microtask drain - the per-socket close-gate property the test exists to pin is exercised exactly as before. ## 3. Mid-request sparing test resolving early Build [89946](https://buildkite.com/bun/bun/builds/89946) hit `error: stop() resolved while a mid-request connection was open` in `a connection mid-request survives stop() until the client closes`. Same root cause: the connection's partial head had not reached the server when `stop()` ran, so the server (correctly) had nothing to count and the drain promise resolved. Fix: the mid-request state is staged as a partial second head on a keep-alive connection that already completed a full request, so the server demonstrably owns the socket. If the sweep still closes it, the head had not arrived and the connection was legitimately idle - that round proves nothing about sparing, so it is voided and retried on a fresh server (bounded, fails loudly if every round races). A resolution while the socket is left open is still reported as the bug it would be. Confirmed-spared rounds run in two variants, and the test requires one success of each: - destroy: the client hangs up and that resolves the drain (the original assertion). - complete: the client finishes the head after `stop()`. The request must still dispatch and be answered, which requires the close-when-idle mark to survive the dispatch's response-state reset (`HTTP_CONNECTION_SCOPED` in uWS `HttpResponseData.h`), and the mark must then close the served connection. This preserves the dispatch-after-stop coverage the old parse-window staging provided incidentally (its partial head completed after `stop()`), which the redesign in section 2 otherwise moves ahead of the stop. ## Verification - `bun bd test test/js/bun/http/bun-server.test.ts`: 74 pass / 3 fail, the 3 failures (`parse source map and fetch small stream`, `rejected promise handled by error method`, `abrubtly close a upload request`) fail identically on an unmodified checkout in this environment. - Drain block run 5x, churn test 2x locally: all pass. - Both redesigned fixtures extracted and run 30x on a macOS 14 CI host under the exact canary from build 90105 (`89d30ad11`): 60/60 pass, plus 40x under a 14-way CPU-spin load: all pass. - Coverage check for the parse-window test: patching the `internalEnd` close gates back to the context-wide `isParsingHttp` bit (the bug oven-sh#37074 guards against) makes the redesigned test fail by timeout, and restoring the per-socket gate makes it pass. - Coverage check for the mid-request test's complete variant: dropping `HTTP_CLOSE_WHEN_IDLE` from `HTTP_CONNECTION_SCOPED` (so the mark is wiped when the post-stop request dispatches) makes it fail by timeout; restored, it passes. The updated fixture also runs 30/30 on the macOS 14 CI host under the build 90105 canary, and hangs as expected under a pre-oven-sh#37074 build. The remaining hazard - any long-running wildcard port-0 listener on darwin CI can have its loopback traffic stolen by a later explicit `127.0.0.1` bind such as `VerdaccioRegistry`'s `randomPort()` (range 1024-65535 overlaps the kernel's ephemeral range) - is worth addressing in the harness separately; noted on oven-sh#36352. <!-- robobun:evidence:begin --> --- **no test proof** · iteration 0 · Platform-specific test-only change; deferring to CI. <!-- robobun:evidence:end -->
server.stop() was blind to open WebSockets: - stop(false) only closed the listener (and, since #37074, idle HTTP connections). Open WebSockets stayed connected and kept serving traffic, so the returned promise never resolved while one was open. - stop(true) tore WebSockets down with a raw socket close, so the server close handler and the peer both observed 1006 (abnormal, no close frame) instead of 1001 Going Away. Add TemplatedApp::endAllWebSockets(code, reason), which snapshots every WebSocket group and calls WebSocket::end() on each open socket (close frame, close handler, FIN), exposed to Rust as NewApp::end_all_websockets. stop_listening() now calls it with 1001 on every stop path (graceful, abrupt, and abrupt after an earlier graceful stop) before closing the listener or the app, under the deinit_running re-entrance guard. node:http servers are exempt: Node's Server#close() leaves upgraded sockets to the user, and the ws shim drains its own clients set. Tests that relied on a graceful stop() leaving an already-open WebSocket alive now reach that state by holding the upgrade in fetch() until after stop(), which keeps their original assertions intact.
…keep the localhost fixtures The stress fixture parked keep-alive connections on a Bun.serve server and stopped it gracefully. Since #37074 that stop() closes idle connections inside the call, so every late request found a closed socket and the fixture's failure branches never ran (0 of 162 late requests dispatched on main, release and debug). node:http close() skips busy connections and leaves them keep-alive afterwards, as in Node, so the fixture now holds one request per connection across close(), drops the binding, GCs with a synchronous sweep, and requires every late request to be answered by the original handler. With the downgrade gate removed (is_drained without the connection term) it fails in round 1, 3 of 3 runs under the debug build; on main 162 of 162 late requests are answered. Keep the three hostname: "localhost" fixtures as they are. They reproduce the loopback resolution asymmetry that #38818 fixes in bun.
…40093) ### Problem - `test/js/bun/http/bun-server.test.ts` takes 9 to 16 s on every CI lane. The time is fixed waits, not work. - The GC stress test runs 400 rounds or 8 s, and since #37074 it tests nothing: a graceful `stop()` closes the idle parked connections, so every late request meets a closed socket (0 of 4770 dispatched on main). - Other fixed costs: 3 x 1000 ms CPU samples during the echo burst, 30-iteration GC loops with 10 ms sleeps, two 100 ms timers in the HEAD probe. ### Fix - The stress test targets node:http, whose `close()` leaves busy connections keep-alive afterwards, as in Node. Each round holds a request per parked connection across `close()`, drops the binding, runs `Bun.gc(true)`, and requires every late request to be answered by the original handler. With the downgrade gate removed it fails in round 1 (3 of 3). On main 162 of 162 late requests are answered. - Every other timer becomes the event it waited for (list in the notes). Top-level tests run concurrently where `test.concurrent(` fits on one line. - Stronger checks on full responses, `pendingRequests` around `stop()`, exact status lines, and piped `stdout`/`stderr`/`exitCode` for every fixture (list in the notes). - Verified, same machine and build: debug+ASAN 88.8 s to 25 s, release 10.4 s to 2.1 s, Windows x64 canary 11.1 s to 2.1 s. Pass set unchanged. ### Background - `Bun.gc(false)` is `Heap::collectSync`: collection only. `Bun.gc(true)` is `Heap::collectNow(Sync)`, which also sweeps, so dead cells with destructors are finalized inside the call. - The downgrade gate (#35130): the JS wrapper is the handlers' only GC root. `is_drained()` lets it go Weak only when no request, listener, websocket, or connection is left. Without it the wrapper is collected while connections are open and its finalizer closes them. - `heapStats().objectTypeCounts` counts prototypes with instances. `count - 1` while one instance is alive is the per-build floor the old 30-iteration drain produced. <details><summary>Notes</summary> Runs of this revision: debug+ASAN `bun bd test` 2 runs (25.5 s, 26.2 s), release `USE_SYSTEM_BUN=1` 5 runs (2.11 to 2.16 s), Windows x64 canary build of the same commit 4 runs (2.10 to 2.12 s, 79 pass). In this container three tests fail on main and on this branch alike (`rejected promise handled by error method`, `parse source map and fetch small stream`, `abrubtly close a upload request`): they bind `hostname: "localhost"`, which lands on `::1` here while the client dials `127.0.0.1`. #38818 fixes that in bun, so the fixtures stay as they are. The local runs unset `HTTP_PROXY`/`HTTPS_PROXY`: this container's egress proxy does not honor a bare `::1` in `NO_PROXY` for the new `http://[::1]:port/` request (#37429). CI has no proxy. Stress fixture evidence: - main's fixture on main (release, 400 rounds): answered 0, closed 4770. Debug build, 40 rounds: answered 0, closed 366. - PR fixture on main: answered 162 of 162, release (about 160 ms) and debug (5.8 s standalone). - gate removed (`is_drained()` returns `is_closed()`), debug build: `FAIL round 1: a parked connection closed`, 3 of 3. The finalizer's `app.close()` lands before the same round's late requests are answered. - bun 1.3.14 is not a control for the new fixture: its node:http `close()` closes busy connections too. - The first revision kept the Bun.serve fixture with `Bun.gc(true)` and 16 rounds (round-2 detection on 1.3.14, 60 of 60, versus round 15 to 153 with `Bun.gc(false)`). Review found the late requests unreachable on main, hence the retarget. Assertions added: `{status, text}` on plain responses, `pendingRequests` before and after `stop()` plus the exact status line in the drain fixtures (`HTTP/1.1 200 OK`, `HTTP/1.1 500 Internal Server Error`, `""` for the force-closed connection), the negotiated subprotocol and the close frame in the custom-protocol test, `AbortError` plus the exact byte count for the truncated upload, `redirected`/`url` on the redirect, `Completed: 10` from the unref fixture, and `{stdout, stderr, exitCode}` via `toEqual` for every spawned fixture. The nitro fixture runs with `PORT=0` instead of port 3000. Concurrency: six long-named tests (or tests with a timeout argument) stay sequential `test(` because prettier reflows `test.concurrent(` calls that exceed 120 columns and would reindent their bodies. The two `await server.stop()` tests also stay sequential: they assert that a fetch to the freed port fails. Timers replaced by events: the abort-signal stream tests await the server-side `abort`; the truncated upload half-closes once the server consumed the chunk; the HEAD probe sends `Connection: close` and reads to `end`; the CPU fixture samples 3 x 500 ms after the last echo; the heapStats loops poll `objectTypeCounts` with a 5 s deadline. `bun test` runs consecutive concurrent tests as one group (20 at a time, 5 under ASAN); a plain `test(` ends the group. Fixed waits removed: the `Bun.sleep(15)`/`Bun.sleep(10)` pairs in the two abort-signal stream tests, `setTimeout(..., 100)` in the upload test and in `doHead`, the `Bun.sleep(10)` in six GC loops, the 8 s floor of the stress test, the 1000 ms CPU windows, and the 30-iteration `drain(0)` baseline loops. The `Bun.sleep(15)` in "abort signal on server should only fire if aborted" stays: a negative check with no event to await, inside a concurrent block. `Bun.spawnSync` in two "Server" tests blocked the event loop of the concurrent block. Both use `Bun.spawn` now. `tryWritePending` kept the written prefix on a partial write (`data.slice(0, written)`). It keeps the unsent tail now. Release per-test after the change: the idle-CPU fixture (1.5 s) bounds the file. Everything else is under 0.2 s. No file under `src/` changes. The edited CPU fixture is referenced only by this test file. </details> <!-- robobun:evidence:begin --> --- **no test proof** · iteration 0 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/bun/http/bun-server.test.ts <!-- robobun:evidence:end -->
### Problem - `server.upgrade()` after an `await` on a `Connection: close` or HTTP/1.0 request sends the 101, then closes the socket: `open()` runs on a dead socket, `close()` never runs, the `ServerWebSocket` leaks. Same after a graceful `server.stop()` during the `await`. - node:http (`ws` `handleUpgrade()` from a later task, HTTP/1.0): `AddressSanitizer: heap-use-after-free ... in us_socket_is_closed`. - Cause: `HttpResponse::upgrade()` ended the 101 through `internalEnd()`, which, uncorked, runs the close gate, then built the WebSocket over the closed socket. ### Fix - `upgrade()` ends the 101 with a new `endUpgradeHandshake()` (`HttpResponse.h:119`): headers terminated, response marked done, no close gate, no uncork. `internalEnd()` loses its `keepCorked` parameter, which only `upgrade()` set. - Correct because the 101 switches protocols: `Connection: close`, HTTP/1.0 and close-when-idle describe the HTTP connection, which ends here, not the WebSocket that takes over the socket. The synchronous path and node's `ws` already do. - #37447 edits the same hunk and asserts the opposite outcome. This lands first, then #37447 drops its post-`internalEnd` re-check and two `Connection: close` tests (Notes). - Verified: `test/js/bun/websocket/websocket-server.test.ts` (4 new, 3 fail on main), `test/js/first_party/ws/ws.test.ts` (1 new, use-after-free on main), plus neighbouring suites (Notes). ### Background - `HttpContext::onData` corks the socket while it parses. A synchronous `server.upgrade()` writes into it. After an `await`, writes reach the kernel at once. - The close gate (`closeIfDoneAndMarked`) shuts an HTTP connection down once the response is complete and flushed and `shouldCloseConnection()` holds: `Connection: close`, HTTP/1.0 (`HttpContext.h:431`) or `HTTP_CLOSE_WHEN_IDLE` (graceful `server.stop()`). - `upgrade()` destructs `HttpResponseData` and adopts the socket into the WebSocket context, which then owns it. <details><summary>Notes</summary> Repro against the released bun (1.4.0): a raw client sends `GET / HTTP/1.1` with `Upgrade: websocket`, `Connection: close` and a valid key to a server whose `fetch()` awaits `setImmediate` before `server.upgrade(req)`: ``` status line: HTTP/1.1 101 Switching Protocols outcome: socket closed by the server events: ["ws open", "upgrade returned true"] // never "ws close" ``` `GET / HTTP/1.0` with `Connection: Upgrade` gives the same. With `Connection: Upgrade` and HTTP/1.1 the frame sent from `open()` arrives. Why the gate fired only here: `internalEnd()` marks the response done and, uncorked, runs `closeIfDoneAndMarked()`. The status line and headers had already reached the kernel, so `hasFullyDrained()` was true. Corked (the synchronous path) the branch is skipped, and `onData` returns through its `upgradedWebSocket` arm, which has no gate. The `Connection: close` and HTTP/1.0 arms are as old as uWS. `HTTP_CLOSE_WHEN_IDLE` arrived in 1.4.0 with #37074, so an upgrade that completes after a graceful `server.stop()` worked in 1.3.x. What happened after the close on main: `us_socket_adopt()` returns a closed socket unchanged, so `upgrade()` read the destructed `HttpResponseData`, placed `WebSocketData` over the closed socket's ext, and ran `open()`. The WebSocket never gets a close event because the HTTP context's `onClose` already ran, so the `ServerWebSocket`'s strong self-ref is never downgraded. `endUpgradeHandshake()` does what `internalEnd({nullptr, 0}, 0, false, false, false, true)` did for the 101 (`writeStatus` was a no-op, the status was written, and the body is empty) minus the gate and the HTTP `resetTimeout()`, which `upgrade()` replaces with the WebSocket timeouts a few lines later. The 101 bytes on the wire are unchanged, Date header included. #37447 (open): it makes `us_socket_adopt()` return NULL for a closed or shut down socket, adds an up-front closed/shut-down check to `upgrade()`, re-checks after `internalEnd()` and returns `nullptr` when the gate closed the socket, and makes the Rust callers free the `ServerWebSocket` on `nullptr`. Its tests "returns false for an async upgrade of a connection marked Connection: close" and "releases the ServerWebSocket of a refused upgrade" assert `upgradeResult: false` for the bytes this PR's tests assert `true` for, with the client left holding a 101. With this PR the gate never runs during `upgrade()`, so the post-`internalEnd` re-check has nothing to catch. The up-front check, the NULL return and the caller handling stay useful for a socket that is already closed or shut down when `upgrade()` runs (a possible peer-FIN path on node:http), so the order is: this PR, then #37447 rebased without the re-check and the two tests. Graceful stop: #34961 (open) rewrites several existing tests to hold the upgrade in `fetch()` until after `server.stop()` and states that an in-flight request may still upgrade after a graceful stop. That is the async path this PR fixes, so the "graceful server.stop() during the await" case pins the behavior #34961 relies on. node:http: a request with `Connection: close` and no `Upgrade` token is a normal request (no `'upgrade'` event, same as node). `Connection: close, Upgrade` does not set the close flag (uWS flags a `Connection` value of exactly 5 bytes), so HTTP/1.0 is the only node:http handshake that reached the gate. That one crashed the unfixed debug build under ASAN. Out of scope, left as they are: whether `server.upgrade()` should refuse an HTTP/1.0 or `Connection: close` handshake (RFC 6455 4.2.1). The synchronous path accepts both today, and #35870 (open) proposes the `Connection: Upgrade` token check. If that lands first, the two `Connection: close` cases in `websocket-server.test.ts` need a rejected-handshake expectation instead. The HTTP/1.0 and graceful-stop cases are unaffected. A peer FIN before the upgrade: Bun.serve closes the socket at once (`HttpContext::onEnd`), so `server.upgrade()` returns `false` through `is_aborted_or_ended()`. node:http marks the socket unreadable and `ws`'s `completeUpgrade()` destroys it before the native upgrade (#39642). Local failures unrelated to this change: the `ServerWebSocket > send()` neighbours of the 30 s benchmark time out when the whole file runs concurrently on the debug build (they pass alone), and `bun-server.test.ts` has 4 tests that need `localhost`, IPv6 or outbound network in this container. </details> <!-- robobun:evidence:begin --> --- **no test proof** · iteration 0 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/bun/websocket/websocket-server.test.ts <!-- robobun:evidence:end -->
server.stop() was blind to open WebSockets: - stop(false) only closed the listener (and, since #37074, idle HTTP connections). Open WebSockets stayed connected and kept serving traffic, so the returned promise never resolved while one was open. - stop(true) tore WebSockets down with a raw socket close, so the server close handler and the peer both observed 1006 (abnormal, no close frame) instead of 1001 Going Away. Add TemplatedApp::endAllWebSockets(code, reason), which snapshots every WebSocket group and calls WebSocket::end() on each open socket (close frame, close handler, FIN), exposed to Rust as NewApp::end_all_websockets. stop_listening() now calls it with 1001 on every stop path (graceful, abrupt, and abrupt after an earlier graceful stop) before closing the listener or the app, under the deinit_running re-entrance guard. node:http servers are exempt: Node's Server#close() leaves upgraded sockets to the user, and the ws shim drains its own clients set. Tests that relied on a graceful stop() leaving an already-open WebSocket alive now reach that state by holding the upgrade in fetch() until after stop(), which keeps their original assertions intact.
2 participants
Requested by @Jarred-Sumner: make
server.stop()close idle connections, and add acloseIdleConnections()method to the HTTP server. The method already existed at runtime (node:http uses it internally) but was missing from the type declarations and docs; the real change is instop().Before
A graceful
stop(false)closed only the listener. Idle keep-alive connections lingered untilidleTimeout(default 10s, forever withidleTimeout: 0), and since #35130 the drain promise waits on open connections, soawait server.stop()hung on them.What changes
Connection state at
stop(false)time:server.closeIdleConnections()keeps its one-shot Node semantics: closes currently idle connections, spares busy ones without marking them, keeps listening. It returns the number of connections it closed, and is now declared inpackages/bun-typesand documented.How
App::closeIdle(closeWhenIdle)keeps the existing idle sweep and, when asked, marks busy connections with a new connection-scopedHTTP_CLOSE_WHEN_IDLEstate bit.shouldCloseConnection()honors the bit only while the socket is actually idle, so mid-request connections are left alone until their work completes. Gracefulstop_listeningruns the sweep with the mark for Bun.serve servers.markDone()used to setisIdlewhile node:http pipelined responses were still queued, so acloseIdleConnections()in that window could kill a connection mid-pipeline;isIdlenow accounts for the queue, andresetResponseState()clears it when a response (re)starts.internalEnd's own uncork releases the cork slot, socork()'s post-uncork close check was skipped.internalEndand the end-without-body shim now run the gate themselves when outside the parser. This also makesConnection: closeteardown reliable on that path.stop()sweep: Node'sclose()closes idle connections exactly once (Bun's_http_server.tsalready callscloseIdleConnections()explicitly), and a connection whose response completes afterclose()stays keep-alive until its timeout reaps it. Verified against Node v26.3.0; the sweep would have closed it early and raced already-queued pipelined requests.A connection that was busy at stop() time and closes after its response also drops a request pipelined behind that response (the connection closes at its first idle moment). That matches the existing policy for async pipelining on Bun.serve, and RFC 9112 9.3.2 requires pipelining clients to retry unanswered requests on a new connection.
Tests
Reworked the drain-promise fixtures in
test/js/bun/http/bun-server.test.tsto pin the new semantics (idle closed by stop, in-flight response delivered then closed, mid-request spared, stop(true) escalation, one-shotcloseIdleConnections()sweep that spares busy connections and keeps listening), and updated the late-keep-alive tests: Bun.serve now closes the drained connection instead of serving a late pipelined request, while the node:http variant still delivers the queued pipelined response.Local runs:
bun-server.test.ts,serve.test.ts,serve-http3.test.ts,bun-serve-static.test.ts,serve-body-leak.test.ts,websocket-server.test.ts,node-http.test.ts, and 46 vendored Node http server/keepalive/pipeline/close tests (includingtest-http-server-close-idle.jsandtest-http-server-close-idle-wait-response.js) pass with the change; the only failures are ones that also fail on an unmodified build in this environment (requestIP v6, privileged port, http proxy, /bun:info loopback, two stream tests).no test proof · iteration 2 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/bun/http/bun-server.test.ts