node:net: defer unref() of a connecting socket to the connect event

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?.();
   }
 }
 

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);
+});