Bun.serve(http3): send CONNECTION_CLOSE on abrupt stop of an idle connection - #34038
Merged
Merged
Conversation
…nection
server.stop(true) routes to us_quic_listen_socket_close(), which called
lsquic_conn_close() on every live connection before closing the UDP fd.
For a server connection, ietf_full_conn_ci_close only schedules
SF_SEND_CONN_CLOSE when conn_ok_to_close() holds, and the tick that
packs the frame (end_write in lsquic_full_conn_ietf.c) additionally
requires IFC_GOAWAY_CLOSE, a received CONNECTION_CLOSE, or scheduled
packets. An idle server conn has none of those, so abrupt stop closed
the fd without ever telling the peer.
The client's pooled session then lingers until the negotiated idle
timeout. If another listener binds the same ephemeral port before that
fires, fetch({protocol:'http3'}) matches and enqueues on the dead
session. Non-streaming bodies recover via retry_or_fail; a
ReadableStream body has already been drained into lsquic's send buffer
and cannot, so it surfaces HTTP3StreamReset once the idle alarm fires.
This is the mechanism behind serve-http3.test.ts intermittently failing
'POST body without Content-Length still reaches the handler' (and
occasionally other tests) since the native H3 client landed.
Use lsquic_conn_abort() instead, which sets IFC_ABORTED and takes the
IFC_IMMEDIATE_CLOSE_FLAGS path that unconditionally packs
CONNECTION_CLOSE. The test file's withCustomServer fixtures are also
given stdin-end handlers so every server process goes through
stop(true) on teardown.
A lifecycle test pins the behaviour: let the server conn go idle,
stop(true), rebind the same port in-process, and POST a
ReadableStream body.
Collaborator
Author
|
Updated 7:51 AM PT - Jul 12th, 2026
❌ @robobun, your commit 3040796 has 2 failures in
🧪 To try this PR locally: bunx bun-pr 34038That installs a local version of the PR into your bun-34038 --bun |
Contributor
WalkthroughChangesThe QUIC listener now aborts live connections to ensure HTTP/3 shutdown lifecycle
Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/js/bun/http/serve-http3.test.ts`:
- Line 340: Replace the duplicated stdin end handlers in the six fixture scripts
with a shared STOP_ON_STDIN_END constant, and make the handler async so it
awaits server.stop(true) before calling process.exit(0). Remove the fixed 50ms
timeout while preserving the existing shutdown behavior across all affected
templates.
- Around line 934-938: Update the restart synchronization around the second
waitForStderr call to use a marker guaranteed to be emitted only by the
restarted start(port) invocation, rather than the stale /PORT=(\d+)/ match
retained in the stderr buffer. Preserve the existing port capture needed for the
initial startup and ensure the subsequent wait proves the restart completed
before continuing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 142a1f1a-c797-411a-a93e-7bcebdc6497e
📒 Files selected for processing (2)
packages/bun-usockets/src/quic.ctest/js/bun/http/serve-http3.test.ts
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
- hoist the stdin-end stop(true) handler into a shared STOP_ON_STDIN_END
constant and use it from every fixture script (including the original
withServer fixture) so teardown stays consistent
- wait on a RESTARTED marker rather than a second /PORT=/ match, which
resolved against the stale startup line in waitForStderr's accumulated
buffer
- read the restart command via process.stdin.on('data') like the sibling
scripts; the previous for-await-console reader is a separate stdin
consumer so the on('end') handler never fired and the fixture was
SIGKILLed after the 1s grace period
Contributor
There was a problem hiding this comment.
LGTM — both of my earlier notes are addressed in 887d0be (the new test now reads commands via process.stdin.on('data') so on('end') fires, and the restart wait uses the unique /RESTARTED/ marker).
Checked that lsquic_conn_abort is only reached on the abrupt-stop path where the UDP fd is closed on the very next line, so the abort-vs-close distinction can't leak into any surviving state. Also confirmed the graceful us_quic_socket_context_shutdown path still uses lsquic_engine_cooldown (GOAWAY) and is untouched.
Extended reasoning...
Overview
Two files: packages/bun-usockets/src/quic.c swaps lsquic_conn_close() → lsquic_conn_abort() in us_quic_listen_socket_close() (the abrupt server.stop(true) path), and test/js/bun/http/serve-http3.test.ts gains a STOP_ON_STDIN_END constant applied to seven fixture scripts, a 1s grace period in withCustomServer teardown, and a new lifecycle test that reproduces the idle-conn / port-reuse race.
Security risks
None. This is server-side shutdown of QUIC connections that are about to have their UDP fd closed regardless. Sending an explicit CONNECTION_CLOSE is strictly more informative to peers than the previous silent fd close; there's no auth, crypto, or input-parsing surface touched.
Level of scrutiny
Medium. The native change is one function call in the abrupt-close path, and the surrounding code already documented the intent ("Send CONNECTION_CLOSE on every live conn before the fd disappears") — this makes the implementation actually match that comment for idle server connections. The immediately-following us_udp_socket_close(ls->udp) means any behavioral difference between close and abort is moot: the fd is gone either way, so abort's "immediate_close → always pack CONNECTION_CLOSE" is the only observable delta. The graceful-drain path (us_quic_socket_context_shutdown) is unchanged and still uses cooldown/GOAWAY.
Other factors
- I reviewed the previous revision and left two inline notes; both are fixed in 887d0be. The new test now uses
process.stdin.on('data')(flowing mode, so the'end'handler inSTOP_ON_STDIN_ENDfires) and waits on a unique/RESTARTED/marker emitted afterstart(port)instead of the vacuous second/PORT=/match. - CodeRabbit's dedup suggestion was taken (
STOP_ON_STDIN_ENDconstant, 7 sites); the author explained whysetTimeout(100)is kept overawait server.stop(true)(flush is synchronous insideus_quic_listen_socket_close; the delay is for loopback delivery, whichstop()'s promise doesn't track). - The PR includes a regression test verified to fail on ASAN without the quic.c change and pass with it, plus a before/after breakdown of client-session close reasons across the whole file.
- The bug hunting system found no issues on this revision.
- No CODEOWNERS entry covers
packages/bun-usockets/.
Contributor
There was a problem hiding this comment.
No issues found after the fixes in 887d0be — the two test-fixture nits I raised (stdin 'end' handler not firing, vacuous /PORT=/ wait) are both resolved. Deferring to a human for the native change: swapping lsquic_conn_close → lsquic_conn_abort in us_quic_listen_socket_close() is small but changes wire-level QUIC shutdown semantics, and correctness hinges on the lsquic IFC_IMMEDIATE_CLOSE_FLAGS reasoning in the PR description.
What was reviewed:
- Traced
lsquic_conn_abortvslsquic_conn_closeagainst theend_write/immediate_closeconditions described in the comment — the rationale is internally consistent. - Checked that the
ctx->connsiteration inus_quic_listen_socket_closeis safe underlsquic_conn_abort(abort only sets a flag;on_conn_closedunlinks later viaus_quic_process, not during the loop). - Verified the new test now uses
process.stdin.on('data')+ sharedSTOP_ON_STDIN_ENDand waits on/RESTARTED/, so teardown and restart sync are non-vacuous.
Extended reasoning...
Overview
Two files: packages/bun-usockets/src/quic.c (one call site changed from lsquic_conn_close to lsquic_conn_abort inside us_quic_listen_socket_close, plus a 10-line explanatory comment) and test/js/bun/http/serve-http3.test.ts (new STOP_ON_STDIN_END constant applied at 7 fixture sites, a 1s grace period in withCustomServer teardown, and a new lifecycle test that stops/restarts an H3 server on the same port and POSTs a ReadableStream body). The goal is to fix intermittent HTTP3StreamReset failures caused by the client's pooled H3 session outliving an abruptly-stopped server whose UDP fd closed without a CONNECTION_CLOSE frame on the wire.
Security risks
None identified. The change doesn't touch input parsing, TLS verification, or auth. lsquic_conn_abort sends CONNECTION_CLOSE with transport error NO_ERROR — same wire frame as a graceful close, just guaranteed to be packed via the IFC_IMMEDIATE_CLOSE_FLAGS path rather than gated on send-ctl state.
Level of scrutiny
This is native C in the QUIC transport glue layer, affecting what goes on the wire when server.stop(true) runs with live H3 connections. The diff is one function call, but its correctness rests on a specific reading of lsquic's ietf_full_conn_ci_close / end_write state machine (documented in the new comment and PR description). I checked that the iteration over ctx->conns doesn't UAF — lsquic_conn_abort only sets IFC_ABORTED; the list mutation happens later in on_conn_closed via us_quic_process after the loop completes. The reasoning looks sound, but someone familiar with the lsquic vendor code should confirm the immediate_close path behaves as described for server-side idle conns.
Other factors
All four prior review threads (two from me, two from coderabbit) were addressed in 887d0be and are resolved: the STOP_ON_STDIN_END dedup, the switch from for await console to process.stdin.on('data') so the end handler actually fires, and the /RESTARTED/ marker replacing the vacuous second /PORT=/ wait. The latest commit (3040796) is an empty CI retrigger. The PR includes before/after evidence showing the new test fails on debug/ASAN without the quic.c change and passes with it, plus a session-close breakdown (idle-timeout closes drop from 29→2 across the file). Given the protocol-level nature of the native change, I'm not comfortable approving without a human look at the lsquic semantics.
Collaborator
Author
|
CI status: The remaining reds are unrelated to this diff, which only touches
Three of the four macOS 14 x64 failures are plain timeouts on a single shard, which points at a slow box rather than a code path this PR reaches. Ready for review/merge. |
Jarred-Sumner
pushed a commit
that referenced
this pull request
Sep 14, 2026
…quest handler (#42619) ### Problem - `server.stop()` or `server.stop(true)` from an HTTP/3 request handler, or from a microtask the handler resolved (the end of a `using server` scope counts), runs inside `lsquic_engine_process_conns` and enters the engine again. lsquic forbids that. A debug build aborts: `lsquic_engine_send_unsent_packets(lsquic_engine_t *): Assertion '!((engine)->pub.enp_flags & ENPUB_PROC)' failed.` - Release, `stop(true)`: `us_quic_listen_socket_close` (`packages/bun-usockets/src/quic.c:917`) closes the UDP fd before the running tick packs the CONNECTION_CLOSE. Every peer waits for its idle timer (10 s to 30 s). - Release, `stop()` on a connection that already served a request: the process spins at 100% CPU and the request in the handler never completes. ### Fix - One enter/leave pair brackets every engine call that can run callbacks (`process_conns`, `send_unsent_packets`). While it is held, `us_quic_socket_context_shutdown` and `us_quic_listen_socket_close` only record the request. `leave` finishes it: GOAWAY or CONNECTION_CLOSE, flush, then the fd. - `us_quic_socket_context_listen` releases a pending fd before it binds to the same port, so `stop(true)` then `Bun.serve` on that port in one turn still works. A stop from a timer is unchanged. - Verified: `test/js/bun/http/serve-http3.test.ts` (9 new tests, release main fails 6, debug main fails 9). - Self-reviewed: the review asked for the graceful path in the same PR. Done. Supersedes #34447. ### Background - lsquic drives every connection from `lsquic_engine_process_conns`. Request handlers and the microtasks they resolve run inside it. - `lsquic_conn_abort` only sets a flag. The connection's tick sees it after the handler returns and packs the CONNECTION_CLOSE. The engine sends it before `process_conns` returns. - `ctx->processing` in `quic.c` is set while the engine is on the stack. <details><summary>Notes</summary> **Where this comes from.** No user report and no CI incident. #34038 made an abrupt stop send CONNECTION_CLOSE for an idle connection when the caller is off the stack. A probe of the same stop with a request in the handler found the cases above. **Timings, release 1.4.3-canary.1, client and server in one process, request in the handler** | call | from a timer | from the handler, before | from the handler, after | | --- | --- | --- | --- | | `stop(true)`, streamed POST | `HTTP3StreamReset` at once | `HTTP3StreamReset` @10 s (warm connection) or @30 s (cold) | at once | | `stop(true)`, GET, old port dead | `HTTP3HandshakeFailed` @10 s | @20 s | @10 s | | `stop()`, cold connection | response arrives | response arrives | same | | `stop()`, warm connection | response arrives | 100% CPU, never completes (stopped after 60 s) | response arrives | The stall for `stop(true)` is the smaller of the server `idleTimeout` (30 s when it is 0) and the client's `max_idle_timeout`, and it hits every connection of the server. A client that re-sends the request adds its 10 s handshake timeout against the closed port. That retry is by design for an idempotent request (`fetch-http3-client.test.ts`, `retries on a fresh session when a pooled session is stale (port reuse)`). After the fix the client log shows `conn_close status=8` at once. **Why the `stop(true)` tests use a `ReadableStream` body.** The fetch client never re-sends a streamed body, so `HTTP3StreamReset` surfaces as soon as the client sees the connection close. With any other body the retry hides the difference behind a 10 s handshake timeout. **Why not keep the fd open in every case.** `stop(true)` promises that the port is free when it returns, and `server.stop(true); server = Bun.serve({ port })` is a common restart pattern. A deferred fd still holds the port, and these sockets do not set `SO_REUSEPORT`. The first version of this change failed that rebind inside a handler with `Failed to listen on UDP port N for HTTP/3`. One test pins it. Only in that case the stopped server's connections go silent, as they did before. A bind to another port leaves the pending fd alone (one more test). **Engine calls outside the pair.** `lsquic_engine_cooldown` (makes no callbacks), `lsquic_engine_packet_in` and `lsquic_engine_connect` (their callbacks stay in C), and `lsquic_engine_destroy` in `us_quic_socket_context_free`. **Excluded, not new.** `server.stop()` followed by `server.stop(true)`: the first call takes `h3_listener`, so the second never reaches `us_quic_listen_socket_close` for HTTP/3. **Relation to #34447.** It wraps the abrupt path's engine calls in `if (!processing)` and defers the graceful path with its own latch and hook. It still closes the fd first on `stop(true)` (its test comment says the client then hangs), and it describes the graceful case as benign on release. This PR keeps one latch per call and one hook, and ports its two graceful tests with a warm-connection variant. **Relation to #42579.** Its new client test stops a server after one `setImmediate` hop to stay off this path. After this PR the hop is not needed. **Suites run on the debug ASAN build:** `serve-http3` (62 pass), `fetch-http3-client` (56), `serve-protocols` (20), `fetch-http3-adversarial` (27), `fetch-http3-cold-post` (2), `fetch-http3-syscall-fault` (3). </details> <!-- robobun:evidence:begin --> --- **[human-review]** gate passed · iteration 0 · 2 files touched <details><summary>fails on main (without fix)</summary> ```console ASAN without fix: 11 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/serve-http3.test.ts" bun test v1.4.3 (6a92015) test/js/bun/http/serve-http3.test.ts: (node:1831379) ExperimentalWarning: quic is an experimental feature and might change at any time (Use `bun-debug --trace-warnings ...` to show where the warning was created) (pass) Bun.serve HTTP/3 > basic GET [1433.51ms] (pass) Bun.serve HTTP/3 > POST echoes body, status, request headers [1535.24ms] (pass) Bun.serve HTTP/3 > 204 with no body [1423.30ms] (pass) Bun.serve HTTP/3 > query string is preserved [1345.95ms] (pass) Bun.serve HTTP/3 > large response body crosses multiple QUIC packets [1369.07ms] (pass) Bun.serve HTTP/3 > concurrent requests across separate connections [1374.92ms] (pass) Bun.serve HTTP/3 > client abort mid-response does not crash the server [1562.40ms] (pass) Bun.serve HTTP/3 > http1: false rejects HTTP/1.1 but accepts HTTP/3 [1396.90ms] (pass) Bun.serve HTTP/3 > http1: false — url/address/stop see the QUIC listener [2251.74ms] (pass) Bun.serve HTTP/3 > maxRequestBodySize is enforced for H3 bodies without C ... (truncated) release without fix: 6 failed, 1 skipped bun test v1.4.3-canary.1 (b7616e5) test/js/bun/http/serve-http3.test.ts: (node:1832208) ExperimentalWarning: quic is an experimental feature and might change at any time (Use `bun --trace-warnings ...` to show where the warning was created) (pass) Bun.serve HTTP/3 > basic GET [133.48ms] (pass) Bun.serve HTTP/3 > POST echoes body, status, request headers [134.22ms] (pass) Bun.serve HTTP/3 > 204 with no body [132.61ms] (pass) Bun.serve HTTP/3 > query string is preserved [133.55ms] (pass) Bun.serve HTTP/3 > large response body crosses multiple QUIC packets [137.20ms] (pass) Bun.serve HTTP/3 > concurrent requests across separate connections [133.74ms] (pass) Bun.serve HTTP/3 > client abort mid-response does not crash the server [131.62ms] (pass) Bun.serve HTTP/3 > http1: false rejects HTTP/1.1 but accepts HTTP/3 [133.16ms] (pass) Bun.serve HTTP/3 > http1: false — url/address/stop see the QUIC listener [1029.34ms] (pass) Bun.serve HTTP/3 > maxRequestBodySize is enforced for H3 bodies without Content-Length [140.71ms] (pass) Bun.serve HTTP/3 > unknown route returns 404 [130.97ms] (pass) Bun.serve HTTP/3 > routes: handler with :params [131.55ms] (pass) Bun.serve HTTP/ ... (truncated) ``` </details> <details><summary>passes on PR (with fix)</summary> ```console ASAN with fix: all passed $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" "test/js/bun/http/serve-http3.test.ts" bun test v1.4.3 (6a92015) test/js/bun/http/serve-http3.test.ts: (node:1834975) ExperimentalWarning: quic is an experimental feature and might change at any time (Use `bun-debug --trace-warnings ...` to show where the warning was created) (pass) Bun.serve HTTP/3 > basic GET [1367.83ms] (pass) Bun.serve HTTP/3 > POST echoes body, status, request headers [1318.78ms] (pass) Bun.serve HTTP/3 > 204 with no body [1425.18ms] (pass) Bun.serve HTTP/3 > query string is preserved [1385.53ms] (pass) Bun.serve HTTP/3 > large response body crosses multiple QUIC packets [1317.12ms] (pass) Bun.serve HTTP/3 > concurrent requests across separate connections [1423.12ms] (pass) Bun.serve HTTP/3 > client abort mid-response does not crash the server [1372.72ms] (pass) Bun.serve HTTP/3 > http1: false rejects HTTP/1.1 but accepts HTTP/3 [1330.87ms] (pass) Bun.serve HTTP/3 > http1: false — url/address/stop see the QUIC listener [2307.59ms] (pass) Bun.serve HTTP/3 > maxRequestBodySize is enforced for H3 bodies without C ... (truncated) release with fix: 1 skipped $ bun scripts/build.ts --profile=release [configured] bun-profile → bun (stripped) in 662ms (unchanged) ninja: Entering directory `/workspace/bun/build/release' [1/124] gen generated_host_exports.rs generated_host_exports.rs: 122 exports (host=5, lazy=10, generic=107, rust=0); 243 extern-C blocks audited [2/124] gen cpp.rs (cppbind) [2/124] cargo bun_runtime → libbun_runtime.a �[1m�[92m Compiling�[0m bun_core v0.0.0 (/workspace/bun/src/bun_core) �[1m�[92m Compiling�[0m bun_errno v0.0.0 (/workspace/bun/src/errno) �[1m�[92m Compiling�[0m bun_ptr v0.0.0 (/workspace/bun/src/ptr) �[1m�[92m Compiling�[0m bun_boringssl_sys v0.0.0 (/workspace/bun/src/boringssl_sys) �[1m�[92m Compiling�[0m bun_safety v0.0.0 (/workspace/bun/src/safety) �[1m�[92m Compiling�[0m bun_base64 v0.0.0 (/workspace/bun/src/base64) �[1m�[92m Compiling�[0m bun_cares_sys v0.0.0 (/workspace/bun/src/cares_sys) �[1m�[92m Compiling�[0m bun_zlib_sys v0.0.0 (/workspace/bun/src/zlib_sys) �[1m�[92m Compiling�[0m bun_zstd v0.0.0 (/workspace/bun/src/zstd) �[1m�[92m Compiling�[0m bun_picohttp v0.0.0 (/workspace/bun/src/picohttp) �[1m�[92m Compiling�[0m bun_brotli v0.0.0 (/workspace/bun/src/ ... (truncated) ``` </details> <details><summary>diff hotspot</summary> ``` packages/bun-usockets/src/quic.c | 116 +++++++++++++++++--- test/js/bun/http/serve-http3.test.ts | 199 +++++++++++++++++++++++++++++++++++ 2 files changed, 300 insertions(+), 15 deletions(-) ``` </details> **gate history** · 1 passed · 0 rejected · iteration 0 <details><summary>evidence per changed file</summary> ``` file reads edits tests packages/bun-usockets/src/quic.c 7 7 31 test/js/bun/http/serve-http3.test.ts 4 5 31 ``` </details> <!-- robobun:evidence:end -->
usrbinkat
pushed a commit
to usrbinkat/bun
that referenced
this pull request
Sep 15, 2026
…quest handler (oven-sh#42619) ### Problem - `server.stop()` or `server.stop(true)` from an HTTP/3 request handler, or from a microtask the handler resolved (the end of a `using server` scope counts), runs inside `lsquic_engine_process_conns` and enters the engine again. lsquic forbids that. A debug build aborts: `lsquic_engine_send_unsent_packets(lsquic_engine_t *): Assertion '!((engine)->pub.enp_flags & ENPUB_PROC)' failed.` - Release, `stop(true)`: `us_quic_listen_socket_close` (`packages/bun-usockets/src/quic.c:917`) closes the UDP fd before the running tick packs the CONNECTION_CLOSE. Every peer waits for its idle timer (10 s to 30 s). - Release, `stop()` on a connection that already served a request: the process spins at 100% CPU and the request in the handler never completes. ### Fix - One enter/leave pair brackets every engine call that can run callbacks (`process_conns`, `send_unsent_packets`). While it is held, `us_quic_socket_context_shutdown` and `us_quic_listen_socket_close` only record the request. `leave` finishes it: GOAWAY or CONNECTION_CLOSE, flush, then the fd. - `us_quic_socket_context_listen` releases a pending fd before it binds to the same port, so `stop(true)` then `Bun.serve` on that port in one turn still works. A stop from a timer is unchanged. - Verified: `test/js/bun/http/serve-http3.test.ts` (9 new tests, release main fails 6, debug main fails 9). - Self-reviewed: the review asked for the graceful path in the same PR. Done. Supersedes oven-sh#34447. ### Background - lsquic drives every connection from `lsquic_engine_process_conns`. Request handlers and the microtasks they resolve run inside it. - `lsquic_conn_abort` only sets a flag. The connection's tick sees it after the handler returns and packs the CONNECTION_CLOSE. The engine sends it before `process_conns` returns. - `ctx->processing` in `quic.c` is set while the engine is on the stack. <details><summary>Notes</summary> **Where this comes from.** No user report and no CI incident. oven-sh#34038 made an abrupt stop send CONNECTION_CLOSE for an idle connection when the caller is off the stack. A probe of the same stop with a request in the handler found the cases above. **Timings, release 1.4.3-canary.1, client and server in one process, request in the handler** | call | from a timer | from the handler, before | from the handler, after | | --- | --- | --- | --- | | `stop(true)`, streamed POST | `HTTP3StreamReset` at once | `HTTP3StreamReset` @10 s (warm connection) or @30 s (cold) | at once | | `stop(true)`, GET, old port dead | `HTTP3HandshakeFailed` @10 s | @20 s | @10 s | | `stop()`, cold connection | response arrives | response arrives | same | | `stop()`, warm connection | response arrives | 100% CPU, never completes (stopped after 60 s) | response arrives | The stall for `stop(true)` is the smaller of the server `idleTimeout` (30 s when it is 0) and the client's `max_idle_timeout`, and it hits every connection of the server. A client that re-sends the request adds its 10 s handshake timeout against the closed port. That retry is by design for an idempotent request (`fetch-http3-client.test.ts`, `retries on a fresh session when a pooled session is stale (port reuse)`). After the fix the client log shows `conn_close status=8` at once. **Why the `stop(true)` tests use a `ReadableStream` body.** The fetch client never re-sends a streamed body, so `HTTP3StreamReset` surfaces as soon as the client sees the connection close. With any other body the retry hides the difference behind a 10 s handshake timeout. **Why not keep the fd open in every case.** `stop(true)` promises that the port is free when it returns, and `server.stop(true); server = Bun.serve({ port })` is a common restart pattern. A deferred fd still holds the port, and these sockets do not set `SO_REUSEPORT`. The first version of this change failed that rebind inside a handler with `Failed to listen on UDP port N for HTTP/3`. One test pins it. Only in that case the stopped server's connections go silent, as they did before. A bind to another port leaves the pending fd alone (one more test). **Engine calls outside the pair.** `lsquic_engine_cooldown` (makes no callbacks), `lsquic_engine_packet_in` and `lsquic_engine_connect` (their callbacks stay in C), and `lsquic_engine_destroy` in `us_quic_socket_context_free`. **Excluded, not new.** `server.stop()` followed by `server.stop(true)`: the first call takes `h3_listener`, so the second never reaches `us_quic_listen_socket_close` for HTTP/3. **Relation to oven-sh#34447.** It wraps the abrupt path's engine calls in `if (!processing)` and defers the graceful path with its own latch and hook. It still closes the fd first on `stop(true)` (its test comment says the client then hangs), and it describes the graceful case as benign on release. This PR keeps one latch per call and one hook, and ports its two graceful tests with a warm-connection variant. **Relation to oven-sh#42579.** Its new client test stops a server after one `setImmediate` hop to stay off this path. After this PR the hop is not needed. **Suites run on the debug ASAN build:** `serve-http3` (62 pass), `fetch-http3-client` (56), `serve-protocols` (20), `fetch-http3-adversarial` (27), `fetch-http3-cold-post` (2), `fetch-http3-syscall-fault` (3). </details> <!-- robobun:evidence:begin --> --- **[human-review]** gate passed · iteration 0 · 2 files touched <details><summary>fails on main (without fix)</summary> ```console ASAN without fix: 11 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/serve-http3.test.ts" bun test v1.4.3 (6a92015) test/js/bun/http/serve-http3.test.ts: (node:1831379) ExperimentalWarning: quic is an experimental feature and might change at any time (Use `bun-debug --trace-warnings ...` to show where the warning was created) (pass) Bun.serve HTTP/3 > basic GET [1433.51ms] (pass) Bun.serve HTTP/3 > POST echoes body, status, request headers [1535.24ms] (pass) Bun.serve HTTP/3 > 204 with no body [1423.30ms] (pass) Bun.serve HTTP/3 > query string is preserved [1345.95ms] (pass) Bun.serve HTTP/3 > large response body crosses multiple QUIC packets [1369.07ms] (pass) Bun.serve HTTP/3 > concurrent requests across separate connections [1374.92ms] (pass) Bun.serve HTTP/3 > client abort mid-response does not crash the server [1562.40ms] (pass) Bun.serve HTTP/3 > http1: false rejects HTTP/1.1 but accepts HTTP/3 [1396.90ms] (pass) Bun.serve HTTP/3 > http1: false — url/address/stop see the QUIC listener [2251.74ms] (pass) Bun.serve HTTP/3 > maxRequestBodySize is enforced for H3 bodies without C ... (truncated) release without fix: 6 failed, 1 skipped bun test v1.4.3-canary.1 (b7616e5) test/js/bun/http/serve-http3.test.ts: (node:1832208) ExperimentalWarning: quic is an experimental feature and might change at any time (Use `bun --trace-warnings ...` to show where the warning was created) (pass) Bun.serve HTTP/3 > basic GET [133.48ms] (pass) Bun.serve HTTP/3 > POST echoes body, status, request headers [134.22ms] (pass) Bun.serve HTTP/3 > 204 with no body [132.61ms] (pass) Bun.serve HTTP/3 > query string is preserved [133.55ms] (pass) Bun.serve HTTP/3 > large response body crosses multiple QUIC packets [137.20ms] (pass) Bun.serve HTTP/3 > concurrent requests across separate connections [133.74ms] (pass) Bun.serve HTTP/3 > client abort mid-response does not crash the server [131.62ms] (pass) Bun.serve HTTP/3 > http1: false rejects HTTP/1.1 but accepts HTTP/3 [133.16ms] (pass) Bun.serve HTTP/3 > http1: false — url/address/stop see the QUIC listener [1029.34ms] (pass) Bun.serve HTTP/3 > maxRequestBodySize is enforced for H3 bodies without Content-Length [140.71ms] (pass) Bun.serve HTTP/3 > unknown route returns 404 [130.97ms] (pass) Bun.serve HTTP/3 > routes: handler with :params [131.55ms] (pass) Bun.serve HTTP/ ... (truncated) ``` </details> <details><summary>passes on PR (with fix)</summary> ```console ASAN with fix: all passed $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" "test/js/bun/http/serve-http3.test.ts" bun test v1.4.3 (6a92015) test/js/bun/http/serve-http3.test.ts: (node:1834975) ExperimentalWarning: quic is an experimental feature and might change at any time (Use `bun-debug --trace-warnings ...` to show where the warning was created) (pass) Bun.serve HTTP/3 > basic GET [1367.83ms] (pass) Bun.serve HTTP/3 > POST echoes body, status, request headers [1318.78ms] (pass) Bun.serve HTTP/3 > 204 with no body [1425.18ms] (pass) Bun.serve HTTP/3 > query string is preserved [1385.53ms] (pass) Bun.serve HTTP/3 > large response body crosses multiple QUIC packets [1317.12ms] (pass) Bun.serve HTTP/3 > concurrent requests across separate connections [1423.12ms] (pass) Bun.serve HTTP/3 > client abort mid-response does not crash the server [1372.72ms] (pass) Bun.serve HTTP/3 > http1: false rejects HTTP/1.1 but accepts HTTP/3 [1330.87ms] (pass) Bun.serve HTTP/3 > http1: false — url/address/stop see the QUIC listener [2307.59ms] (pass) Bun.serve HTTP/3 > maxRequestBodySize is enforced for H3 bodies without C ... (truncated) release with fix: 1 skipped $ bun scripts/build.ts --profile=release [configured] bun-profile → bun (stripped) in 662ms (unchanged) ninja: Entering directory `/workspace/bun/build/release' [1/124] gen generated_host_exports.rs generated_host_exports.rs: 122 exports (host=5, lazy=10, generic=107, rust=0); 243 extern-C blocks audited [2/124] gen cpp.rs (cppbind) [2/124] cargo bun_runtime → libbun_runtime.a �[1m�[92m Compiling�[0m bun_core v0.0.0 (/workspace/bun/src/bun_core) �[1m�[92m Compiling�[0m bun_errno v0.0.0 (/workspace/bun/src/errno) �[1m�[92m Compiling�[0m bun_ptr v0.0.0 (/workspace/bun/src/ptr) �[1m�[92m Compiling�[0m bun_boringssl_sys v0.0.0 (/workspace/bun/src/boringssl_sys) �[1m�[92m Compiling�[0m bun_safety v0.0.0 (/workspace/bun/src/safety) �[1m�[92m Compiling�[0m bun_base64 v0.0.0 (/workspace/bun/src/base64) �[1m�[92m Compiling�[0m bun_cares_sys v0.0.0 (/workspace/bun/src/cares_sys) �[1m�[92m Compiling�[0m bun_zlib_sys v0.0.0 (/workspace/bun/src/zlib_sys) �[1m�[92m Compiling�[0m bun_zstd v0.0.0 (/workspace/bun/src/zstd) �[1m�[92m Compiling�[0m bun_picohttp v0.0.0 (/workspace/bun/src/picohttp) �[1m�[92m Compiling�[0m bun_brotli v0.0.0 (/workspace/bun/src/ ... (truncated) ``` </details> <details><summary>diff hotspot</summary> ``` packages/bun-usockets/src/quic.c | 116 +++++++++++++++++--- test/js/bun/http/serve-http3.test.ts | 199 +++++++++++++++++++++++++++++++++++ 2 files changed, 300 insertions(+), 15 deletions(-) ``` </details> **gate history** · 1 passed · 0 rejected · iteration 0 <details><summary>evidence per changed file</summary> ``` file reads edits tests packages/bun-usockets/src/quic.c 7 7 31 test/js/bun/http/serve-http3.test.ts 4 5 31 ``` </details> <!-- robobun:evidence:end -->
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes
test/js/bun/http/serve-http3.test.tsgoing intermittently red, most often asPOST body without Content-Length still reaches the handlerfailing withHTTP3StreamResetafter ~8-10s.Reproduction
On a release build step 2 usually runs while the server still has ACKs scheduled, so the bug is masked; under debug/ASAN or a loaded CI runner the connection is idle and the POST stalls every time.
Root cause
server.stop(true)reachesus_quic_listen_socket_close(), which calledlsquic_conn_close()on each live conn and then closed the UDP fd. For a server connection,ietf_full_conn_ci_closesetsIFC_CLOSINGand only schedulesSF_SEND_CONN_CLOSEwhenconn_ok_to_close(); the tick that actually packs the frame (theend_writeblock inlsquic_full_conn_ietf.c) additionally requires one of:IFC_GOAWAY_CLOSE, orlsquic_send_ctl_n_scheduled() > 0An idle server conn satisfies none of those, so the UDP fd was closed with nothing on the wire. The client's pooled session stays in
ClientContext.sessionsuntil the negotiated idle timeout. If a later listener binds the same ephemeral port before that fires,fetch({protocol:'http3'})matches the dead session and enqueues on it. Non-stream bodies recover viaretry_or_fail; aReadableStreambody has already been drained into lsquic's send buffer andretry_or_failrefuses onis_streaming_body, so it surfacesHTTP3StreamResetonce the idle alarm fires.The pattern has been present since HTTP/3 support landed (#29768 for the server side, #29795 for the pooled client); it turns into a test failure whenever two ephemeral ports collide inside one idle-timeout window.
Fix
us_quic_listen_socket_close()now callslsquic_conn_abort()instead oflsquic_conn_close().IFC_ABORTEDis inIFC_IMMEDIATE_CLOSE_FLAGS, which takes theimmediate_closepath and unconditionally packs a CONNECTION_CLOSE (transport errorNO_ERROR, "user aborted connection").The test file's
withCustomServerfixtures nowserver.stop(true)on stdin close andwithCustomServergives them a moment to do so beforekill(), so every server process in the file exits through this path.Verification
New lifecycle test lets the server conn go idle, stops abruptly, rebinds the same port in-process, then POSTs a
ReadableStreambody:Across the full file with
BUN_DEBUG_h3_client=1on a debug build, client-session close breakdown:The two remaining timeout closes are the intentional
AbortSignal.timeoutprobes against a stopped port; those sessions are still handshaking and recover on retransmission.[review] gate passed · iteration 1 · 2 files touched
fails on main (without fix)
passes on PR (with fix)
diff hotspot
gate history · 3 passed · 0 rejected · iteration 1
evidence per changed file