mirror of https://github.com/garrytan/gstack.git
fix: isProcessAlive uses signal-0 on all platforms, not tasklist
The Windows branch spawned tasklist per probe. Two failure modes: (1) every call flashed a console window from console-less daemons — the terminal-agent watchdog probes once per 60s tick for the whole session (#1952); (2) node spawnSync reports a timeout via result.error without throwing, so a slow tasklist (>3s under load) read as empty stdout -> "dead" for a LIVE agent. A false "dead" makes the watchdog clear the live agent record and respawn a duplicate around it, while killAgentByRecord consults the same false probe and skips the kill — the split-brain observed in #2151. process.kill(pid, 0) is a pure OpenProcess existence check on Windows under both Node and Bun (verified on Windows 11, node 22 / bun 1.3.14: alive -> true, dead -> false, PID 4 System -> EPERM -> true). Unifying on signal-0 also fixes the old Unix branch collapsing EPERM (exists-but-unsignallable) to "dead", as proposed in #1952. Tests: EPERM-implies-alive case, plus a static tripwire rejecting any tasklist invocation in error-handling.ts. Fixes #1952. Fixes the split-brain trigger in #2151. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
parent
a365ace877
commit
f48623ecac
|
|
@ -7,8 +7,6 @@
|
|||
|
||||
import * as fs from 'fs';
|
||||
|
||||
const IS_WINDOWS = process.platform === 'win32';
|
||||
|
||||
// ─── Filesystem ────────────────────────────────────────────────
|
||||
|
||||
/** Remove a file, ignoring ENOENT (already gone). Rethrows other errors. */
|
||||
|
|
@ -36,23 +34,26 @@ export function safeKill(pid: number, signal: NodeJS.Signals | number): void {
|
|||
}
|
||||
}
|
||||
|
||||
/** Check if a PID is alive. Pure boolean probe — returns false for ALL errors. */
|
||||
/**
|
||||
* Check if a PID is alive. Pure boolean probe — never throws.
|
||||
*
|
||||
* signal-0 on every platform: Node and Bun both implement
|
||||
* `process.kill(pid, 0)` on Windows via an OpenProcess existence check —
|
||||
* no child process, no console window. The previous Windows branch spawned
|
||||
* `tasklist`, which (a) flashed a visible console window from console-less
|
||||
* daemons on every call (#1952), and (b) silently returned false for LIVE
|
||||
* processes when tasklist exceeded its 3s timeout — node's spawnSync
|
||||
* reports timeout via `result.error` without throwing, so the empty-stdout
|
||||
* path read as "dead". A false "dead" makes the terminal-agent watchdog
|
||||
* respawn around a live agent and orphan it (split-brain, #2151).
|
||||
*
|
||||
* EPERM ⇒ alive: the process exists but can't be signalled by this user.
|
||||
*/
|
||||
export function isProcessAlive(pid: number): boolean {
|
||||
if (IS_WINDOWS) {
|
||||
try {
|
||||
const result = Bun.spawnSync(
|
||||
['tasklist', '/FI', `PID eq ${pid}`, '/NH', '/FO', 'CSV'],
|
||||
{ stdout: 'pipe', stderr: 'pipe', timeout: 3000 }
|
||||
);
|
||||
return result.stdout.toString().includes(`"${pid}"`);
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
try {
|
||||
process.kill(pid, 0);
|
||||
return true;
|
||||
} catch {
|
||||
return false;
|
||||
} catch (err: any) {
|
||||
return err?.code === 'EPERM';
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -44,4 +44,28 @@ describe('isProcessAlive', () => {
|
|||
test('returns false for non-existent process', () => {
|
||||
expect(isProcessAlive(99999999)).toBe(false);
|
||||
});
|
||||
|
||||
test('returns true for a live but unsignallable process (EPERM ⇒ alive)', () => {
|
||||
// PID 4 = System on Windows, PID 1 = init/launchd on POSIX. Both are
|
||||
// always alive; signalling them as an unprivileged user throws EPERM,
|
||||
// which must read as "alive" (as root/admin the probe just succeeds).
|
||||
const protectedPid = process.platform === 'win32' ? 4 : 1;
|
||||
expect(isProcessAlive(protectedPid)).toBe(true);
|
||||
});
|
||||
|
||||
test('never spawns a subprocess (no tasklist on Windows)', () => {
|
||||
// Static tripwire: the pre-#2151 Windows branch shelled out to
|
||||
// `tasklist`, which flashed a console window from console-less daemons
|
||||
// on every watchdog tick (#1952) and silently reported live agents as
|
||||
// dead when tasklist exceeded its timeout — triggering split-brain
|
||||
// respawns (#2151). signal-0 is a pure syscall on every platform.
|
||||
const src = fs.readFileSync(
|
||||
path.resolve(import.meta.dir, '..', 'src', 'error-handling.ts'),
|
||||
'utf-8',
|
||||
);
|
||||
// Reject only actual invocations — the doc comment on isProcessAlive
|
||||
// may mention tasklist when explaining what the signal-0 probe replaced.
|
||||
expect(src).not.toMatch(/spawnSync\s*\(\s*\[?\s*['"]tasklist/);
|
||||
expect(src).not.toMatch(/spawn\s*\(\s*\[?\s*['"]tasklist/);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in New Issue