From 9d866d8b2afd4d89bce26e39ade41eec8543b6a5 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 30 May 2026 17:35:59 -0400 Subject: [PATCH] fix(cli): clamp port wait timers --- src/cli/ports.test.ts | 18 +++++++++++++++++- src/cli/ports.ts | 36 +++++++++++++++++++---------------- src/cli/program.force.test.ts | 17 +++++++++++++++++ 3 files changed, 54 insertions(+), 17 deletions(-) diff --git a/src/cli/ports.test.ts b/src/cli/ports.test.ts index 4d911432287..cf7859ea91a 100644 --- a/src/cli/ports.test.ts +++ b/src/cli/ports.test.ts @@ -1,6 +1,6 @@ import { EventEmitter } from "node:events"; import net from "node:net"; -import { describe, expect, it, vi } from "vitest"; +import { afterEach, describe, expect, it, vi } from "vitest"; // Hoist the factory so vi.mock can access it. const mockCreateServer = vi.hoisted(() => vi.fn()); @@ -12,6 +12,10 @@ vi.mock("node:net", async () => { import { probePortFree, waitForPortBindable } from "./ports.js"; +afterEach(() => { + vi.useRealTimers(); +}); + /** Build a minimal fake net.Server that emits a given error code on listen(). */ function makeErrServer(code: string): net.Server { const err = Object.assign(new Error(`bind error: ${code}`), { @@ -129,4 +133,16 @@ describe("waitForPortBindable", () => { // Only one probe should have been attempted — no spinning through the retry loop. expect(mockCreateServer).toHaveBeenCalledTimes(1); }); + + it("bounds oversized bindability intervals by the remaining timeout", async () => { + mockCreateServer.mockReturnValue(makeErrServer("EADDRINUSE")); + + await expect( + waitForPortBindable(9999, { + timeoutMs: 1, + intervalMs: Number.MAX_SAFE_INTEGER, + host: "127.0.0.1", + }), + ).rejects.toThrow(/still not bindable after 1ms/); + }); }); diff --git a/src/cli/ports.ts b/src/cli/ports.ts index 1ee1de8e15e..2c9491dd169 100644 --- a/src/cli/ports.ts +++ b/src/cli/ports.ts @@ -3,6 +3,7 @@ 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 { resolvePositiveTimerTimeoutMs, resolveTimerTimeoutMs } from "../shared/number-coercion.js"; import { sleep } from "../utils.js"; export type PortProcess = { pid: number; command?: string }; @@ -259,9 +260,12 @@ export async function forceFreePortAndWait( sigtermTimeoutMs?: number; } = {}, ): Promise { - const timeoutMs = Math.max(opts.timeoutMs ?? 1500, 0); - const intervalMs = Math.max(opts.intervalMs ?? 100, 1); - const sigtermTimeoutMs = Math.min(Math.max(opts.sigtermTimeoutMs ?? 600, 0), timeoutMs); + const timeoutMs = resolveTimerTimeoutMs(opts.timeoutMs, 1500, 0); + const intervalMs = resolvePositiveTimerTimeoutMs(opts.intervalMs, 100); + const sigtermTimeoutMs = Math.min( + resolveTimerTimeoutMs(opts.sigtermTimeoutMs, 600, 0), + timeoutMs, + ); let killed: PortProcess[] = []; let useFuserFallback = false; @@ -292,13 +296,13 @@ export async function forceFreePortAndWait( } let waitedMs = 0; - const triesSigterm = intervalMs > 0 ? Math.ceil(sigtermTimeoutMs / intervalMs) : 0; - for (let i = 0; i < triesSigterm; i++) { + while (waitedMs < sigtermTimeoutMs) { if (!(await checkBusy())) { return { killed, waitedMs, escalatedToSigkill: false }; } - await sleep(intervalMs); - waitedMs += intervalMs; + const sleepMs = Math.min(intervalMs, sigtermTimeoutMs - waitedMs); + await sleep(sleepMs); + waitedMs += sleepMs; } if (!(await checkBusy())) { @@ -312,14 +316,13 @@ export async function forceFreePortAndWait( killPids(remaining, "SIGKILL"); } - const remainingBudget = Math.max(timeoutMs - waitedMs, 0); - const triesSigkill = intervalMs > 0 ? Math.ceil(remainingBudget / intervalMs) : 0; - for (let i = 0; i < triesSigkill; i++) { + while (waitedMs < timeoutMs) { if (!(await checkBusy())) { return { killed, waitedMs, escalatedToSigkill: true }; } - await sleep(intervalMs); - waitedMs += intervalMs; + const sleepMs = Math.min(intervalMs, timeoutMs - waitedMs); + await sleep(sleepMs); + waitedMs += sleepMs; } if (!(await checkBusy())) { @@ -377,16 +380,17 @@ export async function waitForPortBindable( port: number, opts: { timeoutMs?: number; intervalMs?: number; host?: string } = {}, ): Promise { - const timeoutMs = Math.max(opts.timeoutMs ?? 3000, 0); - const intervalMs = Math.max(opts.intervalMs ?? 150, 1); + const timeoutMs = resolveTimerTimeoutMs(opts.timeoutMs, 3000, 0); + const intervalMs = resolvePositiveTimerTimeoutMs(opts.intervalMs, 150); const host = opts.host; let waited = 0; while (waited < timeoutMs) { if (await probePortFree(port, host)) { return waited; } - await sleep(intervalMs); - waited += intervalMs; + const sleepMs = Math.min(intervalMs, timeoutMs - waited); + await sleep(sleepMs); + waited += sleepMs; } // Final attempt if (await probePortFree(port, host)) { diff --git a/src/cli/program.force.test.ts b/src/cli/program.force.test.ts index 3a092b8add5..6b9ef933fe0 100644 --- a/src/cli/program.force.test.ts +++ b/src/cli/program.force.test.ts @@ -171,6 +171,23 @@ describe("gateway --force helpers", () => { vi.useRealTimers(); }); + it("bounds oversized force-free intervals by the remaining timeout", async () => { + (execFileSync as unknown as Mock).mockReturnValue(["p42", "cnode", ""].join("\n")); + const killMock = vi.fn(); + process.kill = killMock; + + await expect( + forceFreePortAndWait(18789, { + timeoutMs: 2, + intervalMs: Number.MAX_SAFE_INTEGER, + sigtermTimeoutMs: 1, + }), + ).rejects.toThrow(/still has listeners/); + + expect(killMock).toHaveBeenCalledWith(42, "SIGTERM"); + expect(killMock).toHaveBeenCalledWith(42, "SIGKILL"); + }); + it("falls back to fuser when lsof is permission denied", async () => { (execFileSync as unknown as Mock).mockImplementation((cmd: string) => { if (cmd.includes("lsof")) {