From f070d16d380e172912625e9be986f96bd7626ee1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=A8=8B=E5=BA=8F=E5=91=98=E9=98=BF=E6=B1=9F=28Relakkes?= =?UTF-8?q?=29?= Date: Wed, 3 Jun 2026 14:01:50 +0800 Subject: [PATCH] fix(desktop): kill sidecars synchronously on quit to avoid orphans killSidecar gains a sync flag (spawnSync taskkill on Windows); serverRuntime stopAll threads it through; before-quit now shuts down synchronously so the Windows taskkill completes before the process exits. Co-Authored-By: Claude Opus 4.8 (1M context) --- desktop/electron/main.ts | 4 +- desktop/electron/services/serverRuntime.ts | 10 ++--- .../electron/services/sidecarManager.test.ts | 37 ++++++++++++++++++- desktop/electron/services/sidecarManager.ts | 23 ++++++++++-- 4 files changed, 63 insertions(+), 11 deletions(-) diff --git a/desktop/electron/main.ts b/desktop/electron/main.ts index 34b41d3e..69b1f365 100644 --- a/desktop/electron/main.ts +++ b/desktop/electron/main.ts @@ -389,5 +389,7 @@ app.on('before-quit', () => { trayController = null terminalService?.killAll() previewService?.close() - getServerRuntime().stopAll() + // Synchronous on quit so the Windows taskkill completes before the process + // exits, otherwise the fire-and-forget kill can leave orphaned sidecars. + getServerRuntime().stopAll(true) }) diff --git a/desktop/electron/services/serverRuntime.ts b/desktop/electron/services/serverRuntime.ts index 5840d36f..a3c1e815 100644 --- a/desktop/electron/services/serverRuntime.ts +++ b/desktop/electron/services/serverRuntime.ts @@ -64,10 +64,10 @@ export class ElectronServerRuntime { await this.startAdaptersSidecars(serverUrl) } - stopAll() { - this.stopAdaptersSidecars() + stopAll(sync = false) { + this.stopAdaptersSidecars(sync) if (this.server) { - killSidecar(this.server.child) + killSidecar(this.server.child, sync) this.server = null } } @@ -125,9 +125,9 @@ export class ElectronServerRuntime { } } - private stopAdaptersSidecars() { + private stopAdaptersSidecars(sync = false) { for (const child of this.adapters.splice(0)) { - killSidecar(child) + killSidecar(child, sync) } } diff --git a/desktop/electron/services/sidecarManager.test.ts b/desktop/electron/services/sidecarManager.test.ts index c2a69c07..d8d7c3c3 100644 --- a/desktop/electron/services/sidecarManager.test.ts +++ b/desktop/electron/services/sidecarManager.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, it } from 'vitest' +import { describe, expect, it, vi } from 'vitest' import path from 'node:path' import { mkdtempSync, rmSync } from 'node:fs' import { tmpdir } from 'node:os' @@ -7,12 +7,18 @@ import { createAdapterPlan, createServerPlan, httpToWebSocketUrl, + killSidecar, mergeProxyEnv, proxyUrlFromElectronProxyRules, pushStartupLog, resolveHostTriple, + type SidecarChild, } from './sidecarManager' +function fakeChild(pid = 4321) { + return { pid, kill: vi.fn() } as unknown as SidecarChild & { kill: ReturnType } +} + describe('Electron sidecar manager', () => { it('maps host platform to existing sidecar target triples', () => { expect(resolveHostTriple('darwin', 'arm64')).toBe('aarch64-apple-darwin') @@ -113,4 +119,33 @@ describe('Electron sidecar manager', () => { expect(httpToWebSocketUrl('http://127.0.0.1:3456')).toBe('ws://127.0.0.1:3456') expect(httpToWebSocketUrl('https://example.com')).toBe('wss://example.com') }) + + it('kills non-Windows sidecars with a signal', () => { + const child = fakeChild() + const spawnAsync = vi.fn() + const spawnSyncFn = vi.fn() + killSidecar(child, false, { platform: 'darwin', spawnAsync: spawnAsync as never, spawnSyncFn: spawnSyncFn as never }) + expect(child.kill).toHaveBeenCalledTimes(1) + expect(spawnAsync).not.toHaveBeenCalled() + expect(spawnSyncFn).not.toHaveBeenCalled() + }) + + it('uses async taskkill on Windows by default', () => { + const child = fakeChild(777) + const spawnAsync = vi.fn() + const spawnSyncFn = vi.fn() + killSidecar(child, false, { platform: 'win32', spawnAsync: spawnAsync as never, spawnSyncFn: spawnSyncFn as never }) + expect(spawnAsync).toHaveBeenCalledWith('taskkill', ['/F', '/T', '/PID', '777'], { stdio: 'ignore' }) + expect(spawnSyncFn).not.toHaveBeenCalled() + expect(child.kill).not.toHaveBeenCalled() + }) + + it('uses synchronous taskkill on Windows during shutdown to avoid orphaned sidecars', () => { + const child = fakeChild(777) + const spawnAsync = vi.fn() + const spawnSyncFn = vi.fn() + killSidecar(child, true, { platform: 'win32', spawnAsync: spawnAsync as never, spawnSyncFn: spawnSyncFn as never }) + expect(spawnSyncFn).toHaveBeenCalledWith('taskkill', ['/F', '/T', '/PID', '777'], { stdio: 'ignore' }) + expect(spawnAsync).not.toHaveBeenCalled() + }) }) diff --git a/desktop/electron/services/sidecarManager.ts b/desktop/electron/services/sidecarManager.ts index 55ad51a5..e78e63ab 100644 --- a/desktop/electron/services/sidecarManager.ts +++ b/desktop/electron/services/sidecarManager.ts @@ -1,4 +1,4 @@ -import { spawn, type ChildProcessByStdio } from 'node:child_process' +import { spawn, spawnSync, type ChildProcessByStdio } from 'node:child_process' import { mkdirSync, existsSync } from 'node:fs' import type { Readable } from 'node:stream' import net from 'node:net' @@ -213,9 +213,24 @@ export function spawnSidecar(plan: SidecarPlan): SidecarChild { }) } -export function killSidecar(child: SidecarChild) { - if (process.platform === 'win32' && child.pid) { - spawn('taskkill', ['/F', '/T', '/PID', String(child.pid)], { stdio: 'ignore' }) +export type KillSidecarDeps = { + platform?: NodeJS.Platform + spawnAsync?: typeof spawn + spawnSyncFn?: typeof spawnSync +} + +/** + * Terminate a sidecar process. On Windows we shell out to `taskkill /T` to also + * reap the child process tree (the Bun sidecar spawns workers). Pass `sync=true` + * during app shutdown so the kill completes before the process exits — otherwise + * the async `taskkill` is fire-and-forget and can leave orphaned processes. + */ +export function killSidecar(child: SidecarChild, sync = false, deps: KillSidecarDeps = {}) { + const platform = deps.platform ?? process.platform + if (platform === 'win32' && child.pid) { + const args = ['/F', '/T', '/PID', String(child.pid)] + if (sync) (deps.spawnSyncFn ?? spawnSync)('taskkill', args, { stdio: 'ignore' }) + else (deps.spawnAsync ?? spawn)('taskkill', args, { stdio: 'ignore' }) return } child.kill()