Skip to content

uws: make uws_res_end_without_body uncork and run the close gate - #48486

Draft
robobun wants to merge 1 commit into
mainfrom
robobun/7d2c027c/bodyless-end-gates-itself
Draft

robobun wants to merge 1 commit into
mainfrom
robobun/7d2c027c/bodyless-end-gates-itself

Conversation

@robobun

@robobun robobun commented Nov 10, 2026

Copy link
Copy Markdown
Collaborator

Problem

  • A response without a body (304, HEAD) can leave open a connection marked to close by Connection: close, HTTP/1.0 or server.stop(). The client gets no FIN until idleTimeout.
  • uws_res_end_without_body (src/uws_sys/libuwsockets.cpp) marks the response done and runs no uncork and no close gate. HttpResponse::cork() skips its own gate when another socket's cork() flushed the response's slot (HttpResponse.h:968), and callers outside cork() have none.

Fix

  • The shim completes through finishResponseInCorkBuffer(0), the uncork and close gate internalEnd() ends on.
  • node:http res.destroy() drops corked bytes and closes the socket itself. It moves to uws_res_abandon, the shim's old behaviour.
  • Verified: bun-server.test.ts (7 cases), bun-serve-html.test.ts (2), serve.test.ts, node-http-req-complete.test.ts. Suites are in Notes.

Background

  • uWS batches a socket's writes in one of two cork slots per loop. A third cork() flushes the oldest.
  • The close gate closes a connection once its response is complete, marked to close and fully sent.
  • Considered a gate at cork()'s lost-slot return (uws: run the close gate in HttpResponse::cork() when the cork slot was lost #37446): every post-await response with a body exits there, and ends outside cork() stay ungated.

Downsides

Notes

Reported by a maintainer in review of #37074: "HttpResponse::cork() early return skips the close gate ... a completed Connection: close / HTTP/1.0 response on a non-parsing socket is never shut".

Repro of the reported order. An async handler answers a Connection: close POST whose body never arrives:

req.text().catch(() => {
  wsA.cork(() => {
    wsA.send("a");
    wsB.cork(() => wsB.send("b")); // both cork slots hold data: this flushes the response's slot
  });
});
await Bun.sleep(0);
return new Response(null, { status: 304 });

The client receives HTTP/1.1 304 Not Modified and no FIN. Ending the response rejects the unread body, and the microtasks that run then execute while the response still holds its cork slot. No cork() call is needed: four such requests that complete in one microtask drain leave two connections open, because each response completes inside the drain of the one before it.

Callers outside cork(). The request that waited for the bundle of an HTML import is answered from an event-loop task. After 500 Build Failed (production build failed) and after the dev server's 405, the client gets no FIN on main, with no other socket involved.

Also in the diff

  • RequestContext::end_without_body no longer calls close_if_done_and_marked(). That shim stays for the sendfile end.
  • The comments in HTMLBundle.rs and dev_server/deferred_request.rs that said this end cannot close the socket are gone.

Why not the gate at cork()'s lost-slot return

