mirror of
https://github.com/thedotmack/claude-mem.git
synced 2026-09-20 04:23:02 +08:00
fix(windows): bound the port-occupancy probe so a ghost listener cannot hang the launcher (#3989)
isPortInUse()'s Windows fast path is an HTTP probe against /api/health, and it was the one probe the ghost-listener reclaim (#3900) left unbounded. A ghost listener — the dead worker's inherited socket, held open by its chroma sidecar chain (#3603) — completes the TCP handshake and never answers, so the probe never settled. ensureWorkerStarted() calls isPortInUse() BEFORE the reclaim, so the reclaim could never run: instead of healing the ghost, every launcher awaited a promise that never resolved. On CI that hung the gate added by #3900 for the full 600s test timeout in every run (12/12 red on main). The probe now carries the same 5s abort budget as every other probe in the module; on timeout the existing fall-through runs the socket probe, which still reports a bound port as in use, so the launcher proceeds to the reclaim. The gate gains stage lines and a 300s deadline around ensureWorkerStarted() so a future unbounded await fails with a named error instead of a silent bun timeout. Unit coverage pins the probe's abortable-signal contract and the fall-through.
This commit is contained in:
@@ -45,8 +45,19 @@ export async function isPortInUse(port: number): Promise<boolean> {
|
||||
// Fast path: HTTP health check. A live claude-mem worker responds to
|
||||
// /api/health, so this is the cheapest non-disruptive probe for the
|
||||
// common case (worker is running and healthy).
|
||||
//
|
||||
// Bounded like every other probe (HEALTH_PROBE_TIMEOUT_MS): a ghost
|
||||
// listener — the dead worker's inherited socket, held open by its chroma
|
||||
// sidecar chain (plan-15 #3603) — completes the TCP handshake and then
|
||||
// never answers. Unbounded, this fetch would hang forever, and with it
|
||||
// ensureWorkerStarted(), which calls this BEFORE it can reach the reclaim:
|
||||
// the very bug the reclaim exists to fix would instead wedge every
|
||||
// launcher. On timeout the flow falls through to the socket probe below,
|
||||
// which still reports a bound port as in use.
|
||||
try {
|
||||
const response = await fetch(`http://${formatHostForUrl(getWorkerHost())}:${port}/api/health`);
|
||||
const response = await fetch(`http://${formatHostForUrl(getWorkerHost())}:${port}/api/health`, {
|
||||
signal: AbortSignal.timeout(HEALTH_PROBE_TIMEOUT_MS),
|
||||
});
|
||||
if (response.ok) return true;
|
||||
// Non-ok response: port is reachable but the worker is unhealthy.
|
||||
// Fall through to the net.createServer check below so we still report
|
||||
|
||||
@@ -141,6 +141,50 @@ describe('HealthMonitor', () => {
|
||||
}
|
||||
});
|
||||
|
||||
it('should probe Windows health through an abortable signal so a ghost listener cannot hang it (#3603)', async () => {
|
||||
const origPlatform = process.platform;
|
||||
try {
|
||||
Object.defineProperty(process, 'platform', { value: 'win32', configurable: true });
|
||||
|
||||
// A ghost listener completes the TCP handshake and never answers, so a
|
||||
// probe without its own abort budget never settles — and
|
||||
// ensureWorkerStarted(), which runs this check BEFORE it can reach the
|
||||
// ghost reclaim, hangs with it. The abort signal is the fix: capture
|
||||
// it off the call, and model the abort as the rejection it produces.
|
||||
const inits: Array<RequestInit | undefined> = [];
|
||||
const fetchMock = mock((_url: string, init?: RequestInit) => {
|
||||
inits.push(init);
|
||||
const abortError = new Error('The operation was aborted due to timeout');
|
||||
abortError.name = 'TimeoutError';
|
||||
return Promise.reject(abortError);
|
||||
});
|
||||
global.fetch = fetchMock as any;
|
||||
|
||||
const createServerMock = mock(() => ({
|
||||
once: mock((event: string, cb: Function) => {
|
||||
if (event === 'error') setTimeout(() => cb({ code: 'EADDRINUSE' }), 0);
|
||||
}),
|
||||
listen: mock(() => {}),
|
||||
}));
|
||||
|
||||
const netSpy = spyOn(net, 'createServer').mockImplementation(createServerMock as any);
|
||||
|
||||
const result = await isPortInUse(37777);
|
||||
|
||||
expect(inits.length).toBeGreaterThan(0);
|
||||
expect(inits[0]?.signal).toBeInstanceOf(AbortSignal);
|
||||
// An aborted probe is inconclusive, never "free": the flow falls
|
||||
// through to the socket probe, which reports the bound port as in use
|
||||
// so the launcher can go on to reclaim the ghost.
|
||||
expect(result).toBe(true);
|
||||
expect(net.createServer).toHaveBeenCalled();
|
||||
|
||||
netSpy.mockRestore();
|
||||
} finally {
|
||||
Object.defineProperty(process, 'platform', { value: origPlatform, configurable: true });
|
||||
}
|
||||
});
|
||||
|
||||
it('should fall through to socket probe on Windows when health check fails and port is actually free', async () => {
|
||||
const origPlatform = process.platform;
|
||||
try {
|
||||
|
||||
@@ -64,6 +64,14 @@ const RECOVERY_TIMEOUT_MS = 600_000;
|
||||
const GHOST_SETTLE_TIMEOUT_MS = 30_000;
|
||||
const ORPHAN_SETTLE_TIMEOUT_MS = 30_000;
|
||||
|
||||
// Every stage inside ensureWorkerStarted() is supposed to carry its own
|
||||
// deadline (health probes, the reclaim, the readiness wait). The first CI run
|
||||
// of this gate proved how expensive a MISSING one is: the launcher awaited a
|
||||
// ghost's silent socket forever, and the gate died on bun's 600s cap with no
|
||||
// output pointing at the stage. Racing the call itself turns any future
|
||||
// unbounded await into a named failure — and a stage line in the log.
|
||||
const ENSURE_STARTED_DEADLINE_MS = 300_000;
|
||||
|
||||
const here = path.dirname(fileURLToPath(import.meta.url));
|
||||
const FIXTURE = path.join(here, 'fixtures', 'ghost-worker-host.ts');
|
||||
|
||||
@@ -116,6 +124,21 @@ function listeningOwnerPids(port: number): number[] {
|
||||
return [...owners];
|
||||
}
|
||||
|
||||
/** Bound an external await whose internals this test cannot inspect. */
|
||||
async function withDeadline<T>(promise: Promise<T>, timeoutMs: number, label: string): Promise<T> {
|
||||
let timer: ReturnType<typeof setTimeout> | undefined;
|
||||
try {
|
||||
return await Promise.race([
|
||||
promise,
|
||||
new Promise<never>((_, reject) => {
|
||||
timer = setTimeout(() => reject(new Error(`timed out waiting for: ${label}`)), timeoutMs);
|
||||
}),
|
||||
]);
|
||||
} finally {
|
||||
clearTimeout(timer);
|
||||
}
|
||||
}
|
||||
|
||||
async function waitFor(predicate: () => boolean, timeoutMs: number, label: string): Promise<void> {
|
||||
const deadline = Date.now() + timeoutMs;
|
||||
while (!predicate()) {
|
||||
@@ -236,6 +259,7 @@ describe.if(RUN_GATE && IS_WINDOWS)('worker recovers from a ghost listener left
|
||||
it('reclaims the dead worker\'s sidecar chain and starts a new worker', async () => {
|
||||
assertIsolatedDataDir();
|
||||
const fixture = await startFixture();
|
||||
console.log(`[ghost-gate] fixture ready: pid=${fixture.pid} port=${fixture.port} chromaRootPid=${fixture.chromaRootPid}`);
|
||||
|
||||
// Snapshot BEFORE the kill: once the root exits, identity is the only
|
||||
// way to tell the survivors apart from anything that recycled its PID.
|
||||
@@ -275,6 +299,7 @@ describe.if(RUN_GATE && IS_WINDOWS)('worker recovers from a ghost listener left
|
||||
survivorsAfterKill.length > 0,
|
||||
`sidecar chain must survive the out-of-band kill: ${describeProcesses(snapshot)}`
|
||||
).toBe(true);
|
||||
console.log(`[ghost-gate] ghost confirmed: port LISTENING under dead pid=${fixture.pid}; survivors: ${describeProcesses(survivorsAfterKill)}`);
|
||||
|
||||
// Drive the PRODUCTION launcher. On main this returns 'dead' — no
|
||||
// reclaim exists, so the ghost blocks every spawn forever.
|
||||
@@ -282,7 +307,14 @@ describe.if(RUN_GATE && IS_WINDOWS)('worker recovers from a ghost listener left
|
||||
const { resolveWorkerScriptPath } = await import('../../src/shared/worker-utils.js');
|
||||
const scriptPath = resolveWorkerScriptPath();
|
||||
expect(scriptPath).not.toBeNull();
|
||||
const result = await ensureWorkerStarted(fixture.port, scriptPath!);
|
||||
console.log(`[ghost-gate] driving production ensureWorkerStarted() on port ${fixture.port}`);
|
||||
const startedAt = Date.now();
|
||||
const result = await withDeadline(
|
||||
ensureWorkerStarted(fixture.port, scriptPath!),
|
||||
ENSURE_STARTED_DEADLINE_MS,
|
||||
'ensureWorkerStarted'
|
||||
);
|
||||
console.log(`[ghost-gate] ensureWorkerStarted returned '${result}' after ${Date.now() - startedAt}ms`);
|
||||
expect(result, `ensureWorkerStarted must not give up on a reclaimable ghost (was: ${result})`).not.toBe('dead');
|
||||
|
||||
// The reclaim must have taken the sidecar chain down with the ghost.
|
||||
|
||||
Reference in New Issue
Block a user