Bun.serve: close the connection after a sendfile response when the client asked for close

[robobun]

What

Bun.serve never closed the connection after a Connection: close or HTTP/1.0 request when the response body was a Bun.file() whose first sendfile(2) could not complete synchronously. The full body arrived with a correct Content-Length, but no FIN followed, so a client waiting for EOF hung forever and the server pinned the fd and uWS response state for the life of idleTimeout (forever at idleTimeout: 0).

// 32 MiB so loopback's send buffer forces sendfile into the async path.
await Bun.write(path, Buffer.alloc(32 * 1024 * 1024, 0x62));
const srv = Bun.serve({ port: 0, idleTimeout: 0, fetch: () => new Response(Bun.file(path)) });
// raw TCP: GET / HTTP/1.1 + Connection: close, or GET / HTTP/1.0
// body arrives in full; socket stays open forever (no shutdown(2) in strace).

Any Bun.file() response large enough to exceed the socket send buffer takes this path, so a >=1 MiB file to a non-loopback client is affected. The identical bytes sent as a Uint8Array body close correctly. RFC 9112 §9.6 MUST.

Why

uws_res_end_sendfile (src/uws_sys/libuwsockets.cpp) ignored its close_connection argument: it only set offset, HTTP_END_CALLED, and called markDone(). Every other end path routes through internalEnd, which after markDone() checks HTTP_CONNECTION_CLOSE and issues shutdown() + close() when the socket is uncorked and drained.

