diff --git a/src/js/node/net.ts b/src/js/node/net.ts index 81f7b6317db9..38aea8b7394f 100644 --- a/src/js/node/net.ts +++ b/src/js/node/net.ts @@ -156,6 +156,8 @@ const kPerfHooksNetConnectContext = Symbol("kPerfHooksNetConnectContext"); const khandshakeTimer = Symbol("khandshakeTimer"); const kerrorEmitted = Symbol("kerrorEmitted"); const kUserUnrefed = Symbol("kUserUnrefed"); +const kPendingDeferredUnref = Symbol("kPendingDeferredUnref"); +const kPendingDeferredRef = Symbol("kPendingDeferredRef"); // Set when pause() dropped the handle's hold on the loop, so the read paths // only restore a hold they actually removed - re-refing a handle that never // held the loop (a wrapped duplex with no fd) would pin the process. @@ -2502,7 +2504,10 @@ Socket.prototype.ref = function ref() { this[kUserUnrefed] = false; const socket = this._handle; if (!socket) { - this.once("connect", this.ref); + if (!this[kPendingDeferredRef]) { + this[kPendingDeferredRef] = true; + this.once("connect", applyDeferredRef); + } return this; } socket.ref(); @@ -2695,14 +2700,31 @@ Socket.prototype._unrefTimer = function _unrefTimer() { Socket.prototype.unref = function unref() { this[kUserUnrefed] = true; const socket = this._handle; - if (!socket) { - this.once("connect", this.unref); + if (!socket || this.connecting) { + // Node's pending uv_connect_t keeps the loop alive even when the handle is + // unref'd; our handle has no request concept, so apply the unref once + // "connect" fires (this also covers autoSelectFamily retry handles). The + // listener re-checks kUserUnrefed, so one suffices and a later ref() wins. + if (!this[kPendingDeferredUnref]) { + this[kPendingDeferredUnref] = true; + this.once("connect", applyDeferredUnref); + } return this; } socket.unref(); return this; }; +function applyDeferredUnref(this: any) { + this[kPendingDeferredUnref] = false; + if (this[kUserUnrefed]) this.unref(); +} + +function applyDeferredRef(this: any) { + this[kPendingDeferredRef] = false; + if (!this[kUserUnrefed]) this.ref(); +} + // https://github.com/nodejs/node/blob/2eff28fb7a93d3f672f80b582f664a7c701569fb/lib/net.js#L785 Socket.prototype.destroySoon = function destroySoon() { if (this.writable) this.end(); @@ -4089,8 +4111,6 @@ function initSocketHandle(self) { const handle = self._handle; if (handle) { handle[owner_symbol] = self; - // A fresh handle (e.g. an autoSelectFamily retry) inherits a prior unref(). - if (self[kUserUnrefed]) handle.unref?.(); } } diff --git a/test/regression/issue/37086.test.ts b/test/regression/issue/37086.test.ts new file mode 100644 index 000000000000..f23d7e9d12ee --- /dev/null +++ b/test/regression/issue/37086.test.ts @@ -0,0 +1,95 @@ +import { expect, test } from "bun:test"; +import { bunEnv, bunExe } from "harness"; + +// https://github.com/oven-sh/bun/issues/37086 +// A pending connect must keep the event loop alive even when the socket was +// unref'd: Node defers the unref until the "connect" event fires. + +test.concurrent("net.Socket#unref() before connect() does not exit before the connection completes", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + `const net = require("node:net"); + const server = net.createServer(() => {}); + server.listen(0, "127.0.0.1", () => { + const s = new net.Socket(); + s.unref(); // before connect() + s.connect(server.address().port, "127.0.0.1", () => { + console.log("CONNECTED"); + s.destroy(); + server.close(); + }); + }); + server.unref();`, + ], + env: bunEnv, + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + expect(stdout).toBe("CONNECTED\n"); + expect(exitCode).toBe(0); +}); + +test.concurrent("net.Socket#unref() while connecting does not exit before the connection completes", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + `const net = require("node:net"); + const server = net.createServer(() => {}); + server.listen(0, "127.0.0.1", () => { + const s = new net.Socket(); + s.connect(server.address().port, "127.0.0.1", () => { + console.log("CONNECTED"); + s.destroy(); + server.close(); + }); + s.unref(); // handle exists, connection still pending + }); + server.unref();`, + ], + env: bunEnv, + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + expect(stdout).toBe("CONNECTED\n"); + expect(exitCode).toBe(0); +}); + +test.concurrent("net.Socket#ref() after a pre-connect unref() keeps the socket holding the loop", async () => { + // unref() then ref() before connect: the deferred unref must not win over + // the later ref(), so the established socket still holds the event loop + // open until it is destroyed. + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + `const net = require("node:net"); + const server = net.createServer(socket => { + // Close from the server side once the client is established; the + // ref'd client must stay alive to observe it. + socket.end(); + }); + server.listen(0, "127.0.0.1", () => { + const s = new net.Socket(); + s.unref(); + s.ref(); + s.connect(server.address().port, "127.0.0.1", () => console.log("CONNECTED")); + s.on("close", () => { + console.log("CLOSED"); + server.close(); + }); + }); + server.unref();`, + ], + env: bunEnv, + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + expect(stdout).toBe("CONNECTED\nCLOSED\n"); + expect(exitCode).toBe(0); +});