From a2f80f4c27d3ecc3c00ebd10710dbe8d388c497e Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 9 Oct 2026 15:14:17 +0000 Subject: [PATCH 01/11] node:http: a tunnel whose reads are stopped does not hold the event loop A CONNECT or Upgrade tunnel stops its reads when the buffer of its socket is full. It then never reads the EOF of its peer, so it never reached the idle state that releases the event loop, and the process stayed alive after server.close(). Node.js exits there: a handle that does not read and has nothing to send is not active. The tunnel now counts as idle from readStop(), and readStart() takes the event loop back. Bytes left to send hold the loop as before. Over TLS, a socket.end() behind bytes that TLS still holds now waits in the cell for the writable event. usockets sends such a FIN by itself with no writable event behind it, so a tunnel that stayed open behind its FIN was never reported idle. server.unref() stays in force when a tunnel stops being idle: the +3 arm of the connection filter does not take the loop ref back after it. NewServer::poll_ref is a JsCell, so that arm needs no `&mut` and no unsafe block. --- packages/bun-uws/src/HttpResponse.h | 6 +- .../bindings/node/JSNodeHTTPServerSocket.cpp | 36 ++- .../bindings/node/JSNodeHTTPServerSocket.h | 6 +- src/runtime/server/mod.rs | 30 ++- src/runtime/server/server_body.rs | 2 + ...e-http-connect-end-small-sndbuf-fixture.js | 77 ++++++ .../node-http-connect-proxy-pipes-fixture.js | 53 ++++ .../node-http-connect-read-stopped-fixture.js | 235 ++++++++++++++++++ test/js/node/http/node-http-connect.test.ts | 138 +++++++++- 9 files changed, 559 insertions(+), 24 deletions(-) create mode 100644 test/js/node/http/node-http-connect-end-small-sndbuf-fixture.js create mode 100644 test/js/node/http/node-http-connect-proxy-pipes-fixture.js create mode 100644 test/js/node/http/node-http-connect-read-stopped-fixture.js diff --git a/packages/bun-uws/src/HttpResponse.h b/packages/bun-uws/src/HttpResponse.h index 89d1d6f762ac..6e155e944df3 100644 --- a/packages/bun-uws/src/HttpResponse.h +++ b/packages/bun-uws/src/HttpResponse.h @@ -183,8 +183,10 @@ struct HttpResponse : public AsyncSocket { return true; } - /* node:http: an idle tunnel is at read EOF and has nothing left to send. Like a libuv handle in that state, it does not hold - * the event loop. The filter hears -3 when a tunnel becomes idle and +3 when it has bytes to send again. */ + /* node:http: an idle tunnel has reads that do not hold the event loop (they are stopped or at EOF) and has nothing left to + * send. Like a libuv stream handle in that state, it does not hold the event loop: + * https://github.com/nodejs/node/blob/v26.3.0/deps/uv/src/unix/stream.c#L1473-L1485. The filter hears -3 when a tunnel + * becomes idle and +3 when its reads hold the event loop again or it has bytes to send again. */ void setNodeHttpTunnelIdle(bool idle) { HttpResponseData *httpResponseData = getHttpResponseData(); if (!httpResponseData->filteredAccept || httpResponseData->filteredIdleTunnel == idle) { diff --git a/src/jsc/bindings/node/JSNodeHTTPServerSocket.cpp b/src/jsc/bindings/node/JSNodeHTTPServerSocket.cpp index 67990a061c7f..408bc2db7735 100644 --- a/src/jsc/bindings/node/JSNodeHTTPServerSocket.cpp +++ b/src/jsc/bindings/node/JSNodeHTTPServerSocket.cpp @@ -197,11 +197,12 @@ void JSNodeHTTPServerSocket::applyTunnelReads() void JSNodeHTTPServerSocket::readStop() { // After the end of the stream no _read() comes to start the reads again. - if (!isTunnel(this) || tunnelReadEnded) { + if (tunnelReadsStopped || tunnelReadEnded || !isTunnel(this)) { return; } tunnelReadsStopped = true; applyTunnelReads(); + refreshTunnelIdle(); } void JSNodeHTTPServerSocket::readStart() @@ -209,7 +210,11 @@ void JSNodeHTTPServerSocket::readStart() if (!isTunnel(this)) { return; } - tunnelReadsStopped = false; + if (tunnelReadsStopped) [[unlikely]] { + tunnelReadsStopped = false; + // Before applyTunnelReads(): us_socket_resume() closes a socket whose poll it cannot arm again. + refreshTunnelIdle(); + } applyTunnelReads(); } @@ -377,6 +382,11 @@ bool JSNodeHTTPServerSocket::shutdownAfterResponseDrains(bool destroySoon) JSC::EncodedJSValue JSNodeHTTPServerSocket::halfClose(JSC::JSGlobalObject* globalObject) { + // A FIN that usockets holds behind TLS bytes leaves with no writable event, and that event tells a tunnel that nothing is left to send: onDrain() sends the FIN. + if (is_ssl && isTunnel(this) && !reinterpret_cast*>(socket)->hasFullyDrained()) { + tunnelEndAwaitsDrain = true; + return JSValue::encode(JSC::jsUndefined()); + } // onNodeHTTPRequest no longer pauses at dispatch; pause here so the // shutdown+resume below still cycles kqueue's EVFILT_READ (delete then // re-add), without which macOS 26 does not deliver the peer's close. @@ -887,16 +897,24 @@ extern "C" bool Bun__NodeHTTPServerSocket__writeBehindResponse(us_socket_t* sock return is_ssl ? writeBehindResponse(socket, data, length) : writeBehindResponse(socket, data, length); } -void JSNodeHTTPServerSocket::updateTunnelIdle() +void JSNodeHTTPServerSocket::refreshTunnelIdle() { - if (!tunnelReadEnded || upgraded || isClosed()) { + if (!isTunnel(this)) { return; } - const bool sent = streamBuffer.bufferedSize() == 0; + const bool idle = (tunnelReadEnded || tunnelReadsStopped) && streamBuffer.bufferedSize() == 0; if (is_ssl) { - reinterpret_cast*>(socket)->setNodeHttpTunnelIdle(sent && reinterpret_cast*>(socket)->hasFullyDrained()); + reinterpret_cast*>(socket)->setNodeHttpTunnelIdle(idle && reinterpret_cast*>(socket)->hasFullyDrained()); } else { - reinterpret_cast*>(socket)->setNodeHttpTunnelIdle(sent && reinterpret_cast*>(socket)->hasFullyDrained()); + reinterpret_cast*>(socket)->setNodeHttpTunnelIdle(idle && reinterpret_cast*>(socket)->hasFullyDrained()); + } +} + +void JSNodeHTTPServerSocket::updateTunnelIdle() +{ + // A tunnel whose reads hold the event loop is not idle, and a write or a drain does not change that. + if (tunnelReadEnded || tunnelReadsStopped) { + refreshTunnelIdle(); } } @@ -910,6 +928,10 @@ void JSNodeHTTPServerSocket::onDrain() // A read pause or resume arms the writable event too: nothing was buffered, so nothing drained. if (this->streamBuffer.bufferedSize() == 0 && !heldWriteAwaitsDrain) { + if (tunnelEndAwaitsDrain) [[unlikely]] { + tunnelEndAwaitsDrain = false; + halfClose(globalObject); + } updateTunnelIdle(); return; } diff --git a/src/jsc/bindings/node/JSNodeHTTPServerSocket.h b/src/jsc/bindings/node/JSNodeHTTPServerSocket.h index 0942d0b4772d..e300ee339c81 100644 --- a/src/jsc/bindings/node/JSNodeHTTPServerSocket.h +++ b/src/jsc/bindings/node/JSNodeHTTPServerSocket.h @@ -63,6 +63,8 @@ class JSNodeHTTPServerSocket : public JSC::JSDestructibleObject { unsigned heldWriteAwaitsDrain : 1 = 0; /* Set by onClose() for the peerEnded / closeError getters: the peer's FIN, the error of a failed read. */ unsigned peer_ended : 1 = 0; + /* socket.end() came while TLS held bytes of the tunnel: halfClose() to the onDrain() that sends the FIN. */ + unsigned tunnelEndAwaitsDrain : 1 = 0; int closeReadError = 0; /* Tunnel bytes that onData() queued for JS in tasks that have not run yet. */ size_t queuedTunnelBytes = 0; @@ -146,7 +148,9 @@ class JSNodeHTTPServerSocket : public JSC::JSDestructibleObject { void readStop(); void readStart(); bool tunnelReadsPaused() const { return tunnelReadsStopped || tunnelReadsQueuedFull; } - /* Tells uWS whether this tunnel is idle: at read EOF with nothing left to send. See HttpResponse::setNodeHttpTunnelIdle. */ + /* Tells uWS whether this tunnel is idle: its reads do not hold the event loop (readStop() or read EOF) and it has nothing left to send. See HttpResponse::setNodeHttpTunnelIdle. */ + void refreshTunnelIdle(); + /* The same for a caller that did not change what the reads hold: a write, a drain, the end of the stream. */ void updateTunnelIdle(); /* uWS still holds bytes of an HTTP response on this connection. A raw write has to go through the same buffer, or it reaches the wire first. */ bool hasUnsentResponseBytes() const; diff --git a/src/runtime/server/mod.rs b/src/runtime/server/mod.rs index 7284a0a2b9c7..f73d1c00af0a 100644 --- a/src/runtime/server/mod.rs +++ b/src/runtime/server/mod.rs @@ -268,7 +268,7 @@ pub(crate) struct NewServer { /// ([`NewServer::is_drained`]); for Bun.serve it also holds the /// graceful-stop promise open ([`NewServer::is_closed`]). pub(crate) active_connection_count: core::cell::Cell, - /// The node:http tunnels in `active_connection_count` that are at read EOF and have nothing left to send. + /// The node:http tunnels in `active_connection_count` whose reads do not hold the loop (stopped or at EOF) and that have nothing left to send. pub(crate) idle_tunnel_count: core::cell::Cell, /// Live `ServerWebSocket` count. Lives on the server (not the websocket /// context) so a reload's context swap cannot reset it, and sits in a @@ -300,7 +300,9 @@ pub(crate) struct NewServer { /// promise in `deinit_if_we_can`, after which the Strong is dropped. pub(crate) all_closed_promise: jsc::JSPromiseStrong, - pub poll_ref: KeepAlive, + pub poll_ref: bun_jsc::JsCell, + /// `server.unref()` with no `server.ref()` behind it: the loop ref stays dropped when a tunnel stops being idle. + pub(crate) user_unrefed: core::cell::Cell, pub(crate) flags: ServerFlags, @@ -498,7 +500,7 @@ impl NewServer { /// uWS filter: `+2` at TCP accept (before any TLS handshake), `-2` on /// `HttpContext::onClose` / `HttpResponse::upgrade()` — see /// `AsyncSocketData::filteredAccept`. Feeds [`Self::active_connection_count`]. - /// `-3` / `+3`: a node:http tunnel becomes idle / has bytes to send again. `-4`: it closes idle. + /// `-3` / `+3`: a node:http tunnel becomes idle / is not idle any more (`HttpResponse::setNodeHttpTunnelIdle`). `-4`: it closes idle. extern "C" fn on_connection_filter( _socket: *mut uws_sys::us_socket_t, opened: i32, @@ -520,9 +522,8 @@ impl NewServer { 3 => { this.note_tunnel_idle(false); // With a listener open, the loop ref is the user's to drop (`server.unref()`). - if !this.has_listener() { - // SAFETY: `this` is not used again, and a tunnel write never runs inside `app.close()`. - unsafe { &mut *user_data.cast::() }.ref_(); + if !this.has_listener() && !this.user_unrefed.get() { + this.ref_(); } return; } @@ -1624,7 +1625,7 @@ impl NewServer { }); } - /// An idle tunnel is open but, like a libuv handle at EOF with no write pending, does not hold the loop. + /// An idle tunnel is open but, like a libuv handle that does not read and has no write pending, does not hold the loop. fn has_loop_holding_connections(&self) -> bool { self.active_connection_count.get() > self.idle_tunnel_count.get() } @@ -1702,18 +1703,20 @@ impl NewServer { } } - pub(crate) fn ref_(&mut self) { + pub(crate) fn ref_(&self) { // Once `is_closed()`, nothing is left that would ever `unref()` again // (`deinit_if_we_can` already dropped the loop ref), so a ref taken // here would pin the process forever. - if self.poll_ref.is_active() || self.is_closed() { + if self.poll_ref.get().is_active() || self.is_closed() { return; } - self.poll_ref.ref_(self.vm.loop_ctx()); + let loop_ctx = self.vm.loop_ctx(); + self.poll_ref.with_mut(|poll_ref| poll_ref.ref_(loop_ctx)); } - pub(crate) fn unref(&mut self) { - self.poll_ref.unref(self.vm.loop_ctx()); + pub(crate) fn unref(&self) { + let loop_ctx = self.vm.loop_ctx(); + self.poll_ref.with_mut(|poll_ref| poll_ref.unref(loop_ctx)); } pub(crate) fn stop_listening(&mut self, abrupt: bool) { @@ -2231,7 +2234,8 @@ impl NewServer { // ~816 KB mux pool; `listen()` materializes it on demand. mux_request_pool: core::ptr::null_mut(), all_closed_promise: jsc::JSPromiseStrong::default(), - poll_ref: KeepAlive::default(), + poll_ref: bun_jsc::JsCell::new(KeepAlive::default()), + user_unrefed: core::cell::Cell::new(false), flags: ServerFlags::default(), plugins: None, user_routes: Vec::new(), diff --git a/src/runtime/server/server_body.rs b/src/runtime/server/server_body.rs index 102a1c6ca94e..9a3fcfe334b7 100644 --- a/src/runtime/server/server_body.rs +++ b/src/runtime/server/server_body.rs @@ -2660,6 +2660,7 @@ where callframe: &CallFrame, ) -> JsResult { let this_value = callframe.this(); + self.user_unrefed.set(false); self.ref_(); Ok(this_value) } @@ -2671,6 +2672,7 @@ where callframe: &CallFrame, ) -> JsResult { let this_value = callframe.this(); + self.user_unrefed.set(true); self.unref(); Ok(this_value) } diff --git a/test/js/node/http/node-http-connect-end-small-sndbuf-fixture.js b/test/js/node/http/node-http-connect-end-small-sndbuf-fixture.js new file mode 100644 index 000000000000..fa2bd0de0e26 --- /dev/null +++ b/test/js/node/http/node-http-connect-end-small-sndbuf-fixture.js @@ -0,0 +1,77 @@ +// An https server whose 'upgrade' listener ends its socket with bytes, on a connection whose send +// buffer is small: the kernel takes less than one TLS record at a time, so TLS still holds bytes +// when the socket ends. The reads of the tunnel have stopped for a full buffer, and its peer stays +// connected. The process prints what the tunnel saw when it exits, and it must exit by itself: it +// has no timer and calls no process.exit(). +// +// CERT, KEY, LIBC (the libc to dlopen) +"use strict"; +const { dlopen, ptr } = require("bun:ffi"); +const { CERT, KEY, LIBC } = process.env; +const libc = dlopen(LIBC, { + getpeername: { args: ["i32", "ptr", "ptr"], returns: "i32" }, + setsockopt: { args: ["i32", "i32", "i32", "ptr", "u32"], returns: "i32" }, +}).symbols; +const darwin = process.platform === "darwin"; +const AF_INET = 2; +const SOL_SOCKET = darwin ? 0xffff : 1; +const SO_SNDBUF = darwin ? 0x1001 : 7; + +// The socket has no descriptor to ask for: it is the one whose peer has this port. +function descriptorOf(socket) { + // struct sockaddr_in: the family (behind a length byte on macOS), then the port in network order. + const address = new Uint8Array(16); + const length = new Uint32Array(1); + const view = new DataView(address.buffer); + for (let fd = 3; fd < 1024; fd++) { + length[0] = address.length; + if (libc.getpeername(fd, ptr(address), ptr(length)) !== 0) continue; + const family = darwin ? address[1] : view.getUint16(0, true); + if (family === AF_INET && view.getUint16(2, false) === socket.remotePort) return fd; + } + throw new Error("found no descriptor for the connection"); +} + +const server = require("node:https").createServer({ cert: CERT, key: KEY }); +const result = { events: [] }; +// Four TLS records. +const bytes = Buffer.alloc(65536, "y"); + +server.on("upgrade", (req, socket) => { + socket.on("error", error => result.events.push(error.code)); + socket.on("end", () => result.events.push("end")); + socket.on("close", () => result.events.push("close")); + const size = new Int32Array([4096]); + if (libc.setsockopt(descriptorOf(socket), SOL_SOCKET, SO_SNDBUF, ptr(size), 4) !== 0) { + throw new Error("setsockopt(SO_SNDBUF) failed"); + } + socket.write("HTTP/1.1 200 OK\r\n\r\n"); + server.close(() => result.events.push("server close")); + (function untilReadsStop() { + if (socket.readableLength < socket.readableHighWaterMark) return setImmediate(untilReadsStop); + socket.end(bytes, () => result.events.push("finish")); + })(); +}); + +server.listen(0, "127.0.0.1", () => { + const client = require("node:tls").connect({ + port: server.address().port, + host: "127.0.0.1", + rejectUnauthorized: false, + allowHalfOpen: true, + }); + // Only the server's side of the tunnel may keep this process alive. + client.unref(); + client.on("error", error => result.events.push(`peer: ${error.code}`)); + client.once("secureConnect", () => { + client.write("GET / HTTP/1.1\r\nHost: a\r\nConnection: Upgrade\r\nUpgrade: raw\r\n\r\n"); + // The reply: the listener has run, so the socket gets the bytes behind it and not the `head` argument. + client.once("data", () => { + // Twice what the socket buffers before it stops its reads. + client.write(Buffer.alloc(2 * 65536, "x")); + client.resume(); + }); + }); +}); + +process.on("exit", () => console.log(JSON.stringify(result))); diff --git a/test/js/node/http/node-http-connect-proxy-pipes-fixture.js b/test/js/node/http/node-http-connect-proxy-pipes-fixture.js new file mode 100644 index 000000000000..880f60818dcd --- /dev/null +++ b/test/js/node/http/node-http-connect-proxy-pipes-fixture.js @@ -0,0 +1,53 @@ +// The CONNECT proxy of the Node.js documentation for the 'connect' event: net.connect() and two +// pipes. Its upstream is slow and then goes away, its client gives up, and then it shuts down the +// graceful way: server.close(), no process.exit(). The process must exit by itself. +"use strict"; +const http = require("node:http"); +const net = require("node:net"); + +const result = {}; +let client; + +// An upstream that does not read. +let upstreamSocket; +const upstream = net.createServer(socket => { + upstreamSocket = socket.pause().on("error", () => {}); +}); + +const proxy = http.createServer(); +proxy.on("connect", (req, clientSocket, head) => { + const { port, hostname } = new URL(`http://${req.url}`); + const serverSocket = net.connect(port, hostname, () => { + clientSocket.write("HTTP/1.1 200 Connection Established\r\n\r\n"); + serverSocket.write(head); + serverSocket.pipe(clientSocket); + clientSocket.pipe(serverSocket); + }); + clientSocket.on("error", () => {}); + serverSocket.on("error", () => {}); + // pipe() pauses the client's socket because the upstream is slow. Then the upstream goes away. + clientSocket.once("pause", () => { + upstreamSocket.destroy(); + upstream.close(); + }); + serverSocket.on("close", () => { + // pipe() does not resume the socket that it paused. + result.clientSocketIsPaused = clientSocket.isPaused(); + client.on("close", () => proxy.close(() => (result.proxy = "close"))); + client.destroy(); + }); +}); + +upstream.listen(0, "127.0.0.1", () => { + proxy.listen(0, "127.0.0.1", () => { + client = net.connect(proxy.address().port, "127.0.0.1", () => { + client.write(`CONNECT 127.0.0.1:${upstream.address().port} HTTP/1.1\r\nHost: a\r\n\r\n`); + // More than the kernel buffers between the proxy and the upstream take. + client.write(Buffer.alloc(32 * 1024 * 1024, "x")); + }); + client.on("error", () => {}); + client.resume(); + }); +}); + +process.on("exit", () => console.log(JSON.stringify(result))); diff --git a/test/js/node/http/node-http-connect-read-stopped-fixture.js b/test/js/node/http/node-http-connect-read-stopped-fixture.js new file mode 100644 index 000000000000..49fa7fb4022d --- /dev/null +++ b/test/js/node/http/node-http-connect-read-stopped-fixture.js @@ -0,0 +1,235 @@ +// A server whose 'upgrade' and 'connect' listeners never read the socket. The socket buffers what +// the peer sends up to its high-water mark and then stops its reads. The process prints what every +// tunnel saw when it exits, and it must exit by itself: it has no timer and calls no process.exit(). +// +// PROTO=http|https (with CERT and KEY) +// CLOSE=before|after server.close() before or after the reads of the tunnels stop +// READERS=1 the tunnels get a 'data' listener and are read to their end +// SERVER=unref|ref server.unref() comes before server.close(), and server.ref() between them +// for "ref". One tunnel gets a 'data' listener when nothing keeps the process alive +"use strict"; +const { PROTO, CLOSE, READERS, SERVER, CERT, KEY } = process.env; +const secure = PROTO === "https"; +const server = secure + ? require("node:https").createServer({ cert: CERT, key: KEY }) + : require("node:http").createServer(); +const connect = secure ? require("node:tls").connect : require("node:net").connect; + +const requests = { + "upgrade": ["GET / HTTP/1.1\r\nHost: a\r\nConnection: Upgrade\r\nUpgrade: raw\r\n", ""], + "connect": ["CONNECT example.com:443 HTTP/1.1\r\nHost: example.com:443\r\n", ""], + // The tunnel starts behind the body. + "upgrade with a body": [ + "POST / HTTP/1.1\r\nHost: a\r\nConnection: Upgrade\r\nUpgrade: raw\r\nContent-Length: 5\r\n", + "hello", + ], +}; +// The 'connect' listener gives the socket back to the server as a connection, and the server +// parses the Upgrade request on it in JavaScript. +const reemitted = "upgrade on a re-emitted tunnel"; +if (!secure) requests[reemitted] = requests.upgrade; +const reply = "HTTP/1.1 200 OK\r\n\r\n"; +// Twice what the socket buffers before it stops its reads. With SERVER, what it buffers and no +// more: no byte is on its way when the reads stop. +const unread = Buffer.alloc((SERVER ? 1 : 2) * 65536, "x"); +// More than a loopback socket takes in one write. +const large = Buffer.alloc(16 * 1024 * 1024, "y"); +// More than the server reads from a socket in one turn of the event loop. +const upload = Buffer.alloc(8 * 1024 * 1024, "z"); + +// `stopped` runs when the reads of the tunnel have stopped. `peer` is what the peer does when it +// has the reply: by default it sends `unread` and reads what comes. +const stoppedKinds = { + // The peer closes its socket behind its bytes. + "the peer leaves": { + stopped(tunnel) { + tunnel.client.once("close", rest); + tunnel.client.destroySoon(); + }, + }, + // The peer is still connected when the process exits. + "the peer stays": {}, + "the listener ended it at once": { + start: tunnel => tunnel.socket.end(reply), + stopped(tunnel) { + tunnel.client.end(); + rest(); + }, + }, + "the listener ends it later": { + stopped(tunnel) { + tunnel.socket.end(); + tunnel.client.end(); + rest(); + }, + }, + // The bytes that are left to send keep the process alive, and then nothing does. + "the listener writes to it later": { + stopped(tunnel) { + tunnel.socket.write(large, () => tunnel.result.events.push("write callback")); + rest(); + }, + }, + // The peer does not read them until the listener has ended the socket. + "the listener ends it later with bytes": { + peer(client) { + client.write(unread); + client.pause(); + }, + stopped(tunnel) { + tunnel.socket.end(large, () => tunnel.result.events.push("finish")); + setImmediate(() => tunnel.client.resume()); + rest(); + }, + }, +}; +// A tunnel that reads keeps the process alive. `more` is what the peer does when it has read "more". +const readerKinds = { + // The reads of this tunnel never stop for the listener. The server pauses them by itself while + // chunks wait for their turn. It starts when the reads of the other tunnels have stopped. + "a reader that writes for every chunk": { + reads: true, + start({ socket, result }) { + socket.write(reply); + socket.on("data", chunk => { + result.received += chunk.length; + socket.write("a"); + if (result.received === upload.length) socket.write("more"); + }); + }, + peer: client => client.write(upload), + more: client => client.end("tail"), + }, + // Its reader comes when nothing keeps the process alive. + "a reader comes later": { + later({ socket, result }) { + socket.on("data", chunk => { + result.received += chunk.length; + // From here on this tunnel has nothing buffered and nothing to send. + if (result.received === unread.length) socket.write("more"); + }); + }, + more: client => client.end(unread), + }, +}; +// With SERVER, one tunnel of the second kind. In Node.js its socket keeps the process alive up to +// its 'close' in both cases. +if (SERVER) delete readerKinds["a reader that writes for every chunk"]; +const kinds = READERS || SERVER ? readerKinds : stoppedKinds; + +const tunnels = {}; +const results = {}; +for (const request of SERVER ? ["upgrade"] : Object.keys(requests)) { + for (const kind in kinds) { + const name = `${request}, ${kind}`; + tunnels[name] = { name, request, kind: kinds[kind], result: { events: [], received: 0 } }; + } +} +const all = Object.values(tunnels); +const readers = all.filter(tunnel => tunnel.kind.reads); +let handedOff = 0; +let atRest = 0; + +function close() { + if (SERVER) server.unref(); + if (SERVER === "ref") server.ref(); + server.close(() => (results.server = "close")); +} + +// A tunnel whose reads have stopped has done what it does. +function rest() { + if (++atRest < all.length - readers.length) return; + if (CLOSE === "after") close(); + for (const tunnel of readers) tunnel.kind.start(tunnel); +} + +// Nothing keeps the process alive here. A reader that is not at its 'close' yet shows that nothing +// did it for that reader: the process exits, and the rows are as they are. +process.once("beforeExit", () => { + if (readers.some(tunnel => !tunnel.result.events.includes("close"))) return; + for (const tunnel of all) tunnel.kind.later?.(tunnel); +}); + +function start(tunnel) { + const { socket, kind } = tunnel; + if (kind.reads) return; + if (kind.start) kind.start(tunnel); + else socket.write(reply); + (function untilReadsStop() { + if (socket.readableLength < socket.readableHighWaterMark) return setImmediate(untilReadsStop); + if (kind.stopped) kind.stopped(tunnel); + else rest(); + })(); +} + +function onTunnel(req, socket) { + const tunnel = tunnels[req.headers.tunnel]; + tunnel.socket = socket; + results[tunnel.name] = tunnel.result; + socket.on("error", error => tunnel.result.events.push(error.code)); + socket.on("end", () => { + tunnel.result.events.push("end"); + socket.end(); + }); + socket.on("close", () => tunnel.result.events.push("close")); + const handOff = () => { + if (CLOSE === "after") return start(tunnel); + if (++handedOff < all.length) return; + close(); + all.forEach(start); + }; + // The tunnel starts behind the body of the request. + if (requests[tunnel.request][1]) req.on("end", handOff).resume(); + else handOff(); +} +server.on("upgrade", onTunnel).on("connect", (req, socket) => { + if (!req.headers.reemit) return onTunnel(req, socket); + socket.write(reply); + server.emit("connection", socket); +}); + +server.listen(0, "127.0.0.1", () => { + for (const tunnel of all) { + const { name, kind } = tunnel; + const [head, body] = requests[tunnel.request]; + // Half-open: the peer ends its side only where its kind says so. + const client = (tunnel.client = connect({ + port: server.address().port, + host: "127.0.0.1", + rejectUnauthorized: false, + allowHalfOpen: true, + })); + // Only the server's side of a tunnel may keep this process alive. + client.unref(); + const lost = event => { + if (!tunnel.socket) results[name] = `the peer got '${event}' before the listener ran`; + }; + client.on("error", () => lost("error")); + client.on("close", () => lost("close")); + const request = () => { + client.write(`${head}Tunnel: ${name}\r\n\r\n${body}`); + // The reply: the listener has run, so the socket gets the bytes behind it and not the `head` argument. + client.once("data", () => { + if (kind.peer) { + kind.peer(client); + } else { + client.write(unread); + client.resume(); + } + if (!kind.more) return; + let tail = ""; + client.on("data", chunk => { + tail = (tail + chunk.toString("latin1")).slice(-4); + if (tail === "more") kind.more(client); + }); + }); + }; + client.once(secure ? "secureConnect" : "connect", () => { + if (tunnel.request !== reemitted) return request(); + client.write("CONNECT example.com:443 HTTP/1.1\r\nHost: example.com:443\r\nReemit: 1\r\n\r\n"); + client.once("data", request); + }); + } +}); + +process.on("exit", () => console.log(JSON.stringify(results))); diff --git a/test/js/node/http/node-http-connect.test.ts b/test/js/node/http/node-http-connect.test.ts index 95c3ce7310f8..9002ce0ac84a 100644 --- a/test/js/node/http/node-http-connect.test.ts +++ b/test/js/node/http/node-http-connect.test.ts @@ -1,5 +1,15 @@ import { describe, expect, test } from "bun:test"; -import { bunEnv, bunExe, bunRun, isLinux, isWindows, nodeExe, tempDir, tls as tlsCert } from "harness"; +import { + bunEnv, + bunExe, + bunRun, + isLinux, + isWindows, + libcPathForDlopen, + nodeExe, + tempDir, + tls as tlsCert, +} from "harness"; import http from "http"; import { once } from "node:events"; @@ -1924,6 +1934,132 @@ test("a half-open tunnel with bytes left to send keeps the process alive until t } }); +async function runTunnelFixture(exe: string, fixture: string, env: Record = {}) { + await using proc = Bun.spawn({ + cmd: [exe, join(import.meta.dir, fixture)], + env: { + ...bunEnv, + CERT: tlsCert.cert, + KEY: tlsCert.key, + // bun test does not kill the child of a concurrent test that times out. With this, a + // child that hangs exits when the test process does. + BUN_FEATURE_FLAG_NO_ORPHANS: "1", + ...env, + }, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + return { stdout: stdout && JSON.parse(stdout), stderr, exitCode, signalCode: proc.signalCode }; +} +const exitedByItself = { stderr: "", exitCode: 0, signalCode: null }; + +// The fixtures below run under Node.js as they are, and "node" runs them there: every expectation +// is Node v26.3.0's unless it says otherwise. Their peers send the bytes of a tunnel when they have +// the listener's reply, so the socket gets them and not the `head` argument. +describe.concurrent.each(["bun", "node"])("%s: what keeps the process alive after server.close()", runtime => { + const exe = runtime === "node" ? nodeExe() : bunExe(); + const atRest = { events: [], received: 0 }; + + describe.skipIf(!exe).each(["http", "https"])("%s", proto => { + // The server parses the request on a re-emitted tunnel in JavaScript. + const requests = ["upgrade", "connect", "upgrade with a body"]; + if (proto === "http") requests.push("upgrade on a re-emitted tunnel"); + + // In Node.js a socket stops the reads of its handle when its buffer is full, and a handle that + // does not read and has nothing to send is not active: it does not keep the process alive. + describe.each(["before", "after"])("server.close() %s the reads of the unread tunnels stop", close => { + const env = { PROTO: proto, CLOSE: close }; + + test("a tunnel that nothing reads does not keep the process alive", async () => { + // The sockets are still open, so neither they nor the server emit 'close'. + expect(await runTunnelFixture(exe!, "node-http-connect-read-stopped-fixture.js", env)).toEqual({ + stdout: Object.fromEntries( + requests.flatMap(request => [ + [`${request}, the peer leaves`, atRest], + [`${request}, the peer stays`, atRest], + [`${request}, the listener ended it at once`, atRest], + [`${request}, the listener ends it later`, atRest], + [`${request}, the listener writes to it later`, { events: ["write callback"], received: 0 }], + [`${request}, the listener ends it later with bytes`, { events: ["finish"], received: 0 }], + ]), + ), + ...exitedByItself, + }); + }); + + test("a tunnel that reads keeps the process alive up to its 'close'", async () => { + const readers = { ...env, READERS: "1" }; + expect(await runTunnelFixture(exe!, "node-http-connect-read-stopped-fixture.js", readers)).toEqual({ + stdout: { + ...Object.fromEntries( + requests.flatMap(request => [ + // The upload and "tail". + [ + `${request}, a reader that writes for every chunk`, + { events: ["end", "close"], received: 8 * 1024 * 1024 + 4 }, + ], + [`${request}, a reader comes later`, { events: ["end", "close"], received: 4 * 65536 }], + ]), + ), + server: "close", + }, + ...exitedByItself, + }); + }); + }); + + describe("a tunnel that reads again behind server.unref()", () => { + const row = "upgrade, a reader comes later"; + const toItsClose = { + stdout: { [row]: { events: ["end", "close"], received: 2 * 65536 }, server: "close" }, + ...exitedByItself, + }; + const run = (server: string) => + runTunnelFixture(exe!, "node-http-connect-read-stopped-fixture.js", { + PROTO: proto, + CLOSE: "after", + SERVER: server, + }); + + test("keeps the process alive when server.ref() came behind it", async () => { + expect(await run("ref")).toEqual(toItsClose); + }); + + // In Node.js a socket keeps the process alive by itself. Here the server does it for its + // sockets, and server.unref() ends that: the reader gets only what the socket had buffered. + test("keeps the process alive only in Node.js", async () => { + expect(await run("unref")).toEqual( + runtime === "node" ? toItsClose : { stdout: { [row]: { events: [], received: 65536 } }, ...exitedByItself }, + ); + }); + }); + }); + + // pipe() pauses the client's socket while the upstream is slow, and it does not resume it when + // the upstream goes away. + test.skipIf(!exe)("a CONNECT proxy by pipes exits when its upstream went away", async () => { + // The client's socket is still open, so the proxy does not emit 'close'. + expect(await runTunnelFixture(exe!, "node-http-connect-proxy-pipes-fixture.js")).toEqual({ + stdout: { clientSocketIsPaused: true }, + ...exitedByItself, + }); + }); +}); + +// TLS still holds bytes when such a socket ends, and usockets sends the FIN behind them with no +// writable event for the tunnel. Not on Windows: the fixture makes the send buffer small through libc. +test.concurrent.skipIf(isWindows)( + "https: a read-stopped tunnel that ends with bytes behind a small send buffer does not keep the process alive", + async () => { + const env = { LIBC: libcPathForDlopen() }; + expect(await runTunnelFixture(bunExe(), "node-http-connect-end-small-sndbuf-fixture.js", env)).toEqual({ + stdout: { events: ["finish"] }, + ...exitedByItself, + }); + }, +); + // A listener with nothing to send ends its side in the listener. It still reads: an http.Server // socket has allowHalfOpen. Every expectation below is Node v26.3.0's. describe("a tunnel behind a request head that took more than one read", () => { From 5d6cddcbd0cd41d7b2c8eb0140be8b22c64cd002 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 9 Oct 2026 15:26:20 +0000 Subject: [PATCH 02/11] node:http: socket.unref() and socket.ref() act on a tunnel socket socket.unref() and socket.ref() did nothing on the socket of a 'connect' or 'upgrade' listener. They are now a third input of the tunnel's idle state, like the ref state of a libuv handle: the reads of an unref'd tunnel do not hold the event loop, and ref() puts them back. Bytes left to send hold the loop in both states. An unref() that comes before the tunnel is kept for it. The tunnel starts in one place for both paths (the dispatch of the request, or the end of the body of an Upgrade request), and that place applies it. --- packages/bun-uws/src/HttpContext.h | 5 +- packages/bun-uws/src/HttpResponse.h | 4 +- src/js/node/_http_server.ts | 3 + .../bindings/node/JSNodeHTTPServerSocket.cpp | 35 ++- .../bindings/node/JSNodeHTTPServerSocket.h | 6 +- .../node/JSNodeHTTPServerSocketPrototype.cpp | 11 + src/runtime/server/mod.rs | 4 +- ...e-http-connect-end-small-sndbuf-fixture.js | 14 +- .../http/node-http-connect-unref-fixture.js | 259 ++++++++++++++++++ test/js/node/http/node-http-connect.test.ts | 46 +++- 10 files changed, 368 insertions(+), 19 deletions(-) create mode 100644 test/js/node/http/node-http-connect-unref-fixture.js diff --git a/packages/bun-uws/src/HttpContext.h b/packages/bun-uws/src/HttpContext.h index 9c041bdf611c..f68da1ad629e 100644 --- a/packages/bun-uws/src/HttpContext.h +++ b/packages/bun-uws/src/HttpContext.h @@ -37,6 +37,7 @@ extern "C" void Bun__NodeHTTP__onReadsResumable(int ssl, struct us_socket_t *s); +extern "C" void Bun__NodeHTTP__onTunnelStarted(int ssl, struct us_socket_t *s); extern "C" void Bun__NodeHTTP__onReadParsed(int ssl, struct us_socket_t *s); namespace uWS { @@ -705,8 +706,8 @@ struct HttpContext { } if constexpr (IsNodeHttp) { if (switchToTunnelAfterThisChunk) { - /* The response cannot resume reads in tunnel mode: lift the pause the body left. */ - Bun__NodeHTTP__onReadsResumable(SSL, (struct us_socket_t *) user); + /* The response cannot resume reads in tunnel mode: this lifts the pause the body left. */ + Bun__NodeHTTP__onTunnelStarted(SSL, (struct us_socket_t *) user); if (us_socket_is_closed((struct us_socket_t *) user)) { return nullptr; } diff --git a/packages/bun-uws/src/HttpResponse.h b/packages/bun-uws/src/HttpResponse.h index 6e155e944df3..0f2827f8fd75 100644 --- a/packages/bun-uws/src/HttpResponse.h +++ b/packages/bun-uws/src/HttpResponse.h @@ -183,8 +183,8 @@ struct HttpResponse : public AsyncSocket { return true; } - /* node:http: an idle tunnel has reads that do not hold the event loop (they are stopped or at EOF) and has nothing left to - * send. Like a libuv stream handle in that state, it does not hold the event loop: + /* node:http: an idle tunnel has reads that do not hold the event loop (they are stopped, at EOF, or the socket is unref'd) and + * has nothing left to send. Like a libuv stream handle in that state, it does not hold the event loop: * https://github.com/nodejs/node/blob/v26.3.0/deps/uv/src/unix/stream.c#L1473-L1485. The filter hears -3 when a tunnel * becomes idle and +3 when its reads hold the event loop again or it has bytes to send again. */ void setNodeHttpTunnelIdle(bool idle) { diff --git a/src/js/node/_http_server.ts b/src/js/node/_http_server.ts index fae709e723b6..ad24468b152c 100644 --- a/src/js/node/_http_server.ts +++ b/src/js/node/_http_server.ts @@ -2046,7 +2046,9 @@ function getNodeHTTPServerSocket() { return this[kHandle]?.authorizationError ?? null; } + // Like Node's net.Socket, for a tunnel: the handle holds the event loop while it reads and is ref'd. https://github.com/nodejs/node/blob/v26.3.0/lib/net.js#L1621-L1646 ref() { + this[kHandle]?.setRef(true); return this; } @@ -2146,6 +2148,7 @@ function getNodeHTTPServerSocket() { } unref() { + this[kHandle]?.setRef(false); return this; } diff --git a/src/jsc/bindings/node/JSNodeHTTPServerSocket.cpp b/src/jsc/bindings/node/JSNodeHTTPServerSocket.cpp index 408bc2db7735..998d6ac21064 100644 --- a/src/jsc/bindings/node/JSNodeHTTPServerSocket.cpp +++ b/src/jsc/bindings/node/JSNodeHTTPServerSocket.cpp @@ -127,6 +127,28 @@ void JSNodeHTTPServerSocket::reset() template static void onNodeHttpReadsResumable(us_socket_t* socket); +/* The tunnel starts: in the dispatch of a CONNECT or of an Upgrade with no body, behind the body of any other Upgrade (HttpContext). */ +template +static void onNodeHttpTunnelStarted(us_socket_t* socket) +{ + auto* httpResponseData = reinterpret_cast*>(us_socket_ext(socket)); + // socket.unref() came before the tunnel. + if (auto* cell = reinterpret_cast(httpResponseData->socketData); cell && cell->tunnelUnrefed) [[unlikely]] { + cell->updateTunnelIdle(); + } + /* resume() on a response does nothing in tunnel mode: lift what paused reads before it (req.pause(), flood prevention). */ + onNodeHttpReadsResumable(socket); +} + +extern "C" void Bun__NodeHTTP__onTunnelStarted(int ssl, us_socket_t* socket) +{ + if (ssl) { + onNodeHttpTunnelStarted(socket); + } else { + onNodeHttpTunnelStarted(socket); + } +} + template static void upgradeToTunnelModeImpl(us_socket_t* socket, bool afterBody) { @@ -139,8 +161,7 @@ static void upgradeToTunnelModeImpl(us_socket_t* socket, bool afterBody) return; } httpResponseData->isConnectRequest = true; - /* resume() on a response does nothing in tunnel mode: lift what paused reads before it (req.pause(), flood prevention). */ - onNodeHttpReadsResumable(socket); + onNodeHttpTunnelStarted(socket); } void JSNodeHTTPServerSocket::upgradeToTunnelMode(bool afterBody, WebCore::JSNodeHTTPResponse* response) @@ -218,6 +239,12 @@ void JSNodeHTTPServerSocket::readStart() applyTunnelReads(); } +void JSNodeHTTPServerSocket::setRef(bool ref) +{ + tunnelUnrefed = !ref; + refreshTunnelIdle(); +} + void JSNodeHTTPServerSocket::didDeliverQueuedTunnelBytes(size_t length) { queuedTunnelBytes -= length; @@ -902,7 +929,7 @@ void JSNodeHTTPServerSocket::refreshTunnelIdle() if (!isTunnel(this)) { return; } - const bool idle = (tunnelReadEnded || tunnelReadsStopped) && streamBuffer.bufferedSize() == 0; + const bool idle = (tunnelReadEnded || tunnelReadsStopped || tunnelUnrefed) && streamBuffer.bufferedSize() == 0; if (is_ssl) { reinterpret_cast*>(socket)->setNodeHttpTunnelIdle(idle && reinterpret_cast*>(socket)->hasFullyDrained()); } else { @@ -913,7 +940,7 @@ void JSNodeHTTPServerSocket::refreshTunnelIdle() void JSNodeHTTPServerSocket::updateTunnelIdle() { // A tunnel whose reads hold the event loop is not idle, and a write or a drain does not change that. - if (tunnelReadEnded || tunnelReadsStopped) { + if (tunnelReadEnded || tunnelReadsStopped || tunnelUnrefed) { refreshTunnelIdle(); } } diff --git a/src/jsc/bindings/node/JSNodeHTTPServerSocket.h b/src/jsc/bindings/node/JSNodeHTTPServerSocket.h index e300ee339c81..6a9a23255b8a 100644 --- a/src/jsc/bindings/node/JSNodeHTTPServerSocket.h +++ b/src/jsc/bindings/node/JSNodeHTTPServerSocket.h @@ -65,6 +65,8 @@ class JSNodeHTTPServerSocket : public JSC::JSDestructibleObject { unsigned peer_ended : 1 = 0; /* socket.end() came while TLS held bytes of the tunnel: halfClose() to the onDrain() that sends the FIN. */ unsigned tunnelEndAwaitsDrain : 1 = 0; + /* socket.unref() to socket.ref(). The reads of an unref'd tunnel do not hold the event loop. */ + unsigned tunnelUnrefed : 1 = 0; int closeReadError = 0; /* Tunnel bytes that onData() queued for JS in tasks that have not run yet. */ size_t queuedTunnelBytes = 0; @@ -148,10 +150,12 @@ class JSNodeHTTPServerSocket : public JSC::JSDestructibleObject { void readStop(); void readStart(); bool tunnelReadsPaused() const { return tunnelReadsStopped || tunnelReadsQueuedFull; } - /* Tells uWS whether this tunnel is idle: its reads do not hold the event loop (readStop() or read EOF) and it has nothing left to send. See HttpResponse::setNodeHttpTunnelIdle. */ + /* Tells uWS whether this tunnel is idle: its reads do not hold the event loop (readStop(), read EOF, or socket.unref()) and it has nothing left to send. See HttpResponse::setNodeHttpTunnelIdle. */ void refreshTunnelIdle(); /* The same for a caller that did not change what the reads hold: a write, a drain, the end of the stream. */ void updateTunnelIdle(); + /* socket.ref() and socket.unref(), like those of a libuv handle: they change whether the reads of a tunnel hold the event loop. Bytes left to send hold it in both states. Before the tunnel starts, the state is only kept for it. */ + void setRef(bool ref); /* uWS still holds bytes of an HTTP response on this connection. A raw write has to go through the same buffer, or it reaches the wire first. */ bool hasUnsentResponseBytes() const; /* Sends the response bytes that are not in the uWS buffer (the zero-copy tail of a res.write(), the cork buffer) to the kernel or into it. A raw write or a FIN then goes out behind them. */ diff --git a/src/jsc/bindings/node/JSNodeHTTPServerSocketPrototype.cpp b/src/jsc/bindings/node/JSNodeHTTPServerSocketPrototype.cpp index 131ca8458a5e..01773758e491 100644 --- a/src/jsc/bindings/node/JSNodeHTTPServerSocketPrototype.cpp +++ b/src/jsc/bindings/node/JSNodeHTTPServerSocketPrototype.cpp @@ -46,6 +46,7 @@ JSC_DECLARE_HOST_FUNCTION(jsFunctionNodeHTTPServerSocketEnd); JSC_DECLARE_HOST_FUNCTION(jsFunctionNodeHTTPServerSocketUpgradeToTunnel); JSC_DECLARE_HOST_FUNCTION(jsFunctionNodeHTTPServerSocketReadStop); JSC_DECLARE_HOST_FUNCTION(jsFunctionNodeHTTPServerSocketReadStart); +JSC_DECLARE_HOST_FUNCTION(jsFunctionNodeHTTPServerSocketSetRef); JSC_DECLARE_HOST_FUNCTION(jsFunctionNodeHTTPServerSocketSetResponseTrailers); JSC_DECLARE_HOST_FUNCTION(jsFunctionNodeHTTPServerSocketIsRequestTimedOut); JSC_DECLARE_HOST_FUNCTION(jsFunctionNodeHTTPServerSocketStartPipelinedResponse); @@ -89,6 +90,7 @@ static const JSC::HashTableValue JSNodeHTTPServerSocketPrototypeTableValues[] = { "upgradeToTunnel"_s, static_cast(JSC::PropertyAttribute::Function | JSC::PropertyAttribute::DontEnum), JSC::NoIntrinsic, { JSC::HashTableValue::NativeFunctionType, jsFunctionNodeHTTPServerSocketUpgradeToTunnel, 0 } }, { "readStop"_s, static_cast(JSC::PropertyAttribute::Function | JSC::PropertyAttribute::DontEnum), JSC::NoIntrinsic, { JSC::HashTableValue::NativeFunctionType, jsFunctionNodeHTTPServerSocketReadStop, 0 } }, { "readStart"_s, static_cast(JSC::PropertyAttribute::Function | JSC::PropertyAttribute::DontEnum), JSC::NoIntrinsic, { JSC::HashTableValue::NativeFunctionType, jsFunctionNodeHTTPServerSocketReadStart, 0 } }, + { "setRef"_s, static_cast(JSC::PropertyAttribute::Function | JSC::PropertyAttribute::DontEnum), JSC::NoIntrinsic, { JSC::HashTableValue::NativeFunctionType, jsFunctionNodeHTTPServerSocketSetRef, 1 } }, { "setResponseTrailers"_s, static_cast(JSC::PropertyAttribute::Function | JSC::PropertyAttribute::DontEnum), JSC::NoIntrinsic, { JSC::HashTableValue::NativeFunctionType, jsFunctionNodeHTTPServerSocketSetResponseTrailers, 1 } }, { "isRequestTimedOut"_s, static_cast(JSC::PropertyAttribute::Function | JSC::PropertyAttribute::DontEnum), JSC::NoIntrinsic, { JSC::HashTableValue::NativeFunctionType, jsFunctionNodeHTTPServerSocketIsRequestTimedOut, 2 } }, { "startPipelinedResponse"_s, static_cast(JSC::PropertyAttribute::Function | JSC::PropertyAttribute::DontEnum), JSC::NoIntrinsic, { JSC::HashTableValue::NativeFunctionType, jsFunctionNodeHTTPServerSocketStartPipelinedResponse, 3 } }, @@ -166,6 +168,15 @@ JSC_DEFINE_HOST_FUNCTION(jsFunctionNodeHTTPServerSocketReadStart, (JSC::JSGlobal return JSValue::encode(JSC::jsUndefined()); } +// socket.ref() is setRef(true), socket.unref() is setRef(false). +JSC_DEFINE_HOST_FUNCTION(jsFunctionNodeHTTPServerSocketSetRef, (JSC::JSGlobalObject * globalObject, JSC::CallFrame* callFrame)) +{ + if (auto* thisObject = dynamicDowncast(callFrame->thisValue())) [[likely]] { + thisObject->setRef(callFrame->argument(0).isTrue()); + } + return JSValue::encode(JSC::jsUndefined()); +} + // node:http: set the trailer fields (pre-rendered "name: value\r\n" lines) to send // at the end of the current response's chunked body (response.addTrailers()). JSC_DEFINE_HOST_FUNCTION(jsFunctionNodeHTTPServerSocketSetResponseTrailers, (JSC::JSGlobalObject * globalObject, JSC::CallFrame* callFrame)) diff --git a/src/runtime/server/mod.rs b/src/runtime/server/mod.rs index f73d1c00af0a..8e033a2f6d8f 100644 --- a/src/runtime/server/mod.rs +++ b/src/runtime/server/mod.rs @@ -268,7 +268,7 @@ pub(crate) struct NewServer { /// ([`NewServer::is_drained`]); for Bun.serve it also holds the /// graceful-stop promise open ([`NewServer::is_closed`]). pub(crate) active_connection_count: core::cell::Cell, - /// The node:http tunnels in `active_connection_count` whose reads do not hold the loop (stopped or at EOF) and that have nothing left to send. + /// The node:http tunnels in `active_connection_count` whose reads do not hold the loop (stopped, at EOF, or unref'd) and that have nothing left to send. pub(crate) idle_tunnel_count: core::cell::Cell, /// Live `ServerWebSocket` count. Lives on the server (not the websocket /// context) so a reload's context swap cannot reset it, and sits in a @@ -1625,7 +1625,7 @@ impl NewServer { }); } - /// An idle tunnel is open but, like a libuv handle that does not read and has no write pending, does not hold the loop. + /// An idle tunnel is open but, like a libuv handle that does not read or is unref'd, with no write pending, does not hold the loop. fn has_loop_holding_connections(&self) -> bool { self.active_connection_count.get() > self.idle_tunnel_count.get() } diff --git a/test/js/node/http/node-http-connect-end-small-sndbuf-fixture.js b/test/js/node/http/node-http-connect-end-small-sndbuf-fixture.js index fa2bd0de0e26..d1898549e987 100644 --- a/test/js/node/http/node-http-connect-end-small-sndbuf-fixture.js +++ b/test/js/node/http/node-http-connect-end-small-sndbuf-fixture.js @@ -1,13 +1,14 @@ // An https server whose 'upgrade' listener ends its socket with bytes, on a connection whose send // buffer is small: the kernel takes less than one TLS record at a time, so TLS still holds bytes -// when the socket ends. The reads of the tunnel have stopped for a full buffer, and its peer stays +// when the socket ends. The reads of the tunnel do not keep the process alive, and its peer stays // connected. The process prints what the tunnel saw when it exits, and it must exit by itself: it // has no timer and calls no process.exit(). // +// KIND=unref|stopped socket.unref(), or reads that stopped for a full buffer // CERT, KEY, LIBC (the libc to dlopen) "use strict"; const { dlopen, ptr } = require("bun:ffi"); -const { CERT, KEY, LIBC } = process.env; +const { KIND, CERT, KEY, LIBC } = process.env; const libc = dlopen(LIBC, { getpeername: { args: ["i32", "ptr", "ptr"], returns: "i32" }, setsockopt: { args: ["i32", "i32", "i32", "ptr", "u32"], returns: "i32" }, @@ -47,9 +48,14 @@ server.on("upgrade", (req, socket) => { } socket.write("HTTP/1.1 200 OK\r\n\r\n"); server.close(() => result.events.push("server close")); + const end = () => socket.end(bytes, () => result.events.push("finish")); + if (KIND === "unref") { + socket.unref(); + return end(); + } (function untilReadsStop() { if (socket.readableLength < socket.readableHighWaterMark) return setImmediate(untilReadsStop); - socket.end(bytes, () => result.events.push("finish")); + end(); })(); }); @@ -68,7 +74,7 @@ server.listen(0, "127.0.0.1", () => { // The reply: the listener has run, so the socket gets the bytes behind it and not the `head` argument. client.once("data", () => { // Twice what the socket buffers before it stops its reads. - client.write(Buffer.alloc(2 * 65536, "x")); + if (KIND === "stopped") client.write(Buffer.alloc(2 * 65536, "x")); client.resume(); }); }); diff --git a/test/js/node/http/node-http-connect-unref-fixture.js b/test/js/node/http/node-http-connect-unref-fixture.js new file mode 100644 index 000000000000..62f05e44b221 --- /dev/null +++ b/test/js/node/http/node-http-connect-unref-fixture.js @@ -0,0 +1,259 @@ +// 'upgrade' and 'connect' listeners that call socket.unref() and socket.ref(). A tunnel keeps the +// process alive while it reads and is ref'd, or while it has bytes left to send. The process prints +// what every tunnel saw when it exits, and it must exit by itself: it has no timer and calls no +// process.exit(). +// +// PROTO=http|https (with CERT and KEY) +"use strict"; +const { PROTO, CERT, KEY } = process.env; +const secure = PROTO === "https"; +const server = secure + ? require("node:https").createServer({ cert: CERT, key: KEY }) + : require("node:http").createServer(); +const connect = secure ? require("node:tls").connect : require("node:net").connect; + +const upgrade = "GET / HTTP/1.1\r\nHost: a\r\nConnection: Upgrade\r\nUpgrade: raw\r\n"; +// `first` is a request that the peer sends before this one, on the same connection. +const requests = { + "upgrade": { head: upgrade }, + "connect": { head: "CONNECT example.com:443 HTTP/1.1\r\nHost: example.com:443\r\n" }, + // The tunnel starts behind the body, and the listener runs before it. + "upgrade with a body": { + head: "POST / HTTP/1.1\r\nHost: a\r\nConnection: Upgrade\r\nUpgrade: raw\r\nContent-Length: 5\r\n", + body: "hello", + }, + "upgrade behind a request": { head: upgrade, first: "GET / HTTP/1.1\r\nHost: a\r\n\r\n" }, +}; +// The 'connect' listener gives the socket back to the server as a connection, and the server +// parses the Upgrade request on it in JavaScript. +if (!secure) { + requests["upgrade on a re-emitted tunnel"] = { + head: upgrade, + first: "CONNECT example.com:443 HTTP/1.1\r\nHost: example.com:443\r\nReemit: 1\r\n\r\n", + }; +} +const reply = "HTTP/1.1 200 OK\r\n\r\n"; +// What the socket buffers before it stops its reads. +const full = Buffer.alloc(65536, "x"); +// More than a loopback socket takes in one write. +const large = Buffer.alloc(16 * 1024 * 1024, "y"); + +// The server closes when the reads of every tunnel that waits for it have stopped. Up to there the +// listening server keeps the process alive, also for the peers of the tunnels that are unref'd. +let waiting = 0; +function whenReadsStop(socket, then) { + waiting++; + (function poll() { + if (socket.readableLength < socket.readableHighWaterMark) return setImmediate(poll); + then?.(); + if (--waiting === 0) server.close(() => (results.server = "close")); + })(); +} + +function count({ socket, result }) { + socket.on("data", chunk => (result.received += chunk.length)); +} + +// Wraps a callback that the process must stay alive for. +let holding = 0; +function held(callback) { + holding++; + return () => { + holding--; + callback?.(); + }; +} + +// `connection` runs in the 'connection' listener and `listener` in the 'upgrade' or 'connect' +// listener. `start` runs when every tunnel has its listener, before the peer has the reply. The +// `later` steps run one at a time, each when nothing keeps the process alive: from there on, only +// its tunnel can. `more` is what the peer does when it has read "more". +const kinds = { + // The tunnel of a request with a body has not started there. + "unref() in the listener": { + requests: Object.keys(requests), + listener: ({ socket }) => socket.unref(), + }, + "unref() while it reads": { + requests: ["upgrade", "connect", "upgrade with a body"], + start: ({ socket }) => socket.unref(), + }, + // The listener writes nothing to this tunnel. + "unref() before the request": { + requests: ["upgrade", "upgrade with a body", "upgrade behind a request"], + silent: true, + connection: socket => socket.unref(), + }, + // The bytes that are left to send keep the process alive. + "unref(), then a write": { + start({ socket, result }) { + socket.unref(); + socket.write( + large, + held(() => result.events.push("write callback")), + ); + }, + }, + "a write, then unref()": { + start({ socket, result }) { + socket.write( + large, + held(() => result.events.push("write callback")), + ); + socket.unref(); + }, + }, + // The end of the stream goes out behind the bytes. + "unref(), then end() with bytes": { + start({ socket, result }) { + socket.unref(); + socket.end( + large, + held(() => result.events.push("finish")), + ); + }, + }, + "ref() after its reads stopped": { + peerSends: full, + start: ({ socket }) => whenReadsStop(socket, () => socket.ref()), + }, + "ref() at the end of the stream": { + peerEnds: true, + start(tunnel) { + tunnel.atEnd = held(() => tunnel.socket.ref()); + tunnel.socket.resume(); + }, + end: tunnel => tunnel.atEnd(), + }, + "unref(), then its reads stop, then a reader": { + peerSends: full, + start(tunnel) { + tunnel.socket.unref(); + whenReadsStop(tunnel.socket, () => count(tunnel)); + }, + }, + "unref(), then its reads stop, then ref() and a reader": { + peerSends: full, + start({ socket }) { + socket.unref(); + whenReadsStop(socket); + }, + later({ socket, result }) { + socket.ref(); + socket.on("data", chunk => { + result.received += chunk.length; + if (result.received === full.length) socket.write("more"); + }); + }, + more: client => client.end(full), + }, + "a reader, then unref(), then ref()": { + start(tunnel) { + count(tunnel); + tunnel.socket.unref(); + }, + later({ socket }) { + socket.ref(); + socket.write("more"); + }, + more: client => client.end(full), + }, +}; +const all = []; +for (const [name, kind] of Object.entries(kinds)) { + for (const request of kind.requests ?? ["upgrade"]) { + all.push({ name: `${request}, ${name}`, request: requests[request], kind, result: { events: [], received: 0 } }); + } +} +const tunnels = Object.fromEntries(all.map(tunnel => [tunnel.name, tunnel])); +const results = {}; +const later = all.filter(tunnel => tunnel.kind.later); +let handedOff = 0; + +// Nothing keeps the process alive here. When something is not done that the process must stay +// alive for, it exits, and the rows are as they are. +process.on("beforeExit", () => { + if (holding > 0 || later.length === 0) return; + const tunnel = later.shift(); + tunnel.socket.once("close", held()); + tunnel.kind.later(tunnel); +}); + +function onTunnel(req, socket) { + const tunnel = tunnels[req.headers.tunnel]; + const { kind, result } = tunnel; + tunnel.socket = socket; + results[tunnel.name] = result; + socket.on("error", error => result.events.push(error.code)); + socket.on("end", () => { + result.events.push("end"); + if (kind.end) kind.end(tunnel); + else socket.end(); + }); + socket.on("close", () => result.events.push("close")); + kind.listener?.(tunnel); + if (++handedOff < all.length) return; + for (const tunnel of all) { + if (!tunnel.kind.silent) tunnel.socket.write(reply); + tunnel.kind.start?.(tunnel); + } +} +server.on("request", (req, res) => res.end("ok")); +server.on("upgrade", onTunnel).on("connect", (req, socket) => { + if (!req.headers.reemit) return onTunnel(req, socket); + socket.write(reply); + server.emit("connection", socket); +}); + +function open(tunnel) { + const { name, kind } = tunnel; + const { head, body = "", first } = tunnel.request; + // Half-open: the peer ends its side only where its kind says so. + const client = connect({ + port: server.address().port, + host: "127.0.0.1", + rejectUnauthorized: false, + allowHalfOpen: true, + }); + // Only the server's side of a tunnel may keep this process alive. + client.unref(); + const lost = event => { + if (!tunnel.socket) results[name] = `the peer got '${event}' before the listener ran`; + }; + client.on("error", () => lost("error")); + client.on("close", () => lost("close")); + const request = () => { + client.write(`${head}Tunnel: ${name}\r\n\r\n${body}`); + if (kind.silent) return client.resume(); + // The reply: the listener has run, so the socket gets the bytes behind it and not the `head` argument. + client.once("data", () => { + if (kind.peerEnds) return client.end(); + if (kind.peerSends) client.write(kind.peerSends); + if (!kind.more) return client.resume(); + let tail = ""; + client.on("data", chunk => { + tail = (tail + chunk.toString("latin1")).slice(-4); + if (tail === "more") kind.more(client); + }); + }); + }; + client.once(secure ? "secureConnect" : "connect", () => { + if (!first) return request(); + client.write(first); + client.once("data", request); + }); +} + +// One connection at a time, so that the 'connection' listener knows whose socket it has. +const opening = [...all]; +const accepted = new Set(); +server.on("connection", socket => { + // A re-emitted tunnel comes here a second time. + if (accepted.has(socket)) return; + accepted.add(socket); + opening.shift().kind.connection?.(socket); + if (opening.length > 0) open(opening[0]); +}); +server.listen(0, "127.0.0.1", () => open(opening[0])); + +process.on("exit", () => console.log(JSON.stringify(results))); diff --git a/test/js/node/http/node-http-connect.test.ts b/test/js/node/http/node-http-connect.test.ts index 9002ce0ac84a..d4ebe9acbe83 100644 --- a/test/js/node/http/node-http-connect.test.ts +++ b/test/js/node/http/node-http-connect.test.ts @@ -2034,6 +2034,44 @@ describe.concurrent.each(["bun", "node"])("%s: what keeps the process alive afte ); }); }); + + // Like those of a libuv handle: unref() takes the reads of a tunnel out of what keeps the + // process alive, ref() puts them back, and bytes left to send keep it alive in both states. + test("socket.unref() and socket.ref() on a tunnel", async () => { + expect(await runTunnelFixture(exe!, "node-http-connect-unref-fixture.js", { PROTO: proto })).toEqual({ + stdout: { + "upgrade, unref() in the listener": atRest, + "connect, unref() in the listener": atRest, + "upgrade with a body, unref() in the listener": atRest, + "upgrade behind a request, unref() in the listener": atRest, + ...(proto === "http" && { "upgrade on a re-emitted tunnel, unref() in the listener": atRest }), + "upgrade, unref() while it reads": atRest, + "connect, unref() while it reads": atRest, + "upgrade with a body, unref() while it reads": atRest, + "upgrade, unref() before the request": atRest, + "upgrade with a body, unref() before the request": atRest, + "upgrade behind a request, unref() before the request": atRest, + "upgrade, unref(), then a write": { events: ["write callback"], received: 0 }, + "upgrade, a write, then unref()": { events: ["write callback"], received: 0 }, + "upgrade, unref(), then end() with bytes": { events: ["finish"], received: 0 }, + "upgrade, ref() after its reads stopped": atRest, + // Node.js closes a TLS socket when its peer ends it. Here it stays half-open, like a TCP socket. + "upgrade, ref() at the end of the stream": { + events: runtime === "node" && proto === "https" ? ["end", "close"] : ["end"], + received: 0, + }, + // It gets what the socket had buffered, and the process does not wait for more. + "upgrade, unref(), then its reads stop, then a reader": { events: [], received: 65536 }, + // The process waits for the rest. + "upgrade, unref(), then its reads stop, then ref() and a reader": { + events: ["end", "close"], + received: 2 * 65536, + }, + "upgrade, a reader, then unref(), then ref()": { events: ["end", "close"], received: 65536 }, + }, + ...exitedByItself, + }); + }); }); // pipe() pauses the client's socket while the upstream is slow, and it does not resume it when @@ -2049,10 +2087,10 @@ describe.concurrent.each(["bun", "node"])("%s: what keeps the process alive afte // TLS still holds bytes when such a socket ends, and usockets sends the FIN behind them with no // writable event for the tunnel. Not on Windows: the fixture makes the send buffer small through libc. -test.concurrent.skipIf(isWindows)( - "https: a read-stopped tunnel that ends with bytes behind a small send buffer does not keep the process alive", - async () => { - const env = { LIBC: libcPathForDlopen() }; +test.concurrent.skipIf(isWindows).each(["unref", "stopped"])( + "https: a tunnel that ends with bytes behind a small send buffer does not keep the process alive (%s)", + async kind => { + const env = { KIND: kind, LIBC: libcPathForDlopen() }; expect(await runTunnelFixture(bunExe(), "node-http-connect-end-small-sndbuf-fixture.js", env)).toEqual({ stdout: { events: ["finish"] }, ...exitedByItself, From fe3f956438246f89fcfb19fcd2654db5d26cc02c Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 21 Oct 2026 23:15:30 +0000 Subject: [PATCH 03/11] node:http: reduce the read-stopped tunnel fix to readStop() and readStart() reporting the idle state --- packages/bun-uws/src/HttpContext.h | 5 +- packages/bun-uws/src/HttpResponse.h | 7 +- src/js/node/_http_server.ts | 3 - .../bindings/node/JSNodeHTTPServerSocket.cpp | 63 +---- .../bindings/node/JSNodeHTTPServerSocket.h | 10 +- .../node/JSNodeHTTPServerSocketPrototype.cpp | 11 - src/runtime/server/mod.rs | 30 +- src/runtime/server/server_body.rs | 2 - ...e-http-connect-end-small-sndbuf-fixture.js | 83 ------ .../node-http-connect-read-stopped-fixture.js | 235 ---------------- .../http/node-http-connect-unref-fixture.js | 259 ------------------ test/js/node/http/node-http-connect.test.ts | 240 ++++++---------- 12 files changed, 107 insertions(+), 841 deletions(-) delete mode 100644 test/js/node/http/node-http-connect-end-small-sndbuf-fixture.js delete mode 100644 test/js/node/http/node-http-connect-read-stopped-fixture.js delete mode 100644 test/js/node/http/node-http-connect-unref-fixture.js diff --git a/packages/bun-uws/src/HttpContext.h b/packages/bun-uws/src/HttpContext.h index b4c4576018e4..83272d7afbf2 100644 --- a/packages/bun-uws/src/HttpContext.h +++ b/packages/bun-uws/src/HttpContext.h @@ -38,7 +38,6 @@ extern "C" void Bun__NodeHTTP__onReadsResumable(int ssl, struct us_socket_t *s); -extern "C" void Bun__NodeHTTP__onTunnelStarted(int ssl, struct us_socket_t *s); extern "C" void Bun__NodeHTTP__onReadParsed(int ssl, struct us_socket_t *s); namespace uWS { @@ -714,8 +713,8 @@ struct HttpContext { } if constexpr (IsNodeHttp) { if (switchToTunnelAfterThisChunk) { - /* The response cannot resume reads in tunnel mode: this lifts the pause the body left. */ - Bun__NodeHTTP__onTunnelStarted(SSL, (struct us_socket_t *) user); + /* The response cannot resume reads in tunnel mode: lift the pause the body left. */ + Bun__NodeHTTP__onReadsResumable(SSL, (struct us_socket_t *) user); if (us_socket_is_closed((struct us_socket_t *) user)) { return nullptr; } diff --git a/packages/bun-uws/src/HttpResponse.h b/packages/bun-uws/src/HttpResponse.h index fe27252f211f..de3a9560654f 100644 --- a/packages/bun-uws/src/HttpResponse.h +++ b/packages/bun-uws/src/HttpResponse.h @@ -184,10 +184,9 @@ struct HttpResponse : public AsyncSocket { return true; } - /* node:http: an idle tunnel has reads that do not hold the event loop (they are stopped, at EOF, or the socket is unref'd) and - * has nothing left to send. Like a libuv stream handle in that state, it does not hold the event loop: - * https://github.com/nodejs/node/blob/v26.3.0/deps/uv/src/unix/stream.c#L1473-L1485. The filter hears -3 when a tunnel - * becomes idle and +3 when its reads hold the event loop again or it has bytes to send again. */ + /* node:http: an idle tunnel does not read (its reads are stopped or at EOF) and has nothing left to send. Like a libuv handle in + * that state, it does not hold the event loop: https://github.com/nodejs/node/blob/v26.3.0/deps/uv/src/unix/stream.c#L1473-L1485. + * The filter hears -3 when a tunnel becomes idle and +3 when it reads or has bytes to send again. */ void setNodeHttpTunnelIdle(bool idle) { HttpResponseData *httpResponseData = getHttpResponseData(); if (!httpResponseData->filteredAccept || httpResponseData->filteredIdleTunnel == idle) { diff --git a/src/js/node/_http_server.ts b/src/js/node/_http_server.ts index 37554b982f02..5eb47033bec8 100644 --- a/src/js/node/_http_server.ts +++ b/src/js/node/_http_server.ts @@ -2114,9 +2114,7 @@ function getNodeHTTPServerSocket() { return (this[kHandle] ?? this.#peer)?.authorizationError ?? null; } - // Like Node's net.Socket, for a tunnel: the handle holds the event loop while it reads and is ref'd. https://github.com/nodejs/node/blob/v26.3.0/lib/net.js#L1621-L1646 ref() { - this[kHandle]?.setRef(true); return this; } @@ -2216,7 +2214,6 @@ function getNodeHTTPServerSocket() { } unref() { - this[kHandle]?.setRef(false); return this; } diff --git a/src/jsc/bindings/node/JSNodeHTTPServerSocket.cpp b/src/jsc/bindings/node/JSNodeHTTPServerSocket.cpp index 39a00af807aa..85033735e7bc 100644 --- a/src/jsc/bindings/node/JSNodeHTTPServerSocket.cpp +++ b/src/jsc/bindings/node/JSNodeHTTPServerSocket.cpp @@ -127,28 +127,6 @@ void JSNodeHTTPServerSocket::reset() template static void onNodeHttpReadsResumable(us_socket_t* socket); -/* The tunnel starts: in the dispatch of a CONNECT or of an Upgrade with no body, behind the body of any other Upgrade (HttpContext). */ -template -static void onNodeHttpTunnelStarted(us_socket_t* socket) -{ - auto* httpResponseData = reinterpret_cast*>(us_socket_ext(socket)); - // socket.unref() came before the tunnel. - if (auto* cell = reinterpret_cast(httpResponseData->socketData); cell && cell->tunnelUnrefed) [[unlikely]] { - cell->updateTunnelIdle(); - } - /* resume() on a response does nothing in tunnel mode: lift what paused reads before it (req.pause(), flood prevention). */ - onNodeHttpReadsResumable(socket); -} - -extern "C" void Bun__NodeHTTP__onTunnelStarted(int ssl, us_socket_t* socket) -{ - if (ssl) { - onNodeHttpTunnelStarted(socket); - } else { - onNodeHttpTunnelStarted(socket); - } -} - template static void upgradeToTunnelModeImpl(us_socket_t* socket, bool afterBody) { @@ -161,7 +139,8 @@ static void upgradeToTunnelModeImpl(us_socket_t* socket, bool afterBody) return; } httpResponseData->isConnectRequest = true; - onNodeHttpTunnelStarted(socket); + /* resume() on a response does nothing in tunnel mode: lift what paused reads before it (req.pause(), flood prevention). */ + onNodeHttpReadsResumable(socket); } void JSNodeHTTPServerSocket::upgradeToTunnelMode(bool afterBody, JSNodeHTTPResponse* response) @@ -219,12 +198,12 @@ void JSNodeHTTPServerSocket::applyTunnelReads() void JSNodeHTTPServerSocket::readStop() { // After the end of the stream no _read() comes to start the reads again. - if (tunnelReadsStopped || tunnelReadEnded || !isTunnel(this)) { + if (!isTunnel(this) || tunnelReadEnded) { return; } tunnelReadsStopped = true; applyTunnelReads(); - refreshTunnelIdle(); + updateTunnelIdle(); } void JSNodeHTTPServerSocket::readStart() @@ -232,18 +211,9 @@ void JSNodeHTTPServerSocket::readStart() if (!isTunnel(this)) { return; } - if (tunnelReadsStopped) [[unlikely]] { - tunnelReadsStopped = false; - // Before applyTunnelReads(): us_socket_resume() closes a socket whose poll it cannot arm again. - refreshTunnelIdle(); - } + tunnelReadsStopped = false; applyTunnelReads(); -} - -void JSNodeHTTPServerSocket::setRef(bool ref) -{ - tunnelUnrefed = !ref; - refreshTunnelIdle(); + updateTunnelIdle(); } void JSNodeHTTPServerSocket::didDeliverQueuedTunnelBytes(size_t length) @@ -411,11 +381,6 @@ bool JSNodeHTTPServerSocket::shutdownAfterResponseDrains(bool destroySoon) JSC::EncodedJSValue JSNodeHTTPServerSocket::halfClose(JSC::JSGlobalObject* globalObject) { - // A FIN that usockets holds behind TLS bytes leaves with no writable event, and that event tells a tunnel that nothing is left to send: onDrain() sends the FIN. - if (is_ssl && isTunnel(this) && !reinterpret_cast*>(socket)->hasFullyDrained()) { - tunnelEndAwaitsDrain = true; - return JSValue::encode(JSC::jsUndefined()); - } // onNodeHTTPRequest no longer pauses at dispatch; pause here so the // shutdown+resume below still cycles kqueue's EVFILT_READ (delete then // re-add), without which macOS 26 does not deliver the peer's close. @@ -919,12 +884,12 @@ extern "C" bool Bun__NodeHTTPServerSocket__writeBehindResponse(us_socket_t* sock return is_ssl ? writeBehindResponse(socket, data, length) : writeBehindResponse(socket, data, length); } -void JSNodeHTTPServerSocket::refreshTunnelIdle() +void JSNodeHTTPServerSocket::updateTunnelIdle() { if (!isTunnel(this)) { return; } - const bool idle = (tunnelReadEnded || tunnelReadsStopped || tunnelUnrefed) && streamBuffer.bufferedSize() == 0; + const bool idle = (tunnelReadEnded || tunnelReadsStopped) && streamBuffer.bufferedSize() == 0; if (is_ssl) { reinterpret_cast*>(socket)->setNodeHttpTunnelIdle(idle && reinterpret_cast*>(socket)->hasFullyDrained()); } else { @@ -932,14 +897,6 @@ void JSNodeHTTPServerSocket::refreshTunnelIdle() } } -void JSNodeHTTPServerSocket::updateTunnelIdle() -{ - // A tunnel whose reads hold the event loop is not idle, and a write or a drain does not change that. - if (tunnelReadEnded || tunnelReadsStopped || tunnelUnrefed) { - refreshTunnelIdle(); - } -} - void JSNodeHTTPServerSocket::onDrain() { // This function can be called during GC! @@ -950,10 +907,6 @@ void JSNodeHTTPServerSocket::onDrain() // A read pause or resume arms the writable event too: nothing was buffered, so nothing drained. if (this->streamBuffer.bufferedSize() == 0 && !heldWriteAwaitsDrain) { - if (tunnelEndAwaitsDrain) [[unlikely]] { - tunnelEndAwaitsDrain = false; - halfClose(globalObject); - } updateTunnelIdle(); return; } diff --git a/src/jsc/bindings/node/JSNodeHTTPServerSocket.h b/src/jsc/bindings/node/JSNodeHTTPServerSocket.h index b4003fb732e2..7bbc3a202818 100644 --- a/src/jsc/bindings/node/JSNodeHTTPServerSocket.h +++ b/src/jsc/bindings/node/JSNodeHTTPServerSocket.h @@ -65,10 +65,6 @@ class JSNodeHTTPServerSocket : public JSC::JSDestructibleObject { unsigned heldWriteAwaitsDrain : 1 = 0; /* Set by onClose() for the peerEnded / closeError getters: the peer's FIN, the error of a failed read. */ unsigned peer_ended : 1 = 0; - /* socket.end() came while TLS held bytes of the tunnel: halfClose() to the onDrain() that sends the FIN. */ - unsigned tunnelEndAwaitsDrain : 1 = 0; - /* socket.unref() to socket.ref(). The reads of an unref'd tunnel do not hold the event loop. */ - unsigned tunnelUnrefed : 1 = 0; int closeReadError = 0; /* Whether the connection closed before the kernel had taken everything, also after an upgrade to a WebSocket. */ std::optional closedWithUnsentData; @@ -154,12 +150,8 @@ class JSNodeHTTPServerSocket : public JSC::JSDestructibleObject { void readStop(); void readStart(); bool tunnelReadsPaused() const { return tunnelReadsStopped || tunnelReadsQueuedFull; } - /* Tells uWS whether this tunnel is idle: its reads do not hold the event loop (readStop(), read EOF, or socket.unref()) and it has nothing left to send. See HttpResponse::setNodeHttpTunnelIdle. */ - void refreshTunnelIdle(); - /* The same for a caller that did not change what the reads hold: a write, a drain, the end of the stream. */ + /* Tells uWS whether this tunnel is idle: it does not read (readStop() or read EOF) and has nothing left to send. See HttpResponse::setNodeHttpTunnelIdle. */ void updateTunnelIdle(); - /* socket.ref() and socket.unref(), like those of a libuv handle: they change whether the reads of a tunnel hold the event loop. Bytes left to send hold it in both states. Before the tunnel starts, the state is only kept for it. */ - void setRef(bool ref); /* uWS still holds bytes of an HTTP response on this connection. A raw write has to go through the same buffer, or it reaches the wire first. */ bool hasUnsentResponseBytes() const; /* Sends the response bytes that are not in the uWS buffer (the zero-copy tail of a res.write(), the cork buffer) to the kernel or into it. A raw write or a FIN then goes out behind them. */ diff --git a/src/jsc/bindings/node/JSNodeHTTPServerSocketPrototype.cpp b/src/jsc/bindings/node/JSNodeHTTPServerSocketPrototype.cpp index 1a0dc5ea9c0d..bf1cb8923600 100644 --- a/src/jsc/bindings/node/JSNodeHTTPServerSocketPrototype.cpp +++ b/src/jsc/bindings/node/JSNodeHTTPServerSocketPrototype.cpp @@ -46,7 +46,6 @@ JSC_DECLARE_HOST_FUNCTION(jsFunctionNodeHTTPServerSocketEnd); JSC_DECLARE_HOST_FUNCTION(jsFunctionNodeHTTPServerSocketUpgradeToTunnel); JSC_DECLARE_HOST_FUNCTION(jsFunctionNodeHTTPServerSocketReadStop); JSC_DECLARE_HOST_FUNCTION(jsFunctionNodeHTTPServerSocketReadStart); -JSC_DECLARE_HOST_FUNCTION(jsFunctionNodeHTTPServerSocketSetRef); JSC_DECLARE_HOST_FUNCTION(jsFunctionNodeHTTPServerSocketSetResponseTrailers); JSC_DECLARE_HOST_FUNCTION(jsFunctionNodeHTTPServerSocketIsRequestTimedOut); JSC_DECLARE_HOST_FUNCTION(jsFunctionNodeHTTPServerSocketStartPipelinedResponse); @@ -92,7 +91,6 @@ static const JSC::HashTableValue JSNodeHTTPServerSocketPrototypeTableValues[] = { "upgradeToTunnel"_s, static_cast(JSC::PropertyAttribute::Function | JSC::PropertyAttribute::DontEnum), JSC::NoIntrinsic, { JSC::HashTableValue::NativeFunctionType, jsFunctionNodeHTTPServerSocketUpgradeToTunnel, 0 } }, { "readStop"_s, static_cast(JSC::PropertyAttribute::Function | JSC::PropertyAttribute::DontEnum), JSC::NoIntrinsic, { JSC::HashTableValue::NativeFunctionType, jsFunctionNodeHTTPServerSocketReadStop, 0 } }, { "readStart"_s, static_cast(JSC::PropertyAttribute::Function | JSC::PropertyAttribute::DontEnum), JSC::NoIntrinsic, { JSC::HashTableValue::NativeFunctionType, jsFunctionNodeHTTPServerSocketReadStart, 0 } }, - { "setRef"_s, static_cast(JSC::PropertyAttribute::Function | JSC::PropertyAttribute::DontEnum), JSC::NoIntrinsic, { JSC::HashTableValue::NativeFunctionType, jsFunctionNodeHTTPServerSocketSetRef, 1 } }, { "setResponseTrailers"_s, static_cast(JSC::PropertyAttribute::Function | JSC::PropertyAttribute::DontEnum), JSC::NoIntrinsic, { JSC::HashTableValue::NativeFunctionType, jsFunctionNodeHTTPServerSocketSetResponseTrailers, 1 } }, { "isRequestTimedOut"_s, static_cast(JSC::PropertyAttribute::Function | JSC::PropertyAttribute::DontEnum), JSC::NoIntrinsic, { JSC::HashTableValue::NativeFunctionType, jsFunctionNodeHTTPServerSocketIsRequestTimedOut, 2 } }, { "startPipelinedResponse"_s, static_cast(JSC::PropertyAttribute::Function | JSC::PropertyAttribute::DontEnum), JSC::NoIntrinsic, { JSC::HashTableValue::NativeFunctionType, jsFunctionNodeHTTPServerSocketStartPipelinedResponse, 3 } }, @@ -172,15 +170,6 @@ JSC_DEFINE_HOST_FUNCTION(jsFunctionNodeHTTPServerSocketReadStart, (JSC::JSGlobal return JSValue::encode(JSC::jsUndefined()); } -// socket.ref() is setRef(true), socket.unref() is setRef(false). -JSC_DEFINE_HOST_FUNCTION(jsFunctionNodeHTTPServerSocketSetRef, (JSC::JSGlobalObject * globalObject, JSC::CallFrame* callFrame)) -{ - if (auto* thisObject = dynamicDowncast(callFrame->thisValue())) [[likely]] { - thisObject->setRef(callFrame->argument(0).isTrue()); - } - return JSValue::encode(JSC::jsUndefined()); -} - // node:http: set the trailer fields (pre-rendered "name: value\r\n" lines) to send // at the end of the current response's chunked body (response.addTrailers()). JSC_DEFINE_HOST_FUNCTION(jsFunctionNodeHTTPServerSocketSetResponseTrailers, (JSC::JSGlobalObject * globalObject, JSC::CallFrame* callFrame)) diff --git a/src/runtime/server/mod.rs b/src/runtime/server/mod.rs index 4c8c6db680a5..e640487c2ed8 100644 --- a/src/runtime/server/mod.rs +++ b/src/runtime/server/mod.rs @@ -267,7 +267,7 @@ pub(crate) struct NewServer { /// ([`NewServer::is_drained`]); for Bun.serve it also holds the /// graceful-stop promise open ([`NewServer::is_closed`]). pub(crate) active_connection_count: core::cell::Cell, - /// The node:http tunnels in `active_connection_count` whose reads do not hold the loop (stopped, at EOF, or unref'd) and that have nothing left to send. + /// The node:http tunnels in `active_connection_count` that do not read (stopped or at EOF) and have nothing left to send. pub(crate) idle_tunnel_count: core::cell::Cell, /// Live `ServerWebSocket` count. Lives on the server (not the websocket /// context) so a reload's context swap cannot reset it, and sits in a @@ -298,9 +298,7 @@ pub(crate) struct NewServer { /// A `.then(cb)` can reach the wrapper, so `deinit_if_we_can` moves the Strong into the task that resolves the promise. pub(crate) all_closed_promise: jsc::JSPromiseStrong, - pub poll_ref: bun_jsc::JsCell, - /// `server.unref()` with no `server.ref()` behind it: the loop ref stays dropped when a tunnel stops being idle. - pub(crate) user_unrefed: core::cell::Cell, + pub poll_ref: KeepAlive, pub(crate) flags: ServerFlags, @@ -508,7 +506,7 @@ impl NewServer { /// uWS filter: `+2` at TCP accept (before any TLS handshake), `-2` on /// `HttpContext::onClose` / `HttpResponse::upgrade()` — see /// `AsyncSocketData::filteredAccept`. Feeds [`Self::active_connection_count`]. - /// `-3` / `+3`: a node:http tunnel becomes idle / is not idle any more (`HttpResponse::setNodeHttpTunnelIdle`). `-4`: it closes idle. + /// `-3` / `+3`: a node:http tunnel becomes idle / reads or has bytes to send again. `-4`: it closes idle. extern "C" fn on_connection_filter( _socket: *mut uws_sys::us_socket_t, opened: i32, @@ -530,8 +528,9 @@ impl NewServer { 3 => { this.note_tunnel_idle(false); // With a listener open, the loop ref is the user's to drop (`server.unref()`). - if !this.has_listener() && !this.user_unrefed.get() { - this.ref_(); + if !this.has_listener() { + // SAFETY: `this` is not used again, and a tunnel write or read start never runs inside `app.close()`. + unsafe { &mut *user_data.cast::() }.ref_(); } return; } @@ -1616,7 +1615,7 @@ impl NewServer { }); } - /// An idle tunnel is open but, like a libuv handle that does not read or is unref'd, with no write pending, does not hold the loop. + /// An idle tunnel is open but, like a libuv handle that does not read and has no write pending, does not hold the loop. fn has_loop_holding_connections(&self) -> bool { self.active_connection_count.get() > self.idle_tunnel_count.get() } @@ -1694,20 +1693,18 @@ impl NewServer { } } - pub(crate) fn ref_(&self) { + pub(crate) fn ref_(&mut self) { // Once `is_closed()`, nothing is left that would ever `unref()` again // (`deinit_if_we_can` already dropped the loop ref), so a ref taken // here would pin the process forever. - if self.poll_ref.get().is_active() || self.is_closed() { + if self.poll_ref.is_active() || self.is_closed() { return; } - let loop_ctx = self.vm.loop_ctx(); - self.poll_ref.with_mut(|poll_ref| poll_ref.ref_(loop_ctx)); + self.poll_ref.ref_(self.vm.loop_ctx()); } - pub(crate) fn unref(&self) { - let loop_ctx = self.vm.loop_ctx(); - self.poll_ref.with_mut(|poll_ref| poll_ref.unref(loop_ctx)); + pub(crate) fn unref(&mut self) { + self.poll_ref.unref(self.vm.loop_ctx()); } pub(crate) fn stop_listening(&mut self, abrupt: bool) { @@ -2214,8 +2211,7 @@ impl NewServer { // ~816 KB mux pool; `listen()` materializes it on demand. mux_request_pool: core::ptr::null_mut(), all_closed_promise: jsc::JSPromiseStrong::default(), - poll_ref: bun_jsc::JsCell::new(KeepAlive::default()), - user_unrefed: core::cell::Cell::new(false), + poll_ref: KeepAlive::default(), flags: ServerFlags::default(), plugins: None, user_routes: Vec::new(), diff --git a/src/runtime/server/server_body.rs b/src/runtime/server/server_body.rs index 2c90ff7388fb..81a6905cdffb 100644 --- a/src/runtime/server/server_body.rs +++ b/src/runtime/server/server_body.rs @@ -2478,7 +2478,6 @@ where callframe: &CallFrame, ) -> JsResult { let this_value = callframe.this(); - self.user_unrefed.set(false); self.ref_(); Ok(this_value) } @@ -2490,7 +2489,6 @@ where callframe: &CallFrame, ) -> JsResult { let this_value = callframe.this(); - self.user_unrefed.set(true); self.unref(); Ok(this_value) } diff --git a/test/js/node/http/node-http-connect-end-small-sndbuf-fixture.js b/test/js/node/http/node-http-connect-end-small-sndbuf-fixture.js deleted file mode 100644 index d1898549e987..000000000000 --- a/test/js/node/http/node-http-connect-end-small-sndbuf-fixture.js +++ /dev/null @@ -1,83 +0,0 @@ -// An https server whose 'upgrade' listener ends its socket with bytes, on a connection whose send -// buffer is small: the kernel takes less than one TLS record at a time, so TLS still holds bytes -// when the socket ends. The reads of the tunnel do not keep the process alive, and its peer stays -// connected. The process prints what the tunnel saw when it exits, and it must exit by itself: it -// has no timer and calls no process.exit(). -// -// KIND=unref|stopped socket.unref(), or reads that stopped for a full buffer -// CERT, KEY, LIBC (the libc to dlopen) -"use strict"; -const { dlopen, ptr } = require("bun:ffi"); -const { KIND, CERT, KEY, LIBC } = process.env; -const libc = dlopen(LIBC, { - getpeername: { args: ["i32", "ptr", "ptr"], returns: "i32" }, - setsockopt: { args: ["i32", "i32", "i32", "ptr", "u32"], returns: "i32" }, -}).symbols; -const darwin = process.platform === "darwin"; -const AF_INET = 2; -const SOL_SOCKET = darwin ? 0xffff : 1; -const SO_SNDBUF = darwin ? 0x1001 : 7; - -// The socket has no descriptor to ask for: it is the one whose peer has this port. -function descriptorOf(socket) { - // struct sockaddr_in: the family (behind a length byte on macOS), then the port in network order. - const address = new Uint8Array(16); - const length = new Uint32Array(1); - const view = new DataView(address.buffer); - for (let fd = 3; fd < 1024; fd++) { - length[0] = address.length; - if (libc.getpeername(fd, ptr(address), ptr(length)) !== 0) continue; - const family = darwin ? address[1] : view.getUint16(0, true); - if (family === AF_INET && view.getUint16(2, false) === socket.remotePort) return fd; - } - throw new Error("found no descriptor for the connection"); -} - -const server = require("node:https").createServer({ cert: CERT, key: KEY }); -const result = { events: [] }; -// Four TLS records. -const bytes = Buffer.alloc(65536, "y"); - -server.on("upgrade", (req, socket) => { - socket.on("error", error => result.events.push(error.code)); - socket.on("end", () => result.events.push("end")); - socket.on("close", () => result.events.push("close")); - const size = new Int32Array([4096]); - if (libc.setsockopt(descriptorOf(socket), SOL_SOCKET, SO_SNDBUF, ptr(size), 4) !== 0) { - throw new Error("setsockopt(SO_SNDBUF) failed"); - } - socket.write("HTTP/1.1 200 OK\r\n\r\n"); - server.close(() => result.events.push("server close")); - const end = () => socket.end(bytes, () => result.events.push("finish")); - if (KIND === "unref") { - socket.unref(); - return end(); - } - (function untilReadsStop() { - if (socket.readableLength < socket.readableHighWaterMark) return setImmediate(untilReadsStop); - end(); - })(); -}); - -server.listen(0, "127.0.0.1", () => { - const client = require("node:tls").connect({ - port: server.address().port, - host: "127.0.0.1", - rejectUnauthorized: false, - allowHalfOpen: true, - }); - // Only the server's side of the tunnel may keep this process alive. - client.unref(); - client.on("error", error => result.events.push(`peer: ${error.code}`)); - client.once("secureConnect", () => { - client.write("GET / HTTP/1.1\r\nHost: a\r\nConnection: Upgrade\r\nUpgrade: raw\r\n\r\n"); - // The reply: the listener has run, so the socket gets the bytes behind it and not the `head` argument. - client.once("data", () => { - // Twice what the socket buffers before it stops its reads. - if (KIND === "stopped") client.write(Buffer.alloc(2 * 65536, "x")); - client.resume(); - }); - }); -}); - -process.on("exit", () => console.log(JSON.stringify(result))); diff --git a/test/js/node/http/node-http-connect-read-stopped-fixture.js b/test/js/node/http/node-http-connect-read-stopped-fixture.js deleted file mode 100644 index 49fa7fb4022d..000000000000 --- a/test/js/node/http/node-http-connect-read-stopped-fixture.js +++ /dev/null @@ -1,235 +0,0 @@ -// A server whose 'upgrade' and 'connect' listeners never read the socket. The socket buffers what -// the peer sends up to its high-water mark and then stops its reads. The process prints what every -// tunnel saw when it exits, and it must exit by itself: it has no timer and calls no process.exit(). -// -// PROTO=http|https (with CERT and KEY) -// CLOSE=before|after server.close() before or after the reads of the tunnels stop -// READERS=1 the tunnels get a 'data' listener and are read to their end -// SERVER=unref|ref server.unref() comes before server.close(), and server.ref() between them -// for "ref". One tunnel gets a 'data' listener when nothing keeps the process alive -"use strict"; -const { PROTO, CLOSE, READERS, SERVER, CERT, KEY } = process.env; -const secure = PROTO === "https"; -const server = secure - ? require("node:https").createServer({ cert: CERT, key: KEY }) - : require("node:http").createServer(); -const connect = secure ? require("node:tls").connect : require("node:net").connect; - -const requests = { - "upgrade": ["GET / HTTP/1.1\r\nHost: a\r\nConnection: Upgrade\r\nUpgrade: raw\r\n", ""], - "connect": ["CONNECT example.com:443 HTTP/1.1\r\nHost: example.com:443\r\n", ""], - // The tunnel starts behind the body. - "upgrade with a body": [ - "POST / HTTP/1.1\r\nHost: a\r\nConnection: Upgrade\r\nUpgrade: raw\r\nContent-Length: 5\r\n", - "hello", - ], -}; -// The 'connect' listener gives the socket back to the server as a connection, and the server -// parses the Upgrade request on it in JavaScript. -const reemitted = "upgrade on a re-emitted tunnel"; -if (!secure) requests[reemitted] = requests.upgrade; -const reply = "HTTP/1.1 200 OK\r\n\r\n"; -// Twice what the socket buffers before it stops its reads. With SERVER, what it buffers and no -// more: no byte is on its way when the reads stop. -const unread = Buffer.alloc((SERVER ? 1 : 2) * 65536, "x"); -// More than a loopback socket takes in one write. -const large = Buffer.alloc(16 * 1024 * 1024, "y"); -// More than the server reads from a socket in one turn of the event loop. -const upload = Buffer.alloc(8 * 1024 * 1024, "z"); - -// `stopped` runs when the reads of the tunnel have stopped. `peer` is what the peer does when it -// has the reply: by default it sends `unread` and reads what comes. -const stoppedKinds = { - // The peer closes its socket behind its bytes. - "the peer leaves": { - stopped(tunnel) { - tunnel.client.once("close", rest); - tunnel.client.destroySoon(); - }, - }, - // The peer is still connected when the process exits. - "the peer stays": {}, - "the listener ended it at once": { - start: tunnel => tunnel.socket.end(reply), - stopped(tunnel) { - tunnel.client.end(); - rest(); - }, - }, - "the listener ends it later": { - stopped(tunnel) { - tunnel.socket.end(); - tunnel.client.end(); - rest(); - }, - }, - // The bytes that are left to send keep the process alive, and then nothing does. - "the listener writes to it later": { - stopped(tunnel) { - tunnel.socket.write(large, () => tunnel.result.events.push("write callback")); - rest(); - }, - }, - // The peer does not read them until the listener has ended the socket. - "the listener ends it later with bytes": { - peer(client) { - client.write(unread); - client.pause(); - }, - stopped(tunnel) { - tunnel.socket.end(large, () => tunnel.result.events.push("finish")); - setImmediate(() => tunnel.client.resume()); - rest(); - }, - }, -}; -// A tunnel that reads keeps the process alive. `more` is what the peer does when it has read "more". -const readerKinds = { - // The reads of this tunnel never stop for the listener. The server pauses them by itself while - // chunks wait for their turn. It starts when the reads of the other tunnels have stopped. - "a reader that writes for every chunk": { - reads: true, - start({ socket, result }) { - socket.write(reply); - socket.on("data", chunk => { - result.received += chunk.length; - socket.write("a"); - if (result.received === upload.length) socket.write("more"); - }); - }, - peer: client => client.write(upload), - more: client => client.end("tail"), - }, - // Its reader comes when nothing keeps the process alive. - "a reader comes later": { - later({ socket, result }) { - socket.on("data", chunk => { - result.received += chunk.length; - // From here on this tunnel has nothing buffered and nothing to send. - if (result.received === unread.length) socket.write("more"); - }); - }, - more: client => client.end(unread), - }, -}; -// With SERVER, one tunnel of the second kind. In Node.js its socket keeps the process alive up to -// its 'close' in both cases. -if (SERVER) delete readerKinds["a reader that writes for every chunk"]; -const kinds = READERS || SERVER ? readerKinds : stoppedKinds; - -const tunnels = {}; -const results = {}; -for (const request of SERVER ? ["upgrade"] : Object.keys(requests)) { - for (const kind in kinds) { - const name = `${request}, ${kind}`; - tunnels[name] = { name, request, kind: kinds[kind], result: { events: [], received: 0 } }; - } -} -const all = Object.values(tunnels); -const readers = all.filter(tunnel => tunnel.kind.reads); -let handedOff = 0; -let atRest = 0; - -function close() { - if (SERVER) server.unref(); - if (SERVER === "ref") server.ref(); - server.close(() => (results.server = "close")); -} - -// A tunnel whose reads have stopped has done what it does. -function rest() { - if (++atRest < all.length - readers.length) return; - if (CLOSE === "after") close(); - for (const tunnel of readers) tunnel.kind.start(tunnel); -} - -// Nothing keeps the process alive here. A reader that is not at its 'close' yet shows that nothing -// did it for that reader: the process exits, and the rows are as they are. -process.once("beforeExit", () => { - if (readers.some(tunnel => !tunnel.result.events.includes("close"))) return; - for (const tunnel of all) tunnel.kind.later?.(tunnel); -}); - -function start(tunnel) { - const { socket, kind } = tunnel; - if (kind.reads) return; - if (kind.start) kind.start(tunnel); - else socket.write(reply); - (function untilReadsStop() { - if (socket.readableLength < socket.readableHighWaterMark) return setImmediate(untilReadsStop); - if (kind.stopped) kind.stopped(tunnel); - else rest(); - })(); -} - -function onTunnel(req, socket) { - const tunnel = tunnels[req.headers.tunnel]; - tunnel.socket = socket; - results[tunnel.name] = tunnel.result; - socket.on("error", error => tunnel.result.events.push(error.code)); - socket.on("end", () => { - tunnel.result.events.push("end"); - socket.end(); - }); - socket.on("close", () => tunnel.result.events.push("close")); - const handOff = () => { - if (CLOSE === "after") return start(tunnel); - if (++handedOff < all.length) return; - close(); - all.forEach(start); - }; - // The tunnel starts behind the body of the request. - if (requests[tunnel.request][1]) req.on("end", handOff).resume(); - else handOff(); -} -server.on("upgrade", onTunnel).on("connect", (req, socket) => { - if (!req.headers.reemit) return onTunnel(req, socket); - socket.write(reply); - server.emit("connection", socket); -}); - -server.listen(0, "127.0.0.1", () => { - for (const tunnel of all) { - const { name, kind } = tunnel; - const [head, body] = requests[tunnel.request]; - // Half-open: the peer ends its side only where its kind says so. - const client = (tunnel.client = connect({ - port: server.address().port, - host: "127.0.0.1", - rejectUnauthorized: false, - allowHalfOpen: true, - })); - // Only the server's side of a tunnel may keep this process alive. - client.unref(); - const lost = event => { - if (!tunnel.socket) results[name] = `the peer got '${event}' before the listener ran`; - }; - client.on("error", () => lost("error")); - client.on("close", () => lost("close")); - const request = () => { - client.write(`${head}Tunnel: ${name}\r\n\r\n${body}`); - // The reply: the listener has run, so the socket gets the bytes behind it and not the `head` argument. - client.once("data", () => { - if (kind.peer) { - kind.peer(client); - } else { - client.write(unread); - client.resume(); - } - if (!kind.more) return; - let tail = ""; - client.on("data", chunk => { - tail = (tail + chunk.toString("latin1")).slice(-4); - if (tail === "more") kind.more(client); - }); - }); - }; - client.once(secure ? "secureConnect" : "connect", () => { - if (tunnel.request !== reemitted) return request(); - client.write("CONNECT example.com:443 HTTP/1.1\r\nHost: example.com:443\r\nReemit: 1\r\n\r\n"); - client.once("data", request); - }); - } -}); - -process.on("exit", () => console.log(JSON.stringify(results))); diff --git a/test/js/node/http/node-http-connect-unref-fixture.js b/test/js/node/http/node-http-connect-unref-fixture.js deleted file mode 100644 index 62f05e44b221..000000000000 --- a/test/js/node/http/node-http-connect-unref-fixture.js +++ /dev/null @@ -1,259 +0,0 @@ -// 'upgrade' and 'connect' listeners that call socket.unref() and socket.ref(). A tunnel keeps the -// process alive while it reads and is ref'd, or while it has bytes left to send. The process prints -// what every tunnel saw when it exits, and it must exit by itself: it has no timer and calls no -// process.exit(). -// -// PROTO=http|https (with CERT and KEY) -"use strict"; -const { PROTO, CERT, KEY } = process.env; -const secure = PROTO === "https"; -const server = secure - ? require("node:https").createServer({ cert: CERT, key: KEY }) - : require("node:http").createServer(); -const connect = secure ? require("node:tls").connect : require("node:net").connect; - -const upgrade = "GET / HTTP/1.1\r\nHost: a\r\nConnection: Upgrade\r\nUpgrade: raw\r\n"; -// `first` is a request that the peer sends before this one, on the same connection. -const requests = { - "upgrade": { head: upgrade }, - "connect": { head: "CONNECT example.com:443 HTTP/1.1\r\nHost: example.com:443\r\n" }, - // The tunnel starts behind the body, and the listener runs before it. - "upgrade with a body": { - head: "POST / HTTP/1.1\r\nHost: a\r\nConnection: Upgrade\r\nUpgrade: raw\r\nContent-Length: 5\r\n", - body: "hello", - }, - "upgrade behind a request": { head: upgrade, first: "GET / HTTP/1.1\r\nHost: a\r\n\r\n" }, -}; -// The 'connect' listener gives the socket back to the server as a connection, and the server -// parses the Upgrade request on it in JavaScript. -if (!secure) { - requests["upgrade on a re-emitted tunnel"] = { - head: upgrade, - first: "CONNECT example.com:443 HTTP/1.1\r\nHost: example.com:443\r\nReemit: 1\r\n\r\n", - }; -} -const reply = "HTTP/1.1 200 OK\r\n\r\n"; -// What the socket buffers before it stops its reads. -const full = Buffer.alloc(65536, "x"); -// More than a loopback socket takes in one write. -const large = Buffer.alloc(16 * 1024 * 1024, "y"); - -// The server closes when the reads of every tunnel that waits for it have stopped. Up to there the -// listening server keeps the process alive, also for the peers of the tunnels that are unref'd. -let waiting = 0; -function whenReadsStop(socket, then) { - waiting++; - (function poll() { - if (socket.readableLength < socket.readableHighWaterMark) return setImmediate(poll); - then?.(); - if (--waiting === 0) server.close(() => (results.server = "close")); - })(); -} - -function count({ socket, result }) { - socket.on("data", chunk => (result.received += chunk.length)); -} - -// Wraps a callback that the process must stay alive for. -let holding = 0; -function held(callback) { - holding++; - return () => { - holding--; - callback?.(); - }; -} - -// `connection` runs in the 'connection' listener and `listener` in the 'upgrade' or 'connect' -// listener. `start` runs when every tunnel has its listener, before the peer has the reply. The -// `later` steps run one at a time, each when nothing keeps the process alive: from there on, only -// its tunnel can. `more` is what the peer does when it has read "more". -const kinds = { - // The tunnel of a request with a body has not started there. - "unref() in the listener": { - requests: Object.keys(requests), - listener: ({ socket }) => socket.unref(), - }, - "unref() while it reads": { - requests: ["upgrade", "connect", "upgrade with a body"], - start: ({ socket }) => socket.unref(), - }, - // The listener writes nothing to this tunnel. - "unref() before the request": { - requests: ["upgrade", "upgrade with a body", "upgrade behind a request"], - silent: true, - connection: socket => socket.unref(), - }, - // The bytes that are left to send keep the process alive. - "unref(), then a write": { - start({ socket, result }) { - socket.unref(); - socket.write( - large, - held(() => result.events.push("write callback")), - ); - }, - }, - "a write, then unref()": { - start({ socket, result }) { - socket.write( - large, - held(() => result.events.push("write callback")), - ); - socket.unref(); - }, - }, - // The end of the stream goes out behind the bytes. - "unref(), then end() with bytes": { - start({ socket, result }) { - socket.unref(); - socket.end( - large, - held(() => result.events.push("finish")), - ); - }, - }, - "ref() after its reads stopped": { - peerSends: full, - start: ({ socket }) => whenReadsStop(socket, () => socket.ref()), - }, - "ref() at the end of the stream": { - peerEnds: true, - start(tunnel) { - tunnel.atEnd = held(() => tunnel.socket.ref()); - tunnel.socket.resume(); - }, - end: tunnel => tunnel.atEnd(), - }, - "unref(), then its reads stop, then a reader": { - peerSends: full, - start(tunnel) { - tunnel.socket.unref(); - whenReadsStop(tunnel.socket, () => count(tunnel)); - }, - }, - "unref(), then its reads stop, then ref() and a reader": { - peerSends: full, - start({ socket }) { - socket.unref(); - whenReadsStop(socket); - }, - later({ socket, result }) { - socket.ref(); - socket.on("data", chunk => { - result.received += chunk.length; - if (result.received === full.length) socket.write("more"); - }); - }, - more: client => client.end(full), - }, - "a reader, then unref(), then ref()": { - start(tunnel) { - count(tunnel); - tunnel.socket.unref(); - }, - later({ socket }) { - socket.ref(); - socket.write("more"); - }, - more: client => client.end(full), - }, -}; -const all = []; -for (const [name, kind] of Object.entries(kinds)) { - for (const request of kind.requests ?? ["upgrade"]) { - all.push({ name: `${request}, ${name}`, request: requests[request], kind, result: { events: [], received: 0 } }); - } -} -const tunnels = Object.fromEntries(all.map(tunnel => [tunnel.name, tunnel])); -const results = {}; -const later = all.filter(tunnel => tunnel.kind.later); -let handedOff = 0; - -// Nothing keeps the process alive here. When something is not done that the process must stay -// alive for, it exits, and the rows are as they are. -process.on("beforeExit", () => { - if (holding > 0 || later.length === 0) return; - const tunnel = later.shift(); - tunnel.socket.once("close", held()); - tunnel.kind.later(tunnel); -}); - -function onTunnel(req, socket) { - const tunnel = tunnels[req.headers.tunnel]; - const { kind, result } = tunnel; - tunnel.socket = socket; - results[tunnel.name] = result; - socket.on("error", error => result.events.push(error.code)); - socket.on("end", () => { - result.events.push("end"); - if (kind.end) kind.end(tunnel); - else socket.end(); - }); - socket.on("close", () => result.events.push("close")); - kind.listener?.(tunnel); - if (++handedOff < all.length) return; - for (const tunnel of all) { - if (!tunnel.kind.silent) tunnel.socket.write(reply); - tunnel.kind.start?.(tunnel); - } -} -server.on("request", (req, res) => res.end("ok")); -server.on("upgrade", onTunnel).on("connect", (req, socket) => { - if (!req.headers.reemit) return onTunnel(req, socket); - socket.write(reply); - server.emit("connection", socket); -}); - -function open(tunnel) { - const { name, kind } = tunnel; - const { head, body = "", first } = tunnel.request; - // Half-open: the peer ends its side only where its kind says so. - const client = connect({ - port: server.address().port, - host: "127.0.0.1", - rejectUnauthorized: false, - allowHalfOpen: true, - }); - // Only the server's side of a tunnel may keep this process alive. - client.unref(); - const lost = event => { - if (!tunnel.socket) results[name] = `the peer got '${event}' before the listener ran`; - }; - client.on("error", () => lost("error")); - client.on("close", () => lost("close")); - const request = () => { - client.write(`${head}Tunnel: ${name}\r\n\r\n${body}`); - if (kind.silent) return client.resume(); - // The reply: the listener has run, so the socket gets the bytes behind it and not the `head` argument. - client.once("data", () => { - if (kind.peerEnds) return client.end(); - if (kind.peerSends) client.write(kind.peerSends); - if (!kind.more) return client.resume(); - let tail = ""; - client.on("data", chunk => { - tail = (tail + chunk.toString("latin1")).slice(-4); - if (tail === "more") kind.more(client); - }); - }); - }; - client.once(secure ? "secureConnect" : "connect", () => { - if (!first) return request(); - client.write(first); - client.once("data", request); - }); -} - -// One connection at a time, so that the 'connection' listener knows whose socket it has. -const opening = [...all]; -const accepted = new Set(); -server.on("connection", socket => { - // A re-emitted tunnel comes here a second time. - if (accepted.has(socket)) return; - accepted.add(socket); - opening.shift().kind.connection?.(socket); - if (opening.length > 0) open(opening[0]); -}); -server.listen(0, "127.0.0.1", () => open(opening[0])); - -process.on("exit", () => console.log(JSON.stringify(results))); diff --git a/test/js/node/http/node-http-connect.test.ts b/test/js/node/http/node-http-connect.test.ts index 2999157b0b52..3baff409132f 100644 --- a/test/js/node/http/node-http-connect.test.ts +++ b/test/js/node/http/node-http-connect.test.ts @@ -1,15 +1,5 @@ import { describe, expect, test } from "bun:test"; -import { - bunEnv, - bunExe, - bunRun, - isLinux, - isWindows, - libcPathForDlopen, - nodeExe, - tempDir, - tls as tlsCert, -} from "harness"; +import { bunEnv, bunExe, bunRun, isLinux, isWindows, nodeExe, tempDir, tls as tlsCert } from "harness"; import http from "http"; import { once } from "node:events"; @@ -1934,170 +1924,100 @@ test("a half-open tunnel with bytes left to send keeps the process alive until t } }); -async function runTunnelFixture(exe: string, fixture: string, env: Record = {}) { +// A socket that nothing reads stops the reads of its handle when its buffer is full. In Node.js a +// handle that does not read and has nothing to send does not keep the process alive. +async function runToExit(...args: string[]) { await using proc = Bun.spawn({ - cmd: [exe, join(import.meta.dir, fixture)], - env: { - ...bunEnv, - CERT: tlsCert.cert, - KEY: tlsCert.key, - // bun test does not kill the child of a concurrent test that times out. With this, a - // child that hangs exits when the test process does. - BUN_FEATURE_FLAG_NO_ORPHANS: "1", - ...env, - }, + cmd: [bunExe(), ...args], + // A child that hangs exits with the test process. + env: { ...bunEnv, BUN_FEATURE_FLAG_NO_ORPHANS: "1" }, stdout: "pipe", stderr: "pipe", }); const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); - return { stdout: stdout && JSON.parse(stdout), stderr, exitCode, signalCode: proc.signalCode }; + return { stdout, stderr, exitCode, signalCode: proc.signalCode }; } -const exitedByItself = { stderr: "", exitCode: 0, signalCode: null }; - -// The fixtures below run under Node.js as they are, and "node" runs them there: every expectation -// is Node v26.3.0's unless it says otherwise. Their peers send the bytes of a tunnel when they have -// the listener's reply, so the socket gets them and not the `head` argument. -describe.concurrent.each(["bun", "node"])("%s: what keeps the process alive after server.close()", runtime => { - const exe = runtime === "node" ? nodeExe() : bunExe(); - const atRest = { events: [], received: 0 }; - - describe.skipIf(!exe).each(["http", "https"])("%s", proto => { - // The server parses the request on a re-emitted tunnel in JavaScript. - const requests = ["upgrade", "connect", "upgrade with a body"]; - if (proto === "http") requests.push("upgrade on a re-emitted tunnel"); - - // In Node.js a socket stops the reads of its handle when its buffer is full, and a handle that - // does not read and has nothing to send is not active: it does not keep the process alive. - describe.each(["before", "after"])("server.close() %s the reads of the unread tunnels stop", close => { - const env = { PROTO: proto, CLOSE: close }; - - test("a tunnel that nothing reads does not keep the process alive", async () => { - // The sockets are still open, so neither they nor the server emit 'close'. - expect(await runTunnelFixture(exe!, "node-http-connect-read-stopped-fixture.js", env)).toEqual({ - stdout: Object.fromEntries( - requests.flatMap(request => [ - [`${request}, the peer leaves`, atRest], - [`${request}, the peer stays`, atRest], - [`${request}, the listener ended it at once`, atRest], - [`${request}, the listener ends it later`, atRest], - [`${request}, the listener writes to it later`, { events: ["write callback"], received: 0 }], - [`${request}, the listener ends it later with bytes`, { events: ["finish"], received: 0 }], - ]), - ), - ...exitedByItself, - }); - }); - test("a tunnel that reads keeps the process alive up to its 'close'", async () => { - const readers = { ...env, READERS: "1" }; - expect(await runTunnelFixture(exe!, "node-http-connect-read-stopped-fixture.js", readers)).toEqual({ - stdout: { - ...Object.fromEntries( - requests.flatMap(request => [ - // The upload and "tail". - [ - `${request}, a reader that writes for every chunk`, - { events: ["end", "close"], received: 8 * 1024 * 1024 + 4 }, - ], - [`${request}, a reader comes later`, { events: ["end", "close"], received: 4 * 65536 }], - ]), - ), - server: "close", - }, - ...exitedByItself, - }); - }); - }); - - describe("a tunnel that reads again behind server.unref()", () => { - const row = "upgrade, a reader comes later"; - const toItsClose = { - stdout: { [row]: { events: ["end", "close"], received: 2 * 65536 }, server: "close" }, - ...exitedByItself, - }; - const run = (server: string) => - runTunnelFixture(exe!, "node-http-connect-read-stopped-fixture.js", { - PROTO: proto, - CLOSE: "after", - SERVER: server, - }); - - test("keeps the process alive when server.ref() came behind it", async () => { - expect(await run("ref")).toEqual(toItsClose); - }); +test.concurrent("a CONNECT proxy by pipes exits after server.close() when its upstream went away", async () => { + // The client's socket is still open, so the proxy does not emit 'close'. + expect(await runToExit(join(import.meta.dir, "node-http-connect-proxy-pipes-fixture.js"))).toEqual({ + stdout: '{"clientSocketIsPaused":true}\n', + stderr: "", + exitCode: 0, + signalCode: null, + }); +}); - // In Node.js a socket keeps the process alive by itself. Here the server does it for its - // sockets, and server.unref() ends that: the reader gets only what the socket had buffered. - test("keeps the process alive only in Node.js", async () => { - expect(await run("unref")).toEqual( - runtime === "node" ? toItsClose : { stdout: { [row]: { events: [], received: 65536 } }, ...exitedByItself }, - ); - }); - }); +describe.concurrent.each([ + ["http", "connect", "CONNECT example.com:443 HTTP/1.1\r\nHost: example.com:443\r\n\r\n"], + ["https", "connect", "CONNECT example.com:443 HTTP/1.1\r\nHost: example.com:443\r\n\r\n"], + ["http", "upgrade", "GET / HTTP/1.1\r\nHost: a\r\nConnection: Upgrade\r\nUpgrade: raw\r\n\r\n"], + ["https", "upgrade", "GET / HTTP/1.1\r\nHost: a\r\nConnection: Upgrade\r\nUpgrade: raw\r\n\r\n"], +])("%s '%s': a tunnel whose reads stopped, after server.close()", (proto, event, request) => { + // `listener` is the body of the listener and `onPeerData` gets the chunks of the peer behind the reply. + const run = (listener: string, onPeerData = "") => + runToExit( + "-e", + `const server = require("node:${proto}").createServer(${JSON.stringify(proto === "https" ? tlsCert : {})}); + const reply = "HTTP/1.1 200 OK\\r\\n\\r\\n"; + const result = {}; + server.on("${event}", (req, socket) => { + server.close(); + ${listener} + }); + server.listen(0, "127.0.0.1", () => { + const client = require("node:${proto === "https" ? "tls" : "net"}").connect({ + port: server.address().port, + host: "127.0.0.1", + rejectUnauthorized: false, + allowHalfOpen: true, + }); + // Only the server's side of the tunnel may keep the process alive. + client.unref(); + client.write(${JSON.stringify(request)}); + client.once("data", () => { + // What the socket buffers before it stops its reads. + client.write(Buffer.alloc(65536, "a")); + client.on("data", chunk => { + ${onPeerData} + }); + }); + }); + process.on("exit", () => console.log(JSON.stringify(result)));`, + ); - // Like those of a libuv handle: unref() takes the reads of a tunnel out of what keeps the - // process alive, ref() puts them back, and bytes left to send keep it alive in both states. - test("socket.unref() and socket.ref() on a tunnel", async () => { - expect(await runTunnelFixture(exe!, "node-http-connect-unref-fixture.js", { PROTO: proto })).toEqual({ - stdout: { - "upgrade, unref() in the listener": atRest, - "connect, unref() in the listener": atRest, - "upgrade with a body, unref() in the listener": atRest, - "upgrade behind a request, unref() in the listener": atRest, - ...(proto === "http" && { "upgrade on a re-emitted tunnel, unref() in the listener": atRest }), - "upgrade, unref() while it reads": atRest, - "connect, unref() while it reads": atRest, - "upgrade with a body, unref() while it reads": atRest, - "upgrade, unref() before the request": atRest, - "upgrade with a body, unref() before the request": atRest, - "upgrade behind a request, unref() before the request": atRest, - "upgrade, unref(), then a write": { events: ["write callback"], received: 0 }, - "upgrade, a write, then unref()": { events: ["write callback"], received: 0 }, - "upgrade, unref(), then end() with bytes": { events: ["finish"], received: 0 }, - "upgrade, ref() after its reads stopped": atRest, - // Node.js closes a TLS socket when its peer ends it. Here it stays half-open, like a TCP socket. - "upgrade, ref() at the end of the stream": { - events: runtime === "node" && proto === "https" ? ["end", "close"] : ["end"], - received: 0, - }, - // It gets what the socket had buffered, and the process does not wait for more. - "upgrade, unref(), then its reads stop, then a reader": { events: [], received: 65536 }, - // The process waits for the rest. - "upgrade, unref(), then its reads stop, then ref() and a reader": { - events: ["end", "close"], - received: 2 * 65536, - }, - "upgrade, a reader, then unref(), then ref()": { events: ["end", "close"], received: 65536 }, - }, - ...exitedByItself, - }); - }); + test("does not keep the process alive behind the listener's end()", async () => { + // No writable event comes for a socket behind its FIN. + expect( + await run(`socket.end(reply); + process.on("beforeExit", () => (result.buffered = socket.readableLength));`), + ).toEqual({ stdout: '{"buffered":65536}\n', stderr: "", exitCode: 0, signalCode: null }); }); - // pipe() pauses the client's socket while the upstream is slow, and it does not resume it when - // the upstream goes away. - test.skipIf(!exe)("a CONNECT proxy by pipes exits when its upstream went away", async () => { - // The client's socket is still open, so the proxy does not emit 'close'. - expect(await runTunnelFixture(exe!, "node-http-connect-proxy-pipes-fixture.js")).toEqual({ - stdout: { clientSocketIsPaused: true }, - ...exitedByItself, + test("keeps the process alive again while it sends and when it reads", async () => { + // More than a loopback socket takes in one write. + const size = 8 * 1024 * 1024; + expect( + await run( + `socket.write(reply); + Object.assign(result, { sent: false, serverReceived: 0, clientReceived: 0 }); + // 'beforeExit': nothing keeps the process alive. It has to wait for what each step starts. + const steps = [ + () => socket.write(Buffer.alloc(${size}, "x"), () => (result.sent = true)), + () => socket.on("data", chunk => (result.serverReceived += chunk.length)).on("end", () => socket.end()), + ]; + process.on("beforeExit", () => steps.shift()?.());`, + `if ((result.clientReceived += chunk.length) === ${size}) client.end(Buffer.alloc(${size}, "b"));`, + ), + ).toEqual({ + stdout: JSON.stringify({ sent: true, serverReceived: 65536 + size, clientReceived: size }) + "\n", + stderr: "", + exitCode: 0, + signalCode: null, }); }); }); -// TLS still holds bytes when such a socket ends, and usockets sends the FIN behind them with no -// writable event for the tunnel. Not on Windows: the fixture makes the send buffer small through libc. -test.concurrent.skipIf(isWindows).each(["unref", "stopped"])( - "https: a tunnel that ends with bytes behind a small send buffer does not keep the process alive (%s)", - async kind => { - const env = { KIND: kind, LIBC: libcPathForDlopen() }; - expect(await runTunnelFixture(bunExe(), "node-http-connect-end-small-sndbuf-fixture.js", env)).toEqual({ - stdout: { events: ["finish"] }, - ...exitedByItself, - }); - }, -); - // A listener with nothing to send ends its side in the listener. It still reads: an http.Server // socket has allowHalfOpen. Every expectation below is Node v26.3.0's. describe("a tunnel behind a request head that took more than one read", () => { From d74e586ff83747178d4fbb157fa40835d1e99e30 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 22 Oct 2026 00:15:03 +0000 Subject: [PATCH 04/11] node:http: a tunnel that reads again does not take the event loop back behind server.unref() --- packages/bun-uws/src/AsyncSocketData.h | 2 +- packages/bun-uws/src/HttpResponse.h | 7 +- .../bindings/node/JSNodeHTTPServerSocket.cpp | 4 +- src/runtime/server/mod.rs | 13 +- src/runtime/server/server_body.rs | 2 + test/js/node/http/node-http-connect.test.ts | 187 ++++++++++++------ 6 files changed, 140 insertions(+), 75 deletions(-) diff --git a/packages/bun-uws/src/AsyncSocketData.h b/packages/bun-uws/src/AsyncSocketData.h index 9df9166ff664..a0e5be49022f 100644 --- a/packages/bun-uws/src/AsyncSocketData.h +++ b/packages/bun-uws/src/AsyncSocketData.h @@ -177,7 +177,7 @@ struct AsyncSocketData { * that can still reach a handler counts these. */ bool filteredAccept = false; /* Whether it has fired the filter with -3 (HttpResponse::setNodeHttpTunnelIdle). - * +3 undoes it, and an idle socket fires -4 in place of -2. */ + * +3 or +4 undoes it, and an idle socket fires -4 in place of -2. */ bool filteredIdleTunnel = false; }; diff --git a/packages/bun-uws/src/HttpResponse.h b/packages/bun-uws/src/HttpResponse.h index de3a9560654f..a22e54789aab 100644 --- a/packages/bun-uws/src/HttpResponse.h +++ b/packages/bun-uws/src/HttpResponse.h @@ -186,15 +186,16 @@ struct HttpResponse : public AsyncSocket { /* node:http: an idle tunnel does not read (its reads are stopped or at EOF) and has nothing left to send. Like a libuv handle in * that state, it does not hold the event loop: https://github.com/nodejs/node/blob/v26.3.0/deps/uv/src/unix/stream.c#L1473-L1485. - * The filter hears -3 when a tunnel becomes idle and +3 when it reads or has bytes to send again. */ - void setNodeHttpTunnelIdle(bool idle) { + * The filter hears -3 when a tunnel becomes idle. When it is not idle any more, the filter hears +3 at read EOF (it has bytes to + * send) and +4 otherwise (it reads or has bytes to send). */ + void setNodeHttpTunnelIdle(bool idle, bool readEnded) { HttpResponseData *httpResponseData = getHttpResponseData(); if (!httpResponseData->filteredAccept || httpResponseData->filteredIdleTunnel == idle) { return; } httpResponseData->filteredIdleTunnel = idle; for (auto &f : HttpContext::getSocketContextDataS((us_socket_t *) this)->filterHandlers) { - f(this, idle ? -3 : 3); + f(this, idle ? -3 : readEnded ? 3 : 4); } } diff --git a/src/jsc/bindings/node/JSNodeHTTPServerSocket.cpp b/src/jsc/bindings/node/JSNodeHTTPServerSocket.cpp index 85033735e7bc..b553aa47abb3 100644 --- a/src/jsc/bindings/node/JSNodeHTTPServerSocket.cpp +++ b/src/jsc/bindings/node/JSNodeHTTPServerSocket.cpp @@ -891,9 +891,9 @@ void JSNodeHTTPServerSocket::updateTunnelIdle() } const bool idle = (tunnelReadEnded || tunnelReadsStopped) && streamBuffer.bufferedSize() == 0; if (is_ssl) { - reinterpret_cast*>(socket)->setNodeHttpTunnelIdle(idle && reinterpret_cast*>(socket)->hasFullyDrained()); + reinterpret_cast*>(socket)->setNodeHttpTunnelIdle(idle && reinterpret_cast*>(socket)->hasFullyDrained(), tunnelReadEnded); } else { - reinterpret_cast*>(socket)->setNodeHttpTunnelIdle(idle && reinterpret_cast*>(socket)->hasFullyDrained()); + reinterpret_cast*>(socket)->setNodeHttpTunnelIdle(idle && reinterpret_cast*>(socket)->hasFullyDrained(), tunnelReadEnded); } } diff --git a/src/runtime/server/mod.rs b/src/runtime/server/mod.rs index e640487c2ed8..aab2f96c32b2 100644 --- a/src/runtime/server/mod.rs +++ b/src/runtime/server/mod.rs @@ -204,6 +204,8 @@ bitflags::bitflags! { pub struct ServerFlags: u8 { const DEINIT_SCHEDULED = 1 << 0; const TERMINATED = 1 << 1; + /// `server.unref()` with no `server.ref()` behind it. + const UNREFED_BY_USER = 1 << 2; } } @@ -506,7 +508,7 @@ impl NewServer { /// uWS filter: `+2` at TCP accept (before any TLS handshake), `-2` on /// `HttpContext::onClose` / `HttpResponse::upgrade()` — see /// `AsyncSocketData::filteredAccept`. Feeds [`Self::active_connection_count`]. - /// `-3` / `+3`: a node:http tunnel becomes idle / reads or has bytes to send again. `-4`: it closes idle. + /// `-3`: a node:http tunnel becomes idle. `+3` / `+4`: it is not idle any more, at read EOF / not at it. `-4`: it closes idle. extern "C" fn on_connection_filter( _socket: *mut uws_sys::us_socket_t, opened: i32, @@ -525,10 +527,13 @@ impl NewServer { this.note_connection_opened(); return; } - 3 => { + 3 | 4 => { this.note_tunnel_idle(false); // With a listener open, the loop ref is the user's to drop (`server.unref()`). - if !this.has_listener() { + // Behind `server.unref()` reads do not hold the loop, and only at read EOF is it sure that the tunnel has bytes to send. + if !this.has_listener() + && (opened == 3 || !this.flags.contains(ServerFlags::UNREFED_BY_USER)) + { // SAFETY: `this` is not used again, and a tunnel write or read start never runs inside `app.close()`. unsafe { &mut *user_data.cast::() }.ref_(); } @@ -1606,7 +1611,7 @@ impl NewServer { fn note_tunnel_idle(&self, idle: bool) { let count = self.idle_tunnel_count.get(); - // `AsyncSocketData::filteredIdleTunnel` pairs every -3 with one +3 or -4. + // `AsyncSocketData::filteredIdleTunnel` pairs every -3 with one +3, +4 or -4. debug_assert!(idle || count > 0); self.idle_tunnel_count.set(if idle { count + 1 diff --git a/src/runtime/server/server_body.rs b/src/runtime/server/server_body.rs index 81a6905cdffb..b94e3e7fd31c 100644 --- a/src/runtime/server/server_body.rs +++ b/src/runtime/server/server_body.rs @@ -2478,6 +2478,7 @@ where callframe: &CallFrame, ) -> JsResult { let this_value = callframe.this(); + self.flags.remove(ServerFlags::UNREFED_BY_USER); self.ref_(); Ok(this_value) } @@ -2489,6 +2490,7 @@ where callframe: &CallFrame, ) -> JsResult { let this_value = callframe.this(); + self.flags.insert(ServerFlags::UNREFED_BY_USER); self.unref(); Ok(this_value) } diff --git a/test/js/node/http/node-http-connect.test.ts b/test/js/node/http/node-http-connect.test.ts index 3baff409132f..7a323fa52b13 100644 --- a/test/js/node/http/node-http-connect.test.ts +++ b/test/js/node/http/node-http-connect.test.ts @@ -1937,84 +1937,141 @@ async function runToExit(...args: string[]) { const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); return { stdout, stderr, exitCode, signalCode: proc.signalCode }; } +const exitedWith = (result: object) => ({ + stdout: JSON.stringify(result) + "\n", + stderr: "", + exitCode: 0, + signalCode: null, +}); test.concurrent("a CONNECT proxy by pipes exits after server.close() when its upstream went away", async () => { // The client's socket is still open, so the proxy does not emit 'close'. - expect(await runToExit(join(import.meta.dir, "node-http-connect-proxy-pipes-fixture.js"))).toEqual({ - stdout: '{"clientSocketIsPaused":true}\n', - stderr: "", - exitCode: 0, - signalCode: null, - }); + expect(await runToExit(join(import.meta.dir, "node-http-connect-proxy-pipes-fixture.js"))).toEqual( + exitedWith({ clientSocketIsPaused: true }), + ); }); -describe.concurrent.each([ - ["http", "connect", "CONNECT example.com:443 HTTP/1.1\r\nHost: example.com:443\r\n\r\n"], - ["https", "connect", "CONNECT example.com:443 HTTP/1.1\r\nHost: example.com:443\r\n\r\n"], - ["http", "upgrade", "GET / HTTP/1.1\r\nHost: a\r\nConnection: Upgrade\r\nUpgrade: raw\r\n\r\n"], - ["https", "upgrade", "GET / HTTP/1.1\r\nHost: a\r\nConnection: Upgrade\r\nUpgrade: raw\r\n\r\n"], -])("%s '%s': a tunnel whose reads stopped, after server.close()", (proto, event, request) => { - // `listener` is the body of the listener and `onPeerData` gets the chunks of the peer behind the reply. - const run = (listener: string, onPeerData = "") => - runToExit( - "-e", - `const server = require("node:${proto}").createServer(${JSON.stringify(proto === "https" ? tlsCert : {})}); - const reply = "HTTP/1.1 200 OK\\r\\n\\r\\n"; - const result = {}; - server.on("${event}", (req, socket) => { - server.close(); - ${listener} +// `listener` is the body of the listener for `event`. `peer` runs when `client` has the reply. The process prints `result`. +function runTunnel(proto: string, event: string, listener: string, peer = "client.write(full);") { + const request = + event === "connect" + ? "CONNECT example.com:443 HTTP/1.1\r\nHost: example.com:443\r\n\r\n" + : "GET / HTTP/1.1\r\nHost: a\r\nConnection: Upgrade\r\nUpgrade: raw\r\n\r\n"; + return runToExit( + "-e", + `const server = require("node:${proto}").createServer(${JSON.stringify(proto === "https" ? tlsCert : {})}); + const reply = "HTTP/1.1 200 OK\\r\\n\\r\\n"; + // What a socket buffers before it stops its reads. + const full = Buffer.alloc(65536, "a"); + const result = {}; + let client; + server.on("${event}", (req, socket) => { + ${listener} + }); + server.listen(0, "127.0.0.1", () => { + client = require("node:${proto === "https" ? "tls" : "net"}").connect({ + port: server.address().port, + host: "127.0.0.1", + rejectUnauthorized: false, + allowHalfOpen: true, }); - server.listen(0, "127.0.0.1", () => { - const client = require("node:${proto === "https" ? "tls" : "net"}").connect({ - port: server.address().port, - host: "127.0.0.1", - rejectUnauthorized: false, - allowHalfOpen: true, - }); - // Only the server's side of the tunnel may keep the process alive. - client.unref(); - client.write(${JSON.stringify(request)}); - client.once("data", () => { - // What the socket buffers before it stops its reads. - client.write(Buffer.alloc(65536, "a")); - client.on("data", chunk => { - ${onPeerData} - }); - }); + // Only the server's side of the tunnel may keep the process alive. + client.unref(); + client.write(${JSON.stringify(request)}); + client.once("data", () => { + ${peer} }); - process.on("exit", () => console.log(JSON.stringify(result)));`, - ); + }); + process.on("exit", () => console.log(JSON.stringify(result)));`, + ); +} +// More than a loopback socket takes in one write. +const large = 8 * 1024 * 1024; +describe.concurrent.each([ + ["http", "connect"], + ["https", "connect"], + ["http", "upgrade"], + ["https", "upgrade"], +])("%s '%s': a tunnel whose reads stopped, after server.close()", (proto, event) => { test("does not keep the process alive behind the listener's end()", async () => { // No writable event comes for a socket behind its FIN. - expect( - await run(`socket.end(reply); - process.on("beforeExit", () => (result.buffered = socket.readableLength));`), - ).toEqual({ stdout: '{"buffered":65536}\n', stderr: "", exitCode: 0, signalCode: null }); + const listener = ` + server.close(); + socket.end(reply); + process.on("beforeExit", () => (result.buffered = socket.readableLength));`; + expect(await runTunnel(proto, event, listener)).toEqual(exitedWith({ buffered: 65536 })); }); test("keeps the process alive again while it sends and when it reads", async () => { - // More than a loopback socket takes in one write. - const size = 8 * 1024 * 1024; - expect( - await run( - `socket.write(reply); - Object.assign(result, { sent: false, serverReceived: 0, clientReceived: 0 }); - // 'beforeExit': nothing keeps the process alive. It has to wait for what each step starts. - const steps = [ - () => socket.write(Buffer.alloc(${size}, "x"), () => (result.sent = true)), - () => socket.on("data", chunk => (result.serverReceived += chunk.length)).on("end", () => socket.end()), - ]; - process.on("beforeExit", () => steps.shift()?.());`, - `if ((result.clientReceived += chunk.length) === ${size}) client.end(Buffer.alloc(${size}, "b"));`, - ), - ).toEqual({ - stdout: JSON.stringify({ sent: true, serverReceived: 65536 + size, clientReceived: size }) + "\n", - stderr: "", - exitCode: 0, - signalCode: null, - }); + const listener = ` + server.close(); + socket.write(reply); + Object.assign(result, { sent: false, serverReceived: 0, clientReceived: 0 }); + // 'beforeExit': nothing keeps the process alive. It has to wait for what each step starts. + const steps = [ + () => socket.write(Buffer.alloc(${large}, "x"), () => (result.sent = true)), + () => socket.on("data", chunk => (result.serverReceived += chunk.length)).on("end", () => socket.end()), + ]; + process.on("beforeExit", () => steps.shift()?.());`; + const peer = ` + client.write(full); + client.on("data", chunk => { + if ((result.clientReceived += chunk.length) === ${large}) client.end(Buffer.alloc(${large}, "b")); + });`; + expect(await runTunnel(proto, event, listener, peer)).toEqual( + exitedWith({ sent: true, serverReceived: 65536 + large, clientReceived: large }), + ); + }); +}); + +// The server holds the event loop for its sockets, so server.unref() gives it up for their reads too. +// In Node.js a socket holds it by itself: there the process of the first test does not exit. +describe.concurrent.each(["http", "https"])("%s: a tunnel behind server.unref() and server.close()", proto => { + // `hold` keeps the process alive until the reads have stopped. Then the tunnel gets a reader. + const readsAgain = (ref: string, whenRead: string) => ` + socket.write(reply); + server.unref(); + ${ref} + server.close(); + const hold = setInterval(() => { + if (socket.readableLength === full.length) clearInterval(hold); + }, 1); + result.received = 0; + process.once("beforeExit", () => { + socket.on("data", chunk => { + if ((result.received += chunk.length) === full.length) { + ${whenRead} + } + }); + socket.on("end", () => { + result.end = true; + socket.end(); + }); + });`; + + test("does not keep the process alive when it reads again", async () => { + // The peer stays connected and sends nothing more. + expect(await runTunnel(proto, "connect", readsAgain("", ""))).toEqual(exitedWith({ received: 65536 })); + }); + + test("keeps the process alive when it reads again behind server.ref()", async () => { + expect(await runTunnel(proto, "connect", readsAgain("server.ref();", "client.end();"))).toEqual( + exitedWith({ received: 65536, end: true }), + ); + }); + + test("keeps the process alive while it sends at the end of the stream", async () => { + const listener = ` + socket.write(reply); + server.unref(); + server.close(); + const hold = setInterval(() => {}, 1000); + socket.on("end", () => clearInterval(hold)).resume(); + process.once("beforeExit", () => socket.end(Buffer.alloc(${large}, "x"), () => (result.sent = true)));`; + expect(await runTunnel(proto, "connect", listener, "client.end(); client.resume();")).toEqual( + exitedWith({ sent: true }), + ); }); }); From c43473943d16e37053b2c67a01e6caf720534fda Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 22 Oct 2026 00:35:26 +0000 Subject: [PATCH 05/11] node:http: the read-stopped tunnel tests do not depend on how much a loopback socket takes in one write --- test/js/node/http/node-http-connect.test.ts | 34 +++++++++++---------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/test/js/node/http/node-http-connect.test.ts b/test/js/node/http/node-http-connect.test.ts index 7a323fa52b13..278211199d09 100644 --- a/test/js/node/http/node-http-connect.test.ts +++ b/test/js/node/http/node-http-connect.test.ts @@ -1985,7 +1985,7 @@ function runTunnel(proto: string, event: string, listener: string, peer = "clien process.on("exit", () => console.log(JSON.stringify(result)));`, ); } -// More than a loopback socket takes in one write. +// More than a loopback socket takes in one write, except on Windows. const large = 8 * 1024 * 1024; describe.concurrent.each([ @@ -2003,26 +2003,28 @@ describe.concurrent.each([ expect(await runTunnel(proto, event, listener)).toEqual(exitedWith({ buffered: 65536 })); }); - test("keeps the process alive again while it sends and when it reads", async () => { + // 'beforeExit': nothing keeps the process alive. It has to wait for what the listener starts there. + test("keeps the process alive again while it sends", async () => { const listener = ` server.close(); socket.write(reply); - Object.assign(result, { sent: false, serverReceived: 0, clientReceived: 0 }); - // 'beforeExit': nothing keeps the process alive. It has to wait for what each step starts. - const steps = [ - () => socket.write(Buffer.alloc(${large}, "x"), () => (result.sent = true)), - () => socket.on("data", chunk => (result.serverReceived += chunk.length)).on("end", () => socket.end()), - ]; - process.on("beforeExit", () => steps.shift()?.());`; - const peer = ` - client.write(full); - client.on("data", chunk => { - if ((result.clientReceived += chunk.length) === ${large}) client.end(Buffer.alloc(${large}, "b")); - });`; - expect(await runTunnel(proto, event, listener, peer)).toEqual( - exitedWith({ sent: true, serverReceived: 65536 + large, clientReceived: large }), + process.once("beforeExit", () => socket.write(Buffer.alloc(${large}, "x"), () => (result.sent = true)));`; + expect(await runTunnel(proto, event, listener, "client.write(full); client.resume();")).toEqual( + exitedWith({ sent: true }), ); }); + + test("keeps the process alive again when it reads", async () => { + const listener = ` + server.close(); + socket.write(reply); + result.received = 0; + process.once("beforeExit", () => { + socket.on("data", chunk => (result.received += chunk.length)).on("end", () => socket.end()); + client.end(Buffer.alloc(${large}, "b")); + });`; + expect(await runTunnel(proto, event, listener)).toEqual(exitedWith({ received: 65536 + large })); + }); }); // The server holds the event loop for its sockets, so server.unref() gives it up for their reads too. From 37dd19b15278306d33d0343a630e0943c6242b25 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 22 Oct 2026 02:07:23 +0000 Subject: [PATCH 06/11] node:http: the read-stopped tunnel tests send one highWaterMark of bytes, which is 16 KiB on Windows --- test/js/node/http/node-http-connect.test.ts | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/test/js/node/http/node-http-connect.test.ts b/test/js/node/http/node-http-connect.test.ts index 278211199d09..65386d931e64 100644 --- a/test/js/node/http/node-http-connect.test.ts +++ b/test/js/node/http/node-http-connect.test.ts @@ -8,7 +8,7 @@ import type { AddressInfo } from "node:net"; import net from "node:net"; import { join } from "node:path"; import type { Duplex } from "node:stream"; -import { Writable } from "node:stream"; +import { getDefaultHighWaterMark, Writable } from "node:stream"; import tls from "node:tls"; import { WebSocketServer } from "ws"; function connectClient(proxyAddress: AddressInfo, targetAddress: AddressInfo, add_http_prefix: boolean) { @@ -1961,8 +1961,8 @@ function runTunnel(proto: string, event: string, listener: string, peer = "clien "-e", `const server = require("node:${proto}").createServer(${JSON.stringify(proto === "https" ? tlsCert : {})}); const reply = "HTTP/1.1 200 OK\\r\\n\\r\\n"; - // What a socket buffers before it stops its reads. - const full = Buffer.alloc(65536, "a"); + // What a socket buffers before it stops its reads, however the bytes arrive. + const full = Buffer.alloc(${highWaterMark}, "a"); const result = {}; let client; server.on("${event}", (req, socket) => { @@ -1985,6 +1985,8 @@ function runTunnel(proto: string, event: string, listener: string, peer = "clien process.on("exit", () => console.log(JSON.stringify(result)));`, ); } +// 16 KiB on Windows, 64 KiB elsewhere. +const highWaterMark = getDefaultHighWaterMark(false); // More than a loopback socket takes in one write, except on Windows. const large = 8 * 1024 * 1024; @@ -2000,7 +2002,7 @@ describe.concurrent.each([ server.close(); socket.end(reply); process.on("beforeExit", () => (result.buffered = socket.readableLength));`; - expect(await runTunnel(proto, event, listener)).toEqual(exitedWith({ buffered: 65536 })); + expect(await runTunnel(proto, event, listener)).toEqual(exitedWith({ buffered: highWaterMark })); }); // 'beforeExit': nothing keeps the process alive. It has to wait for what the listener starts there. @@ -2023,7 +2025,7 @@ describe.concurrent.each([ socket.on("data", chunk => (result.received += chunk.length)).on("end", () => socket.end()); client.end(Buffer.alloc(${large}, "b")); });`; - expect(await runTunnel(proto, event, listener)).toEqual(exitedWith({ received: 65536 + large })); + expect(await runTunnel(proto, event, listener)).toEqual(exitedWith({ received: highWaterMark + large })); }); }); @@ -2054,12 +2056,12 @@ describe.concurrent.each(["http", "https"])("%s: a tunnel behind server.unref() test("does not keep the process alive when it reads again", async () => { // The peer stays connected and sends nothing more. - expect(await runTunnel(proto, "connect", readsAgain("", ""))).toEqual(exitedWith({ received: 65536 })); + expect(await runTunnel(proto, "connect", readsAgain("", ""))).toEqual(exitedWith({ received: highWaterMark })); }); test("keeps the process alive when it reads again behind server.ref()", async () => { expect(await runTunnel(proto, "connect", readsAgain("server.ref();", "client.end();"))).toEqual( - exitedWith({ received: 65536, end: true }), + exitedWith({ received: highWaterMark, end: true }), ); }); From 2c26d0103541eb8f9d7fbac91f3ca1019ad8efdb Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 22 Oct 2026 02:54:16 +0000 Subject: [PATCH 07/11] node:http: the CONNECT proxy fixture waits for its upstream's connection, and the send tests leave bytes to send on every platform --- .../http/node-http-connect-proxy-pipes-fixture.js | 8 ++++---- test/js/node/http/node-http-connect.test.ts | 15 ++++++++++++--- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/test/js/node/http/node-http-connect-proxy-pipes-fixture.js b/test/js/node/http/node-http-connect-proxy-pipes-fixture.js index 880f60818dcd..9e233c810f10 100644 --- a/test/js/node/http/node-http-connect-proxy-pipes-fixture.js +++ b/test/js/node/http/node-http-connect-proxy-pipes-fixture.js @@ -9,9 +9,9 @@ const result = {}; let client; // An upstream that does not read. -let upstreamSocket; +const upstreamSocket = Promise.withResolvers(); const upstream = net.createServer(socket => { - upstreamSocket = socket.pause().on("error", () => {}); + upstreamSocket.resolve(socket.pause().on("error", () => {})); }); const proxy = http.createServer(); @@ -26,8 +26,8 @@ proxy.on("connect", (req, clientSocket, head) => { clientSocket.on("error", () => {}); serverSocket.on("error", () => {}); // pipe() pauses the client's socket because the upstream is slow. Then the upstream goes away. - clientSocket.once("pause", () => { - upstreamSocket.destroy(); + clientSocket.once("pause", async () => { + (await upstreamSocket.promise).destroy(); upstream.close(); }); serverSocket.on("close", () => { diff --git a/test/js/node/http/node-http-connect.test.ts b/test/js/node/http/node-http-connect.test.ts index 65386d931e64..bad9d2751be7 100644 --- a/test/js/node/http/node-http-connect.test.ts +++ b/test/js/node/http/node-http-connect.test.ts @@ -1987,8 +1987,11 @@ function runTunnel(proto: string, event: string, listener: string, peer = "clien } // 16 KiB on Windows, 64 KiB elsewhere. const highWaterMark = getDefaultHighWaterMark(false); -// More than a loopback socket takes in one write, except on Windows. const large = 8 * 1024 * 1024; +// Writes until the kernel takes no more. The peer is in the same process, so it reads nothing meanwhile. +const writeUntilBytesAreLeftToSend = ` + const chunk = Buffer.alloc(1024 * 1024, "x"); + while (socket.writable && socket.writableLength === 0) socket.write(chunk);`; describe.concurrent.each([ ["http", "connect"], @@ -2010,7 +2013,10 @@ describe.concurrent.each([ const listener = ` server.close(); socket.write(reply); - process.once("beforeExit", () => socket.write(Buffer.alloc(${large}, "x"), () => (result.sent = true)));`; + process.once("beforeExit", () => { + ${writeUntilBytesAreLeftToSend} + socket.write(chunk, () => (result.sent = true)); + });`; expect(await runTunnel(proto, event, listener, "client.write(full); client.resume();")).toEqual( exitedWith({ sent: true }), ); @@ -2072,7 +2078,10 @@ describe.concurrent.each(["http", "https"])("%s: a tunnel behind server.unref() server.close(); const hold = setInterval(() => {}, 1000); socket.on("end", () => clearInterval(hold)).resume(); - process.once("beforeExit", () => socket.end(Buffer.alloc(${large}, "x"), () => (result.sent = true)));`; + process.once("beforeExit", () => { + ${writeUntilBytesAreLeftToSend} + socket.end(() => (result.sent = true)); + });`; expect(await runTunnel(proto, "connect", listener, "client.end(); client.resume();")).toEqual( exitedWith({ sent: true }), ); From 835a3ed453433b4a42632fb276ba3863c0c8eced Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 22 Oct 2026 09:12:52 +0000 Subject: [PATCH 08/11] node:http: a tunnel with bytes left to send holds the event loop behind server.unref(), and over TLS gives it up once end() has sent them --- packages/bun-usockets/src/crypto/openssl.c | 6 +- packages/bun-uws/src/AsyncSocketData.h | 4 +- packages/bun-uws/src/HttpContext.h | 6 + packages/bun-uws/src/HttpResponse.h | 22 +- .../bindings/node/JSNodeHTTPServerSocket.cpp | 24 +- .../bindings/node/JSNodeHTTPServerSocket.h | 2 +- src/runtime/server/mod.rs | 41 ++- test/js/node/http/node-http-connect.test.ts | 238 +++++++++++++++++- .../node-http-server-socket-end-drain.test.ts | 26 ++ 9 files changed, 347 insertions(+), 22 deletions(-) diff --git a/packages/bun-usockets/src/crypto/openssl.c b/packages/bun-usockets/src/crypto/openssl.c index c19e06354d9c..9158366adf1f 100644 --- a/packages/bun-usockets/src/crypto/openssl.c +++ b/packages/bun-usockets/src/crypto/openssl.c @@ -2485,6 +2485,7 @@ static struct us_socket_t *ssl_on_writable(struct us_socket_t *s) { loop_ssl_data->ssl_send_error = 0; if (!ssl_drain_spill(loop_ssl_data, s)) return s; if (loop_ssl_data->ssl_send_error) return ssl_close_after_rejected_send(s, loop_ssl_data->ssl_send_error); + int sent_deferred_shutdown = s->ssl_shutdown_after_spill; if (s->ssl_shutdown_after_spill) { s->ssl_shutdown_after_spill = 0; us_internal_ssl_shutdown(s); @@ -2500,8 +2501,9 @@ static struct us_socket_t *ssl_on_writable(struct us_socket_t *s) { /* uWS HTTP sockets keep the pre-existing SENT_SHUTDOWN suppression: their * onWritable clears the teardown timeout armed at shutdown. node sockets * still get write-completion dispatch after a half-close in either - * direction. */ - if (ssl_is_uws_http_tls(s) && us_internal_ssl_is_shut_down(s)) return s; + * direction. The event that sent a shutdown held behind the spill goes through: + * nothing else tells the owner that the bytes it counts as written have left. */ + if (ssl_is_uws_http_tls(s) && us_internal_ssl_is_shut_down(s) && !sent_deferred_shutdown) return s; if (s->ssl_handshake_state != HANDSHAKE_PENDING) { s->ssl_write_parked = 0; diff --git a/packages/bun-uws/src/AsyncSocketData.h b/packages/bun-uws/src/AsyncSocketData.h index a0e5be49022f..ec1921d40f46 100644 --- a/packages/bun-uws/src/AsyncSocketData.h +++ b/packages/bun-uws/src/AsyncSocketData.h @@ -177,8 +177,10 @@ struct AsyncSocketData { * that can still reach a handler counts these. */ bool filteredAccept = false; /* Whether it has fired the filter with -3 (HttpResponse::setNodeHttpTunnelIdle). - * +3 or +4 undoes it, and an idle socket fires -4 in place of -2. */ + * +3 undoes it, and an idle socket fires -4 in place of -2. */ bool filteredIdleTunnel = false; + /* Whether it has fired the filter with +5 (HttpResponse::setNodeHttpTunnelSending). -5 undoes it, on the paths that balance +2 too. */ + bool filteredSendingTunnel = false; }; } diff --git a/packages/bun-uws/src/HttpContext.h b/packages/bun-uws/src/HttpContext.h index 14c58b249928..1e84351de030 100644 --- a/packages/bun-uws/src/HttpContext.h +++ b/packages/bun-uws/src/HttpContext.h @@ -280,6 +280,7 @@ struct HttpContext { f((HttpResponse *) s, -1); } } + ((HttpResponse *) s)->setNodeHttpTunnelSending(false); if (httpResponseData->filteredAccept) { for (auto &f : httpContextData->filterHandlers) { f((HttpResponse *) s, httpResponseData->filteredIdleTunnel ? -4 : -2); @@ -893,6 +894,8 @@ struct HttpContext { static us_socket_t *onWritable(us_socket_t *s) { auto *asyncSocket = reinterpret_cast *>(s); auto *httpResponseData = reinterpret_cast *>(asyncSocket->getAsyncSocketData()); + /* Behind its FIN, a TLS socket gets this event only so that a tunnel hears that what TLS held has left (ssl_on_writable). */ + const bool behindFin = SSL && us_socket_is_shut_down(s); /* Attempt to drain the socket buffer before triggering onWritable callback */ size_t bufferedAmount = asyncSocket->getBufferedAmount(); @@ -930,6 +933,9 @@ struct HttpContext { if (httpResponseData->isConnectRequest && httpResponseData->socketData && httpContextData->onSocketDrain) { httpContextData->onSocketDrain(httpResponseData->socketData, SSL, (struct us_socket_t *) s); } + if (behindFin) { + return s; + } /* Ask the developer to write data and return success (true) or failure (false), OR skip sending anything and return success (true). */ if (httpResponseData->onWritable) { /* We are now writable, so hang timeout again, the user does not have to do anything so we should hang until end or tryEnd rearms timeout */ diff --git a/packages/bun-uws/src/HttpResponse.h b/packages/bun-uws/src/HttpResponse.h index a22e54789aab..d1899f21a630 100644 --- a/packages/bun-uws/src/HttpResponse.h +++ b/packages/bun-uws/src/HttpResponse.h @@ -186,16 +186,29 @@ struct HttpResponse : public AsyncSocket { /* node:http: an idle tunnel does not read (its reads are stopped or at EOF) and has nothing left to send. Like a libuv handle in * that state, it does not hold the event loop: https://github.com/nodejs/node/blob/v26.3.0/deps/uv/src/unix/stream.c#L1473-L1485. - * The filter hears -3 when a tunnel becomes idle. When it is not idle any more, the filter hears +3 at read EOF (it has bytes to - * send) and +4 otherwise (it reads or has bytes to send). */ - void setNodeHttpTunnelIdle(bool idle, bool readEnded) { + * The filter hears -3 when a tunnel becomes idle and +3 when it reads or has bytes to send again. */ + void setNodeHttpTunnelIdle(bool idle) { HttpResponseData *httpResponseData = getHttpResponseData(); if (!httpResponseData->filteredAccept || httpResponseData->filteredIdleTunnel == idle) { return; } httpResponseData->filteredIdleTunnel = idle; for (auto &f : HttpContext::getSocketContextDataS((us_socket_t *) this)->filterHandlers) { - f(this, idle ? -3 : readEnded ? 3 : 4); + f(this, idle ? -3 : 3); + } + } + + /* node:http: a tunnel with bytes left to send holds the event loop whatever is unref'd, like a libuv write request: + * https://github.com/nodejs/node/blob/v26.3.0/deps/uv/src/unix/core.c#L393-L398. + * The filter hears +5 when a tunnel has bytes left to send and -5 when it has none any more. */ + void setNodeHttpTunnelSending(bool sending) { + HttpResponseData *httpResponseData = getHttpResponseData(); + if (!httpResponseData->filteredAccept || httpResponseData->filteredSendingTunnel == sending) { + return; + } + httpResponseData->filteredSendingTunnel = sending; + for (auto &f : HttpContext::getSocketContextDataS((us_socket_t *) this)->filterHandlers) { + f(this, sending ? 5 : -5); } } @@ -496,6 +509,7 @@ struct HttpResponse : public AsyncSocket { f((HttpResponse *) this, -1); } } + setNodeHttpTunnelSending(false); if (((AsyncSocketData *) responseData)->filteredAccept) { for (auto &f : httpContextData->filterHandlers) { f((HttpResponse *) this, ((AsyncSocketData *) responseData)->filteredIdleTunnel ? -4 : -2); diff --git a/src/jsc/bindings/node/JSNodeHTTPServerSocket.cpp b/src/jsc/bindings/node/JSNodeHTTPServerSocket.cpp index 800e9be07a98..24f530c737c8 100644 --- a/src/jsc/bindings/node/JSNodeHTTPServerSocket.cpp +++ b/src/jsc/bindings/node/JSNodeHTTPServerSocket.cpp @@ -93,6 +93,8 @@ static void flushPartialResponseBeforeClose(us_socket_t* socket) } } +static bool isTunnel(const JSNodeHTTPServerSocket* self); + void JSNodeHTTPServerSocket::close() { if (socket) { @@ -114,6 +116,10 @@ void JSNodeHTTPServerSocket::close() } // Forceful: code 0 defers the fd close until a close_notify reply that a half-open peer never sends. us_socket_close(socket, LIBUS_SOCKET_CLOSE_CODE_FAST_SHUTDOWN, nullptr); + // TLS holds the close behind the bytes it has left. As in Node, destroy() does not keep the process alive for them. + if (is_ssl && isTunnel(this)) { + reinterpret_cast*>(socket)->setNodeHttpTunnelSending(false); + } } } @@ -398,6 +404,8 @@ JSC::EncodedJSValue JSNodeHTTPServerSocket::halfClose(JSC::JSGlobalObject* globa if (socket && cycleReads) { us_socket_resume(socket); } + // The shutdown can send what TLS still held, and no writable event comes behind the FIN. + updateTunnelIdle(); return result; } @@ -886,16 +894,26 @@ extern "C" bool Bun__NodeHTTPServerSocket__writeBehindResponse(us_socket_t* sock return is_ssl ? writeBehindResponse(socket, data, length) : writeBehindResponse(socket, data, length); } +template +static void updateTunnelIdleImpl(us_socket_t* socket, bool reads, bool buffered) +{ + auto* response = reinterpret_cast*>(socket); + const bool sending = buffered || !response->hasFullyDrained(); + response->setNodeHttpTunnelSending(sending); + response->setNodeHttpTunnelIdle(!reads && !sending); +} + void JSNodeHTTPServerSocket::updateTunnelIdle() { if (!isTunnel(this)) { return; } - const bool idle = (tunnelReadEnded || tunnelReadsStopped) && streamBuffer.bufferedSize() == 0; + const bool reads = !tunnelReadEnded && !tunnelReadsStopped; + const bool buffered = streamBuffer.bufferedSize() > 0; if (is_ssl) { - reinterpret_cast*>(socket)->setNodeHttpTunnelIdle(idle && reinterpret_cast*>(socket)->hasFullyDrained(), tunnelReadEnded); + updateTunnelIdleImpl(socket, reads, buffered); } else { - reinterpret_cast*>(socket)->setNodeHttpTunnelIdle(idle && reinterpret_cast*>(socket)->hasFullyDrained(), tunnelReadEnded); + updateTunnelIdleImpl(socket, reads, buffered); } } diff --git a/src/jsc/bindings/node/JSNodeHTTPServerSocket.h b/src/jsc/bindings/node/JSNodeHTTPServerSocket.h index 7bbc3a202818..8194d5a0b5a6 100644 --- a/src/jsc/bindings/node/JSNodeHTTPServerSocket.h +++ b/src/jsc/bindings/node/JSNodeHTTPServerSocket.h @@ -150,7 +150,7 @@ class JSNodeHTTPServerSocket : public JSC::JSDestructibleObject { void readStop(); void readStart(); bool tunnelReadsPaused() const { return tunnelReadsStopped || tunnelReadsQueuedFull; } - /* Tells uWS whether this tunnel is idle: it does not read (readStop() or read EOF) and has nothing left to send. See HttpResponse::setNodeHttpTunnelIdle. */ + /* Tells uWS whether this tunnel has bytes left to send, and whether it is idle: it has none and does not read (readStop() or read EOF). See HttpResponse::setNodeHttpTunnelIdle. */ void updateTunnelIdle(); /* uWS still holds bytes of an HTTP response on this connection. A raw write has to go through the same buffer, or it reaches the wire first. */ bool hasUnsentResponseBytes() const; diff --git a/src/runtime/server/mod.rs b/src/runtime/server/mod.rs index aab2f96c32b2..f9c9671fdcf7 100644 --- a/src/runtime/server/mod.rs +++ b/src/runtime/server/mod.rs @@ -271,6 +271,9 @@ pub(crate) struct NewServer { pub(crate) active_connection_count: core::cell::Cell, /// The node:http tunnels in `active_connection_count` that do not read (stopped or at EOF) and have nothing left to send. pub(crate) idle_tunnel_count: core::cell::Cell, + /// The node:http tunnels that have bytes left to send. They hold the loop through `sending_tunnel_ref`, whatever `server.unref()` did to `poll_ref`. + sending_tunnel_count: core::cell::Cell, + sending_tunnel_ref: jsc::JsCell, /// Live `ServerWebSocket` count. Lives on the server (not the websocket /// context) so a reload's context swap cannot reset it, and sits in a /// `Cell` because the open/close accounting arrives through shared @@ -508,7 +511,7 @@ impl NewServer { /// uWS filter: `+2` at TCP accept (before any TLS handshake), `-2` on /// `HttpContext::onClose` / `HttpResponse::upgrade()` — see /// `AsyncSocketData::filteredAccept`. Feeds [`Self::active_connection_count`]. - /// `-3`: a node:http tunnel becomes idle. `+3` / `+4`: it is not idle any more, at read EOF / not at it. `-4`: it closes idle. + /// A node:http tunnel: `-3` / `+3` it becomes idle / is not idle any more, `-4` it closes idle, `+5` / `-5` it has bytes left to send / has none any more. extern "C" fn on_connection_filter( _socket: *mut uws_sys::us_socket_t, opened: i32, @@ -527,18 +530,19 @@ impl NewServer { this.note_connection_opened(); return; } - 3 | 4 => { + 3 => { this.note_tunnel_idle(false); - // With a listener open, the loop ref is the user's to drop (`server.unref()`). - // Behind `server.unref()` reads do not hold the loop, and only at read EOF is it sure that the tunnel has bytes to send. - if !this.has_listener() - && (opened == 3 || !this.flags.contains(ServerFlags::UNREFED_BY_USER)) - { + // With a listener open, the loop ref is the user's to drop (`server.unref()`), and what they dropped stays dropped. + if !this.has_listener() && !this.flags.contains(ServerFlags::UNREFED_BY_USER) { // SAFETY: `this` is not used again, and a tunnel write or read start never runs inside `app.close()`. unsafe { &mut *user_data.cast::() }.ref_(); } return; } + 5 | -5 => { + this.note_tunnel_sending(opened == 5); + return; + } -3 => this.note_tunnel_idle(true), -4 => { this.note_tunnel_idle(false); @@ -1611,7 +1615,7 @@ impl NewServer { fn note_tunnel_idle(&self, idle: bool) { let count = self.idle_tunnel_count.get(); - // `AsyncSocketData::filteredIdleTunnel` pairs every -3 with one +3, +4 or -4. + // `AsyncSocketData::filteredIdleTunnel` pairs every -3 with one +3 or -4. debug_assert!(idle || count > 0); self.idle_tunnel_count.set(if idle { count + 1 @@ -1620,6 +1624,25 @@ impl NewServer { }); } + fn note_tunnel_sending(&self, sending: bool) { + let count = self.sending_tunnel_count.get(); + // `AsyncSocketData::filteredSendingTunnel` pairs every +5 with one -5. + debug_assert!(sending || count > 0); + let count = if sending { + count + 1 + } else { + count.saturating_sub(1) + }; + self.sending_tunnel_count.set(count); + self.sending_tunnel_ref.with_mut(|keep_alive| { + if count > 0 { + keep_alive.ref_(self.vm.loop_ctx()); + } else { + keep_alive.unref(self.vm.loop_ctx()); + } + }); + } + /// An idle tunnel is open but, like a libuv handle that does not read and has no write pending, does not hold the loop. fn has_loop_holding_connections(&self) -> bool { self.active_connection_count.get() > self.idle_tunnel_count.get() @@ -2207,6 +2230,8 @@ impl NewServer { pending_requests: core::cell::Cell::new(0), active_connection_count: core::cell::Cell::new(0), idle_tunnel_count: core::cell::Cell::new(0), + sending_tunnel_count: core::cell::Cell::new(0), + sending_tunnel_ref: jsc::JsCell::new(KeepAlive::default()), active_websocket_count: core::cell::Cell::new(0), deinit_running: core::cell::Cell::new(false), abort_handle: jsc::AbortHandle::for_owner::(), diff --git a/test/js/node/http/node-http-connect.test.ts b/test/js/node/http/node-http-connect.test.ts index bad9d2751be7..e92fa31e1f26 100644 --- a/test/js/node/http/node-http-connect.test.ts +++ b/test/js/node/http/node-http-connect.test.ts @@ -1956,7 +1956,7 @@ function runTunnel(proto: string, event: string, listener: string, peer = "clien const request = event === "connect" ? "CONNECT example.com:443 HTTP/1.1\r\nHost: example.com:443\r\n\r\n" - : "GET / HTTP/1.1\r\nHost: a\r\nConnection: Upgrade\r\nUpgrade: raw\r\n\r\n"; + : "GET / HTTP/1.1\r\nHost: a\r\nConnection: Upgrade\r\nUpgrade: websocket\r\nSec-WebSocket-Version: 13\r\nSec-WebSocket-Key: dGhlIHNhbXBsZSBub25jZQ==\r\n\r\n"; return runToExit( "-e", `const server = require("node:${proto}").createServer(${JSON.stringify(proto === "https" ? tlsCert : {})}); @@ -1968,8 +1968,8 @@ function runTunnel(proto: string, event: string, listener: string, peer = "clien server.on("${event}", (req, socket) => { ${listener} }); - server.listen(0, "127.0.0.1", () => { - client = require("node:${proto === "https" ? "tls" : "net"}").connect({ + function connect() { + const client = require("node:${proto === "https" ? "tls" : "net"}").connect({ port: server.address().port, host: "127.0.0.1", rejectUnauthorized: false, @@ -1978,6 +1978,10 @@ function runTunnel(proto: string, event: string, listener: string, peer = "clien // Only the server's side of the tunnel may keep the process alive. client.unref(); client.write(${JSON.stringify(request)}); + return client; + } + server.listen(0, "127.0.0.1", () => { + client = connect(); client.once("data", () => { ${peer} }); @@ -2088,6 +2092,234 @@ describe.concurrent.each(["http", "https"])("%s: a tunnel behind server.unref() }); }); +// In Node.js a write request holds the event loop until it is done, whatever is unref'd. +describe.concurrent.each([ + ["http", "connect"], + ["https", "connect"], + ["http", "upgrade"], + ["https", "upgrade"], +])("%s '%s': a tunnel with bytes left to send, behind server.unref()", (proto, event) => { + // `lost` is what the tunnel wrote less what the peer read. + const peerReads = ` + result.lost = 0; + client.on("data", chunk => (result.lost -= chunk.length)).on("end", () => client.destroy());`; + // From here on the peer keeps the process alive, until it has read up to the FIN. + const whenSent = `() => { + result.lost += socket.bytesWritten - reply.length; + result.sent = true; + client.ref(); + socket.end(); + }`; + // `hold` keeps the process alive until the reads have stopped. 'beforeExit': nothing keeps it alive. + const whenReadsHaveStopped = (close: string, then: string) => ` + socket.write(reply); + server.unref(); + ${close} + const hold = setInterval(() => { + if (socket.readableLength === full.length) clearInterval(hold); + }, 1); + process.once("beforeExit", () => { + ${writeUntilBytesAreLeftToSend} + ${then} + });`; + + test.each([ + ["after server.close()", "server.close();"], + ["while the server listens", ""], + ])("keeps the process alive until they are sent, with its reads stopped, %s", async (_, close) => { + const listener = whenReadsHaveStopped(close, `socket.write(chunk, ${whenSent});`); + expect(await runTunnel(proto, event, listener, `client.write(full); ${peerReads}`)).toEqual( + exitedWith({ lost: 0, sent: true }), + ); + }); + + test("keeps the process alive until they are sent, at the end of the stream, before server.close()", async () => { + const listener = ` + socket.write(reply); + server.unref(); + const hold = setInterval(() => {}, 1000); + socket.on("end", () => clearInterval(hold)).resume(); + process.once("beforeExit", () => { + ${writeUntilBytesAreLeftToSend} + socket.write(chunk, ${whenSent}); + server.close(); + });`; + expect(await runTunnel(proto, event, listener, `client.end(); ${peerReads}`)).toEqual( + exitedWith({ lost: 0, sent: true }), + ); + }); + + // In Node.js the reads hold the event loop too: there this process does not exit. + test("does not keep the process alive once they are sent, when it reads again meanwhile", async () => { + const listener = whenReadsHaveStopped( + "server.close();", + `socket.write(chunk, () => (result.sent = true)); + socket.resume();`, + ); + expect(await runTunnel(proto, event, listener, "client.write(full); client.resume();")).toEqual( + exitedWith({ sent: true }), + ); + }); + + test("does not keep the process alive once it is destroyed", async () => { + const listener = whenReadsHaveStopped( + "server.close();", + `result.left = socket.writableLength > 0; + socket.destroy();`, + ); + expect(await runTunnel(proto, event, listener, "client.write(full); client.pause();")).toEqual( + exitedWith({ left: true }), + ); + }); + + test("keeps the process alive for the second of two, once the first has sent its own", async () => { + const listener = ` + socket.write(reply); + if (!server.first) return void (server.first = socket); + server.unref(); + server.close(); + Object.assign(result, { first: false, second: false }); + const both = [server.first, socket]; + for (const socket of both) { + ${writeUntilBytesAreLeftToSend} + } + server.first.end(() => (result.first = true)); + socket.end(() => (result.second = true));`; + // The second peer reads nothing before the first has read up to the FIN. + const peer = ` + const second = connect(); + second.once("data", () => second.pause()).on("end", () => second.destroy()); + client.on("end", () => (client.destroy(), second.resume())).resume();`; + expect(await runTunnel(proto, event, listener, peer)).toEqual(exitedWith({ first: true, second: true })); + }); +}); + +test.concurrent.each(["http", "https"])( + "%s: a tunnel with bytes left to send does not keep the process alive once a WebSocket adopts its connection", + async proto => { + const listener = ` + server.unref(); + server.close(); + ${writeUntilBytesAreLeftToSend} + result.left = socket.writableLength > 0; + const wss = new (require("ws").WebSocketServer)({ noServer: true }); + wss.handleUpgrade(req, socket, Buffer.alloc(0), () => (result.adopted = true));`; + expect(await runTunnel(proto, "upgrade", listener, "client.pause();")).toEqual( + exitedWith({ left: true, adopted: true }), + ); + }, +); + +// With a send buffer smaller than a TLS record, TLS holds a part of what the socket counts as written. +const shrinkSendBuffer = `require("bun:internal-for-testing").setSocketOptions(socket, 1, 4096);`; + +describe.concurrent("https: a tunnel with bytes that only TLS holds", () => { + // Not where the kernel takes a write whole whatever its send buffer: there TLS holds nothing. + test.skipIf(!isLinux)("keeps the process alive until they are sent, behind server.unref()", async () => { + // The second 'beforeExit' comes when they are sent. From there on the peer keeps the process alive, up to the FIN. + const listener = ` + socket.write(reply); + server.unref(); + server.close(); + const hold = setInterval(() => { + if (socket.readableLength === full.length) clearInterval(hold); + }, 1); + process.once("beforeExit", () => { + ${shrinkSendBuffer} + socket.write(full); + result.lost += full.length; + result.buffered = socket.writableLength; + process.once("beforeExit", () => { + client.ref(); + socket.end(); + }); + });`; + const peer = ` + client.write(full); + result.lost = 0; + client.on("data", chunk => (result.lost -= chunk.length)).on("end", () => client.destroy());`; + expect(await runTunnel("https", "connect", listener, peer)).toEqual(exitedWith({ lost: 0, buffered: 0 })); + }); + + // TLS holds the close of destroy() behind them. The server waits for that close as for any other. + test("emits 'close' when it is destroyed, and so does its server", async () => { + const listener = ` + socket.write(reply); + server.close(); + socket.on("close", () => (result.socketClosed = true)); + server.on("close", () => (result.serverClosed = true)); + process.once("beforeExit", () => { + ${shrinkSendBuffer} + socket.write(full); + result.buffered = socket.writableLength; + socket.destroy(); + });`; + expect(await runTunnel("https", "connect", listener, "client.write(full); client.resume();")).toEqual( + exitedWith({ buffered: 0, socketClosed: true, serverClosed: true }), + ); + }); +}); + +// The FIN of end() waits behind what TLS holds. The peer reads up to the FIN and stays connected. +describe.each([ + ["server.close()", ""], + ["server.unref() and server.close()", "server.unref();"], +])("https: a tunnel whose reads stopped, after %s", (_, unref) => { + test.concurrent.each([64 * 1024, 1024 * 1024])( + "sends the %d bytes of end() and then does not keep the process alive", + async size => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + `const server = require("node:https").createServer(${JSON.stringify(tlsCert)}); + server.on("connect", (req, socket) => { + socket.write("HTTP/1.1 200 OK\\r\\n\\r\\n"); + ${unref} + server.close(); + const hold = setInterval(() => { + if (socket.readableLength < ${highWaterMark}) return; + clearInterval(hold); + ${shrinkSendBuffer} + socket.end(Buffer.alloc(${size}, "x")); + }, 1); + }); + server.listen(0, "127.0.0.1", () => console.log(server.address().port));`, + ], + env: { ...bunEnv, BUN_FEATURE_FLAG_NO_ORPHANS: "1" }, + stdout: "pipe", + stderr: "pipe", + }); + let port = ""; + for await (const chunk of proc.stdout) { + port += Buffer.from(chunk).toString(); + if (port.includes("\n")) break; + } + + const client = tls.connect({ + port: Number(port), + host: "127.0.0.1", + rejectUnauthorized: false, + allowHalfOpen: true, + }); + using _ = { [Symbol.dispose]: () => void client.destroy() }; + const ended = Promise.withResolvers(); + client.on("error", ended.reject).on("close", () => ended.reject(new Error("closed before the FIN"))); + client.on("end", ended.resolve); + let received = 0; + client.once("data", () => { + client.on("data", chunk => (received += chunk.length)); + client.write(Buffer.alloc(highWaterMark, "a")); + }); + client.write("CONNECT example.com:443 HTTP/1.1\r\nHost: example.com:443\r\n\r\n"); + await ended.promise; + + const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); + expect({ received, stderr, exitCode }).toEqual({ received: size, stderr: "", exitCode: 0 }); + }, + ); +}); + // A listener with nothing to send ends its side in the listener. It still reads: an http.Server // socket has allowHalfOpen. Every expectation below is Node v26.3.0's. describe("a tunnel behind a request head that took more than one read", () => { diff --git a/test/js/node/http/node-http-server-socket-end-drain.test.ts b/test/js/node/http/node-http-server-socket-end-drain.test.ts index 0f979699f5ff..ff29f0911db0 100644 --- a/test/js/node/http/node-http-server-socket-end-drain.test.ts +++ b/test/js/node/http/node-http-server-socket-end-drain.test.ts @@ -1,3 +1,4 @@ +import { setSocketOptions } from "bun:internal-for-testing"; import { describe, expect, test } from "bun:test"; import { bunEnv, bunExe, tls as tlsCert } from "harness"; import { once } from "node:events"; @@ -179,6 +180,31 @@ describe.each(["http", "https"] as const)("%s: the request body behind the FIN o ["chunked", "Transfer-Encoding: chunked", `3e8\r\n${payload}\r\n0\r\n\r\n`], ]; + // TLS holds a part of the response, and the FIN behind it. Without TLS uWS holds that part, and closes behind it. + test + .skipIf(protocol === "http") + .concurrent("the body reaches the request behind a response that the send buffer does not take whole", async () => { + const { promise, resolve } = Promise.withResolvers>(); + await using server = createServer((req, res) => { + resolve(observe(req)); + setSocketOptions(req.socket, 1, 4096); + res.writeHead(401, { "Content-Length": 64 * 1024, Connection: "close" }); + res.end(Buffer.alloc(64 * 1024, "n")); + req.socket.end(); + }); + + const client = await connectTo(server); + try { + const serverEnded = once(client, "end"); + client.write(requestHead(`Content-Length: ${payload.length}`)); + await serverEnded; + client.end(payload); + expect(await promise).toEqual({ body: payload, events: ["end", "close"] }); + } finally { + client.destroy(); + } + }); + describe.each(rejections)("%s", (_how, reject) => { describe.each(bodies)("%s", (_framing, framing, encoded) => { test.concurrent.each([ From eabb827d4d5e12e8aa539d341667830e58800bac Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 22 Oct 2026 10:07:56 +0000 Subject: [PATCH 09/11] node:http: the writable event behind the FIN of a TLS socket only tells a tunnel, also when uWS still holds bytes --- packages/bun-uws/src/HttpContext.h | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/packages/bun-uws/src/HttpContext.h b/packages/bun-uws/src/HttpContext.h index 1e84351de030..f58c7a15acaa 100644 --- a/packages/bun-uws/src/HttpContext.h +++ b/packages/bun-uws/src/HttpContext.h @@ -894,8 +894,15 @@ struct HttpContext { static us_socket_t *onWritable(us_socket_t *s) { auto *asyncSocket = reinterpret_cast *>(s); auto *httpResponseData = reinterpret_cast *>(asyncSocket->getAsyncSocketData()); + /* Behind its FIN, a TLS socket gets this event only so that a tunnel hears that what TLS held has left (ssl_on_writable). */ - const bool behindFin = SSL && us_socket_is_shut_down(s); + if (SSL && us_socket_is_shut_down(s)) { + auto *httpContextData = getSocketContextDataS(s); + if (httpResponseData->isConnectRequest && httpResponseData->socketData && httpContextData->onSocketDrain) { + httpContextData->onSocketDrain(httpResponseData->socketData, SSL, (struct us_socket_t *) s); + } + return s; + } /* Attempt to drain the socket buffer before triggering onWritable callback */ size_t bufferedAmount = asyncSocket->getBufferedAmount(); @@ -933,9 +940,6 @@ struct HttpContext { if (httpResponseData->isConnectRequest && httpResponseData->socketData && httpContextData->onSocketDrain) { httpContextData->onSocketDrain(httpResponseData->socketData, SSL, (struct us_socket_t *) s); } - if (behindFin) { - return s; - } /* Ask the developer to write data and return success (true) or failure (false), OR skip sending anything and return success (true). */ if (httpResponseData->onWritable) { /* We are now writable, so hang timeout again, the user does not have to do anything so we should hang until end or tryEnd rearms timeout */ From 66a58c9de4cfdd31030677a4cd7f3e11737929e2 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 22 Oct 2026 10:52:58 +0000 Subject: [PATCH 10/11] node:http: a tunnel holds the event loop for the bytes that TLS has left until its close is done, also when it is destroyed --- .../bindings/node/JSNodeHTTPServerSocket.cpp | 6 -- src/runtime/server/mod.rs | 6 +- test/js/node/http/node-http-connect.test.ts | 100 ++++++++++-------- .../node-http-server-socket-end-drain.test.ts | 5 +- 4 files changed, 59 insertions(+), 58 deletions(-) diff --git a/src/jsc/bindings/node/JSNodeHTTPServerSocket.cpp b/src/jsc/bindings/node/JSNodeHTTPServerSocket.cpp index 24f530c737c8..e33104983829 100644 --- a/src/jsc/bindings/node/JSNodeHTTPServerSocket.cpp +++ b/src/jsc/bindings/node/JSNodeHTTPServerSocket.cpp @@ -93,8 +93,6 @@ static void flushPartialResponseBeforeClose(us_socket_t* socket) } } -static bool isTunnel(const JSNodeHTTPServerSocket* self); - void JSNodeHTTPServerSocket::close() { if (socket) { @@ -116,10 +114,6 @@ void JSNodeHTTPServerSocket::close() } // Forceful: code 0 defers the fd close until a close_notify reply that a half-open peer never sends. us_socket_close(socket, LIBUS_SOCKET_CLOSE_CODE_FAST_SHUTDOWN, nullptr); - // TLS holds the close behind the bytes it has left. As in Node, destroy() does not keep the process alive for them. - if (is_ssl && isTunnel(this)) { - reinterpret_cast*>(socket)->setNodeHttpTunnelSending(false); - } } } diff --git a/src/runtime/server/mod.rs b/src/runtime/server/mod.rs index f9c9671fdcf7..611aeaa1790f 100644 --- a/src/runtime/server/mod.rs +++ b/src/runtime/server/mod.rs @@ -1628,11 +1628,7 @@ impl NewServer { let count = self.sending_tunnel_count.get(); // `AsyncSocketData::filteredSendingTunnel` pairs every +5 with one -5. debug_assert!(sending || count > 0); - let count = if sending { - count + 1 - } else { - count.saturating_sub(1) - }; + let count = if sending { count + 1 } else { count - 1 }; self.sending_tunnel_count.set(count); self.sending_tunnel_ref.with_mut(|keep_alive| { if count > 0 { diff --git a/test/js/node/http/node-http-connect.test.ts b/test/js/node/http/node-http-connect.test.ts index e92fa31e1f26..1f609f3a440d 100644 --- a/test/js/node/http/node-http-connect.test.ts +++ b/test/js/node/http/node-http-connect.test.ts @@ -2161,13 +2161,14 @@ describe.concurrent.each([ ); }); + // The peer reads: TLS holds the close behind the bytes that it has left. test("does not keep the process alive once it is destroyed", async () => { const listener = whenReadsHaveStopped( "server.close();", `result.left = socket.writableLength > 0; socket.destroy();`, ); - expect(await runTunnel(proto, event, listener, "client.write(full); client.pause();")).toEqual( + expect(await runTunnel(proto, event, listener, "client.write(full); client.resume();")).toEqual( exitedWith({ left: true }), ); }); @@ -2194,6 +2195,7 @@ describe.concurrent.each([ }); }); +// The WebSocket closes the connection, so that only what the tunnel left behind can keep the process alive. test.concurrent.each(["http", "https"])( "%s: a tunnel with bytes left to send does not keep the process alive once a WebSocket adopts its connection", async proto => { @@ -2203,8 +2205,11 @@ test.concurrent.each(["http", "https"])( ${writeUntilBytesAreLeftToSend} result.left = socket.writableLength > 0; const wss = new (require("ws").WebSocketServer)({ noServer: true }); - wss.handleUpgrade(req, socket, Buffer.alloc(0), () => (result.adopted = true));`; - expect(await runTunnel(proto, "upgrade", listener, "client.pause();")).toEqual( + wss.handleUpgrade(req, socket, Buffer.alloc(0), ws => { + result.adopted = true; + ws.terminate(); + });`; + expect(await runTunnel(proto, "upgrade", listener, "client.resume();")).toEqual( exitedWith({ left: true, adopted: true }), ); }, @@ -2260,64 +2265,69 @@ describe.concurrent("https: a tunnel with bytes that only TLS holds", () => { }); }); -// The FIN of end() waits behind what TLS holds. The peer reads up to the FIN and stays connected. +// The FIN of end() waits behind what TLS holds. The peer reads up to the FIN and does not close. +// At the end of the stream, 'finish' destroys the socket, and TLS holds that close behind its bytes too. describe.each([ ["server.close()", ""], ["server.unref() and server.close()", "server.unref();"], -])("https: a tunnel whose reads stopped, after %s", (_, unref) => { - test.concurrent.each([64 * 1024, 1024 * 1024])( - "sends the %d bytes of end() and then does not keep the process alive", - async size => { - await using proc = Bun.spawn({ - cmd: [ - bunExe(), - "-e", - `const server = require("node:https").createServer(${JSON.stringify(tlsCert)}); +])("https: after %s", (_, unref) => { + test.concurrent.each([ + ["whose reads stopped", 64 * 1024], + ["whose reads stopped", 1024 * 1024], + ["at the end of the stream", 256 * 1024], + ])("a tunnel %s sends the %d bytes of end() and then does not keep the process alive", async (reads, size) => { + const atEnd = reads === "at the end of the stream"; + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + `const server = require("node:https").createServer(${JSON.stringify(tlsCert)}); server.on("connect", (req, socket) => { socket.write("HTTP/1.1 200 OK\\r\\n\\r\\n"); ${unref} server.close(); + if (${atEnd}) socket.resume(); const hold = setInterval(() => { - if (socket.readableLength < ${highWaterMark}) return; + if (${atEnd} ? !socket.readableEnded : socket.readableLength < ${highWaterMark}) return; clearInterval(hold); ${shrinkSendBuffer} socket.end(Buffer.alloc(${size}, "x")); }, 1); }); server.listen(0, "127.0.0.1", () => console.log(server.address().port));`, - ], - env: { ...bunEnv, BUN_FEATURE_FLAG_NO_ORPHANS: "1" }, - stdout: "pipe", - stderr: "pipe", - }); - let port = ""; - for await (const chunk of proc.stdout) { - port += Buffer.from(chunk).toString(); - if (port.includes("\n")) break; - } + ], + env: { ...bunEnv, BUN_FEATURE_FLAG_NO_ORPHANS: "1" }, + stdout: "pipe", + stderr: "pipe", + }); + let port = ""; + for await (const chunk of proc.stdout) { + port += Buffer.from(chunk).toString(); + if (port.includes("\n")) break; + } - const client = tls.connect({ - port: Number(port), - host: "127.0.0.1", - rejectUnauthorized: false, - allowHalfOpen: true, - }); - using _ = { [Symbol.dispose]: () => void client.destroy() }; - const ended = Promise.withResolvers(); - client.on("error", ended.reject).on("close", () => ended.reject(new Error("closed before the FIN"))); - client.on("end", ended.resolve); - let received = 0; - client.once("data", () => { - client.on("data", chunk => (received += chunk.length)); - client.write(Buffer.alloc(highWaterMark, "a")); - }); - client.write("CONNECT example.com:443 HTTP/1.1\r\nHost: example.com:443\r\n\r\n"); - await ended.promise; + const client = tls.connect({ + port: Number(port), + host: "127.0.0.1", + rejectUnauthorized: false, + allowHalfOpen: true, + }); + using _ = { [Symbol.dispose]: () => void client.destroy() }; + const ended = Promise.withResolvers(); + client.on("error", ended.reject).on("close", () => ended.reject(new Error("closed before the FIN"))); + client.on("end", ended.resolve); + let received = 0; + client.once("data", () => { + client.on("data", chunk => (received += chunk.length)); + if (atEnd) client.end(); + else client.write(Buffer.alloc(highWaterMark, "a")); + }); + client.write("CONNECT example.com:443 HTTP/1.1\r\nHost: example.com:443\r\n\r\n"); + await ended.promise; - const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); - expect({ received, stderr, exitCode }).toEqual({ received: size, stderr: "", exitCode: 0 }); - }, - ); + const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); + expect({ received, stderr, exitCode }).toEqual({ received: size, stderr: "", exitCode: 0 }); + }); }); // A listener with nothing to send ends its side in the listener. It still reads: an http.Server diff --git a/test/js/node/http/node-http-server-socket-end-drain.test.ts b/test/js/node/http/node-http-server-socket-end-drain.test.ts index ff29f0911db0..4c2727520a9b 100644 --- a/test/js/node/http/node-http-server-socket-end-drain.test.ts +++ b/test/js/node/http/node-http-server-socket-end-drain.test.ts @@ -1,6 +1,6 @@ import { setSocketOptions } from "bun:internal-for-testing"; import { describe, expect, test } from "bun:test"; -import { bunEnv, bunExe, tls as tlsCert } from "harness"; +import { bunEnv, bunExe, isWindows, tls as tlsCert } from "harness"; import { once } from "node:events"; import http, { type IncomingMessage, type ServerResponse } from "node:http"; import https from "node:https"; @@ -181,8 +181,9 @@ describe.each(["http", "https"] as const)("%s: the request body behind the FIN o ]; // TLS holds a part of the response, and the FIN behind it. Without TLS uWS holds that part, and closes behind it. + // Windows takes a write whole whatever the send buffer: the response is out at once, and uWS closes behind it. test - .skipIf(protocol === "http") + .skipIf(protocol === "http" || isWindows) .concurrent("the body reaches the request behind a response that the send buffer does not take whole", async () => { const { promise, resolve } = Promise.withResolvers>(); await using server = createServer((req, res) => { From 8f729a776d52fe527ad6b0088e29f751e3422002 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 22 Oct 2026 11:21:52 +0000 Subject: [PATCH 11/11] node:http: the test of a request body behind a FIN that TLS held runs on Linux only, where the kernel leaves a part of the write every time --- .../js/node/http/node-http-server-socket-end-drain.test.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test/js/node/http/node-http-server-socket-end-drain.test.ts b/test/js/node/http/node-http-server-socket-end-drain.test.ts index 4c2727520a9b..acb258d2d6ae 100644 --- a/test/js/node/http/node-http-server-socket-end-drain.test.ts +++ b/test/js/node/http/node-http-server-socket-end-drain.test.ts @@ -1,6 +1,6 @@ import { setSocketOptions } from "bun:internal-for-testing"; import { describe, expect, test } from "bun:test"; -import { bunEnv, bunExe, isWindows, tls as tlsCert } from "harness"; +import { bunEnv, bunExe, isLinux, tls as tlsCert } from "harness"; import { once } from "node:events"; import http, { type IncomingMessage, type ServerResponse } from "node:http"; import https from "node:https"; @@ -181,9 +181,10 @@ describe.each(["http", "https"] as const)("%s: the request body behind the FIN o ]; // TLS holds a part of the response, and the FIN behind it. Without TLS uWS holds that part, and closes behind it. - // Windows takes a write whole whatever the send buffer: the response is out at once, and uWS closes behind it. + // Only on Linux does the kernel leave a part of the write every time. When it takes it whole (Windows, at times macOS), + // the response is out at once, and uWS closes behind it. test - .skipIf(protocol === "http" || isWindows) + .skipIf(protocol === "http" || !isLinux) .concurrent("the body reaches the request behind a response that the send buffer does not take whole", async () => { const { promise, resolve } = Promise.withResolvers>(); await using server = createServer((req, res) => {