diff --git a/apps/desktop/src/main/lib/terminal/port-manager.ts b/apps/desktop/src/main/lib/terminal/port-manager.ts index fd5955404e7..4b659b89b98 100644 --- a/apps/desktop/src/main/lib/terminal/port-manager.ts +++ b/apps/desktop/src/main/lib/terminal/port-manager.ts @@ -1,6 +1,6 @@ import { EventEmitter } from "node:events"; import type { DetectedPort } from "shared/types"; -import { treeKillWithEscalation } from "../tree-kill-with-escalation"; +import { treeKillWithEscalation } from "../tree-kill"; import { getListeningPortsForPids, getProcessTree, diff --git a/apps/desktop/src/main/lib/tree-kill-with-escalation.ts b/apps/desktop/src/main/lib/tree-kill.ts similarity index 90% rename from apps/desktop/src/main/lib/tree-kill-with-escalation.ts rename to apps/desktop/src/main/lib/tree-kill.ts index e6f9c1264ec..793b7b56ea0 100644 --- a/apps/desktop/src/main/lib/tree-kill-with-escalation.ts +++ b/apps/desktop/src/main/lib/tree-kill.ts @@ -3,6 +3,20 @@ import treeKill from "tree-kill"; const DEFAULT_ESCALATION_TIMEOUT_MS = 2000; const POLL_INTERVAL_MS = 50; +export function treeKillAsync(pid: number, signal: string): Promise { + return new Promise((resolve) => { + treeKill(pid, signal, (err) => { + if (err) { + console.warn( + `[treeKillAsync] Failed to ${signal} pid ${pid}:`, + err.message, + ); + } + resolve(); + }); + }); +} + /** * Kill a process tree with escalation to SIGKILL if the process survives. * Sends SIGTERM, polls for exit, escalates to SIGKILL after timeout. diff --git a/apps/desktop/src/main/terminal-host/index.ts b/apps/desktop/src/main/terminal-host/index.ts index 9faed98bf07..c1c7d18d6de 100644 --- a/apps/desktop/src/main/terminal-host/index.ts +++ b/apps/desktop/src/main/terminal-host/index.ts @@ -759,14 +759,13 @@ async function startServer(): Promise { }); } -function stopServer(): Promise { - return new Promise((resolve) => { - // Dispose terminal host (kills all sessions) - if (terminalHost) { - terminalHost.dispose(); - log("info", "Terminal host disposed"); - } +async function stopServer(): Promise { + if (terminalHost) { + await terminalHost.dispose(); + log("info", "Terminal host disposed"); + } + await new Promise((resolve) => { if (server) { server.close(() => { log("info", "Server closed"); @@ -775,15 +774,14 @@ function stopServer(): Promise { } else { resolve(); } - - // Clean up socket and PID files - try { - if (existsSync(SOCKET_PATH)) unlinkSync(SOCKET_PATH); - if (existsSync(PID_PATH)) unlinkSync(PID_PATH); - } catch { - // Best effort cleanup - } }); + + try { + if (existsSync(SOCKET_PATH)) unlinkSync(SOCKET_PATH); + if (existsSync(PID_PATH)) unlinkSync(PID_PATH); + } catch { + // Best effort cleanup + } } // ============================================================================= diff --git a/apps/desktop/src/main/terminal-host/session.ts b/apps/desktop/src/main/terminal-host/session.ts index fd0bf30143f..f0874b6aa50 100644 --- a/apps/desktop/src/main/terminal-host/session.ts +++ b/apps/desktop/src/main/terminal-host/session.ts @@ -22,6 +22,7 @@ import type { TerminalExitEvent, TerminalSnapshot, } from "../lib/terminal-host/types"; +import { treeKillAsync } from "../lib/tree-kill"; import { createFrameHeader, PtySubprocessFrameDecoder, @@ -340,24 +341,7 @@ export class Session { this.ptyReadyResolve = null; } - this.subprocess = null; - this.subprocessReady = false; - this.subprocessDecoder = null; - this.subprocessStdinQueue = []; - this.subprocessStdinQueuedBytes = 0; - this.subprocessStdinDrainArmed = false; - this.subprocessStdoutPaused = false; - - this.emulatorWriteQueue = []; - this.emulatorWriteQueuedBytes = 0; - this.emulatorWriteScheduled = false; - this.snapshotBoundaryIndex = null; - const waiters = this.emulatorFlushWaiters; - this.emulatorFlushWaiters = []; - for (const resolve of waiters) resolve(); - const boundaryWaiters = this.snapshotBoundaryWaiters; - this.snapshotBoundaryWaiters = []; - for (const resolve of boundaryWaiters) resolve(); + this.resetProcessState(); } /** @@ -814,33 +798,46 @@ export class Session { } } - /** - * Dispose of the session - */ - dispose(): void { - if (this.disposed) return; + /** Callers that don't need to wait can fire-and-forget. */ + dispose(): Promise { + if (this.disposed) return Promise.resolve(); this.disposed = true; + const pidsToKill = this.collectProcessPids(); + if (this.subprocess) { - // Capture reference before nullifying - the timeout needs it - const subprocess = this.subprocess; this.sendDisposeToSubprocess(); - // Force kill after timeout if dispose frame didn't terminate it - const killTimer = setTimeout(() => { - try { - subprocess.kill("SIGKILL"); - } catch { - // Process may already be dead - } - }, 1000); - killTimer.unref(); // Don't keep daemon alive for this timer - this.subprocess = null; } + + this.resetProcessState(); + this.emulator.dispose(); + this.attachedClients.clear(); + this.clientSocketsWaitingForDrain.clear(); + + if (pidsToKill.length === 0) return Promise.resolve(); + + // Must await: treeKill enumerates descendants via ps/pgrep before signaling + return Promise.all( + pidsToKill.map((pid) => treeKillAsync(pid, "SIGKILL")), + ).then(() => {}); + } + + /** Includes PTY PID as safety net in case the shell was reparented after subprocess exit. */ + private collectProcessPids(): number[] { + const pids: number[] = []; + if (this.subprocess?.pid) pids.push(this.subprocess.pid); + if (this.ptyPid) pids.push(this.ptyPid); + return pids; + } + + private resetProcessState(): void { + this.subprocess = null; this.subprocessReady = false; this.subprocessDecoder = null; this.subprocessStdinQueue = []; this.subprocessStdinQueuedBytes = 0; this.subprocessStdinDrainArmed = false; + this.subprocessStdoutPaused = false; this.emulatorWriteQueue = []; this.emulatorWriteQueuedBytes = 0; @@ -852,11 +849,6 @@ export class Session { const boundaryWaiters = this.snapshotBoundaryWaiters; this.snapshotBoundaryWaiters = []; for (const resolve of boundaryWaiters) resolve(); - - this.emulator.dispose(); - this.attachedClients.clear(); - this.clientSocketsWaitingForDrain.clear(); - this.subprocessStdoutPaused = false; } /** diff --git a/apps/desktop/src/main/terminal-host/terminal-host.ts b/apps/desktop/src/main/terminal-host/terminal-host.ts index 5f24c6a4fdf..95119036de1 100644 --- a/apps/desktop/src/main/terminal-host/terminal-host.ts +++ b/apps/desktop/src/main/terminal-host/terminal-host.ts @@ -90,14 +90,14 @@ export class TerminalHost { // Force-dispose terminating sessions to prevent race conditions if (session?.isTerminating) { - session.dispose(); + void session.dispose(); this.sessions.delete(sessionId); this.clearKillTimer(sessionId); session = undefined; } if (session && !session.isAlive) { - session.dispose(); + void session.dispose(); this.sessions.delete(sessionId); session = undefined; } @@ -137,7 +137,7 @@ export class TerminalHost { } if (!session.isAlive) { - session.dispose(); + void session.dispose(); throw new Error("Session spawn failed: PTY process exited immediately"); } @@ -207,7 +207,7 @@ export class TerminalHost { if (session) { session.detach(socket); if (!session.isAlive && session.clientCount === 0) { - session.dispose(); + void session.dispose(); this.sessions.delete(request.sessionId); } } @@ -253,7 +253,7 @@ export class TerminalHost { console.warn( `[TerminalHost] Force disposing stuck session ${sessionId} after ${KILL_TIMEOUT_MS}ms`, ); - s.dispose(); + void s.dispose(); this.sessions.delete(sessionId); } this.killTimers.delete(sessionId); @@ -318,22 +318,27 @@ export class TerminalHost { session.detach(socket); // Clean up dead sessions when last client detaches if (!session.isAlive && session.clientCount === 0) { - session.dispose(); + void session.dispose(); this.sessions.delete(sessionId); } } } - dispose(): void { + async dispose(): Promise { for (const timer of this.killTimers.values()) { clearTimeout(timer); } this.killTimers.clear(); - for (const session of this.sessions.values()) { - session.dispose(); - } + const sessions = [...this.sessions.values()]; this.sessions.clear(); + + if (sessions.length === 0) return; + + await Promise.race([ + Promise.all(sessions.map((s) => s.dispose())), + new Promise((resolve) => setTimeout(resolve, 5000)), + ]); } /** @@ -393,7 +398,7 @@ export class TerminalHost { } if (session.clientCount === 0) { - session.dispose(); + void session.dispose(); this.sessions.delete(sessionId); } else { this.scheduleSessionCleanup(sessionId);