diff --git a/packages/cli/src/server/portUtils.test.ts b/packages/cli/src/server/portUtils.test.ts new file mode 100644 index 0000000000..8f64704dd5 --- /dev/null +++ b/packages/cli/src/server/portUtils.test.ts @@ -0,0 +1,62 @@ +import { afterEach, describe, expect, it } from "vitest"; +import { createServer, type Server } from "node:net"; +import { testPortOnAllHosts } from "./portUtils.js"; + +// A genuinely-free port on every OS we care about. Picked from the high +// IANA-ephemeral range with enough runway that random collisions are rare; +// if a test still flakes on a loaded CI shard we bump `BASE`. +const BASE = 45_000; + +const openServers: Server[] = []; + +function allocFreePort(): number { + // Each test picks its own port out of a counter to avoid cross-test reuse. + return BASE + Math.floor(Math.random() * 1_000); +} + +afterEach(async () => { + await Promise.all( + openServers.splice(0).map( + (s) => + new Promise((resolve) => { + s.close(() => resolve()); + }), + ), + ); +}); + +describe("testPortOnAllHosts", () => { + it("returns true for a genuinely free port (regression: #309)", async () => { + // The bug: running the four probes in parallel caused the `0.0.0.0` bind + // to collide with the still-open `127.0.0.1` socket, so `every port is + // occupied` on Linux. This test binds NOTHING and asserts the scanner + // sees the port as free — which only works when the internal probes are + // sequential and each socket is fully closed before the next opens. + const port = allocFreePort(); + const result = await testPortOnAllHosts(port); + expect(result).toBe(true); + }); + + it("returns false when the port is occupied on 0.0.0.0", async () => { + const port = allocFreePort(); + const blocker = createServer(); + openServers.push(blocker); + await new Promise((resolve, reject) => { + blocker.once("error", reject); + blocker.listen({ port, host: "0.0.0.0" }, () => resolve()); + }); + const result = await testPortOnAllHosts(port); + expect(result).toBe(false); + }); + + it("releases each probe socket before starting the next (no wildcard-vs-loopback race)", async () => { + // Sanity-check the shape of the bug itself: two back-to-back calls for + // the same free port must both return true. If the first call left a + // socket lingering, the second would see EADDRINUSE. + const port = allocFreePort(); + const first = await testPortOnAllHosts(port); + const second = await testPortOnAllHosts(port); + expect(first).toBe(true); + expect(second).toBe(true); + }); +}); diff --git a/packages/cli/src/server/portUtils.ts b/packages/cli/src/server/portUtils.ts index d66a74e7dc..d681e12e55 100644 --- a/packages/cli/src/server/portUtils.ts +++ b/packages/cli/src/server/portUtils.ts @@ -50,14 +50,28 @@ function isPortAvailableOnHost(port: number, host: string): Promise { } /** - * Test a port across IPv4 and IPv6 interfaces in parallel. A port is only - * unavailable if ANY host reports EADDRINUSE. This catches the devbox bug - * where a port is free on localhost but occupied on 0.0.0.0 via SSH forwarding. + * Test a port across IPv4 and IPv6 interfaces. A port is only available if + * EVERY host binds and releases cleanly — that catches the devbox class of + * bug where a port is free on `127.0.0.1` but held on `0.0.0.0` via SSH + * forwarding. + * + * **Must be sequential, not Promise.all.** Binding `127.0.0.1` holds the + * socket open until `server.close()` resolves on the next event-loop tick. + * In parallel, the wildcard `0.0.0.0` / `::` tests race that still-open + * socket and return spurious `EADDRINUSE` — which makes every port in the + * scan range look occupied and the preview server refuse to start. Repro + * on Linux (Crostini on ChromeOS in the reporting environment, issue #309) + * is deterministic; on macOS/Windows the behaviour is less consistent but + * the race is there all the same. Serializing each bind past its close + * callback eliminates the window entirely. */ export async function testPortOnAllHosts(port: number): Promise { const hosts = ["127.0.0.1", "0.0.0.0", "::1", "::"]; - const results = await Promise.all(hosts.map((h) => isPortAvailableOnHost(port, h))); - return results.every(Boolean); + for (const host of hosts) { + const available = await isPortAvailableOnHost(port, host); + if (!available) return false; + } + return true; } // ── Existing instance detection ────────────────────────────────────────────