From edad317a55482bddfd84201e3c28a26ac431da19 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 15 Aug 2026 09:55:07 +0000 Subject: [PATCH 1/4] node:net: reset the stream state when connect() reuses a half-closed socket Socket.prototype.connect only ran initSocketHandle (which _undestroy()s the Duplex) when it created a new handle, i.e. on a fresh socket or one that had already been destroyed. A socket whose previous connection was still being torn down (end() called, or the peer's FIN received on an allowHalfOpen socket) kept its handle, so the native side replaced the connection but the stream kept the ended/finished flags of the previous one: 'connect' fired and the first write failed with ERR_STREAM_WRITE_AFTER_END, and a readable side that had ended never emitted 'end' again. Run initSocketHandle on every connect(), and also drop the cached _peername there so remoteAddress/remotePort describe the new peer. The "should allow reconnecting after end()" test reconnected 3ms after end()'s callback and only passed when the previous connection had finished closing by then (it flaked on slow debug builds); it now reconnects from the callback itself, which reuses the half-closed handle deterministically. --- src/js/node/net.ts | 14 +- test/js/node/net/node-net.test.ts | 224 +++++++++++++++++++++++++++--- 2 files changed, 216 insertions(+), 22 deletions(-) diff --git a/src/js/node/net.ts b/src/js/node/net.ts index 1e9d83b4dda9..a454daa1ff76 100644 --- a/src/js/node/net.ts +++ b/src/js/node/net.ts @@ -2134,8 +2134,15 @@ Socket.prototype.connect = function connect(...args) { if (!this._handle) { this._handle = newDetachedSocket(typeof this[bunTlsSymbol] === "function"); - initSocketHandle(this); } + // Unlike node (where connecting a handle that is still connected fails with + // EISCONN), a handle whose previous connection is still open or half-closed is + // reused: doConnect closes that connection and connects the same handle again. + // The stream state has to be re-initialized for it as well, otherwise a + // connect() issued after end() but before the previous connection finished + // closing keeps its ended/finished state and the first write on the new + // connection fails with ERR_STREAM_WRITE_AFTER_END. + initSocketHandle(this); if (!pipe) { lookupAndConnect(this, options); @@ -4261,10 +4268,13 @@ function normalizeArgs(args: unknown[]): [options: Record, cb: return arr; } -// Called when creating new Socket, or when re-using a closed Socket +// Called before every connect(): on a new Socket, and whenever a Socket is +// re-used for another connection (after a close, or while the previous +// connection is still being torn down). function initSocketHandle(self) { self._undestroy(); self._sockname = null; + self._peername = null; self[kclosed] = false; self[kended] = false; diff --git a/test/js/node/net/node-net.test.ts b/test/js/node/net/node-net.test.ts index eadca761d75b..e8c5488854f8 100644 --- a/test/js/node/net/node-net.test.ts +++ b/test/js/node/net/node-net.test.ts @@ -463,33 +463,217 @@ describe("net.Socket write", () => { }), ); + function listen(server: Server) { + return new Promise(resolve => + server.listen(0, "127.0.0.1", () => resolve((server.address() as import("node:net").AddressInfo).port)), + ); + } + it("should allow reconnecting after end()", async () => { - const server = new Server(socket => socket.end()); - const port = await new Promise(resolve => { - server.once("listening", () => resolve(server.address().port)); - server.listen(); + // #7325: the same net.Socket is reconnected from end()'s callback. That + // callback runs before the peer's FIN has been read, so the socket is not + // destroyed yet and connect() reuses the handle of the half-closed + // connection. (This used to reconnect 3ms after the callback instead, which + // only passed when the previous connection had finished closing by then.) + const iterations = 10; + const connections: Socket[] = []; + const server = createServer(c => { + connections.push(c); + c.on("error", () => {}); + c.end(); }); - const socket = new Socket(); - socket.on("data", data => console.log(data.toString())); - socket.on("error", err => console.error(err)); - - async function run() { - return new Promise((resolve, reject) => { - socket.once("connect", (...args) => { - socket.write("script\n", err => { - if (err) return reject(err); - socket.end(() => setTimeout(resolve, 3)); - }); + const errors: Error[] = []; + socket.on("error", err => errors.push(err)); + socket.resume(); + const results: unknown[] = []; + try { + const port = await listen(server); + for (let i = 0; i < iterations; i++) { + socket.connect(port, "127.0.0.1"); + await once(socket, "connect"); + const readyState = socket.readyState; + const writeError = await new Promise(resolve => socket.write("script\n", resolve)); + const endError = await new Promise(resolve => socket.end(resolve)); + results.push({ readyState, writeError: writeError?.message ?? null, endError: endError?.message ?? null }); + } + expect(results).toEqual(Array(iterations).fill({ readyState: "open", writeError: null, endError: null })); + expect(errors).toEqual([]); + } finally { + socket.destroy(); + for (const c of connections) c.destroy(); + server.close(); + } + }); + + describe("connect() while the previous connection is half-closed", () => { + // connect() on a socket that has not been destroyed replaces the connection + // underneath the same handle; the per-connection stream state has to start + // over with it. One side of each connection stays open (allowHalfOpen on + // the server, or on the client) so that the client socket is still not + // destroyed when it is reconnected. + it("re-opens the writable side after end()", async () => { + const connections: Socket[] = []; + const received: { connection: number; data: string }[] = []; + let onData: ((data: string) => void) | undefined; + const server = createServer({ allowHalfOpen: true }, c => { + connections.push(c); + const connection = connections.length; + c.on("error", () => {}); + c.setEncoding("utf8"); + c.on("data", (data: string) => { + received.push({ connection, data }); + onData?.(data); }); + }); + const socket = new Socket(); + const errors: Error[] = []; + socket.on("error", err => errors.push(err)); + function writeAndWaitForServer(data: string) { + const { promise, resolve } = Promise.withResolvers(); + onData = resolve; + socket.write(data); + return promise; + } + try { + const port = await listen(server); + socket.connect(port, "127.0.0.1"); + await once(socket, "connect"); + await writeAndWaitForServer("first"); + socket.end(); + await once(socket, "finish"); + expect({ + destroyed: socket.destroyed, + writableEnded: socket.writableEnded, + readyState: socket.readyState, + }).toEqual({ destroyed: false, writableEnded: true, readyState: "readOnly" }); + + socket.connect(port, "127.0.0.1"); + await once(socket, "connect"); + expect({ + writableEnded: socket.writableEnded, + writableFinished: socket.writableFinished, + readyState: socket.readyState, + }).toEqual({ writableEnded: false, writableFinished: false, readyState: "open" }); + await writeAndWaitForServer("second"); + expect(received).toEqual([ + { connection: 1, data: "first" }, + { connection: 2, data: "second" }, + ]); + expect(errors).toEqual([]); + } finally { + socket.destroy(); + for (const c of connections) c.destroy(); + server.close(); + } + }); + + it("re-opens the writable side after end() (unix socket path)", async () => { + const connections: Socket[] = []; + const server = createServer({ allowHalfOpen: true }, c => { + connections.push(c); + c.on("error", () => {}); }); - } + const socketPath = join(socket_domain, "reconnect.sock"); + const socket = new Socket(); + const errors: Error[] = []; + socket.on("error", err => errors.push(err)); + try { + await new Promise(r => server.listen(socketPath, r)); + + socket.connect(socketPath); + await once(socket, "connect"); + socket.end(); + await once(socket, "finish"); + + socket.connect(socketPath); + await once(socket, "connect"); + const writeError = await new Promise(resolve => socket.write("again", resolve)); + expect({ readyState: socket.readyState, writeError: writeError?.message ?? null }).toEqual({ + readyState: "open", + writeError: null, + }); + expect(errors).toEqual([]); + } finally { + socket.destroy(); + for (const c of connections) c.destroy(); + server.close(); + } + }); - for (let i = 0; i < 10; i++) { - await run(); - } - server.close(); + it("re-opens the readable side after the peer ended an allowHalfOpen socket", async () => { + const connections: Socket[] = []; + const server = createServer(c => { + connections.push(c); + c.on("error", () => {}); + c.end(); + }); + const socket = new Socket({ allowHalfOpen: true }); + const errors: Error[] = []; + socket.on("error", err => errors.push(err)); + socket.resume(); + try { + const port = await listen(server); + + socket.connect(port, "127.0.0.1"); + await once(socket, "end"); + expect({ + destroyed: socket.destroyed, + readableEnded: socket.readableEnded, + readyState: socket.readyState, + }).toEqual({ destroyed: false, readableEnded: true, readyState: "writeOnly" }); + + const endedAgain = once(socket, "end"); + socket.connect(port, "127.0.0.1"); + await once(socket, "connect"); + expect({ readableEnded: socket.readableEnded, readyState: socket.readyState }).toEqual({ + readableEnded: false, + readyState: "open", + }); + // The new connection's FIN is reported as a fresh 'end'. + await endedAgain; + expect(connections).toHaveLength(2); + expect(errors).toEqual([]); + } finally { + socket.destroy(); + for (const c of connections) c.destroy(); + server.close(); + } + }); + + it("reports the address of the new peer", async () => { + const connections: Socket[] = []; + const onConnection = (c: Socket) => { + connections.push(c); + c.on("error", () => {}); + }; + const server1 = createServer({ allowHalfOpen: true }, onConnection); + const server2 = createServer({ allowHalfOpen: true }, onConnection); + const socket = new Socket(); + const errors: Error[] = []; + socket.on("error", err => errors.push(err)); + try { + const port1 = await listen(server1); + const port2 = await listen(server2); + + socket.connect(port1, "127.0.0.1"); + await once(socket, "connect"); + expect(socket.remotePort).toBe(port1); + socket.end(); + await once(socket, "finish"); + + socket.connect(port2, "127.0.0.1"); + await once(socket, "connect"); + expect(socket.remotePort).toBe(port2); + expect(errors).toEqual([]); + } finally { + socket.destroy(); + for (const c of connections) c.destroy(); + server1.close(); + server2.close(); + } + }); }); // Client-mode `Handlers.markInactive()` frees the per-connection Handlers From 44e6a05caa3e5d52fa4a64fb71dff38eab8cc52d Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 15 Aug 2026 10:18:24 +0000 Subject: [PATCH 2/4] node:net: drop the name clears initSocketHandle now covers; reject on listen errors in the reconnect tests --- src/js/node/net.ts | 2 -- test/js/node/net/node-net.test.ts | 12 ++++++++---- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/js/node/net.ts b/src/js/node/net.ts index a454daa1ff76..971f3a9d0b8e 100644 --- a/src/js/node/net.ts +++ b/src/js/node/net.ts @@ -2122,8 +2122,6 @@ Socket.prototype.connect = function connect(...args) { } if (this.destroyed) { this._handle = null; - this._peername = null; - this._sockname = null; } this.connecting = true; diff --git a/test/js/node/net/node-net.test.ts b/test/js/node/net/node-net.test.ts index e8c5488854f8..43e4fd9e9a6a 100644 --- a/test/js/node/net/node-net.test.ts +++ b/test/js/node/net/node-net.test.ts @@ -464,9 +464,10 @@ describe("net.Socket write", () => { ); function listen(server: Server) { - return new Promise(resolve => - server.listen(0, "127.0.0.1", () => resolve((server.address() as import("node:net").AddressInfo).port)), - ); + return new Promise((resolve, reject) => { + server.once("error", reject); + server.listen(0, "127.0.0.1", () => resolve((server.address() as import("node:net").AddressInfo).port)); + }); } it("should allow reconnecting after end()", async () => { @@ -580,7 +581,10 @@ describe("net.Socket write", () => { const errors: Error[] = []; socket.on("error", err => errors.push(err)); try { - await new Promise(r => server.listen(socketPath, r)); + await new Promise((resolve, reject) => { + server.once("error", reject); + server.listen(socketPath, resolve); + }); socket.connect(socketPath); await once(socket, "connect"); From 33e982c92f7776ee0917f682e2660c5ae430cb67 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 15 Aug 2026 12:10:43 +0000 Subject: [PATCH 3/4] node:net: shorten the initSocketHandle comments --- src/js/node/net.ts | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/src/js/node/net.ts b/src/js/node/net.ts index 971f3a9d0b8e..301e831b885e 100644 --- a/src/js/node/net.ts +++ b/src/js/node/net.ts @@ -2133,13 +2133,8 @@ Socket.prototype.connect = function connect(...args) { if (!this._handle) { this._handle = newDetachedSocket(typeof this[bunTlsSymbol] === "function"); } - // Unlike node (where connecting a handle that is still connected fails with - // EISCONN), a handle whose previous connection is still open or half-closed is - // reused: doConnect closes that connection and connects the same handle again. - // The stream state has to be re-initialized for it as well, otherwise a - // connect() issued after end() but before the previous connection finished - // closing keeps its ended/finished state and the first write on the new - // connection fails with ERR_STREAM_WRITE_AFTER_END. + // Also for a reused handle: doConnect replaces the connection it still + // carries, so the stream state of that connection has to go with it. initSocketHandle(this); if (!pipe) { @@ -4266,9 +4261,7 @@ function normalizeArgs(args: unknown[]): [options: Record, cb: return arr; } -// Called before every connect(): on a new Socket, and whenever a Socket is -// re-used for another connection (after a close, or while the previous -// connection is still being torn down). +// Called on every connect(): a new Socket, or one re-used for another connection. function initSocketHandle(self) { self._undestroy(); self._sockname = null; From 5314d9e5b12f8eb52abcf1871843f7f724130de3 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 15 Aug 2026 12:12:13 +0000 Subject: [PATCH 4/4] node:net: one-line comment on the unconditional initSocketHandle call --- src/js/node/net.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/js/node/net.ts b/src/js/node/net.ts index 301e831b885e..d1a65fc69963 100644 --- a/src/js/node/net.ts +++ b/src/js/node/net.ts @@ -2133,8 +2133,7 @@ Socket.prototype.connect = function connect(...args) { if (!this._handle) { this._handle = newDetachedSocket(typeof this[bunTlsSymbol] === "function"); } - // Also for a reused handle: doConnect replaces the connection it still - // carries, so the stream state of that connection has to go with it. + // A reused handle gets a new connection from doConnect, so it is reset as well. initSocketHandle(this); if (!pipe) {