fix(http): preserve server reference across close() for closeAllConnections()

src/js/node/_http_server.ts

@@ -75,6 +75,11 @@
 const { kIncomingMessage } = require("node:_http_common");
 const kConnectionsCheckingInterval = Symbol("http.server.connectionsCheckingInterval");
 const kTrackedConnections = Symbol("http.server.trackedConnections");
+// Set synchronously in close(); tracks whether the server has entered
+// the shutdown sequence, so address()/close() behave like Node even
+// while the underlying Bun.serve reference is still populated for
+// closeAllConnections()/unref() to reach.
+const kServerClosed = Symbol("http.server.closed");
 
 const getBunServerAllClosedPromise = $newZigFunction("node_http_binding.zig", "getBunServerAllClosedPromise", 1);
 const sendHelper = $newZigFunction("node_cluster_binding.zig", "sendHelperChild", 3);
@@ -95,7 +100,22 @@
   callCloseCallback(self);
   self.emit("close");
 }
-function emitCloseNTServer(this: Server) {
+function emitCloseNTServer(this: Server, closingServer) {
+  // The underlying Bun server has finished shutting down — drop our
+  // reference so the allClosed promise chain can settle and the event
+  // loop can exit. msal and other consumers call
+  // `close() → closeAllConnections() → unref()` in sequence; the
+  // underlying reference has to survive past `close()` so the follow-up
+  // calls can still reach the native layer.
+  //
+  // Guard against re-listen: if the caller already started a fresh
+  // `listen()` before this callback fired, `this[serverSymbol]` now
+  // points at a NEW Bun.serve handle — nulling it would trash the new
+  // listener. Only clear when the handle still matches the one whose
+  // allClosed promise we're resolving.
+  if (this[serverSymbol] === closingServer) {
+    this[serverSymbol] = undefined;
+  }
   process.nextTick(emitCloseServer, this);
 }
 
@@ -351,10 +371,15 @@
   if (!server) {
     return;
   }
-  this[serverSymbol] = undefined;
+  this[kServerClosed] = true;
   clearInterval(this[kConnectionsCheckingInterval]);
   this.listening = false;
 
+  // Abrupt stop — force-closes any open connections, including the ones
+  // a graceful `close()` would have left open. We keep `this[serverSymbol]`
+  // populated so subsequent calls (e.g. `unref()`) still reach the native
+  // server; it's cleared in `emitCloseNTServer` once everything actually
+  // drains.
   server.stop(true);
 };
 
@@ -378,15 +403,20 @@
   // Node.js's httpServerPreClose clears the connections-checking interval
   // even when the server was never listening.
   clearInterval(this[kConnectionsCheckingInterval]);
-  if (!server) {
+  if (!server || this[kServerClosed]) {
     if (typeof optionalCallback === "function") process.nextTick(optionalCallback, $ERR_SERVER_NOT_RUNNING());
     return;
   }
-  this[serverSymbol] = undefined;
+  this[kServerClosed] = true;
   if (typeof optionalCallback === "function") setCloseCallback(this, optionalCallback);
   this.listening = false;
+  // Graceful stop: stop accepting, close idle connections. Callers like
+  // `@azure/msal-node` follow this with `closeAllConnections()` to force-
+  // close remaining in-flight sockets, so we must NOT null out
+  // `this[serverSymbol]` here — that reference is still needed by the
+  // subsequent `closeAllConnections()` / `unref()` calls.
   server.closeIdleConnections();
   server.stop();
 };
 
 Server.prototype[EventEmitter.captureRejectionSymbol] = function (err, event, ...args) {
@@ -426,7 +456,12 @@
 };
 
 Server.prototype.address = function () {
-  if (!this[serverSymbol]) return null;
+  // Node returns null from address() once close() has been called, even
+  // if draining isn't finished. We keep `this[serverSymbol]` populated
+  // past `close()` so the msal pattern `close() → closeAllConnections()`
+  // still reaches the native layer — `kServerClosed` is the "has close
+  // been called" signal.
+  if (!this[serverSymbol] || this[kServerClosed]) return null;
   return this[serverSymbol].address;
 };
 
@@ -541,6 +576,10 @@
     if (tls) {
       this.serverName = tls.serverName || host || "localhost";
     }
+    // Reset the "has close been called" flag — Node allows listening
+    // again after close, so address()/close() need to behave normally
+    // on the fresh listen.
+    this[kServerClosed] = false;
     this[serverSymbol] = Bun.serve<any>({
       idleTimeout: 0, // nodejs dont have a idleTimeout by default
       tls,
@@ -877,8 +916,13 @@
       // },
     });
 
