mirror of
https://github.com/AutoMaker-Org/automaker.git
synced 2026-02-02 20:43:36 +00:00
fix: unify PTY process termination handling across platforms
Refactored the process termination logic in both ClaudeUsageService and TerminalService to use a centralized method for killing PTY processes. This ensures consistent handling of process termination across Windows and Unix-like systems, improving reliability and maintainability of the code.
This commit is contained in:
@@ -23,6 +23,22 @@ export class ClaudeUsageService {
|
|||||||
private isWindows = os.platform() === 'win32';
|
private isWindows = os.platform() === 'win32';
|
||||||
private isLinux = os.platform() === 'linux';
|
private isLinux = os.platform() === 'linux';
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Kill a PTY process with platform-specific handling.
|
||||||
|
* Windows doesn't support Unix signals like SIGTERM, so we call kill() without arguments.
|
||||||
|
* On Unix-like systems (macOS, Linux), we can specify the signal.
|
||||||
|
*
|
||||||
|
* @param ptyProcess - The PTY process to kill
|
||||||
|
* @param signal - The signal to send on Unix-like systems (default: 'SIGTERM')
|
||||||
|
*/
|
||||||
|
private killPtyProcess(ptyProcess: pty.IPty, signal: string = 'SIGTERM'): void {
|
||||||
|
if (this.isWindows) {
|
||||||
|
ptyProcess.kill();
|
||||||
|
} else {
|
||||||
|
ptyProcess.kill(signal);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Check if Claude CLI is available on the system
|
* Check if Claude CLI is available on the system
|
||||||
*/
|
*/
|
||||||
@@ -211,7 +227,7 @@ export class ClaudeUsageService {
|
|||||||
if (!settled) {
|
if (!settled) {
|
||||||
settled = true;
|
settled = true;
|
||||||
if (ptyProcess && !ptyProcess.killed) {
|
if (ptyProcess && !ptyProcess.killed) {
|
||||||
ptyProcess.kill();
|
this.killPtyProcess(ptyProcess);
|
||||||
}
|
}
|
||||||
// Don't fail if we have data - return it instead
|
// Don't fail if we have data - return it instead
|
||||||
if (output.includes('Current session')) {
|
if (output.includes('Current session')) {
|
||||||
@@ -253,7 +269,7 @@ export class ClaudeUsageService {
|
|||||||
if (!settled) {
|
if (!settled) {
|
||||||
settled = true;
|
settled = true;
|
||||||
if (ptyProcess && !ptyProcess.killed) {
|
if (ptyProcess && !ptyProcess.killed) {
|
||||||
ptyProcess.kill();
|
this.killPtyProcess(ptyProcess);
|
||||||
}
|
}
|
||||||
reject(
|
reject(
|
||||||
new Error(
|
new Error(
|
||||||
@@ -277,14 +293,10 @@ export class ClaudeUsageService {
|
|||||||
ptyProcess.write('\x1b'); // Send escape key
|
ptyProcess.write('\x1b'); // Send escape key
|
||||||
|
|
||||||
// Fallback: if ESC doesn't exit (Linux), use SIGTERM after 2s
|
// Fallback: if ESC doesn't exit (Linux), use SIGTERM after 2s
|
||||||
// Windows doesn't support signals, so just call kill() without args
|
// Windows doesn't support signals, so killPtyProcess handles platform differences
|
||||||
setTimeout(() => {
|
setTimeout(() => {
|
||||||
if (!settled && ptyProcess && !ptyProcess.killed) {
|
if (!settled && ptyProcess && !ptyProcess.killed) {
|
||||||
if (this.isWindows) {
|
this.killPtyProcess(ptyProcess);
|
||||||
ptyProcess.kill();
|
|
||||||
} else {
|
|
||||||
ptyProcess.kill('SIGTERM');
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
}, 2000);
|
}, 2000);
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -70,6 +70,23 @@ export class TerminalService extends EventEmitter {
|
|||||||
private sessions: Map<string, TerminalSession> = new Map();
|
private sessions: Map<string, TerminalSession> = new Map();
|
||||||
private dataCallbacks: Set<DataCallback> = new Set();
|
private dataCallbacks: Set<DataCallback> = new Set();
|
||||||
private exitCallbacks: Set<ExitCallback> = new Set();
|
private exitCallbacks: Set<ExitCallback> = new Set();
|
||||||
|
private isWindows = os.platform() === 'win32';
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Kill a PTY process with platform-specific handling.
|
||||||
|
* Windows doesn't support Unix signals like SIGTERM/SIGKILL, so we call kill() without arguments.
|
||||||
|
* On Unix-like systems (macOS, Linux), we can specify the signal.
|
||||||
|
*
|
||||||
|
* @param ptyProcess - The PTY process to kill
|
||||||
|
* @param signal - The signal to send on Unix-like systems (default: 'SIGTERM')
|
||||||
|
*/
|
||||||
|
private killPtyProcess(ptyProcess: pty.IPty, signal: string = 'SIGTERM'): void {
|
||||||
|
if (this.isWindows) {
|
||||||
|
ptyProcess.kill();
|
||||||
|
} else {
|
||||||
|
ptyProcess.kill(signal);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Detect the best shell for the current platform
|
* Detect the best shell for the current platform
|
||||||
@@ -477,8 +494,9 @@ export class TerminalService extends EventEmitter {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// First try graceful SIGTERM to allow process cleanup
|
// First try graceful SIGTERM to allow process cleanup
|
||||||
|
// On Windows, killPtyProcess calls kill() without signal since Windows doesn't support Unix signals
|
||||||
logger.info(`Session ${sessionId} sending SIGTERM`);
|
logger.info(`Session ${sessionId} sending SIGTERM`);
|
||||||
session.pty.kill('SIGTERM');
|
this.killPtyProcess(session.pty, 'SIGTERM');
|
||||||
|
|
||||||
// Schedule SIGKILL fallback if process doesn't exit gracefully
|
// Schedule SIGKILL fallback if process doesn't exit gracefully
|
||||||
// The onExit handler will remove session from map when it actually exits
|
// The onExit handler will remove session from map when it actually exits
|
||||||
@@ -486,7 +504,7 @@ export class TerminalService extends EventEmitter {
|
|||||||
if (this.sessions.has(sessionId)) {
|
if (this.sessions.has(sessionId)) {
|
||||||
logger.info(`Session ${sessionId} still alive after SIGTERM, sending SIGKILL`);
|
logger.info(`Session ${sessionId} still alive after SIGTERM, sending SIGKILL`);
|
||||||
try {
|
try {
|
||||||
session.pty.kill('SIGKILL');
|
this.killPtyProcess(session.pty, 'SIGKILL');
|
||||||
} catch {
|
} catch {
|
||||||
// Process may have already exited
|
// Process may have already exited
|
||||||
}
|
}
|
||||||
@@ -588,7 +606,8 @@ export class TerminalService extends EventEmitter {
|
|||||||
if (session.flushTimeout) {
|
if (session.flushTimeout) {
|
||||||
clearTimeout(session.flushTimeout);
|
clearTimeout(session.flushTimeout);
|
||||||
}
|
}
|
||||||
session.pty.kill();
|
// Use platform-specific kill to ensure proper termination on Windows
|
||||||
|
this.killPtyProcess(session.pty);
|
||||||
} catch {
|
} catch {
|
||||||
// Ignore errors during cleanup
|
// Ignore errors during cleanup
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user