Question for a maintainer. Is it acceptable that uws_res_end_without_body closes the socket before it returns? The note said callers keep using the response. Today the callers either return or call clear_aborted / clear_on_writable / clear_timeout, which are no-ops on a closed socket because end() and tryEnd() already close in-call. If the end has to stay non-closing, the alternative is to make uws_res_close_if_done_and_marked uncork (as #41341 does) and to add a gate call at each raw end outside the parser.

Tests

  • bun-server.test.ts, "a response without a body closes a connection that is marked to close": request Connection: close, HTTP/1.0, response Connection: close, server.stop() in flight, TLS, and the two eviction orders (two corked WebSockets, two streaming responses). All 7 time out on main.
  • bun-serve-html.test.ts: the two HTML import cases above. Both time out on main.
  • serve.test.ts, "share one send() until a handler runs JavaScript": new rows for responses without a body. On main mixedHead is 3 segments. fetchHead 8, staticHead 1, fetch304 8 and static304 1 are the same before and after.
  • node-http-req-complete.test.ts, "res.destroy() drops the chunk that res.write() queued in the same turn": passes on main, under Node v26.3.0 and with this change. It fails (the client receives the dropped chunk) when uws_res_abandon is made to gate, and no existing test did.

Measurements

  • Instruction counts are path counts from llvm-objdump -d of src/uws_sys/libuwsockets.cpp compiled with the release flags and without LTO, TCP arm, over uws_res_end_without_body, finishResponseInCorkBuffer and uncorkCompletedResponse (calls to the one-line usockets accessors not included: 7 on main, 14 here). The 110 is what an end with a body already pays at completion. perf, valgrind, strace and bloaty are not installed in the build container, so there is no dynamic count and no pair of full release builds.
  • In that object 550 of 551 functions are instruction-identical, among them HttpResponse::cork(), internalEnd(), uws_res_end, uws_res_try_end, uws_res_end_stream, uws_res_cork and HttpContext::onData. uws_res_end_without_body goes from 450 to 264 bytes. New: uws_res_abandon (488 bytes) and two out-of-line finishResponseInCorkBuffer instantiations (199 bytes each, which the NodeHTTP.cpp unit already has).
  • Segments are counted with getsockopt(TCP_INFO) in the test above.
  • Reads after an in-call close: a local build poisoned (ASAN) the response ext of every socket that the bodyless end closed. 31 such closes across the suites below, 0 reads of a poisoned ext.

Suites run on the debug build: bun-server, serve, bun-serve-html, bun-serve-static, bun-serve-file, bun-serve-routes, serve-directory-routes, request-smuggling, websocket-server, node-http, node-http-req-complete, node-http-server-abort-events, node-http-closed-response, node-http-server-close-drain, node-http-upgrade-body, test/bake/dev/html, test/bake/dev/plugins, test/bake/serve-plugins-dev-server. Failures that also occur without this change: the privileged port test (the container runs as root), one stack-shape assertion in node-http-closed-response on debug builds, and timeouts of subprocess-heavy tests under the debug build that pass alone.

Not changed here

Earlier attempts: #35555 (the end shims gate themselves) and #37446 (gate at cork()'s tail, the source of the two eviction tests).

…at is marked to close

uws_res_end_without_body marked the response done and left the uncork and
the close gate to whoever held the socket's cork. Every other end runs both
itself. A bodyless end (a 304, a HEAD response, the bodyless error
responses) that completed outside the read of its request could therefore
leave a connection open that `Connection: close`, HTTP/1.0 or a graceful
server.stop() had marked to close: when another socket's cork() flushed its
cork slot first, and whenever the caller ran no gate of its own (the 500 of
an HTML import whose build failed, the 405 of the dev server).

The shim now completes through HttpResponse::finishResponseInCorkBuffer(),
the uncork and close gate that internalEnd() ends on. A response that
JavaScript produced is sent when it completes, with or without a body.

node:http res.destroy() ended its response through the same shim. It drops
what is still corked and closes the socket itself, so it gets
uws_res_abandon, which does what the shim did before.
@robobun

robobun commented Nov 10, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 11:44 AM PT - Nov 10th, 2026

✅ @robobun, your commit 76f3a768876351845b2f157143c85dfb2771e9bf passed in Build #139804! 🎉


🧪   To try this PR locally:

bunx bun-pr 48486

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

bun-48486 --bun

@robobun

robobun commented Nov 10, 2026

Copy link
Copy Markdown
Collaborator Author

Status

How it was reproduced, on main (89452d2) and on canary 1.4.3:

  • A Bun.serve async handler answers a Connection: close POST, whose body never arrives, with a 304. While the response completes, two corked WebSockets (or two streaming responses) take its cork slot. The client receives HTTP/1.1 304 Not Modified and no FIN until idleTimeout.
  • Four such requests that complete together leave two connections open, with no cork() call in user code.
  • The request that waited for an HTML import's bundle gets 500 Build Failed (production build failed) or the dev server's 405, and no FIN.

With this PR each connection closes right after the response. The tests are in test/js/bun/http/bun-server.test.ts, test/js/bun/http/bun-serve-html.test.ts and test/js/bun/http/serve.test.ts.

PR: #48486. It replaces #37446, which a stale sweep closed.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants