diff --git a/src/main/mobile/cloudflared-tunnel.ts b/src/main/mobile/cloudflared-tunnel.ts index 5066faf6d..36744ff3f 100644 --- a/src/main/mobile/cloudflared-tunnel.ts +++ b/src/main/mobile/cloudflared-tunnel.ts @@ -184,6 +184,29 @@ export interface CloudflareTunnelInstance extends InternetTunnelInstance { provider: 'cloudflare' } +/** + * Stop a spawned child with a SIGTERM grace period, escalating to SIGKILL + * only while the process is still running. `ChildProcess.killed` becomes true + * as soon as kill() is *called* — it reports intent, not liveness — so an + * escalation guarded by `!child.killed` is dead code and a cloudflared that + * ignores SIGTERM would survive every cleanup. Exit status (`exitCode`/ + * `signalCode`) is the liveness signal Node actually maintains. + */ +export function terminateChildProcess( + child: { + exitCode: number | null + signalCode: NodeJS.Signals | null + kill(signal: NodeJS.Signals): boolean + }, + graceMs = 2000 +): void { + if (child.exitCode !== null || child.signalCode !== null) return + child.kill('SIGTERM') + setTimeout(() => { + if (child.exitCode === null && child.signalCode === null) child.kill('SIGKILL') + }, graceMs).unref?.() +} + export async function startCloudflareQuickTunnel(options: { port: number binaryPath: string @@ -246,12 +269,7 @@ export async function startCloudflareQuickTunnel(options: { const cleanup = () => { try { - if (!child.killed) { - child.kill('SIGTERM') - setTimeout(() => { - if (!child.killed) child.kill('SIGKILL') - }, 2000).unref?.() - } + terminateChildProcess(child) } catch {} } }) diff --git a/test/lan-mobile-tunnel.test.ts b/test/lan-mobile-tunnel.test.ts index be912db48..90de6d627 100644 --- a/test/lan-mobile-tunnel.test.ts +++ b/test/lan-mobile-tunnel.test.ts @@ -13,7 +13,8 @@ import { ensureCloudflaredBinary, extractTryCloudflareUrl, resolveCurrentAssetSpec, - sha256OfFile + sha256OfFile, + terminateChildProcess } from '../src/main/mobile/cloudflared-tunnel' import { startTunnelWithFallback, @@ -64,6 +65,56 @@ describe('Cloudflare Quick Tunnel utilities', () => { expect(CLOUDFLARED_VERSION).toBeTruthy() }) + + describe('terminateChildProcess escalation', () => { + interface FakeChild { + exitCode: number | null + signalCode: NodeJS.Signals | null + killed: boolean + kill: (signal: NodeJS.Signals) => boolean + } + + function fakeChild(): FakeChild { + const child = { + exitCode: null, + signalCode: null, + killed: false, + kill: vi.fn((signal: NodeJS.Signals) => { + child.killed = true + return true + }) + } + return child + } + + it('escalates to SIGKILL when SIGTERM did not stop the child', async () => { + const child = fakeChild() + terminateChildProcess(child, 10) + expect(child.kill).toHaveBeenCalledWith('SIGTERM') + await new Promise((resolve) => setTimeout(resolve, 40)) + expect(child.kill).toHaveBeenCalledWith('SIGKILL') + }) + + it('never escalates once the child has exited after SIGTERM', async () => { + const child = fakeChild() + vi.mocked(child.kill).mockImplementationOnce(() => { + child.killed = true + child.exitCode = 143 + return true + }) + terminateChildProcess(child, 10) + await new Promise((resolve) => setTimeout(resolve, 40)) + expect(child.kill).toHaveBeenCalledTimes(1) + expect(child.kill).toHaveBeenCalledWith('SIGTERM') + }) + + it('leaves an already-exited child untouched', () => { + const child = fakeChild() + child.exitCode = 0 + terminateChildProcess(child, 10) + expect(child.kill).not.toHaveBeenCalled() + }) + }) }) describe('Pinggy Tunnel utilities', () => {