diff --git a/cli/src/utils/process.ts b/cli/src/utils/process.ts index 9e800724..f19b0877 100644 --- a/cli/src/utils/process.ts +++ b/cli/src/utils/process.ts @@ -43,12 +43,91 @@ export async function killProcess(pid: number, force: boolean = false): Promise< try { process.kill(pid, force ? 'SIGKILL' : 'SIGTERM'); + await waitForProcessToDie(pid, force); return true; } catch { return false; } } +/** + * Recursively collects all descendant PIDs of a process (depth-first). + * Returns PIDs in child-first order (leaves first, root last). + */ +function collectProcessTree(pid: number): number[] { + const pids: number[] = []; + + try { + const result = spawn.sync('pgrep', ['-P', pid.toString()], { encoding: 'utf8' }); + if (result.stdout) { + const childPids = result.stdout.trim().split('\n').filter(Boolean).map(Number); + for (const childPid of childPids) { + pids.push(...collectProcessTree(childPid)); + } + } + } catch { + // pgrep may not be available + } + + pids.push(pid); + return pids; +} + +/** + * Kills a process and all its descendants. + * Signals are sent synchronously (children first) to work in exit handlers, + * then waits asynchronously for processes to die. + */ +async function killProcessTree(pid: number, force: boolean): Promise { + // Collect all PIDs first (sync) - returns in child-first order + const pids = collectProcessTree(pid); + + // Signal all processes synchronously (children first, then root) + const signal = force ? 'SIGKILL' : 'SIGTERM'; + for (const p of pids) { + try { + process.kill(p, signal); + } catch { + // Process may have already exited + } + } + + // Wait for processes to die (async) - wait for root last + for (const p of pids) { + await waitForProcessToDie(p, force); + } + + return true; +} + +/** + * Waits for a process to die, escalating to SIGKILL if SIGTERM doesn't work. + */ +async function waitForProcessToDie(pid: number, force: boolean): Promise { + const maxWait = 2000; + const pollInterval = 20; + let waited = 0; + + while (isProcessAlive(pid) && waited < maxWait) { + await new Promise(r => setTimeout(r, pollInterval)); + waited += pollInterval; + } + + // If SIGTERM didn't work and we haven't tried SIGKILL yet, escalate + if (!force && isProcessAlive(pid)) { + try { + process.kill(pid, 'SIGKILL'); + } catch { + return; + } + waited = 0; + while (isProcessAlive(pid) && waited < 1000) { + await new Promise(r => setTimeout(r, pollInterval)); + waited += pollInterval; + } + } +} + export async function killProcessByChildProcess( child: ChildProcess, force: boolean = false @@ -59,13 +138,10 @@ export async function killProcessByChildProcess( } if (isWindows()) { + // Windows taskkill /T already kills the entire process tree return killProcess(pid, force); } - try { - child.kill(force ? 'SIGKILL' : 'SIGTERM'); - return true; - } catch { - return false; - } + // Kill entire process tree on Unix to prevent orphan processes + return killProcessTree(pid, force); } diff --git a/cli/src/utils/spawnWithAbort.ts b/cli/src/utils/spawnWithAbort.ts index 0b4462ff..22749262 100644 --- a/cli/src/utils/spawnWithAbort.ts +++ b/cli/src/utils/spawnWithAbort.ts @@ -43,9 +43,12 @@ export async function spawnWithAbort(options: SpawnWithAbortOptions): Promise((resolve, reject) => { + // Note: We intentionally do NOT pass signal to spawn() because Node.js's + // built-in abort handling only kills the direct child, not grandchildren. + // Instead, we handle abort ourselves using killProcessByChildProcess which + // kills the entire process tree to prevent orphan processes. const child = spawn(options.command, options.args, { stdio, - signal: options.signal, cwd: options.cwd, env: options.env, shell: options.shell @@ -57,6 +60,14 @@ export async function spawnWithAbort(options: SpawnWithAbortOptions): Promise { if (child.exitCode === null && !child.killed) { logDebug('Abort timeout reached, sending SIGKILL');