From ea13e645da554c0ccadd3a7e6207ef580aeb9b56 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 21 Sep 2026 16:10:37 +0000 Subject: [PATCH 1/3] node:net: finish an accepted socket whose native close leaves a write parked A send() that fails with the peer's reset consumes the socket error. The loop then sees a plain hangup, so the native close carries no error. The server-side close path failed a parked write only when the close had an error or the socket was already destroyed, because the same function also serves a half-open 'end', whose write can still drain. With neither, the socket emitted 'end' and nothing else: no write callback, no 'error', no 'close', and the server counted it until the process exited. Tell the two apart by kclosed, which only the native close handlers set. A parked write on a closed handle now fails with ERR_SOCKET_CLOSED, as it already does on the client side, and 'error' and 'close' follow. --- src/js/node/net.ts | 4 +- test/js/node/tls/node-tls-server.test.ts | 61 ++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 1 deletion(-) diff --git a/src/js/node/net.ts b/src/js/node/net.ts index 27fa8b16ffd0..904facfe16b6 100644 --- a/src/js/node/net.ts +++ b/src/js/node/net.ts @@ -874,8 +874,10 @@ function SocketEmitEndNT(self, _err?) { } // A write that was waiting on the native drain can never complete once the // socket is gone - fail it so 'finish'/destroy are not stuck behind it. + // kclosed, not _err: a native close can carry no error. A half-open 'end' + // also lands here, and its write stays parked because it can still drain. const pendingWrite = self[kwriteCallback]; - if (pendingWrite && (self.destroyed || _err)) { + if (pendingWrite && (self[kclosed] || self.destroyed)) { self[kwriteCallback] = null; pendingWrite(_err ?? $ERR_SOCKET_CLOSED()); } diff --git a/test/js/node/tls/node-tls-server.test.ts b/test/js/node/tls/node-tls-server.test.ts index 707474890f20..8cd700f94115 100644 --- a/test/js/node/tls/node-tls-server.test.ts +++ b/test/js/node/tls/node-tls-server.test.ts @@ -2685,3 +2685,64 @@ describe("pauseOnConnect", () => { } }); }); + +it("an accepted socket emits 'close' when a write is the first to see the peer's reset", async () => { + // A send() that fails with the reset consumes the socket error, so the loop then sees a + // plain hangup and the native close carries no error. With the rest of the write still + // waiting for a drain, the socket emitted 'end' and nothing else: no write callback, no + // 'error', no 'close', and the server counted it forever. + // + // The server is a child process so that it can stop polling: it reports the accepted + // socket, then blocks on stdin until this process has reset the connection. + const serverScript = ` + const tls = require("node:tls"); + const fs = require("node:fs"); + const events = []; + const server = tls.createServer(${JSON.stringify(COMMON_CERT)}, socket => { + socket.on("error", () => events.push("error")); + socket.on("close", hadError => { + events.push("close:" + hadError); + server.getConnections((err, connections) => { + console.log(JSON.stringify({ events, connections })); + server.close(); + }); + }); + socket.resume(); + fs.writeSync(1, "accepted\\n"); + fs.readSync(0, Buffer.alloc(1)); + // More than the TLS layer takes once the wire rejects a record, so the rest is parked. + socket.write(Buffer.alloc(1 << 20)); + }); + server.listen(0, "127.0.0.1", () => fs.writeSync(1, "port=" + server.address().port + "\\n")); + `; + await using proc = Bun.spawn({ + cmd: [bunExe(), "-e", serverScript], + env: bunEnv, + stdin: "pipe", + stdout: "pipe", + stderr: "pipe", + }); + let stdout = ""; + let raw: net.Socket | undefined; + let reset = false; + for await (const chunk of proc.stdout) { + stdout += Buffer.from(chunk).toString(); + const port = /port=(\d+)/.exec(stdout); + if (port && !raw) { + raw = net.connect(Number(port[1]), "127.0.0.1"); + raw.on("error", () => {}); + connect({ socket: raw, rejectUnauthorized: false }).on("error", () => {}); + } + if (raw && !reset && stdout.includes("accepted\n")) { + reset = true; + raw.resetAndDestroy(); + proc.stdin.write("x"); + proc.stdin.flush(); + } + } + const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + const lines = stdout.trim().split("\n"); + expect(JSON.parse(lines[lines.length - 1])).toEqual({ events: ["error", "close:true"], connections: 0 }); + expect(exitCode).toBe(0); +}); From 9f37921079cf3861c8536be2d0bf86ecec5bbfda Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 21 Sep 2026 17:58:59 +0000 Subject: [PATCH 2/3] test: drain the child's stderr while its stdout is scanned The test read stderr only after stdout ended, so a child that logs more than the pipe holds would block on it and the test would hang instead of failing on the stderr assertion. --- test/js/node/tls/node-tls-server.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/js/node/tls/node-tls-server.test.ts b/test/js/node/tls/node-tls-server.test.ts index 8cd700f94115..8200822096e0 100644 --- a/test/js/node/tls/node-tls-server.test.ts +++ b/test/js/node/tls/node-tls-server.test.ts @@ -2722,6 +2722,8 @@ it("an accepted socket emits 'close' when a write is the first to see the peer's stdout: "pipe", stderr: "pipe", }); + // Drain stderr while stdout is scanned, so a child that logs a lot cannot block on it. + const stderrText = proc.stderr.text(); let stdout = ""; let raw: net.Socket | undefined; let reset = false; @@ -2740,7 +2742,7 @@ it("an accepted socket emits 'close' when a write is the first to see the peer's proc.stdin.flush(); } } - const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); + const [stderr, exitCode] = await Promise.all([stderrText, proc.exited]); expect(stderr).toBe(""); const lines = stdout.trim().split("\n"); expect(JSON.parse(lines[lines.length - 1])).toEqual({ events: ["error", "close:true"], connections: 0 }); From 1740dfa71e4871dac1a4c85350737bd43ef3d5f7 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 21 Sep 2026 19:39:02 +0000 Subject: [PATCH 3/3] test: report a socket that never closes from a second connection Without the fix the accepted socket never emits 'close', so the test had no event to wait for and ran into its timeout. The server now reports when a second connection arrives. That handshake takes several turns of the server's loop, and the reset socket closes in the first of them or not at all. The test then fails with the recorded events and the connection count, and it checks that the child exited on its own. --- test/js/node/tls/node-tls-server.test.ts | 29 ++++++++++++++++++++---- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/test/js/node/tls/node-tls-server.test.ts b/test/js/node/tls/node-tls-server.test.ts index 8200822096e0..a672381930ae 100644 --- a/test/js/node/tls/node-tls-server.test.ts +++ b/test/js/node/tls/node-tls-server.test.ts @@ -2694,24 +2694,35 @@ it("an accepted socket emits 'close' when a write is the first to see the peer's // // The server is a child process so that it can stop polling: it reports the accepted // socket, then blocks on stdin until this process has reset the connection. + // + // A socket that never closes gives no event to wait for, so a second connection asks. + // Its handshake takes several turns of the server's loop, and the reset socket closes in + // the first of them or not at all. The server reports when the second connection arrives. const serverScript = ` const tls = require("node:tls"); const fs = require("node:fs"); const events = []; + let accepted; const server = tls.createServer(${JSON.stringify(COMMON_CERT)}, socket => { - socket.on("error", () => events.push("error")); - socket.on("close", hadError => { - events.push("close:" + hadError); + if (accepted) { server.getConnections((err, connections) => { console.log(JSON.stringify({ events, connections })); + // Also the first socket, so that this process exits when it never closed. + accepted.destroy(); + socket.destroy(); server.close(); }); - }); + return; + } + accepted = socket; + socket.on("error", () => events.push("error")); + socket.on("close", hadError => events.push("close:" + hadError)); socket.resume(); fs.writeSync(1, "accepted\\n"); fs.readSync(0, Buffer.alloc(1)); // More than the TLS layer takes once the wire rejects a record, so the rest is parked. socket.write(Buffer.alloc(1 << 20)); + fs.writeSync(1, "wrote\\n"); }); server.listen(0, "127.0.0.1", () => fs.writeSync(1, "port=" + server.address().port + "\\n")); `; @@ -2726,6 +2737,7 @@ it("an accepted socket emits 'close' when a write is the first to see the peer's const stderrText = proc.stderr.text(); let stdout = ""; let raw: net.Socket | undefined; + let probe: net.Socket | undefined; let reset = false; for await (const chunk of proc.stdout) { stdout += Buffer.from(chunk).toString(); @@ -2741,10 +2753,17 @@ it("an accepted socket emits 'close' when a write is the first to see the peer's proc.stdin.write("x"); proc.stdin.flush(); } + if (port && !probe && stdout.includes("wrote\n")) { + probe = connect({ port: Number(port[1]), host: "127.0.0.1", rejectUnauthorized: false }); + probe.on("error", () => {}); + } } const [stderr, exitCode] = await Promise.all([stderrText, proc.exited]); + probe?.destroy(); expect(stderr).toBe(""); const lines = stdout.trim().split("\n"); - expect(JSON.parse(lines[lines.length - 1])).toEqual({ events: ["error", "close:true"], connections: 0 }); + // The one connection left is the second one. + expect(JSON.parse(lines[lines.length - 1])).toEqual({ events: ["error", "close:true"], connections: 1 }); + expect(proc.signalCode).toBeNull(); expect(exitCode).toBe(0); });