Skip to content

fix(desktop): kill all terminal child processes on close #892

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Kitenite merged 4 commits into from
Jan 22, 2026
Merged

fix(desktop): kill all terminal child processes on close #892

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e1551ad
fix(desktop): kill all terminal child processes on close using TTY
chasemcdo Jan 22, 2026
427e342
refactor(desktop): use execFileSync to avoid shell interpolation
chasemcdo Jan 22, 2026
a10385f
refactor(desktop): use tree-kill instead of TTY-based process cleanup
chasemcdo Jan 22, 2026
d386714
fix(desktop): add error logging to tree-kill callbacks
Kitenite Jan 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions apps/desktop/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@
"strip-ansi": "^7.1.2",
"superjson": "^2.2.5",
"tailwind-merge": "^3.4.0",
"tree-kill": "^1.2.2",
"trpc-electron": "^0.1.2",
"tw-animate-css": "^1.4.0",
"zod": "^4.3.5",
Expand Down
37 changes: 21 additions & 16 deletions apps/desktop/src/main/terminal-host/pty-subprocess.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import { write as fsWrite } from "node:fs";
import type { IPty } from "node-pty";
import * as pty from "node-pty";
import treeKill from "tree-kill";
import {
PtySubprocessFrameDecoder,
PtySubprocessIpcType,
Expand Down Expand Up @@ -367,23 +368,24 @@ function handleKill(payload: Buffer): void {

const pid = ptyProcess.pid;

// Step 1: Send the requested signal (usually SIGTERM for graceful shutdown)
try {
ptyProcess.kill(signal);
} catch {
// Process may already be dead
}
// Step 1: Kill the process tree using tree-kill
// tree-kill traverses by PPID to find all descendants
treeKill(pid, signal, (err) => {
if (err) {
console.error("[pty-subprocess] Failed to kill process tree:", err);
}
});

// Step 2: Escalate to SIGKILL if still alive after 2 seconds
// node-pty's onExit callback may not fire reliably after pty.kill()
const escalationTimer = setTimeout(() => {
if (!ptyProcess) return; // Already exited via onExit

try {
ptyProcess.kill("SIGKILL");
} catch {
// Process may already be dead
}
treeKill(pid, "SIGKILL", (err) => {
if (err) {
console.error("[pty-subprocess] Failed to SIGKILL process tree:", err);
}
});

// Step 3: Force completion if onExit still hasn't fired after another 1 second
// This ensures the subprocess exits even if node-pty never emits onExit
Expand Down Expand Up @@ -440,11 +442,14 @@ function handleDispose(): void {
ptyFd = null;

if (ptyProcess) {
try {
ptyProcess.kill("SIGKILL");
} catch {
// Ignore
}
const pid = ptyProcess.pid;

// Kill the process tree - fire and forget since we're exiting
treeKill(pid, "SIGKILL", (err) => {
if (err) {
console.error("[pty-subprocess] Failed to kill process tree:", err);
}
});
Copy link
Copy Markdown

@cursor cursor Bot Jan 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Async tree-kill skipped by immediate process.exit

Medium Severity

In handleDispose(), treeKill() is called asynchronously but process.exit(0) executes synchronously on the next line. Since tree-kill internally spawns child processes to discover and kill the process tree, calling process.exit(0) immediately terminates the Node.js event loop before tree-kill can complete its work. The comment "fire and forget since we're exiting" is misleading—async operations cannot complete if the process exits synchronously. Child processes won't be killed in this code path.

Fix in Cursor Fix in Web

ptyProcess = null;
}

Expand Down
5 changes: 4 additions & 1 deletion bun.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.