mirror of
https://github.com/openclaw/openclaw.git
synced 2026-06-24 10:58:37 +00:00
fix(ports): route isPortBusy through checkPortInUse to catch IPv4-only occupants (#94949)
* fix(ports): route isPortBusy through checkPortInUse to catch IPv4-only occupants * fix(ports): treat PortUsageStatus unknown as busy in isPortBusy Per ClawSweeper review: checkPortInUse returns 'unknown' when every host probe fails for a non-EADDRINUSE reason. Treating unknown as 'not busy' could cause forceFreePortAndWait to exit before lsof/fuser inspects the port. Conservative fix: only 'free' means not busy; everything else (busy or unknown) triggers further inspection. * fix(ports): reuse canonical multi-address probe * fix(ports): reuse canonical multi-address probe --------- Co-authored-by: Vincent Koc <25068+vincentkoc@users.noreply.github.com>
This commit is contained in:
@@ -3,7 +3,7 @@ import { execFileSync } from "node:child_process";
|
||||
import { createServer } from "node:net";
|
||||
import { formatErrorMessage } from "../infra/errors.js";
|
||||
import { resolveLsofCommandSync } from "../infra/ports-lsof.js";
|
||||
import { tryListenOnPort } from "../infra/ports-probe.js";
|
||||
import { probePortUsage } from "../infra/ports-probe.js";
|
||||
import { getWindowsSystem32ExePath } from "../infra/windows-install-roots.js";
|
||||
import { resolvePositiveTimerTimeoutMs, resolveTimerTimeoutMs } from "../shared/number-coercion.js";
|
||||
import { sleep } from "../utils.js";
|
||||
@@ -131,16 +131,12 @@ function killPortWithFuser(port: number, signal: "SIGTERM" | "SIGKILL"): PortPro
|
||||
}
|
||||
|
||||
async function isPortBusy(port: number): Promise<boolean> {
|
||||
try {
|
||||
await tryListenOnPort({ port, exclusive: true });
|
||||
return false;
|
||||
} catch (err: unknown) {
|
||||
const code = (err as NodeJS.ErrnoException).code;
|
||||
if (code === "EADDRINUSE") {
|
||||
return true;
|
||||
}
|
||||
throw err instanceof Error ? err : new Error(String(err));
|
||||
}
|
||||
// Route through probePortUsage which probes all four endpoints
|
||||
// (127.0.0.1, 0.0.0.0, ::1, ::) instead of a single hostless bind
|
||||
// that defaults to IPv6 wildcard and misses IPv4-only occupants.
|
||||
// Treat "unknown" as busy — inconclusive probe failures must not cause
|
||||
// forceFreePortAndWait to exit early before lsof/fuser can inspect.
|
||||
return (await probePortUsage(port)) !== "free";
|
||||
}
|
||||
|
||||
export function parseLsofOutput(output: string): PortProcess[] {
|
||||
|
||||
@@ -9,10 +9,10 @@ vi.mock("node:child_process", async () => {
|
||||
};
|
||||
});
|
||||
|
||||
const tryListenOnPortMock = vi.hoisted(() => vi.fn());
|
||||
const probePortUsageMock = vi.hoisted(() => vi.fn());
|
||||
|
||||
vi.mock("../infra/ports-probe.js", () => ({
|
||||
tryListenOnPort: (...args: unknown[]) => tryListenOnPortMock(...args),
|
||||
probePortUsage: (...args: unknown[]) => probePortUsageMock(...args),
|
||||
}));
|
||||
|
||||
import { execFileSync } from "node:child_process";
|
||||
@@ -33,10 +33,8 @@ describe("gateway --force helpers", () => {
|
||||
vi.clearAllMocks();
|
||||
originalKill = process.kill.bind(process);
|
||||
originalPlatform = process.platform;
|
||||
tryListenOnPortMock.mockReset();
|
||||
tryListenOnPortMock.mockRejectedValue(
|
||||
Object.assign(new Error("in use"), { code: "EADDRINUSE" }),
|
||||
);
|
||||
probePortUsageMock.mockReset();
|
||||
probePortUsageMock.mockResolvedValue("busy");
|
||||
// Pin to linux so all lsof tests are platform-invariant.
|
||||
Object.defineProperty(process, "platform", { value: "linux", configurable: true });
|
||||
});
|
||||
@@ -65,7 +63,7 @@ describe("gateway --force helpers", () => {
|
||||
});
|
||||
|
||||
it("skips lsof when the port is already bindable", async () => {
|
||||
tryListenOnPortMock.mockResolvedValue(undefined);
|
||||
probePortUsageMock.mockResolvedValue("free");
|
||||
|
||||
const result = await forceFreePortAndWait(18789, { timeoutMs: 500, intervalMs: 100 });
|
||||
|
||||
@@ -199,9 +197,7 @@ describe("gateway --force helpers", () => {
|
||||
}
|
||||
return "18789/tcp: 4242\n";
|
||||
});
|
||||
tryListenOnPortMock
|
||||
.mockRejectedValueOnce(Object.assign(new Error("in use"), { code: "EADDRINUSE" }))
|
||||
.mockResolvedValue(undefined);
|
||||
probePortUsageMock.mockResolvedValueOnce("busy").mockResolvedValue("free");
|
||||
|
||||
const result = await forceFreePortAndWait(18789, { timeoutMs: 500, intervalMs: 100 });
|
||||
|
||||
@@ -231,13 +227,12 @@ describe("gateway --force helpers", () => {
|
||||
return "";
|
||||
});
|
||||
|
||||
const busyErr = Object.assign(new Error("in use"), { code: "EADDRINUSE" });
|
||||
tryListenOnPortMock
|
||||
.mockRejectedValueOnce(busyErr)
|
||||
.mockRejectedValueOnce(busyErr)
|
||||
.mockRejectedValueOnce(busyErr)
|
||||
.mockRejectedValueOnce(busyErr)
|
||||
.mockResolvedValueOnce(undefined);
|
||||
probePortUsageMock
|
||||
.mockResolvedValueOnce("busy")
|
||||
.mockResolvedValueOnce("busy")
|
||||
.mockResolvedValueOnce("busy")
|
||||
.mockResolvedValueOnce("busy")
|
||||
.mockResolvedValue("free");
|
||||
|
||||
const promise = forceFreePortAndWait(18789, {
|
||||
timeoutMs: 300,
|
||||
@@ -258,6 +253,8 @@ describe("gateway --force helpers", () => {
|
||||
});
|
||||
|
||||
it("throws when lsof is unavailable and fuser is missing", async () => {
|
||||
// An inconclusive four-host probe must continue into the cleanup tools.
|
||||
probePortUsageMock.mockResolvedValue("unknown");
|
||||
(execFileSync as unknown as Mock).mockImplementation((cmd: string) => {
|
||||
const err = new Error(`spawnSync ${cmd} ENOENT`) as NodeJS.ErrnoException;
|
||||
err.code = "ENOENT";
|
||||
|
||||
Reference in New Issue
Block a user