Bun.serve: keep the event loop alive while a request is in flight

[robobun]

Repro

// server.mjs
const s = Bun.serve({ port: 0, async fetch() {
  s.unref();
  await new Promise(r => setTimeout(r, 200).unref());
  return new Response("ok");
} });
console.log("PORT " + s.port);

From a separate process, fetch(\http://127.0.0.1:\${PORT}/\`)` gets ECONNRESET: the server process exits mid-handler with code 0, no error. Deterministic 3/3 on both release-asan and canary. Node v26 delivers the response (the accepted socket is its own handle).

Cause

NewServer has a single poll_ref: KeepAlive; server.unref() clears it. RequestContext (the per-request state for the fetch handler) has no event-loop ref of its own. pending_requests is only a destructor gate, so once the listener is unref'd and the handler awaits something that is itself unref'd, nothing holds the loop.

The node:net face of this was fixed in Listener::on_create (each accepted socket refs its own KeepAlive), and node:http's NodeHTTPResponse already carries a per-request jsc::Ref. The Bun.serve fetch path did neither.

Fix

Add a second KeepAlive on the server, ref'd on the pending_requests 0 -> nonzero edge and unref'd on the nonzero -> 0 edge, separate from the listener's poll_ref. server.unref() then only releases the listener's hold; in-flight requests keep the process alive on their own. This sits in on_pending_request / on_request_complete, so it covers the fetch handler, static/file/directory routes, and the node:http dispatch uniformly. It uses KeepAlive (loop ref) rather than jsc::Ref so Windows uv_run keeps driving socket I/O while the drain is in flight (same reasoning as the existing comment in stop_listening).

Also routed the websocket-upgrade fallback through on_pending_request() instead of pending_requests += 1 so the ref stays balanced with the on_request_complete() decrement in RequestContext::deinit.

Verification

USE_SYSTEM_BUN=1 bun test test/js/bun/http/bun-server.test.ts -t "in-flight request keeps"
  (fail) ECONNRESET

bun bd test test/js/bun/http/bun-server.test.ts -t "in-flight request keeps"
  (pass) 200 "ok", server exits cleanly after responding

Existing unref tests, websocket and routes, and bun-serve-static all pass.

---

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

[robobun]

Reproduced with the fixture at test/js/bun/http/unref-fixture-3.ts (server subprocess, server.unref() inside the handler, handler awaits an unref'd timer; parent process fetches). System bun: ECONNRESET. With this change: 200 ok, then the server process exits cleanly.

3d9b699 adds a stop(true) release for in_flight_keep_alive: pending_requests does not drain when a client aborts while a direct-stream pull() is suspended (pre-existing; see the comment in RequestContext::deinit), so an abrupt stop has to drop the in-flight ref itself, same as it already drops poll_ref. That fixed the serve-body-leak.test.ts timeout from the first CI run.

Known edge: an unref'd server whose direct-stream pull() is parked on a promise that never resolves, where the client disconnects and stop(true) is never called, now stays alive instead of exiting (previously it exited with a leaked pending_requests). The underlying counter leak is pre-existing and out of scope here.

CI: diff is green. Build 86321 had all test lanes pass (including the 7 where serve-body-leak.test.ts previously timed out); only red was darwin-14-aarch64 expiring in queue. Build 86374 (retrigger): all test-failure items are flakes that passed on retry; the one failed lane is darwin-14-x64 where the Node test harness itself crashed with uv_os_get_passwd ENOENT on darwin-rye-x64-1 (runner infra, unrelated to this diff). Ready for review.

[coderabbitai]

Walkthrough

Changes

The server now uses a dedicated KeepAlive handle for pending requests. The fetch fallback uses the shared request-tracking helper. An end-to-end test verifies process liveness during an unreferenced in-flight request.

In-flight request lifecycle

Layer / File(s)	Summary
KeepAlive lifecycle wiring src/runtime/server/mod.rs, src/runtime/server/server_body.rs	NewServer initializes and manages in_flight_keep_alive. The fetch fallback records pending requests through on_pending_request().
Unref behavior validation test/js/bun/http/unref-fixture-3.ts, test/js/bun/http/bun-server.test.ts	The fixture unreferences the server and timer during a request. The subprocess test validates the response and clean exit.

Possibly related issues

• Windows: uv_run's alive-guard wedges teardown states with zero ref'd handles (server.close() waits forever class) #34158 — Both changes update KeepAlive and pending-request lifecycle handling for active server work.

Possibly related PRs

• oven-sh/bun#35130 — Both modify NewServer request lifecycle tracking and shutdown keep-alive behavior.
• oven-sh/bun#36181 — Both address event-loop lifetime for unreferenced servers with active connections.
• oven-sh/bun#36247 — Both update runtime keep-alive behavior for queued or in-flight work.

Suggested reviewers: jarred-sumner

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly summarizes the main change: keeping the event loop alive during in-flight Bun.serve requests.
Description check	✅ Passed	The description explains the problem, cause, fix, and verification, although it uses different headings from the template.

---

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

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@test/js/bun/http/bun-server.test.ts`:
- Around line 509-527: Update the subprocess test around the child process setup
to start draining proc.stderr and waiting on proc.exited immediately after
spawning, while retaining the stdout reader for port discovery. Await the fetch
response, then assert the response body together with the already-started stderr
and exit-code results as one combined outcome, ensuring stdout, stderr, and
process completion are handled concurrently.

🪄 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: 00fc0ce0-b84f-4855-84df-0fe4568553f4

📥 Commits

Reviewing files that changed from the base of the PR and between 37a7767 and 4ec7acb.

📒 Files selected for processing (4)

• src/runtime/server/mod.rs
• src/runtime/server/server_body.rs
• test/js/bun/http/bun-server.test.ts
• test/js/bun/http/unref-fixture-3.ts

[claude]

No issues found — the ref/unref edges look balanced and KeepAlive is idempotent, so I don't see a leak or double-unref path. Deferring because this is a user-visible semantic change to server.unref() (in-flight requests now hold the loop independently) on the hot request-dispatch path; worth a maintainer confirming that's the intended contract.

What was reviewed:

• Audited every pending_requests mutation site — all now go through on_pending_request/on_request_complete except the fully-synchronous on_bun_info_request pair, which never yields.
• Checked stop(true) / deinit_if_we_can / schedule_deinit — all gate on pending_requests == 0, so in_flight_keep_alive is already unref'd before deinit; poll_ref has the same (no explicit disable in Drop) so no new lifecycle gap.
• Confirmed server.ref()/unref() only touch poll_ref, so the new keep-alive is independent as intended.

Extended reasoning...

Overview

Adds a second KeepAlive (in_flight_keep_alive) to NewServer, ref'd on the pending_requests 0→nonzero edge in on_pending_request() and unref'd on the nonzero→0 edge in on_request_complete(). Also routes the websocket-upgrade fallback in server_body.rs through on_pending_request() instead of a raw pending_requests += 1 so it participates in the edge-triggered ref. Ships a subprocess test + fixture that reproduces the ECONNRESET on system bun and verifies a 200 + clean exit with the fix.

Security risks

None. No new inputs, no parsing, no auth/crypto surface. The unsafe blocks match the existing // SAFETY idiom used two lines away for poll_ref (dereferencing the per-thread VM singleton backref).

Level of scrutiny

Medium-high. The diff is small (~15 native lines) and mechanically simple, but it sits on the per-request hot path of Bun.serve and changes what server.unref() means: previously it released the server's only loop hold, now in-flight requests hold the loop on their own. That matches Node's behavior (each accepted socket is its own handle) and node:http in Bun already does this via NodeHTTPResponse's per-request jsc::Ref, but for Bun.serve's native fetch path it's a new contract.

Other factors

• I traced every pending_requests += 1 / -= 1 site. After this PR the only raw pair left is on_bun_info_request (server_body.rs:2900/2923), which is fully synchronous — no event-loop yield between the increment and decrement, and no interleaving with on_request_complete — so skipping the keep-alive there is fine and can't unbalance the edge trigger.
• KeepAlive::ref_/unref are state-gated (Active/Inactive/Done), not counted, so an unmatched call is a no-op rather than a corruption; the edge-trigger design means the state and the counter stay in sync as long as every async increment goes through on_pending_request, which it now does.
• deinit_if_we_can and schedule_deinit both gate on pending_requests == 0, and on_request_complete unrefs before calling deinit_if_we_can, so the new field is always Inactive by the time the server box is dropped. poll_ref follows the same pattern (no explicit disable() in deinit/Drop), so this introduces no new teardown gap.
• Test looks solid: port 0, subprocess with bunEnv, stderr/exit drained from spawn, combined toEqual assertion. The 200 ms unref'd timer in the fixture is the whole point of the repro (nothing else holds the loop), not a flaky sleep-then-check.
• Both prior review comments (concurrent stderr drain, long doc comment) are addressed in ebae380 / 88d2b6b.

[claude]

I didn't find any bugs. Deferring because this changes event-loop lifetime semantics in the core Bun.serve request path and carries an acknowledged behavioral tradeoff (the parked direct-stream + client-disconnect edge now keeps the process alive instead of exiting with a leaked counter) — worth a maintainer sign-off on that and on the server-level edge-triggered KeepAlive vs. a per-RequestContext ref.

What was reviewed:

• on_pending_request/on_request_complete edge-triggering vs. KeepAlive's Active/Inactive/Done state machine — balanced, and disable() on abrupt stop makes later ref_()/unref() no-ops.
• The one remaining raw pending_requests += 1 site (on_bun_info_request) is fully synchronous, so skipping the helper there is harmless.
• The stop(true)-after-stop(false) early-return (listener already None, non-H3) leaves in_flight_keep_alive untouched — same as pre-existing behavior for poll_ref/TERMINATED on that path; ruled out as a regression.

Extended reasoning...

Overview

Adds a second KeepAlive (in_flight_keep_alive) to NewServer, ref'd on the pending_requests 0→nonzero edge in on_pending_request() and unref'd on nonzero→0 in on_request_complete(), so server.unref() (which only clears poll_ref) no longer lets the process exit while a handler is suspended on an unref'd awaitable. Also disable()s it on abrupt stop (both the H3 and listener-present arms of stop_listening), and routes the websocket-upgrade fallback (server_body.rs:3425) through on_pending_request() so its increment pairs with the on_request_complete() decrement in RequestContext::deinit. Test: subprocess fixture that unref's the server inside the handler and awaits an unref'd timer; parent asserts 200 body + clean exit.

Security risks

None. This is event-loop ref-counting; no untrusted input parsing, auth, or data exposure.

Level of scrutiny

High. This is process-lifetime behavior in the core HTTP server. KeepAlive is not a counter — it's a tri-state (Active/Inactive/Done), so the edge-triggered ref/unref must be exactly right, and every pending_requests mutation site must either use the helper or be provably synchronous. I verified: the only remaining raw += 1 (on_bun_info_request) is bracketed by a raw -= 1 in the same frame with no yield point, so it can't affect loop liveness. All other increment sites go through on_pending_request(), and all decrements go through on_request_complete() / on_static_request_complete(). deinit_if_we_can() only schedules teardown when pending_requests == 0, at which point in_flight_keep_alive is already Inactive (or Done after abrupt stop), so no leaked loop ref at destructor time.

Other factors

• The PR body openly documents a behavior change on a pre-existing counter leak (unref'd server + parked direct-stream pull() + client disconnect + no stop(true)): previously the process exited with a leaked pending_requests; now it stays alive. That's arguably more correct (Node semantics: in-flight work holds the loop), but it's a user-observable tradeoff a maintainer should ack.
• Architecture question worth a human eye: the fix places the ref on the server (edge-triggered on the aggregate counter) rather than per-RequestContext, unlike NodeHTTPResponse's per-response jsc::Ref or Listener::on_create's per-socket KeepAlive. The PR justifies KeepAlive over jsc::Ref for Windows uv_run I/O; the server-level placement is simpler but couples correctness to every pending_requests mutation going through the helper.
• CodeRabbit's stderr-drain feedback and the comment-cop note were both addressed; the first CI failure (serve-body-leak.test.ts) was fixed by 3d9b699's abrupt-stop disable().
• Test follows harness conventions (bunEnv/bunExe, port: 0, concurrent pipe drain, combined-object assertion).

[claude]

No bugs found, but this changes process-lifetime semantics for Bun.serve (and acknowledges a new edge case where an unref'd server with a parked direct-stream now stays alive instead of exiting), so it's worth a human look.

What was reviewed:

• KeepAlive state machine — edge-triggered ref/unref on pending_requests 0↔nonzero is idempotent; disable() on abrupt stop makes later ref_() a no-op.
• The one remaining raw pending_requests += 1 (on_bun_info_request) is fully synchronous, so skipping the keep-alive there is harmless.
• stop(true) after graceful stop() on the non-H3 listener-already-None path returns early without disable() — matches pre-existing behavior for poll_ref/TERMINATED on that path.

Extended reasoning...

Overview

Adds a second KeepAlive field (in_flight_keep_alive) to NewServer, ref'd on the pending_requests 0→nonzero edge in on_pending_request() and unref'd on the nonzero→0 edge in on_request_complete(), so that server.unref() (which only touches poll_ref) no longer lets the process exit while a request handler is suspended. Also routes the websocket-upgrade fallback in server_body.rs through on_pending_request() instead of a raw counter increment, and adds in_flight_keep_alive.disable() on both abrupt-stop branches in stop_listening. A subprocess-based test with an external client fetch validates the fix.

Security risks

None. No parsing of untrusted input, no auth/crypto, no new user-facing surface.

Level of scrutiny

High. This touches the core Bun.serve request lifecycle and event-loop keep-alive accounting, which is cross-platform-sensitive (the existing comment in stop_listening calls out Windows uv_run behavior with zero ref'd handles) and historically subtle (serve-body-leak.test.ts already forced one follow-up commit here). The author explicitly notes a behavioral change: an unref'd server whose direct-stream pull() is parked on a never-resolving promise, with the client disconnected and no stop(true), now keeps the process alive where it previously exited. Whether that trade-off is acceptable is a maintainer call.

Other factors

• The bug-hunting system raised and refuted the stop() → stop(true) early-return path not disabling in_flight_keep_alive. I checked: on the non-H3 branch with listener already taken, stop_listening(true) returns before the new disable() — but that path also skips TERMINATED and the poll_ref unref today, so this preserves existing shape rather than introducing a new hole.
• KeepAlive is a tri-state (Inactive/Active/Done), not a counter; the 0↔nonzero edge-triggering keeps ref/unref balanced, and disable() correctly no-ops future ref_() calls after abrupt stop.
• on_bun_info_request still does raw pending_requests += 1 / -= 1, but it's fully synchronous so the process can't exit between them.
• The test follows repo conventions (subprocess, concurrent stderr/exit drain, combined toEqual assertion) and was verified to fail on system bun and pass on the debug build per the PR body.
• All prior review comments (CodeRabbit, comment-cop) are resolved.