-    getBunServerAllClosedPromise(this[serverSymbol]).$then(emitCloseNTServer.bind(this));
-    isHTTPS = this[serverSymbol].protocol === "https";
+    // Capture the Bun.serve handle for this listen generation so the
+    // close callback can tell whether it's still the current one (see
+    // `emitCloseNTServer`). Without this, a fresh `listen()` that races
+    // the previous shutdown would have its new handle nulled out.
+    const bunServer = this[serverSymbol];
+    getBunServerAllClosedPromise(bunServer).$then(emitCloseNTServer.bind(this, bunServer));
+    isHTTPS = bunServer.protocol === "https";
     // always set strict method validation to true for node.js compatibility
     setServerCustomOptions(
       this[serverSymbol],

src/runtime/server/mod.rs

@@ -1539,6 +1539,28 @@ impl<const SSL: bool, const DEBUG: bool> NewServer<SSL, DEBUG> {
         }
 
         let Some(listener) = self.listener.take() else {
+            // Upgrading from graceful (`stop(false)`) to abrupt:
+            // `stop(false)` already took the listener, but the uWS App
+            // still owns open/idle keep-alive connections that would
+            // otherwise pin the event loop. `app.close()` force-closes
+            // every remaining socket on the app. This is the path that
+            // makes `server.close(); server.closeAllConnections();`
+            // actually drain msal-style loopback servers.
+            //
+            // This runs BEFORE the h3 branch below, because the h3 branch
+            // flips `TERMINATED` on abrupt stops, which would make our
+            // `!TERMINATED` guard false and skip the app close for
+            // h3-enabled servers (HTTPS + h3: true).
+            if abrupt && !self.flags.contains(ServerFlags::TERMINATED) {
+                if let Some(app) = self.app {
+                    if let Some(ws) = self.config.websocket.as_mut() {
+                        ws.handler.app = None;
+                    }
+                    self.flags.insert(ServerFlags::TERMINATED);
+                    // S012: `NewApp<SSL>` is a ZST opaque — safe deref.
+                    bun_opaque::opaque_deref_mut(app).close();
+                }
+            }
             if Self::HAS_H3 && self.h3_app.is_some() {
                 self.unref();
                 self.notify_inspector_server_stopped();

src/runtime/server/server_body.rs

@@ -2499,23 +2499,39 @@ where
     pub fn stop_from_js(&mut self, abruptly: Option<JSValue>) -> JSValue {
         let rc = self.get_all_closed_promise(&self.global());
 
-        if self.has_listener() {
-            let abrupt = 'brk: {
-                if let Some(val) = abruptly {
-                    if val.is_boolean() && val.to_boolean() {
-                        break 'brk true;
-                    }
+        let abrupt = 'brk: {
+            if let Some(val) = abruptly {
+                if val.is_boolean() && val.to_boolean() {
+                    break 'brk true;
                 }
-                false
-            };
-            self.stop(abrupt);
+            }
+            false
+        };
+
+        // For an abrupt stop, the app may still have open connections
+        // even after a graceful `stop(false)` cleared `self.listener`.
+        // `stop_listening` force-closes those via `app.close()` only when
+        // it actually enters the abrupt branch, so we can't gate on
+        // `has_listener()` here. `stop`/`stop_listening` are idempotent:
+        // second calls short-circuit via the `terminated` flag and the
+        // `deinit_scheduled` flag, so this is safe.
+        if abrupt {
+            if self.has_listener()
+                || (self.app.is_some() && !self.flags.contains(ServerFlags::TERMINATED))
+            {
+                self.stop(true);
+            }
+        } else if self.has_listener() {
+            self.stop(false);
         }
 
         rc
     }
 
     pub fn dispose_from_js(&mut self) -> JSValue {
-        if self.has_listener() {
+        if self.has_listener()
+            || (self.app.is_some() && !self.flags.contains(ServerFlags::TERMINATED))
+        {
             self.stop(true);
         }
         JSValue::UNDEFINED

test/js/node/http/node-http-close-all-connections.test.ts

@@ -0,0 +1,120 @@
+import { expect, test } from "bun:test";
+import { bunEnv, bunExe } from "harness";
+import { once } from "node:events";
+import http from "node:http";
+import type { AddressInfo } from "node:net";
+import { connect } from "node:net";
+
+// Regression: `@azure/msal-node`'s `LoopbackClient.closeServer` calls
+//   server.close();
+//   server.closeAllConnections();
+//   server.unref();
+// in sequence. Bun used to null out the internal server reference in
+// `close()`, so the subsequent `closeAllConnections()` was a no-op —
+// the keep-alive socket kept the event loop alive and the process hung.
+// Issue: https://github.com/oven-sh/bun/issues/30501
+test("closeAllConnections() after close() force-closes in-flight sockets", async () => {
+  const { promise: requestReceived, resolve: resolveReceived } = Promise.withResolvers<void>();
+  const server = http.createServer((req, _res) => {
+    // Signal receipt but DO NOT reply — socket is "in flight" (not idle)
+    // when the teardown sequence runs. This is the case where close()
+    // alone (which only closes idle connections) cannot reclaim the
+    // socket, and closeAllConnections() must do it.
+    resolveReceived();
+  });
+  await once(server.listen(0, "127.0.0.1"), "listening");
+  const { port } = server.address() as AddressInfo;
+
+  const sock = connect(port, "127.0.0.1");
+  const { promise: sockClosed, resolve: resolveClosed } = Promise.withResolvers<void>();
+  sock.on("close", () => resolveClosed());
+  sock.on("error", () => {});
+  await once(sock, "connect");
+  sock.write("GET / HTTP/1.1\r\nHost: localhost\r\nConnection: keep-alive\r\n\r\n");
+  await requestReceived;
+
+  // msal-style teardown — no waiting between calls
+  server.close();
+  server.closeAllConnections();
+  server.unref();
+
+  // The client socket must be force-closed by closeAllConnections().
+  // Without the fix, the socket stays open indefinitely (msal hang).
+  await sockClosed;
+});
+
+// Regression: when a caller does close() and then listen() again before
+// the previous shutdown's allClosed promise has fulfilled, the stale
+// callback must not null out the newly-created server handle.
+test("listen() during an in-flight close() doesn't corrupt the new server", async () => {
+  const server = http.createServer((_req, res) => res.end("ok"));
+
+  await once(server.listen(0, "127.0.0.1"), "listening");
+
+  // Fire-and-forget close; don't wait for the allClosed callback.
+  server.close();
+
+  // Re-listen immediately while the previous shutdown is still settling.
+  await once(server.listen(0, "127.0.0.1"), "listening");
+  const secondAddress = server.address() as AddressInfo | null;
+  expect(secondAddress).not.toBeNull();
+  const secondPort = secondAddress!.port;
+  expect(secondPort).toBeInteger();
+
+  // Drain the new server — address() must still return a port after
+  // microtasks run (this is where the stale close callback would have
+  // hit, pre-fix).
+  await new Promise(r => setImmediate(r));
+  expect((server.address() as AddressInfo | null)?.port).toBe(secondPort);
+
+  // Confirm the new server actually serves requests, not just has a port.
+  const res = await fetch(`http://127.0.0.1:${secondPort}`);
+  expect(await res.text()).toBe("ok");
+
+  await new Promise<void>(r => server.close(() => r()));
+});
+
+// End-to-end: spawn a child that opens an HTTP server, accepts a
+// keep-alive connection, and calls the msal teardown. Must exit
+// immediately — not wait for the keep-alive idle timeout to reclaim
+// the in-flight socket.
+test("process exits after close() + closeAllConnections() + unref() teardown", async () => {
+  await using proc = Bun.spawn({
+    cmd: [
+      bunExe(),
+      "-e",
+      `
+        const http = require("node:http");
+        const net = require("node:net");
+        const server = http.createServer((req, _res) => {
+          // Never reply — keep the socket in-flight (not idle) so that
+          // only closeAllConnections() (abrupt) can reclaim it.
+          server.close();
+          server.closeAllConnections();
+          server.unref();
+          process.stdout.write("TEARDOWN_DONE\\n");
+        });
+        server.listen(0, "127.0.0.1", () => {
+          const port = server.address().port;
+          const sock = net.connect(port, "127.0.0.1", () => {
+            sock.write("GET / HTTP/1.1\\r\\nHost: localhost\\r\\nConnection: keep-alive\\r\\n\\r\\n");
+          });
+          sock.on("data", () => {});
+          sock.on("error", () => {});
+        });
+      `,
+    ],
+    env: bunEnv,
+    stdout: "pipe",
+    stderr: "pipe",
+  });
+  // If the teardown path is broken, the subprocess never exits and
+  // `proc.exited` never resolves — the bun:test runner's default 5s
+  // timeout catches that.
+  const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+  // Surface any uncaught exception / ASAN trace before the exit-code
+  // assertion so failures point at the real cause.
+  expect(stderr).toBe("");
+  expect(stdout).toContain("TEARDOWN_DONE");
+  expect(exitCode).toBe(0);
+});