Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 31 additions & 12 deletions bin/cli/commands/serve.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -246,18 +246,7 @@ export async function runServe(opts = {}) {
// BEFORE any pid file is written or any child is spawned. Otherwise the
// doomed child's EADDRINUSE arrives only after this process has rewritten
// the pid files of the healthy instance that actually owns the port.
// findListeningPids() returning null means the discovery tool itself is
// missing or unusable (Termux, slim containers, #14518) — fall back to a
// bind probe so the guard still answers before spawning the doomed child.
let busyPids = await findListeningPids(dashboardPort);
if (busyPids === null) {
// Discovery tool missing/unusable (#14518): the bind probe is the guard.
if (!(await probePortFree(dashboardPort))) busyPids = [null];
} else if (busyPids.length === 0) {
// Discovery ran and saw nothing, but that window can race a starting
// instance; a bind probe costs nothing and doubles as confirmation.
if (!(await probePortFree(dashboardPort))) busyPids = [null];
}
const busyPids = await resolveServeBusyPids(dashboardPort);
if (busyPids.length > 0) {
reportPortInUse(dashboardPort, busyPids);
process.exit(1);
Expand Down Expand Up @@ -341,6 +330,36 @@ export async function runServe(opts = {}) {
);
}

/**
* Listeners blocking `serve` on `port`. Discovery returning null means the
* tool is missing (#14518); the bind probe then decides. A free port is an
* empty list — leaving null throws on the caller's `.length` (#14800).
*
* @param {number} port
* @param {object} [deps]
* @param {typeof findListeningPids} [deps.findListeningPids]
* @param {typeof probePortFree} [deps.probePortFree]
* @returns {Promise<Array<number|null>>}
*/
export async function resolveServeBusyPids(port, deps = {}) {
const discover = deps.findListeningPids ?? findListeningPids;
const probe = deps.probePortFree ?? probePortFree;
// findListeningPids() returning null means the discovery tool itself is
// missing or unusable (Termux, slim containers, #14518) — fall back to a
// bind probe so the guard still answers before spawning the doomed child.
let busyPids = await discover(port);
if (busyPids === null) {
// Discovery tool missing/unusable (#14518): the bind probe is the guard.
if (!(await probe(port))) busyPids = [null];
else busyPids = [];
} else if (busyPids.length === 0) {
// Discovery ran and saw nothing, but that window can race a starting
// instance; a bind probe costs nothing and doubles as confirmation.
if (!(await probe(port))) busyPids = [null];
}
return busyPids;
}

/**
* Explain a port conflict in terms the operator can act on: who owns the port,
* and the two ways out. Exported for unit tests.
Expand Down
18 changes: 13 additions & 5 deletions bin/cli/utils/pid.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -89,11 +89,19 @@ export async function findListeningPids(port, deps = {}) {
.split("\n")
.map((entry) => parseInt(entry, 10))
.filter((entry) => Number.isFinite(entry) && entry > 0);
} catch {
// Tool missing (ENOENT) or unusable: "no listener" cannot be distinguished
// from "cannot look" here, so report null and let the caller decide. The
// serve preflight bind-probes the port in that case (#14518) — a false
// "busy" would block a legitimate start, the worse failure of the two.
} catch (err) {
// POSIX lsof exits 1 with empty output when there are simply no matches.
// That is the normal "port is free" result, not a discovery failure.
if (
platform !== "win32" &&
err?.code === 1 &&
!String(err?.stdout ?? "").trim()
) {
return [];
}
// Tool missing (ENOENT) or genuinely unusable: "no listener" cannot be
// distinguished from "cannot look" here, so report null and let the serve
// preflight bind-probe the port instead (#14518).
return null;
}
}
Expand Down
26 changes: 26 additions & 0 deletions tests/unit/cli-serve-port-in-use-preflight.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,20 @@ test("findListeningPids reports the PID holding the port (posix lsof)", async ()
assert.deepEqual(pids, [4242, 4243]);
});

test("findListeningPids treats an empty lsof result as a free port", async () => {
const noMatch = Object.assign(new Error("lsof exited with no matches"), {
code: 1,
stdout: "",
});
const pids = await findListeningPids(20128, {
platform: "darwin",
execFileAsync: async () => {
throw noMatch;
},
});
assert.deepEqual(pids, [], "lsof exit 1 with empty output means nothing is listening");
});

test("findListeningPids returns null when discovery is unavailable (#14518)", async () => {
const pids = await findListeningPids(20128, {
platform: "win32",
Expand Down Expand Up @@ -144,6 +158,18 @@ test("reportPortInUse degrades gracefully when the owner pid is unknown", async
assert.match(out, /omniroute stop/, "must keep the resolution path");
});

test("serve preflight treats a free port as no listeners when pid discovery returns null", async () => {
const { resolveServeBusyPids } = await import("../../bin/cli/commands/serve.mjs");
const busyPids = await resolveServeBusyPids(20128, {
findListeningPids: async () => null,
probePortFree: async () => true,
});
// #14800: null discovery + a free bind probe used to leave busyPids null, and
// the next `.length` threw on any host without lsof/netstat. Free means [].
assert.equal(busyPids.length, 0);
assert.deepEqual(busyPids, []);
});

test("serve preflight rejects a busy port even without any discovery tool (end-to-end for #14518)", async () => {
const { probePortFree, findListeningPids } = await import("../../bin/cli/utils/pid.mjs");
const server = net.createServer();
Expand Down
Loading