mirror of
https://github.com/NanmiCoder/cc-haha
synced 2026-07-16 13:03:31 +08:00
fix: prevent orphaned desktop CLI sidecars
Packaged desktop shutdown was killing the server sidecar with SIGKILL on Unix, which skipped server-side cleanup for CLI subprocesses spawned by active chat sessions. The native layer now gives sidecars a SIGTERM grace window, and the server waits for active CLI sessions to exit before completing signal-driven shutdown. Constraint: Tauri shell CommandChild::kill maps to SIGKILL on Unix. Rejected: Only clean stale processes on next launch | leaves user-owned sessions and sockets in an unknown state after every unclean app exit. Confidence: high Scope-risk: moderate Directive: Do not replace the SIGTERM grace path with direct CommandChild::kill without rechecking packaged-app process-tree shutdown. Tested: bun test src/server/__tests__/conversation-service.test.ts Tested: bun run check:server Tested: bun run check:native Tested: git diff --check Not-tested: Fresh DMG install plus manual quit/reopen process-tree smoke.
This commit is contained in:
parent
fd040e3798
commit
4b3eb16347
@ -231,6 +231,7 @@ const APP_MODE_FILE: &str = "app-mode.json";
|
||||
const MIN_WINDOW_WIDTH: u32 = 960;
|
||||
const MIN_WINDOW_HEIGHT: u32 = 640;
|
||||
const MIN_VISIBLE_PIXELS: i64 = 64;
|
||||
const SIDECAR_GRACEFUL_TERMINATION_TIMEOUT: Duration = Duration::from_millis(3_000);
|
||||
|
||||
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)]
|
||||
#[serde(rename_all = "lowercase")]
|
||||
@ -1819,9 +1820,53 @@ fn kill_sidecar_child(child: CommandChild) {
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
{
|
||||
terminate_unix_sidecar_child(child);
|
||||
}
|
||||
|
||||
#[cfg(not(unix))]
|
||||
{
|
||||
let _ = child.kill();
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
fn terminate_unix_sidecar_child(child: CommandChild) {
|
||||
let pid = child.pid();
|
||||
let pid_text = pid.to_string();
|
||||
|
||||
// tauri-plugin-shell's CommandChild::kill() maps to SIGKILL on Unix.
|
||||
// Give bundled sidecars a SIGTERM window first so the server can stop
|
||||
// CLI sessions it spawned before the native app exits.
|
||||
let _ = StdCommand::new("kill")
|
||||
.args(["-TERM", &pid_text])
|
||||
.stdout(Stdio::null())
|
||||
.stderr(Stdio::null())
|
||||
.status();
|
||||
|
||||
let deadline = Instant::now() + SIDECAR_GRACEFUL_TERMINATION_TIMEOUT;
|
||||
while Instant::now() < deadline {
|
||||
if !is_unix_process_running(pid) {
|
||||
return;
|
||||
}
|
||||
std::thread::sleep(Duration::from_millis(50));
|
||||
}
|
||||
|
||||
let _ = child.kill();
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
fn is_unix_process_running(pid: u32) -> bool {
|
||||
StdCommand::new("kill")
|
||||
.args(["-0", &pid.to_string()])
|
||||
.stdout(Stdio::null())
|
||||
.stderr(Stdio::null())
|
||||
.status()
|
||||
.map(|status| status.success())
|
||||
.unwrap_or(false)
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
fn kill_stale_unix_adapter_sidecars() {
|
||||
let current_pid = std::process::id();
|
||||
|
||||
@ -599,6 +599,54 @@ describe('ConversationService', () => {
|
||||
expect(args).toContain('--worktree-base-ref')
|
||||
expect(args).toContain('feature/rail')
|
||||
})
|
||||
|
||||
test('stopAllSessionsAndWait kills every active CLI subprocess and waits for exits', async () => {
|
||||
const service = new ConversationService() as any
|
||||
const killed: string[] = []
|
||||
const drained: string[] = []
|
||||
|
||||
const makeSession = (sessionId: string) => {
|
||||
let resolveExit: (code: number) => void = () => {}
|
||||
const exited = new Promise<number>((resolve) => {
|
||||
resolveExit = resolve
|
||||
})
|
||||
|
||||
return {
|
||||
proc: {
|
||||
kill: () => {
|
||||
killed.push(sessionId)
|
||||
resolveExit(0)
|
||||
},
|
||||
exited,
|
||||
},
|
||||
outputCallbacks: [],
|
||||
workDir: tmpDir,
|
||||
permissionMode: 'default',
|
||||
sdkToken: `${sessionId}-token`,
|
||||
sdkSocket: null,
|
||||
pendingOutbound: [],
|
||||
startupPending: false,
|
||||
startupExitCode: null,
|
||||
stdoutLines: [],
|
||||
stderrLines: [],
|
||||
outputDrain: Promise.resolve().then(() => {
|
||||
drained.push(sessionId)
|
||||
}),
|
||||
sdkMessages: [],
|
||||
initMessage: null,
|
||||
pendingPermissionRequests: new Map(),
|
||||
}
|
||||
}
|
||||
|
||||
service.sessions.set('session-a', makeSession('session-a'))
|
||||
service.sessions.set('session-b', makeSession('session-b'))
|
||||
|
||||
await service.stopAllSessionsAndWait(500)
|
||||
|
||||
expect(killed.sort()).toEqual(['session-a', 'session-b'])
|
||||
expect(drained.sort()).toEqual(['session-a', 'session-b'])
|
||||
expect(service.getActiveSessions()).toEqual([])
|
||||
})
|
||||
})
|
||||
|
||||
function sanitizeMemoryPath(value: string): string {
|
||||
|
||||
@ -454,26 +454,46 @@ export function startServer(port = PORT, host = HOST) {
|
||||
|
||||
// ─── Graceful shutdown: kill all CLI subprocesses on exit ────────────────────
|
||||
|
||||
let shutdownInProgress: Promise<void> | null = null
|
||||
|
||||
function cleanupAllSessions() {
|
||||
const active = conversationService.getActiveSessions()
|
||||
if (active.length > 0) {
|
||||
console.log(`[Server] Shutting down — killing ${active.length} CLI subprocess(es)`)
|
||||
for (const sessionId of active) {
|
||||
conversationService.stopSession(sessionId)
|
||||
}
|
||||
conversationService.stopAllSessions()
|
||||
}
|
||||
}
|
||||
|
||||
async function cleanupAllSessionsAndWait() {
|
||||
const active = conversationService.getActiveSessions()
|
||||
if (active.length > 0) {
|
||||
console.log(`[Server] Shutting down — killing ${active.length} CLI subprocess(es)`)
|
||||
await conversationService.stopAllSessionsAndWait()
|
||||
}
|
||||
}
|
||||
|
||||
function shutdownAndExit(signal: 'SIGTERM' | 'SIGINT', exitCode: number) {
|
||||
if (shutdownInProgress) return
|
||||
|
||||
shutdownInProgress = (async () => {
|
||||
console.log(`[Server] Received ${signal}`)
|
||||
await cleanupAllSessionsAndWait()
|
||||
process.exit(exitCode)
|
||||
})().catch((error) => {
|
||||
console.error(
|
||||
`[Server] ${signal} shutdown cleanup failed:`,
|
||||
error instanceof Error ? error.message : error,
|
||||
)
|
||||
process.exit(1)
|
||||
})
|
||||
}
|
||||
|
||||
process.on('SIGTERM', () => {
|
||||
console.log('[Server] Received SIGTERM')
|
||||
cleanupAllSessions()
|
||||
process.exit(0)
|
||||
shutdownAndExit('SIGTERM', 0)
|
||||
})
|
||||
|
||||
process.on('SIGINT', () => {
|
||||
console.log('[Server] Received SIGINT')
|
||||
cleanupAllSessions()
|
||||
process.exit(0)
|
||||
shutdownAndExit('SIGINT', 0)
|
||||
})
|
||||
|
||||
process.on('exit', () => {
|
||||
|
||||
@ -733,10 +733,10 @@ export class ConversationService {
|
||||
|
||||
stopSession(sessionId: string): void {
|
||||
const session = this.sessions.get(sessionId)
|
||||
if (session) {
|
||||
session.proc.kill()
|
||||
this.sessions.delete(sessionId)
|
||||
}
|
||||
if (!session) return
|
||||
|
||||
this.sessions.delete(sessionId)
|
||||
this.killProcess(sessionId, session)
|
||||
}
|
||||
|
||||
async stopSessionAndWait(sessionId: string, timeoutMs = 2_000): Promise<void> {
|
||||
@ -744,15 +744,64 @@ export class ConversationService {
|
||||
if (!session) return
|
||||
|
||||
this.sessions.delete(sessionId)
|
||||
session.proc.kill()
|
||||
await this.stopProcessAndWait(sessionId, session, timeoutMs)
|
||||
}
|
||||
|
||||
await Promise.race([
|
||||
session.proc.exited.catch(() => undefined),
|
||||
new Promise<void>((resolve) => setTimeout(resolve, timeoutMs)),
|
||||
stopAllSessions(): void {
|
||||
for (const sessionId of this.getActiveSessions()) {
|
||||
this.stopSession(sessionId)
|
||||
}
|
||||
}
|
||||
|
||||
async stopAllSessionsAndWait(timeoutMs = 2_000): Promise<void> {
|
||||
const activeSessions = Array.from(this.sessions.entries())
|
||||
if (activeSessions.length === 0) return
|
||||
|
||||
this.sessions.clear()
|
||||
await Promise.all(
|
||||
activeSessions.map(([sessionId, session]) =>
|
||||
this.stopProcessAndWait(sessionId, session, timeoutMs),
|
||||
),
|
||||
)
|
||||
}
|
||||
|
||||
private async stopProcessAndWait(
|
||||
sessionId: string,
|
||||
session: SessionProcess,
|
||||
timeoutMs: number,
|
||||
): Promise<void> {
|
||||
this.killProcess(sessionId, session, 'SIGTERM')
|
||||
|
||||
const exited = await Promise.race([
|
||||
session.proc.exited.then(() => true, () => true),
|
||||
new Promise<boolean>((resolve) => setTimeout(() => resolve(false), timeoutMs)),
|
||||
])
|
||||
if (!exited) {
|
||||
this.killProcess(sessionId, session, 'SIGKILL')
|
||||
await Promise.race([
|
||||
session.proc.exited.catch(() => undefined),
|
||||
new Promise<void>((resolve) => setTimeout(resolve, 500)),
|
||||
])
|
||||
}
|
||||
await this.waitForProcessOutputDrain(session, timeoutMs)
|
||||
}
|
||||
|
||||
private killProcess(
|
||||
sessionId: string,
|
||||
session: SessionProcess,
|
||||
signal?: NodeJS.Signals,
|
||||
): void {
|
||||
try {
|
||||
session.proc.kill(signal)
|
||||
} catch (error) {
|
||||
console.warn(
|
||||
`[ConversationService] Failed to kill CLI subprocess for ${sessionId}: ${
|
||||
error instanceof Error ? error.message : String(error)
|
||||
}`,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
markSessionDeleted(sessionId: string): void {
|
||||
this.deletedSessions.add(sessionId)
|
||||
this.stopSession(sessionId)
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user