From 701e3af56ef54adbe2c0c6183dba36a2e401dd78 Mon Sep 17 00:00:00 2001 From: A <258483684+la14-1@users.noreply.github.com> Date: Wed, 4 Mar 2026 19:04:47 -0800 Subject: [PATCH] fix: prevent timer leaks and event-loop stalls in SSH timeout handling (#2200) - Unref the SIGKILL timer in killWithTimeout() so it doesn't keep the event loop alive for 5 extra seconds after a timed-out process exits - Wrap all setTimeout/clearTimeout pairs in try/finally across 6 cloud providers (12 call sites) to guarantee cleanup on exceptions - Add missing 60s timeout guard to runSpriteSilent() which could hang indefinitely on unresponsive sprite processes Agent: code-health Co-authored-by: B <6723574+louisgv@users.noreply.github.com> Co-authored-by: Claude Sonnet 4.5 --- packages/cli/src/aws/aws.ts | 34 ++++++++------ packages/cli/src/daytona/daytona.ts | 44 ++++++++++--------- packages/cli/src/digitalocean/digitalocean.ts | 36 ++++++++------- packages/cli/src/gcp/gcp.ts | 34 ++++++++------ packages/cli/src/hetzner/hetzner.ts | 36 ++++++++------- packages/cli/src/shared/ssh.ts | 5 ++- packages/cli/src/sprite/sprite.ts | 31 +++++++++---- 7 files changed, 129 insertions(+), 91 deletions(-) diff --git a/packages/cli/src/aws/aws.ts b/packages/cli/src/aws/aws.ts index 1b4cb7b4..e2bd18f4 100644 --- a/packages/cli/src/aws/aws.ts +++ b/packages/cli/src/aws/aws.ts @@ -1047,10 +1047,13 @@ export async function runServer(cmd: string, timeoutSecs?: number): Promise killWithTimeout(proc), timeout); - const exitCode = await proc.exited; - clearTimeout(timer); - if (exitCode !== 0) { - throw new Error(`run_server failed (exit ${exitCode}): ${cmd}`); + try { + const exitCode = await proc.exited; + if (exitCode !== 0) { + throw new Error(`run_server failed (exit ${exitCode}): ${cmd}`); + } + } finally { + clearTimeout(timer); } } @@ -1075,17 +1078,20 @@ export async function runServerCapture(cmd: string, timeoutSecs?: number): Promi ); const timeout = (timeoutSecs || 300) * 1000; const timer = setTimeout(() => killWithTimeout(proc), timeout); - // Drain both pipes before awaiting exit to prevent pipe buffer deadlock - const [stdout] = await Promise.all([ - new Response(proc.stdout).text(), - new Response(proc.stderr).text(), - ]); - const exitCode = await proc.exited; - clearTimeout(timer); - if (exitCode !== 0) { - throw new Error(`run_server_capture failed (exit ${exitCode})`); + try { + // Drain both pipes before awaiting exit to prevent pipe buffer deadlock + const [stdout] = await Promise.all([ + new Response(proc.stdout).text(), + new Response(proc.stderr).text(), + ]); + const exitCode = await proc.exited; + if (exitCode !== 0) { + throw new Error(`run_server_capture failed (exit ${exitCode})`); + } + return stdout.trim(); + } finally { + clearTimeout(timer); } - return stdout.trim(); } export async function uploadFile(localPath: string, remotePath: string): Promise { diff --git a/packages/cli/src/daytona/daytona.ts b/packages/cli/src/daytona/daytona.ts index f6e33523..ee5b20b8 100644 --- a/packages/cli/src/daytona/daytona.ts +++ b/packages/cli/src/daytona/daytona.ts @@ -389,14 +389,15 @@ export async function runServer(cmd: string, timeoutSecs?: number): Promise killWithTimeout(proc), timeout); - const exitCode = await proc.exited; - clearTimeout(timer); - - // Brief sleep to let gateway release connection slot - await sleep(1000); - - if (exitCode !== 0) { - throw new Error(`run_server failed (exit ${exitCode}): ${cmd}`); + try { + const exitCode = await proc.exited; + // Brief sleep to let gateway release connection slot + await sleep(1000); + if (exitCode !== 0) { + throw new Error(`run_server failed (exit ${exitCode}): ${cmd}`); + } + } finally { + clearTimeout(timer); } } @@ -426,20 +427,21 @@ export async function runServerCapture(cmd: string, timeoutSecs?: number): Promi } const timeout = (timeoutSecs || 300) * 1000; const timer = setTimeout(() => killWithTimeout(proc), timeout); - // Drain both pipes before awaiting exit to prevent pipe buffer deadlock - const [stdout] = await Promise.all([ - new Response(proc.stdout).text(), - new Response(proc.stderr).text(), - ]); - const exitCode = await proc.exited; - clearTimeout(timer); - - await sleep(1000); - - if (exitCode !== 0) { - throw new Error(`run_server_capture failed (exit ${exitCode})`); + try { + // Drain both pipes before awaiting exit to prevent pipe buffer deadlock + const [stdout] = await Promise.all([ + new Response(proc.stdout).text(), + new Response(proc.stderr).text(), + ]); + const exitCode = await proc.exited; + await sleep(1000); + if (exitCode !== 0) { + throw new Error(`run_server_capture failed (exit ${exitCode})`); + } + return stdout.trim(); + } finally { + clearTimeout(timer); } - return stdout.trim(); } /** diff --git a/packages/cli/src/digitalocean/digitalocean.ts b/packages/cli/src/digitalocean/digitalocean.ts index be8e024f..dbdd8d9f 100644 --- a/packages/cli/src/digitalocean/digitalocean.ts +++ b/packages/cli/src/digitalocean/digitalocean.ts @@ -984,11 +984,13 @@ export async function runServer(cmd: string, timeoutSecs?: number, ip?: string): const timeout = (timeoutSecs || 300) * 1000; const timer = setTimeout(() => killWithTimeout(proc), timeout); - const exitCode = await proc.exited; - clearTimeout(timer); - - if (exitCode !== 0) { - throw new Error(`run_server failed (exit ${exitCode}): ${cmd}`); + try { + const exitCode = await proc.exited; + if (exitCode !== 0) { + throw new Error(`run_server failed (exit ${exitCode}): ${cmd}`); + } + } finally { + clearTimeout(timer); } } @@ -1016,18 +1018,20 @@ export async function runServerCapture(cmd: string, timeoutSecs?: number, ip?: s const timeout = (timeoutSecs || 300) * 1000; const timer = setTimeout(() => killWithTimeout(proc), timeout); - // Drain both pipes before awaiting exit to prevent pipe buffer deadlock - const [stdout] = await Promise.all([ - new Response(proc.stdout).text(), - new Response(proc.stderr).text(), - ]); - const exitCode = await proc.exited; - clearTimeout(timer); - - if (exitCode !== 0) { - throw new Error(`run_server_capture failed (exit ${exitCode})`); + try { + // Drain both pipes before awaiting exit to prevent pipe buffer deadlock + const [stdout] = await Promise.all([ + new Response(proc.stdout).text(), + new Response(proc.stderr).text(), + ]); + const exitCode = await proc.exited; + if (exitCode !== 0) { + throw new Error(`run_server_capture failed (exit ${exitCode})`); + } + return stdout.trim(); + } finally { + clearTimeout(timer); } - return stdout.trim(); } export async function uploadFile(localPath: string, remotePath: string, ip?: string): Promise { diff --git a/packages/cli/src/gcp/gcp.ts b/packages/cli/src/gcp/gcp.ts index dd511ad7..6e6257a7 100644 --- a/packages/cli/src/gcp/gcp.ts +++ b/packages/cli/src/gcp/gcp.ts @@ -848,10 +848,13 @@ export async function runServer(cmd: string, timeoutSecs?: number): Promise killWithTimeout(proc), timeout); - const exitCode = await proc.exited; - clearTimeout(timer); - if (exitCode !== 0) { - throw new Error(`run_server failed (exit ${exitCode}): ${cmd}`); + try { + const exitCode = await proc.exited; + if (exitCode !== 0) { + throw new Error(`run_server failed (exit ${exitCode}): ${cmd}`); + } + } finally { + clearTimeout(timer); } } @@ -879,17 +882,20 @@ export async function runServerCapture(cmd: string, timeoutSecs?: number): Promi ); const timeout = (timeoutSecs || 300) * 1000; const timer = setTimeout(() => killWithTimeout(proc), timeout); - // Drain both pipes before awaiting exit to prevent pipe buffer deadlock - const [stdout] = await Promise.all([ - new Response(proc.stdout).text(), - new Response(proc.stderr).text(), - ]); - const exitCode = await proc.exited; - clearTimeout(timer); - if (exitCode !== 0) { - throw new Error(`run_server_capture failed (exit ${exitCode})`); + try { + // Drain both pipes before awaiting exit to prevent pipe buffer deadlock + const [stdout] = await Promise.all([ + new Response(proc.stdout).text(), + new Response(proc.stderr).text(), + ]); + const exitCode = await proc.exited; + if (exitCode !== 0) { + throw new Error(`run_server_capture failed (exit ${exitCode})`); + } + return stdout.trim(); + } finally { + clearTimeout(timer); } - return stdout.trim(); } export async function uploadFile(localPath: string, remotePath: string): Promise { diff --git a/packages/cli/src/hetzner/hetzner.ts b/packages/cli/src/hetzner/hetzner.ts index a1ee624f..bfdc2720 100644 --- a/packages/cli/src/hetzner/hetzner.ts +++ b/packages/cli/src/hetzner/hetzner.ts @@ -511,11 +511,13 @@ export async function runServer(cmd: string, timeoutSecs?: number, ip?: string): const timeout = (timeoutSecs || 300) * 1000; const timer = setTimeout(() => killWithTimeout(proc), timeout); - const exitCode = await proc.exited; - clearTimeout(timer); - - if (exitCode !== 0) { - throw new Error(`run_server failed (exit ${exitCode}): ${cmd}`); + try { + const exitCode = await proc.exited; + if (exitCode !== 0) { + throw new Error(`run_server failed (exit ${exitCode}): ${cmd}`); + } + } finally { + clearTimeout(timer); } } @@ -543,18 +545,20 @@ export async function runServerCapture(cmd: string, timeoutSecs?: number, ip?: s const timeout = (timeoutSecs || 300) * 1000; const timer = setTimeout(() => killWithTimeout(proc), timeout); - // Drain both pipes before awaiting exit to prevent pipe buffer deadlock - const [stdout] = await Promise.all([ - new Response(proc.stdout).text(), - new Response(proc.stderr).text(), - ]); - const exitCode = await proc.exited; - clearTimeout(timer); - - if (exitCode !== 0) { - throw new Error(`run_server_capture failed (exit ${exitCode})`); + try { + // Drain both pipes before awaiting exit to prevent pipe buffer deadlock + const [stdout] = await Promise.all([ + new Response(proc.stdout).text(), + new Response(proc.stderr).text(), + ]); + const exitCode = await proc.exited; + if (exitCode !== 0) { + throw new Error(`run_server_capture failed (exit ${exitCode})`); + } + return stdout.trim(); + } finally { + clearTimeout(timer); } - return stdout.trim(); } export async function uploadFile(localPath: string, remotePath: string, ip?: string): Promise { diff --git a/packages/cli/src/shared/ssh.ts b/packages/cli/src/shared/ssh.ts index 3f0cc901..c8fdef81 100644 --- a/packages/cli/src/shared/ssh.ts +++ b/packages/cli/src/shared/ssh.ts @@ -120,7 +120,7 @@ export function killWithTimeout( } catch { return; } - setTimeout(() => { + const sigkillTimer = setTimeout(() => { try { if (!proc.killed) { proc.kill(9); @@ -129,6 +129,9 @@ export function killWithTimeout( /* already dead */ } }, gracePeriodMs); + // Don't let this timer keep the event loop alive — the process may already + // be dead from SIGTERM, so there's no reason to block exit for 5 seconds. + sigkillTimer.unref(); } // ─── TCP Pre-Check ─────────────────────────────────────────────────────────── diff --git a/packages/cli/src/sprite/sprite.ts b/packages/cli/src/sprite/sprite.ts index a4ccdce7..98bfcdb0 100644 --- a/packages/cli/src/sprite/sprite.ts +++ b/packages/cli/src/sprite/sprite.ts @@ -461,10 +461,13 @@ export async function runSprite(cmd: string, timeoutSecs?: number): Promise killWithTimeout(proc), timeout); - const exitCode = await proc.exited; - clearTimeout(timer); - if (exitCode !== 0) { - throw new Error(`sprite exec failed (exit ${exitCode}): ${cmd.slice(0, 80)}`); + try { + const exitCode = await proc.exited; + if (exitCode !== 0) { + throw new Error(`sprite exec failed (exit ${exitCode}): ${cmd.slice(0, 80)}`); + } + } finally { + clearTimeout(timer); } }); } @@ -492,9 +495,15 @@ async function runSpriteSilent(cmd: string): Promise { ], }, ); - const exitCode = await proc.exited; - if (exitCode !== 0) { - throw new Error(`sprite exec (silent) failed (exit ${exitCode})`); + // 60s timeout — silent commands should not hang indefinitely + const timer = setTimeout(() => killWithTimeout(proc), 60_000); + try { + const exitCode = await proc.exited; + if (exitCode !== 0) { + throw new Error(`sprite exec (silent) failed (exit ${exitCode})`); + } + } finally { + clearTimeout(timer); } } @@ -629,8 +638,12 @@ export async function destroyServer(name?: string): Promise { const stderrText = new Response(proc.stderr).text(); // 60s timeout — sprite destroy should not hang indefinitely const timer = setTimeout(() => killWithTimeout(proc), 60_000); - const exitCode = await proc.exited; - clearTimeout(timer); + let exitCode: number; + try { + exitCode = await proc.exited; + } finally { + clearTimeout(timer); + } if (exitCode !== 0) { logError(`Failed to destroy sprite '${target}'`); logError(`Delete it manually: sprite destroy ${target}`);