node:http: remove the dispatch promise so that a 'close' emitted by user code acts as in Node.js
Problem
res.emit('close')on an opennode:httpresponse makes the server answer with a bare\r\n. The next request throwsERR_HTTP_SOCKET_ASSIGNED. Node.js sends the full response. Found in #44217, not by a user.onNodeHTTPRequest(src/js/node/_http_server.ts) returned a promise and resolved it at any 'close' of the response.node_http_request_on_resolve(src/runtime/server/NodeHTTPResponse.rs) then ended the open response.
Fix
onNodeHTTPRequestreturns nothing. An open response completes through its 'finish' listener and native handle. Deleted: the close slot, the ownServerResponse.prototype.emit, the hand-off promises, both native promise reactions.- A hand-emitted socket 'close' closes the open response and aborts an incomplete request, like Node's
socketOnClose. A 'connect' or 'upgrade' hand-off removes that listener. - Verified:
test/js/node/http/node-http.test.ts(4 new tests, main fails 3, values from Node.js v26.3.0). Suites: Notes. - Self-reviewed: 9 concerns raised, 7 addressed (Notes).
Background
- uWS calls the dispatcher
onNodeHTTPRequestonce per request. Native code keeps a response pending until JS ends it or the connection closes. - The promise also made native code run the listener's ticks and promise jobs, then report unhandled rejections. Response flags do that now.
- Considered: delete only the own
emit(21 lines). That fixesres.emit('close')but not a socket 'close', and the promise stays.
Downsides
- A hand-emitted socket 'close' closes the open response, as in Node.js. A later
end()sends no body. main sent it. - Closed this way before its header, a response still answers with an empty 200. Node.js sends nothing (#39366, Notes).
- Other callers: nothing added. 1 promise and 1 closure fewer per open response,
.text5,632 bytes smaller.
Notes
Repro. #44217 lists this case under "Found, not fixed here". No user report exists. Node.js v26.3.0 prints the full HTTP/1.1 200 OK ... Content-Length: 4\r\n\r\nbody. Bun 1.4.3 and main print "\r\n".
import http from "node:http";
import net from "node:net";
const server = http.createServer((req, res) => {
setImmediate(() => {
res.emit("close");
setImmediate(() => res.end("body"));
});
});
server.listen(0, "127.0.0.1", () => {
const c = net.connect(server.address().port, "127.0.0.1", () => c.write("GET / HTTP/1.1\r\nHost: t\r\n\r\n"));
let got = "";
c.on("data", d => { got += d.toString("latin1"); });
setTimeout(() => { console.log(JSON.stringify(got.replace(/Date: [^\r]+\r\n/, ""))); c.destroy(); server.close(); }, 500);
});
Effects of the same signal on main. Each is a scenario of node-http-emitted-close-fixture.js.
| User code | main | Node.js v26.3.0 and this PR |
|---|---|---|
res.emit('close') after the listener returned, then res.end('body') | \r\n, then ERR_HTTP_SOCKET_ASSIGNED for the next request | the full response |
the same after res.write('a') | no terminating chunk | 1\r\na\r\n4\r\nbody\r\n0\r\n\r\n |
res.emit('close') in the listener, then a second request | response 2 leaves first, res.end() of response 1 throws ERR_HTTP_HEADERS_SENT | response 1, then response 2 |
res.emit('close') while an ended response drains | emit() returns false, 'finish' fires before the client reads | emit() returns true, 'finish' after the client reads |
socket.emit('close') in a 'connect' or 'upgrade' tunnel | \r\n written into the tunnel | nothing written |
socket.emit('close') on the socket of an open response | the parser stops, and the response still goes out | request aborted, connection closed |
| the same after the request body was read (Bun 1.4.3) | the response stays open and sends the chunk of a later end() | response closed, end() sends no chunk, the socket stays |
socket.emit('close'), response class with its own assignSocket() | \r\n, connection stays open | request aborted, connection closed |
The JS-only cut. A preload that deletes the own ServerResponse.prototype.emit (21 lines in the source) on the merge-base binary makes 21 of the 25 TCP rows of node-http-emitted-close-fixture.js equal Node.js. The merge base has 3, this PR 25 (fixture as of 09df76ac2b). The 4 rows left have a hand-emitted socket 'close': two with an own assignSocket() (\r\n, and the connection stays open) and the CONNECT and upgrade tunnels (\r\n written into the tunnel). They need the hand-off promises gone. The fixture has six more rows now: a socket 'close' with the default classes (2), a tunnel on a connection that served a request before, and a socket 'close' after the request body was read (3). I did not run the cut on them. When JS returns nothing, the native promise arm has no caller, so the same change deletes it.
What else the promise did. Native code read a pending promise as "this response is not complete". It then ran the ticks and promise jobs of the listener before its last steps, and reported unhandled rejections after them. The dispatch now reads the same from the flags of the response: CURRENT and not REQUEST_HAS_COMPLETED. Nothing else depended on the promise. node-http-completion-paths-fixture.js runs 13 ways an exchange ends: end in the listener, after an await, in a later turn, a 32 MB body against a client that does not read, a client that leaves, res.destroy(), a body that nothing reads, half of a body (then the end of the response, or the client leaves), two pipelined requests, a 'connect' and an 'upgrade' hand-off, and a ws upgrade. main and this PR print the same lines: the same order of 'finish' / 'close' / 'aborted', and 0 event loop refs left after server.close(). The test for it passes on main too.
Native dispatch. A breakpoint count (gdb, 200 dispatches per mode, 20 for the body that drains) gives the same numbers for the merge base 95690fc530 and this PR.
| Mode | set_on_aborted_handler calls | drain before the last steps | drain after them |
|---|---|---|---|
| end in the listener | 0 | 0 | 200 |
await null, then end | 0 | 200 | 0 |
end from setImmediate | 200 | 200 | 0 |
| two pipelined requests per write | 200 | 100 | 100 |
| CONNECT hand-off | 200 | 200 | 0 |
| 'upgrade' hand-off | 200 | 200 | 0 |
res.destroy() in the listener | 200 | 200 | 0 |
| end in the listener, 16 MB body that drains | 0 | 20 | 0 |
'unhandledRejection'. A listener queues a tick, a 0 ms timer and an immediate, and leaves a rejection. The order is the same on the merge base and on this PR in 11 cases: response left open, after write(), an async listener that throws, end after an await, res.destroy(), end in the listener, end with a body that drains, a 'connect' listener, an 'upgrade' listener, two pipelined requests, two connections in one poll. In 9 of them the rejection comes right after the tick, as in Node.js. After an end in the listener it comes after the timer. With two pipelined requests it comes before the second listener. The commits of this PR before 09df76ac2b reported every rejection after the timer: the dispatch had lost its handle_rejected_promises() call. The fixture scenario "an unhandled rejection in a listener that leaves the response open" fails on them (tick, timer, rejection).
Measurements. Taken with the first commit of this PR on bf42a525d5. I did not repeat them after the later commits and the merges of main. A later commit moves the last block of the dispatch into NodeHTTPResponse::on_dispatch_returned. main has since renamed the dispatch function to on_node_http_request (#45223). The sizes of the binary and of the dispatch function are from before that move.
| What | main | PR | How |
|---|---|---|---|
onNodeHTTPRequest bytecode: create_lexical_environment / new_func / new_promise | 1 / 1 / 3 | 0 / 0 / 0 | BUN_JSC_dumpGeneratedBytecodes=1, debug builds |
onNodeHTTPRequest bytecode size | 749 instructions, 4,079 bytes | 697 instructions, 3,831 bytes | the same dump |
| cells per open response: Promise / JSLexicalEnvironment / Function | 1.00 / 1.00 / 5.00 | 0.00 / 0.00 / 4.00 | heapStats(), N=500, 3 runs each, debug builds |
calls of the own ServerResponse.prototype.emit per response | 4 | 0 | counter, 1000 requests |
Object.hasOwn(ServerResponse.prototype, "emit") | true | false (as Node.js) | |
close-slot reads per response, end in the listener / from setImmediate | 2 / 2 | 0 / 0 | counter, 1000 requests |
| event loop refs / native responses left over 13 completion paths | 0 / 0 | 0 / 0 | activeTasks, heapStats() after Bun.gc(true), Linux x64 |
| the same over 6 paths with a hand-emitted 'close' | hangs | 0 / 0 | |
| release binary, stripped | 80,844,360 bytes | 80,836,168 (-8,192) | ls -l, release builds, same toolchain |
.text | 58,159,221 bytes | 58,153,589 (-5,632) | llvm-size -A |
Bun__NodeHTTPRequest__onResolve + onReject | 957 + 1,010 bytes | absent | llvm-nm -S |
on_node_http_request_with_upgrade_ctx, each of 4 instances | 3,852 bytes, 869 instructions, 58 calls | 3,134 bytes, 715 instructions, 47 calls | llvm-nm -S, llvm-objdump -d |
PromiseFunctions entries | 56 | 54 | ZigGlobalObject.h |
size_of::<NodeHTTPResponse>() | 544 bytes | 536 | debug info (gdb) |
bundled builtins: node/_http_server.js / internal/http.js | 96,531 / 13,075 bytes | 94,890 / 12,841 | wc -c, release builds |
user CPU per request, end in the listener / after await / from setImmediate | median 10.25 / 10.45 / 10.45 us (9.6 to 11.4) | median 9.88 / 9.80 / 9.69 us (9.0 to 11.3) | process.cpuUsage(), 100,000 keep-alive requests, 8 interleaved runs. The ranges overlap: the host was loaded |
| instructions per request | not measured | not measured | perf and valgrind cannot be installed in the container (perf_event_paranoid is 4) |
Suites. Debug+ASAN builds, one file at a time, on Linux x64. CI runs the tests on macOS and Windows.
- This head (f4753eb40a, with main at 5eab34dacb merged):
node-http.test.ts296 pass, 1 skip, 0 fail with--timeout 120000. The other 37 files oftest/js/node/httpand the 21 files oftest/js/first_party/wspass.node-http-emitted-close-fixture.jsprints the lines of Node.js v26.3.0 on all 31 TCP rows and 25 TLS rows. - The same head: 115 of Node's
test-http-*andtest-https-*files about upgrade, CONNECT, close, abort, destroy, pipelining and keep-alive, all pass. - fe005a21c4 (main at 47ba1c8d85): the same runs, on a slower debug build. At the default 5 s, 13 tests of
node-http.test.tsthat exist on main timed out: each needed up to 9 s. "should not crash on abort (node-http-uaf-fixture.ts)" needed 63 s and allows 60 s.test-http-agent-keepalive.jsfailed: the 'close' of the client socket came later than the 1 ms timer of the test, and it passed with a 100 ms timer. CI passed on that head (build 125546, 181 of 181 jobs). - 09df76ac2b against the merge base 95690fc530: 64
bun:testfiles (test/js/node/httpand the express, body-parser, hono, st and socket.io tests), 1,424 tests pass. 9 tests in 4 third-party files fail, and they fail on the merge base too in this container (express-memory-leak 4, socket.io-handshake 3, express 1, socket.io-close 1). - First commit: 463 of Node's
test-http-*andtest-https-*files, 456 pass. 6 of the other 7 pass when they run alone, on both builds.test-https-timeout.jstimes out on both. The new tests also pass withBUN_JSC_validateExceptionChecks=1.
Windows. The scenario with a draining response does not run there. Winsock takes the 32 MB body in one send() on loopback, so no response is left that drains. node-http-pinned-write.test.ts skips its Windows cases for the same reason.
The count of native responses is gone. Earlier commits also counted the NodeHTTPResponse cells that heapStats() reports after a full GC. For a cell that stayed counted, the fixture looked for a path from a root in the debugging heap snapshot, and it reported 0 when it found none. That check could report 0 for a cell that was alive (review finding), so fe005a21c4 removes the count. The fixture reports the order of the events and the event loop refs, like the other node:http leak tests (node-http-ondata-reregister-leak, node-http-with-ws).
- What the plain count showed (one exchange per path, then 64 turns with a full GC each): 0 after every path on the Linux and macOS lanes (build 122201) and on Windows 2019 x64 (build 122167).
- On Windows 11 aarch64 (build 122167), one cell stayed counted after the path "the client leaves" and through the next four paths. The count was 0 again from "two pipelined requests" on (3 of 4 attempts). With a hand-emitted 'close', one cell stayed counted after "res.emit('close'), then the client leaves" only (4 of 4 attempts).
- I did not find what holds that cell. I cannot run Windows. Nothing holds it for good: later exchanges in the same process free it, and the event loop refs are 0 in every row.
The kPendingFinish branch of the deleted emit. It finished a draining response when its 'close' came first. The close path of the socket (flushPendingFinish in #onClose) and the HTTP/1 fallback already do that before they emit 'close'. A counter in that branch on main had 3 hits over the 64 test files. All 3 came from the new fixture, which emits 'close' by hand. An earlier run of the 463 Node files had none.
Tests that used the old behaviour.
node-http-displaced-response: the triggeremit-closeand the scenariocompletedButPendingare gone. A 'close' emitted by user code no longer gives the connection to the next response. The state "request complete, not ended, still pending" has no producer left:on_request_complete()now only runs for a response that ended, whose socket closed, or that upgraded.node-http-ondata-reregister-leak.test.ts: the scenario withres.emit('close')stays and still expects 0 refs. Its comments no longer describe a handler promise.- The completed-response branch of
handle_abort_or_timeoutstays. A connection that closes after its response completed enters it, and it releases the pending request of an ended response whose body reader is still armed (gdb backtrace in the path "half of a body, then the end of the response").
Still different from Node.js, not changed here.
- A dispatch that finds
socket._httpMessageset throwsERR_HTTP_SOCKET_ASSIGNED(assignSocketInternal). Node.js queues that response (parserOnIncoming). This PR removes the producer that a hand-emitted 'close' was. 'unhandledRejection'after an end of the response in the listener comes after a due timer. With two pipelined requests in one read it comes before the second listener. Node.js reports it before the timer, and runs both listeners first (#43736 is about that order). Both are as on main.res.emit('finish')from user code: Node.js runs itsresOnFinish. Bun detaches the socket through other code.res.destroy()andreq.destroy()before the headers go out answer withHTTP/1.1 200 OKandConnection: close(#39366).- A hand-emitted socket 'close' after the request body was read, with no header sent, then
res.end('body'): Node.js sends nothing, because itswrite_()returns before_implicitHeader(). Bun sends a 200 withContent-Length: 0, as both do forres.end()without a chunk. The socket stays open in both. To send nothing, the response must finish in JS and stay open in native code until the connection closes. This PR removes that state. uws_res_end_without_bodywrites no status line whenclose_connectionis false. It is shared withBun.serve. The node:http caller that reached it with an open header section is deleted.
Other effects.
- Found, not changed here: after a
wsconnection of anhttpsserver closed on both sides, the process can exit before the callback ofserver.close(callback)when nothing else holds the event loop. The merge base does the same. Bun 1.4.3 calls back. - The steps that the dispatch runs after the handler returned are
NodeHTTPResponse::on_dispatch_returnednow. Without the promise, that block reads only the response. As a method it compiles once, not once per server type (SSLxDEBUG). Thegeneric_body_not_genericlint of mordant reported the block. - The internal
Bun.serve({ onNodeHTTPRequest })(typed insrc/js/private.d.tsonly) no longer waits for a returned promise. - #39366 and #43736 edit the same native dispatch function. #39366's note that
node_http_request_on_resolvestays unchanged no longer applies.
Self-review. A review of this diff ran for about 13 hours on 09df76ac2b. The machines that run it restarted four times, and its final report did not complete. Its answer to "should this change exist" was: wanted, keep the PR open. The 9 concerns that I could read from it, and what I did:
'unhandledRejection'came after due timers and after other connections: fixed in 09df76ac2b (section above).- The dispatch drained after its last steps for a response that ends while its body drains: fixed in the same commit.
- The repro was described as "from the report": corrected. It is the case from #44217.
- The smallest fix was not named: "The JS-only cut" above.
ERR_HTTP_SOCKET_ASSIGNEDas a class is not fixed: stated under "Still different from Node.js".- The abort for a hand-emitted socket 'close' covered only a response with its own
assignSocket(): fixed in e72a47cf24. The listener that every connection has for the 'close' of its socket does it now, as Node'ssocketOnClose. - The count of native responses decided by the heap snapshot: the count is removed (section above).
- Not done: two PRs (JS, then native). The native promise arm would be dead code between them.
- Not done: the sizes of the release binary are from the first commit.
Review of e72a47cf24. Three findings.
- The hand-off to 'connect' and 'upgrade' left the
socketOnCloselistener on the socket: 2525782846 removes it, as Node'sonParserExecuteCommondoes. On Bun 1.4.3 the 'connect' listener of a connection that served a request before sees 2 'close' listeners. Node.js and this PR have 0. - The count of native responses could report 0 for a cell that was alive: removed (section above).
- A socket 'close' that is emitted after the request body was read: Node.js closes that response too, and three new fixture rows equal Node.js. One case differs and is not changed ("Still different from Node.js").
no test proof · iteration 1 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/node/http/node-http.test.ts