Bun.serve: free a gracefully stopped server at VM teardown

src/runtime/server/mod.rs

@@ -276,9 +276,10 @@ pub struct NewServer<const SSL: bool, const DEBUG: bool> {
     /// `AnyServer` handles on the JS thread.
     pub(crate) active_websocket_count: core::cell::Cell<u32>,
     /// Set across [`NewServer::deinit_if_we_can`] and the synchronous
-    /// `app.close()` drain in `stop_listening`; lets a nested call (reached
-    /// via a callback the body fires) early-return instead of re-running the
-    /// downgrade/teardown while the outer frame still holds `&mut self`.
+    /// `app.close()` drains in `stop_listening` and `deinit`; lets a nested
+    /// call (reached via a callback the body fires) early-return instead of
+    /// re-running the downgrade/teardown while the outer frame still holds
+    /// `&mut self`.
     deinit_running: core::cell::Cell<bool>,
     pub(crate) request_pool:
         *mut request_context::RequestContextStackAllocator<Self, SSL, DEBUG, false>,
@@ -2095,6 +2096,20 @@ impl<const SSL: bool, const DEBUG: bool> NewServer<SSL, DEBUG> {
     /// must not be used after this returns.
     pub(super) fn deinit(this: *mut Self) {
         httplog!("deinit");
+        // `NewApp::destroy` below closes nothing itself; close while `this` is
+        // still a raw pointer, since close callbacks re-derive `&Self` from it.
+        // SAFETY: live, uniquely owned server (fn contract); nothing borrows it
+        // across the `close()` call.
+        unsafe {
+            if !(*this).flags.contains(ServerFlags::TERMINATED)
+                && let Some(app) = (*this).app
+            {
+                (*this).flags.insert(ServerFlags::TERMINATED);
+                (*this).deinit_running.set(true);
+                // S012: `NewApp<SSL>` is a ZST opaque — safe `*mut → &mut` deref.
+                bun_opaque::opaque_deref_mut(app).close();
+            }
+        }
         // SAFETY: paired with heap::alloc in `init()`; `this` is uniquely
         // owned here, so reclaim the Box up front and let its `&mut` drive
         // the teardown (dropped — and freed — at scope exit).
@@ -2116,7 +2131,8 @@ impl<const SSL: bool, const DEBUG: bool> NewServer<SSL, DEBUG> {
             }
         }
         if let Some(app) = server.app.take() {
-            // SAFETY: live uws App handle owned by this server.
+            // SAFETY: live uws App handle owned by this server, closed above or
+            // by whoever set `TERMINATED`.
             unsafe { uws_sys::NewApp::<SSL>::destroy(app) };
         }
     }

src/runtime/server/server_body.rs

@@ -2815,13 +2815,12 @@ where
         // unconditional. `Box::into_raw` (not `heap::release`) keeps raw-owner
         // provenance for the `deinit()` dealloc — a `&mut self`-derived tag
         // there would be Stacked-Borrows UB. JSC-handle Drops are no-ops past
-        // `is_shutting_down()`; `TERMINATED` means `app.close()` already ran,
-        // so `NewApp::destroy` can't orphan a keep-alive socket.
+        // `is_shutting_down()`, and teardown has closed every socket by then,
+        // so `deinit()` may destroy the app of a gracefully stopped server too.
         unsafe {
             (*this_ptr).js_value.finalize();
             (*this_ptr).deinit_if_we_can();
             if (*this_ptr).flags.contains(ServerFlags::DEINIT_SCHEDULED)
-                && (*this_ptr).flags.contains(ServerFlags::TERMINATED)
                 && (*this_ptr).vm().is_shutting_down()
             {
                 Self::deinit(this_ptr);

test/js/bun/http/bun-serve-html-405.test.ts

@@ -1,4 +1,4 @@
-import { expect, test } from "bun:test";
+import { describe, expect, test } from "bun:test";
 import { bunEnv, bunExe, isASAN, isWindows, tempDir } from "harness";
 import { join } from "path";
 
@@ -44,6 +44,30 @@ test("dev server html route: non-GET/HEAD requests complete without hanging", as
   }
 });
 
+// The tests below exit a child under LeakSanitizer with BUN_DESTRUCT_VM_ON_EXIT,
+// the way the ASAN CI lane runs every test, and expect a silent, clean exit.
+const leaksanSupp = join(import.meta.dirname, "../../../leaksan.supp");
+
+// LSan symbolizes its report through llvm-symbolizer on failure, which is slow
+// against the debug binary.
+const LEAK_TEST_TIMEOUT = 30_000;
+
+async function exitUnderLeakSanitizer(script: string, { args = [] as string[], suppressions = leaksanSupp } = {}) {
+  await using proc = Bun.spawn({
+    cmd: [bunExe(), "-e", script, ...args],
+    env: {
+      ...bunEnv,
+      BUN_DESTRUCT_VM_ON_EXIT: "1",
+      ASAN_OPTIONS: [bunEnv.ASAN_OPTIONS, "detect_leaks=1"].filter(Boolean).join(":"),
+      LSAN_OPTIONS: `malloc_context_size=30:print_suppressions=0:suppressions=${suppressions}`,
+    },
+    stdout: "pipe",
+    stderr: "pipe",
+  });
+  const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+  return { stdout, stderr, exitCode };
+}
+
 // A server whose JS wrapper survives to lastChanceToFinalize used to leak its
 // NewServer Box: finalize() -> schedule_deinit() enqueued a ManagedTask that
 // the now-exiting event loop never ran. The html_bundle::Route.server
@@ -55,33 +79,20 @@ test.skipIf(!isASAN || isWindows)(
     await using dir = tempDir("serve-static-route-teardown-leak", {
       "index.html": `<!DOCTYPE html><html><body>hi</body></html>`,
     });
-    await using proc = Bun.spawn({
-      cmd: [
-        bunExe(),
-        "-e",
-        `
-          const { default: html } = await import(process.argv[1]);
-          const server = Bun.serve({
-            port: 0,
-            development: true,
-            routes: { "/": html },
-            fetch: () => new Response("no"),
-          });
-          await (await fetch(server.url)).text();
-          server.stop(true);
-        `,
-        join(dir, "index.html"),
-      ],
-      env: {
-        ...bunEnv,
-        BUN_DESTRUCT_VM_ON_EXIT: "1",
-        ASAN_OPTIONS: [bunEnv.ASAN_OPTIONS, "detect_leaks=1"].filter(Boolean).join(":"),
-        LSAN_OPTIONS: `print_suppressions=0:suppressions=${join(import.meta.dirname, "../../../leaksan.supp")}`,
-      },
-      stdout: "pipe",
-      stderr: "pipe",
-    });
-    const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+    const { stdout, stderr, exitCode } = await exitUnderLeakSanitizer(
+      `
+        const { default: html } = await import(process.argv[1]);
+        const server = Bun.serve({
+          port: 0,
+          development: true,
+          routes: { "/": html },
+          fetch: () => new Response("no"),
+        });
+        await (await fetch(server.url)).text();
+        server.stop(true);
+      `,
+      { args: [join(dir, "index.html")] },
+    );
     // The dev-server bundler prints "Bundled page in Nms: <path>" to stderr on
     // first request; strip it so the assertion shows only LSan's report.
     const filtered = stderr
@@ -91,40 +102,115 @@ test.skipIf(!isASAN || isWindows)(
       .trim();
     expect({ stdout, stderr: filtered, exitCode }).toEqual({ stdout: "", stderr: "", exitCode: 0 });
   },
-  // LSan symbolizes through llvm-symbolizer on failure, which is slow against
-  // the debug binary.
-  30_000,
+  LEAK_TEST_TIMEOUT,
 );
 
-// Graceful stop() leaves the keep-alive socket in the uws app's socket group.
-// A NewApp::destroy at lastChanceToFinalize would unlink that group from the
-// loop and orphan the socket, so the synchronous deinit path above is gated
-// on TERMINATED (set only by an abrupt stop or an app.close() that already
-// drained the group).
+// The common shape of the scenarios below (nothing pins the server, stop() is
+// not awaited), run under the CI lane's full suppression list: freeing the
+// gracefully stopped server at teardown has to leave nothing behind there
+// either, in particular the keep-alive socket that stop() closed (a socket
+// still linked into the app would be a direct leak no suppression covers).
 test.skipIf(!isASAN || isWindows)(
   "gracefully-stopped Bun.serve does not orphan a keep-alive socket at VM teardown",
   async () => {
-    await using proc = Bun.spawn({
-      cmd: [
-        bunExe(),
-        "-e",
+    const result = await exitUnderLeakSanitizer(`
+      const server = Bun.serve({ port: 0, fetch: () => new Response("ok") });
+      await (await fetch(server.url)).text();
+      server.stop();
+    `);
+    expect(result).toEqual({ stdout: "", stderr: "", exitCode: 0 });
+  },
+  LEAK_TEST_TIMEOUT,
+);
+
+// After a graceful stop() (or node:http server.close()) the drained server is
+// only freed once its JS object is finalized. When that happens during VM
+// teardown, finalize() has to free it inline (no event-loop tick will run the
+// deferred deinit); it used to do so only after an abrupt stop, so every
+// gracefully stopped server still referenced at exit leaked its NewServer Box,
+// uws app and routers.
+//
+// That leak hides behind `leak:uws_create_app` in leaksan.supp: LSan also
+// suppresses everything reachable from a suppressed block, so the whole cycle
+// vanishes and only the llvm-symbolizer pass LSan needs to match the
+// suppression remains (about 15s per such child on the ASAN lane). These
+// children therefore run against the suppression list minus that entry. They
+// create the server only after a timer tick, because a Bun.serve() call made
+// synchronously from the module body has JSModuleLoader::evaluateNonVirtual
+// (suppressed as well) on every allocation's stack, and they keep the server
+// on globalThis so that its wrapper is finalized by the teardown and never by
+// an earlier GC.
+describe.skipIf(!isASAN || isWindows)(
+  "a gracefully stopped server still referenced at exit is freed at VM teardown",
+  () => {
+    const scenarios: [name: string, script: string][] = [
+      [
+        "Bun.serve stop() after serving a request",
         `
+          await Bun.sleep(0);
           const server = Bun.serve({ port: 0, fetch: () => new Response("ok") });
+          globalThis.keep = server;
           await (await fetch(server.url)).text();
+          await server.stop();
+        `,
+      ],
+      [
+        "Bun.serve stop() before any connection",
+        `
+          await Bun.sleep(0);
+          const server = Bun.serve({ port: 0, fetch: () => new Response("ok") });
+          globalThis.keep = server;
+          await server.stop();
+        `,
+      ],
+      [
+        "node:http server.close()",
+        `
+          import { createServer } from "node:http";
+          await Bun.sleep(0);
+          const server = createServer((req, res) => res.end("ok"));
+          globalThis.keep = server;
+          await new Promise(resolve => server.listen(0, resolve));
+          await (await fetch("http://127.0.0.1:" + server.address().port + "/")).text();
+          await new Promise(resolve => server.close(resolve));
+        `,
+      ],
+      [
+        // The websocket keeps the server draining past stop(); the teardown's
+        // stop phase closes it (the stop() promise never settles, script is
+        // forbidden by then) and the finalizer then finds the server drained.
+        "Bun.serve stop() with a websocket still open at process.exit()",
+        `
+          await Bun.sleep(0);
+          const server = Bun.serve({
+            port: 0,
+            fetch: (req, server) => (server.upgrade(req) ? undefined : new Response("no")),
+            websocket: { message() {} },
+          });
+          globalThis.keep = server;
+          const ws = new WebSocket("ws://127.0.0.1:" + server.port + "/");
+          const { promise, resolve, reject } = Promise.withResolvers();
+          ws.onopen = resolve;
+          ws.onclose = reject;
+          await promise;
           server.stop();
+          process.exit(0);
         `,
       ],
-      env: {
-        ...bunEnv,
-        BUN_DESTRUCT_VM_ON_EXIT: "1",
-        ASAN_OPTIONS: [bunEnv.ASAN_OPTIONS, "detect_leaks=1"].filter(Boolean).join(":"),
-        LSAN_OPTIONS: `print_suppressions=0:suppressions=${join(import.meta.dirname, "../../../leaksan.supp")}`,
+    ];
+
+    test.concurrent.each(scenarios)(
+      "%s",
+      async (_name, script) => {
+        const suppressions = (await Bun.file(leaksanSupp).text())
+          .split("\n")
+          .filter(line => line.trim() !== "leak:uws_create_app")
+          .join("\n");
+        using dir = tempDir("serve-teardown-leak", { "leaksan.supp": suppressions });
+        const result = await exitUnderLeakSanitizer(script, { suppressions: join(String(dir), "leaksan.supp") });
+        expect(result).toEqual({ stdout: "", stderr: "", exitCode: 0 });
       },
-      stdout: "pipe",
-      stderr: "pipe",
-    });
-    const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
-    expect({ stdout, stderr, exitCode }).toEqual({ stdout: "", stderr: "", exitCode: 0 });
+      LEAK_TEST_TIMEOUT,
+    );
   },
-  30_000,
 );

test/js/bun/http/serve-http3.test.ts

@@ -492,6 +492,51 @@ describe("Bun.serve HTTP/3", () => {
     expect(stdout).toContain("listening");
     expect(exitCode).toBe(0);
   });
+
+  // The TCP listener is bound before the UDP one, and a fixed port is not
+  // retried, so a UDP bind failure tears down a server that is already
+  // listening on TCP. That teardown used to destroy the uws app with the
+  // listener still in it: the TCP port stayed bound (and debug builds abort in
+  // us_socket_group_deinit).
+  test("http3 UDP bind failure releases the TCP port the server had bound", async () => {
+    await using proc = Bun.spawn({
+      cmd: [
+        bunExe(),
+        "-e",
+        `
+          // Occupy a UDP port whose TCP side is free, then ask for http3 on it.
+          // Retried in case the kernel-chosen port is also busy on TCP.
+          for (let attempt = 0; ; attempt++) {
+            const udp = await Bun.udpSocket({ hostname: "127.0.0.1", port: 0, socket: { data() {} } });
+            const { port } = udp;
+            let error;
+            try {
+              Bun.serve({ port, hostname: "127.0.0.1", tls: ${JSON.stringify(tls)}, http3: true, fetch: () => new Response("x") });
+            } catch (e) {
+              error = e;
+            }
+            if (error?.code === "EADDRINUSE" && attempt < 5) {
+              udp.close();
+              continue;
+            }
+            using tcp = Bun.serve({ port, hostname: "127.0.0.1", fetch: () => new Response("x") });
+            console.log(String(error?.message).replace(port, "PORT"), "| TCP port rebound:", tcp.port === port);
+            udp.close();
+            break;
+          }
+        `,
+      ],
+      env: bunEnv,
+      stdout: "pipe",
+      stderr: "pipe",
+    });
+    const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+    expect({ stdout, stderr, exitCode }).toEqual({
+      stdout: "Failed to listen on UDP port PORT for HTTP/3 | TCP port rebound: true\n",
+      stderr: "",
+      exitCode: 0,
+    });
+  });
 });
 
 // Cases ported from h2o t/40http3 and aioquic interop. Each test gets its own