On async completion the call arrives from FileResponseStream::on_writable → end_sendfile → resp.end_send_file(offset, resp.should_close_connection()), inside HttpContext::onWritable. The Rust handler returns false after completing, so onWritable takes the early return at if (!success) return s; and never reaches its own HTTP_CONNECTION_CLOSE check. No shutdown is ever issued. (Synchronous completion happens inside the request handler's cork, whose uncork path does the check, which is why small files were fine.)

Fix

uws_res_end_sendfile now mirrors internalEnd: it records close_connection into HTTP_CONNECTION_CLOSE, and after markDone() clears HTTP_RESPONSE_PENDING it runs the same uncorked + drained check and issues shutdown() + close().

Verification

New Bun.file() sendfile closes connection when requested describe block in test/js/bun/http/bun-serve-file.test.ts: raw TCP request for a 32 MiB Bun.file() body with Connection: close and with HTTP/1.0, asserting the full body arrives and the server then sends FIN. Both hang on the system bun (5 s timeout) and pass with this change. The full file (69 tests) and serve.test.ts show no new failures.

Related: #33005 fixes the same bug class for the other end paths (internalEnd, uws_res_end_without_body) but does not touch uws_res_end_sendfile; this change is independent of it.

---

no test proof · iteration 8 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/bun/http/bun-serve-file.test.ts

[coderabbitai]

Walkthrough

Modifies uws_res_end_sendfile in libuwsockets.cpp to honor a close_connection flag by setting HTTP_CONNECTION_CLOSE and conditionally shutting down/closing the underlying socket in both SSL and non-SSL paths. Adds a new test suite validating connection shutdown behavior for Bun.file() sendfile responses.

Changes

Sendfile close_connection handling

Layer / File(s)	Summary
SSL and non-SSL close_connection logic src/uws_sys/libuwsockets.cpp	In both SSL and non-SSL branches of uws_res_end_sendfile, sets HTTP_CONNECTION_CLOSE when close_connection is set, then after HTTP_END_CALLED/markDone/resetTimeout, conditionally calls shutdown() and close() on the AsyncSocket when not corked, no response is pending, and buffered amount is zero.
Socket-level shutdown test test/js/bun/http/bun-serve-file.test.ts	Adds node:net import and a describe.skipIf(isWindows) suite that uses raw TCP sockets against a Bun.file() sendfile server, verifying full body delivery and proper connection ending (end vs close) for Connection: close and HTTP/1.0 requests.

Possibly related PRs

• oven-sh/bun#32798: Related change to HTTP client retry logic avoiding replay of sendfile bodies on disconnects, touching the same sendfile/close-connection interaction area.

🚥 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 matches the change: closing Bun.serve sendfile connections when the client asks to close.
Description check	✅ Passed	The description explains the bug, fix, and verification, so it mostly satisfies the template requirements.

---

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

[robobun]

^{Updated 12:55 AM PT - Jul 11th, 2026}

❌ @robobun, your commit 74c8f26 has 2 failures in Build #71818 (All Failures):

• test/js/third_party/grpc-js/test-tonic.test.ts - code 1 on 🍎 14 aarch64
• test/js/bun/http/proxy-stress-concurrent.test.ts - code 1 on 🍎 14 x64

---

🧪   To try this PR locally:

bunx bun-pr 33698

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

bun-33698 --bun

[claude]

Thanks for addressing the cleanup nit. The change mirrors internalEnd's close sequence verbatim and looks correct to me, but since it adds shutdown()/close() inside the onWritable callback path for the sendfile completion, I'd like a human to confirm the socket-lifecycle ordering.

Extended reasoning...

Overview

The PR modifies uws_res_end_sendfile in src/uws_sys/libuwsockets.cpp (both SSL and non-SSL branches) to honour its previously-ignored close_connection argument: it now records HTTP_CONNECTION_CLOSE into the response state and, after markDone(), runs the same uncorked + no-pending + drained → shutdown()/close() sequence that internalEnd and four other sites in HttpResponse.h/HttpContext.h already use. A new describe block in test/js/bun/http/bun-serve-file.test.ts sends a raw-TCP Connection: close request and an HTTP/1.0 request for a 32 MiB Bun.file() body and asserts the full body arrives followed by FIN.

Security risks

None apparent. The change only affects when the server issues FIN/close after a completed sendfile response; it does not touch parsing, auth, or untrusted input. The added logic is gated behind the same four conditions used everywhere else in uWS for connection-close.

Level of scrutiny

Medium-high. The added code is a verbatim copy of a well-established pattern (five existing occurrences across HttpResponse.h:157-166, HttpResponse.h:223-232, HttpResponse.h:742-750, HttpContext.h:441-449, HttpContext.h:537-545), so the risk of the logic itself being wrong is low. However, this is Bun.serve connection-lifecycle code in the C++ uWS FFI layer — a hot, production-critical path where an incorrect close() from inside an onWritable callback could cause UAF, double-close, or hung connections. The PR description traces the callback chain (FileResponseStream::on_writable → end_sendfile → returns false → HttpContext::onWritable early-returns s) and I verified that internalEnd is already invoked from the same position via tryEnd, so there is precedent for closing from here — but a maintainer familiar with usockets' close semantics should confirm.

Other factors

• My previous nit (32 MiB temp dir not cleaned up in afterAll) was addressed in 04b54cb and the thread is resolved.
• The bug-hunting system found no issues on the current revision.
• Tests cover both Connection: close and HTTP/1.0 variants and assert both the full body length and ended: true (i.e., FIN received via node:net's end event before close).
• The change is small and self-contained, but does not fall into the "simple, mechanical, or obvious" bucket — it's a behavioural change to socket teardown ordering.

[robobun]

On the lifecycle ordering: calling shutdown()/close() from inside callOnWritable is the existing pattern. StaticRoute::on_writable → on_writable_bytes → resp.try_end(bytes, len, resp.should_close_connection()) → tryEnd → internalEnd already performs the identical shutdown() + close() at the same call depth for buffered bodies (StaticRoute.rs, HttpResponse.h internalEnd). us_socket_close defers the free to the end of the loop iteration via closed_head, and markDone() has already nulled onWritable/onAborted before the close fires, so callOnWritable's epilogue and HttpContext::onWritable's return s; see the same state they already do for the buffered path.

[robobun]

CI (post-rebase onto 18cfc1a): build #71818 finished with 284 lanes green. bun-serve-file.test.ts (the new tests) passed on every lane that ran it, including ASAN. The two red lanes are unrelated to this diff:

• :darwin: 14 x64: proxy-stress-concurrent.test.ts → https-proxy → https-origin mode=redirect ×1200 had 1/1200 iterations fail. That test does not use Bun.file() or touch the sendfile path; this change only affects uws_res_end_sendfile, reached exclusively when a Bun.file() body completes sendfile.
• :darwin: 14 aarch64: grpc-js/test-tonic.test.ts failed because the agent's rustup has no default toolchain (rustup could not choose a version of cargo to run). The test's beforeAll never built the server, so no bun code executed.

Diff is ready for review.

Previous runs (pre-rebase)

#70198: 284 green; red lanes were agent darwin-aarch64-26-5-1-1 (buildkite-agent artifact download timed out after 120s, also on #70047 and #70056) and a darwin-14-aarch64 autobahn docker exec format error (amd64 image on arm64 host).

[claude]

No issues found and the nit is addressed — deferring only because this adds a shutdown()/close() call inside the uWS onWritable path, which is socket-lifecycle code worth a human sanity check.

Extended reasoning...

Overview

This PR fixes uws_res_end_sendfile in src/uws_sys/libuwsockets.cpp to honour its previously-ignored close_connection argument: it now sets HTTP_CONNECTION_CLOSE and, after markDone(), runs the same uncorked/no-pending/drained check as internalEnd and issues shutdown() + close(). Both SSL and non-SSL branches are updated identically. A new test suite in test/js/bun/http/bun-serve-file.test.ts uses raw TCP to verify a 32 MiB Bun.file() response delivers the full body and then sends FIN for both Connection: close and HTTP/1.0.

I verified the added block against internalEnd in packages/bun-uws/src/HttpResponse.h:157-168 — it is a byte-for-byte match of the four-guard nesting (!isCorked → HTTP_CONNECTION_CLOSE → !HTTP_RESPONSE_PENDING → getBufferedAmount() == 0 → shutdown()+close()). The author's lifecycle-ordering comment is consistent with what I read: markDone() nulls the callbacks first, us_socket_close defers the free via closed_head, and tryEnd→internalEnd already performs this exact sequence at the same call depth for buffered bodies.

Security risks

None identified. This does not touch auth, crypto, or input parsing. The only behavioural change is that connections which the client already asked to close now actually close after the body is delivered — strictly a resource-leak fix, not a new attack surface.

Level of scrutiny

Medium-high. Bun.serve is production-critical, and adding a close() inside the onWritable callback path is the class of change where an ordering mistake becomes a use-after-free. The mitigations are strong — the added code is a verbatim copy of an existing, exercised pattern from internalEnd, the PR description traces the exact call chain (FileResponseStream::on_writable → end_sendfile → here, inside HttpContext::onWritable), and CI passed on 281 lanes including ASAN — but socket lifecycle in uWS is subtle enough that a maintainer familiar with this layer should confirm.

Other factors

• My earlier nit (32 MiB temp dir not cleaned up) was addressed in 04b54cb; the afterAll now calls rmScope(dir).
• No bugs from the bug-hunting system on the current revision.
• The test is well-constructed: it distinguishes graceful FIN (end event → ended: true) from abrupt close, wires error to reject, cleans up the socket in finally, and covers both Connection: close and HTTP/1.0.
• CI's single red lane was an artifact-download timeout on one darwin agent, unrelated to the change.

[claude]

No issues found; the earlier temp-dir cleanup nit is addressed. Deferring to a human because this adds shutdown()/close() inside the uWS onWritable path — HTTP server socket lifecycle is critical enough to warrant a maintainer's eyes even though the change looks correct.

Checked: the new guard in uws_res_end_sendfile mirrors internalEnd (HttpResponse.h:157-168) exactly — same isCorked/HTTP_CONNECTION_CLOSE/HTTP_RESPONSE_PENDING/getBufferedAmount sequence.
Checked: the caller FileResponseStream::end_sendfile runs detach_resp() before this, so uWS callbacks are already nulled when close() fires.
Checked: the new test wires error to reject, awaits FIN via end vs close, and cleans up its 32 MiB temp dir.

Extended reasoning...

Overview

The PR fixes uws_res_end_sendfile in src/uws_sys/libuwsockets.cpp to honor its previously-ignored close_connection argument. It now sets HTTP_CONNECTION_CLOSE and, after markDone(), runs the same uncorked/drained check as internalEnd to issue shutdown() + close(). Both SSL and non-SSL branches receive the identical change. A new raw-TCP test in bun-serve-file.test.ts covers Connection: close and HTTP/1.0 against a 32 MiB Bun.file() body.

Security risks

None identified. This is server-side connection-close semantics; it does not touch auth, input parsing, or trust boundaries. The only risk class is memory safety (premature socket free), addressed under scrutiny below.

Level of scrutiny

High. uws_res_end_sendfile is called from FileResponseStream::on_writable inside HttpContext::onWritable — adding a close() here means the socket may be queued for free while the Rust caller and uWS's onWritable epilogue are still on the stack. The author's lifecycle argument checks out against source: (1) the guard sequence is byte-identical to internalEnd at HttpResponse.h:157-168; (2) StaticRoute::on_writable → tryEnd → internalEnd already performs the same shutdown()/close() at the same call depth for buffered bodies, so this is not a new pattern; (3) us_socket_close defers the actual free via closed_head; (4) FileResponseStream::end_sendfile calls detach_resp() first, so onWritable/onAborted are already nulled. That said, uWS socket lifecycle is exactly the area CLAUDE.md flags as most-blocked in review ("anything that can run user JS can synchronously free your state", ref/deref bracketing around teardown), and I don't think a bot should be the final sign-off on it.

Other factors

• CI build #70198: 284 lanes green including ASAN; the two red lanes are documented infra failures unrelated to the diff.
• My earlier nit (leaked 32 MiB temp dir) was fixed in 74c8f26 — afterAll now uses rmScope(dir).
• The bug hunting system found nothing.
• Related PR #33005 touches the same bug class for other end paths; a maintainer coordinating that work should confirm the two changes compose.

[robobun]

This landed on main through #37074. It added a closeIfDoneAndMarked completion gate (uws_res_close_if_done_and_marked in src/uws_sys/libuwsockets.cpp) and runs it from FileResponseStream right after end_send_file, precisely because the sendfile end bypasses internalEnd and so never reached the Connection: close check. Connection: close and HTTP/1.0 requests served from the sendfile path now close the socket once the file has been written.

Checked by running this PR's version of test/js/bun/http/bun-serve-file.test.ts against a debug build of current main (05dd45e) on Linux, where the sendfile path is still used: the two cases this PR added (Connection: close and HTTP/1.0) pass in 3 separate runs, and 2 runs of the whole file give 84 pass / 0 fail each time.

Nothing left for this PR to add, so closing it.