Skip to content

Commit 314eb99

Browse files
roomoteroomote
committed
fix: improve process termination for pnpm and other package managers
- Enhanced TerminalProcess abort() to handle package managers more aggressively - Added multi-stage termination strategy for stubborn processes - Improved ExecaTerminalProcess to kill entire process tree - Added forceClose() method to Terminal class for stuck terminals - Uses SIGTERM first for graceful shutdown, then SIGKILL if needed Fixes #7455
1 parent c479678 commit 314eb99

File tree

3 files changed

+145
-46
lines changed

3 files changed

+145
-46
lines changed

‎src/integrations/terminal/ExecaTerminalProcess.ts‎

Lines changed: 98 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -159,62 +159,116 @@ export class ExecaTerminalProcess extends BaseTerminalProcess {
159159
public override abort() {
160160
this.aborted = true
161161

162+
// Function to kill entire process tree
163+
const killProcessTree = (pid: number, signal: NodeJS.Signals = "SIGKILL") => {
164+
return new Promise<void>((resolve) => {
165+
psTree(pid, (err, children) => {
166+
if (!err && children.length > 0) {
167+
const pids = children.map((p) => parseInt(p.PID))
168+
console.log(
169+
`[ExecaTerminalProcess#abort] Killing process tree for PID ${pid}: ${pids.join(", ")}`,
170+
)
171+
172+
// Kill children first (bottom-up approach)
173+
for (const childPid of pids.reverse()) {
174+
try {
175+
process.kill(childPid, signal)
176+
} catch (e) {
177+
// Process might already be dead
178+
console.debug(
179+
`[ExecaTerminalProcess#abort] Failed to send ${signal} to child PID ${childPid}: ${e instanceof Error ? e.message : String(e)}`,
180+
)
181+
}
182+
}
183+
}
184+
185+
// Then kill the parent
186+
try {
187+
process.kill(pid, signal)
188+
} catch (e) {
189+
console.debug(
190+
`[ExecaTerminalProcess#abort] Failed to send ${signal} to parent PID ${pid}: ${e instanceof Error ? e.message : String(e)}`,
191+
)
192+
}
193+
194+
resolve()
195+
})
196+
})
197+
}
198+
162199
// Function to perform the kill operations
163-
const performKill = () => {
164-
// Try to kill using the subprocess object
165-
if (this.subprocess) {
166-
try {
167-
this.subprocess.kill("SIGKILL")
168-
} catch (e) {
169-
console.warn(
170-
`[ExecaTerminalProcess#abort] Failed to kill subprocess: ${e instanceof Error ? e.message : String(e)}`,
171-
)
200+
const performKill = async () => {
201+
const command = this.command?.toLowerCase() || ""
202+
const needsAggressiveTermination =
203+
command.includes("pnpm") ||
204+
command.includes("npm") ||
205+
command.includes("yarn") ||
206+
command.includes("bun")
207+
208+
if (needsAggressiveTermination) {
209+
console.log(
210+
`[ExecaTerminalProcess#abort] Detected package manager command, using aggressive termination`,
211+
)
212+
213+
// First try SIGTERM to allow graceful shutdown
214+
if (this.subprocess) {
215+
try {
216+
this.subprocess.kill("SIGTERM")
217+
} catch (e) {
218+
console.debug(
219+
`[ExecaTerminalProcess#abort] Failed to send SIGTERM to subprocess: ${e instanceof Error ? e.message : String(e)}`,
220+
)
221+
}
172222
}
173-
}
174223

175-
// Kill the stored PID (which should be the actual command after our update)
176-
if (this.pid) {
177-
try {
178-
process.kill(this.pid, "SIGKILL")
179-
} catch (e) {
180-
console.warn(
181-
`[ExecaTerminalProcess#abort] Failed to kill process ${this.pid}: ${e instanceof Error ? e.message : String(e)}`,
182-
)
224+
// Kill the entire process tree with SIGTERM first
225+
if (this.pid) {
226+
await killProcessTree(this.pid, "SIGTERM")
227+
}
228+
229+
// Wait a bit for graceful shutdown
230+
await new Promise((resolve) => setTimeout(resolve, 500))
231+
232+
// Then force kill with SIGKILL if still running
233+
if (this.subprocess && !this.subprocess.killed) {
234+
try {
235+
this.subprocess.kill("SIGKILL")
236+
} catch (e) {
237+
console.debug(
238+
`[ExecaTerminalProcess#abort] Failed to send SIGKILL to subprocess: ${e instanceof Error ? e.message : String(e)}`,
239+
)
240+
}
241+
}
242+
243+
// Force kill the entire process tree
244+
if (this.pid) {
245+
await killProcessTree(this.pid, "SIGKILL")
246+
}
247+
} else {
248+
// For regular commands, use the standard approach
249+
if (this.subprocess) {
250+
try {
251+
this.subprocess.kill("SIGKILL")
252+
} catch (e) {
253+
console.warn(
254+
`[ExecaTerminalProcess#abort] Failed to kill subprocess: ${e instanceof Error ? e.message : String(e)}`,
255+
)
256+
}
257+
}
258+
259+
// Kill the stored PID and its children
260+
if (this.pid) {
261+
await killProcessTree(this.pid, "SIGKILL")
183262
}
184263
}
185264
}
186265

187266
// If PID update is in progress, wait for it before killing
188267
if (this.pidUpdatePromise) {
189-
this.pidUpdatePromise.then(performKill).catch(() => performKill())
268+
this.pidUpdatePromise.then(() => performKill()).catch(() => performKill())
190269
} else {
191270
performKill()
192271
}
193-
194-
// Continue with the rest of the abort logic
195-
if (this.pid) {
196-
// Also check for any child processes
197-
psTree(this.pid, async (err, children) => {
198-
if (!err) {
199-
const pids = children.map((p) => parseInt(p.PID))
200-
console.error(`[ExecaTerminalProcess#abort] SIGKILL children -> ${pids.join(", ")}`)
201-
202-
for (const pid of pids) {
203-
try {
204-
process.kill(pid, "SIGKILL")
205-
} catch (e) {
206-
console.warn(
207-
`[ExecaTerminalProcess#abort] Failed to send SIGKILL to child PID ${pid}: ${e instanceof Error ? e.message : String(e)}`,
208-
)
209-
}
210-
}
211-
} else {
212-
console.error(
213-
`[ExecaTerminalProcess#abort] Failed to get process tree for PID ${this.pid}: ${err.message}`,
214-
)
215-
}
216-
})
217-
}
218272
}
219273

220274
public override hasUnretrievedOutput() {

‎src/integrations/terminal/Terminal.ts‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,18 @@ export class Terminal extends BaseTerminal {
4040
return this.terminal.exitStatus !== undefined
4141
}
4242

43+
/**
44+
* Force close the terminal if it's stuck
45+
*/
46+
public forceClose(): void {
47+
try {
48+
// Try to dispose the terminal directly
49+
this.terminal.dispose()
50+
} catch (error) {
51+
console.error(`[Terminal] Failed to force close terminal ${this.id}:`, error)
52+
}
53+
}
54+
4355
public override runCommand(command: string, callbacks: RooTerminalCallbacks): RooTerminalProcessResultPromise {
4456
// We set busy before the command is running because the terminal may be
4557
// waiting on terminal integration, and we must prevent another instance

‎src/integrations/terminal/TerminalProcess.ts‎

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -266,8 +266,41 @@ export class TerminalProcess extends BaseTerminalProcess {
266266

267267
public override abort() {
268268
if (this.isListening) {
269-
// Send SIGINT using CTRL+C
270-
this.terminal.terminal.sendText("\x03")
269+
// For pnpm and similar tools that spawn child processes,
270+
// we need to be more aggressive with termination
271+
const command = this.command?.toLowerCase() || ""
272+
const needsAggressiveTermination =
273+
command.includes("pnpm") ||
274+
command.includes("npm") ||
275+
command.includes("yarn") ||
276+
command.includes("bun")
277+
278+
if (needsAggressiveTermination) {
279+
// First try SIGINT (CTRL+C)
280+
this.terminal.terminal.sendText("\x03")
281+
282+
// Then send SIGTERM after a short delay
283+
setTimeout(() => {
284+
// Send SIGTERM (CTRL+\)
285+
this.terminal.terminal.sendText("\x1c")
286+
}, 100)
287+
288+
// Finally, if the terminal supports it, try to kill the process tree
289+
setTimeout(() => {
290+
// Send SIGKILL as last resort (may not work in all terminals)
291+
// This is a more aggressive approach for stubborn processes
292+
this.terminal.terminal.sendText("\x03")
293+
// Try to send 'exit' command to close the terminal if process is stuck
294+
setTimeout(() => {
295+
if (this.terminal.busy) {
296+
this.terminal.terminal.sendText("exit\n")
297+
}
298+
}, 500)
299+
}, 1000)
300+
} else {
301+
// Send SIGINT using CTRL+C for regular commands
302+
this.terminal.terminal.sendText("\x03")
303+
}
271304
}
272305
}
273306

0 commit comments

Comments
 (0)