From fe7042b94850db9ff64b9d183c8a72caff6851db Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Tue, 26 May 2026 20:24:42 -0700 Subject: [PATCH] fix: clean production launcher startup listeners (#2885) --- src/main/daemon/production-launcher.test.ts | 57 ++++++++++++++++++++ src/main/daemon/production-launcher.ts | 58 +++++++++++++++------ 2 files changed, 99 insertions(+), 16 deletions(-) diff --git a/src/main/daemon/production-launcher.test.ts b/src/main/daemon/production-launcher.test.ts index 832f6c949..721ead728 100644 --- a/src/main/daemon/production-launcher.test.ts +++ b/src/main/daemon/production-launcher.test.ts @@ -7,6 +7,15 @@ import { startDaemon, type DaemonHandle } from './daemon-main' import { DaemonClient } from './client' import type { SubprocessHandle } from './session' +const { forkMock } = vi.hoisted(() => ({ + forkMock: vi.fn() +})) + +vi.mock('child_process', async () => { + const actual = await vi.importActual>('child_process') + return { ...actual, fork: forkMock } +}) + function createTestDir(): string { return mkdtempSync(join(tmpdir(), 'prod-launcher-test-')) } @@ -41,6 +50,7 @@ describe('createProductionLauncher', () => { for (const h of handles) { await h.shutdown().catch(() => {}) } + forkMock.mockReset() rmSync(dir, { recursive: true, force: true }) }) @@ -75,4 +85,51 @@ describe('createProductionLauncher', () => { await handle.shutdown() handles.pop() }) + + it('removes startup child listeners after readiness', async () => { + const handlers: Record void)[]> = { + message: [], + error: [], + exit: [] + } + const child = { + pid: 12345, + killed: false, + on: vi.fn((event: string, cb: (arg?: unknown) => void) => { + handlers[event]?.push(cb) + return child + }), + off: vi.fn((event: string, cb: (arg?: unknown) => void) => { + handlers[event] = handlers[event]?.filter((handler) => handler !== cb) ?? [] + return child + }), + kill: vi.fn(), + disconnect: vi.fn(), + unref: vi.fn() + } + forkMock.mockReturnValueOnce(child) + + const launcher = createProductionLauncher({ + getDaemonEntryPath: () => join(dir, 'daemon-entry.js') + }) + + const launch = launcher(socketPathFor(dir), tokenPathFor(dir)) + handlers.message[0]?.({ type: 'ready' }) + const handle = await launch + + expect(handle.shutdown).toEqual(expect.any(Function)) + expect(handlers.message).toHaveLength(0) + expect(handlers.error).toHaveLength(0) + expect(handlers.exit).toHaveLength(0) + expect(child.disconnect).toHaveBeenCalled() + expect(child.unref).toHaveBeenCalled() + }) }) + +function socketPathFor(dir: string): string { + return join(dir, 'test.sock') +} + +function tokenPathFor(dir: string): string { + return join(dir, 'test.token') +} diff --git a/src/main/daemon/production-launcher.ts b/src/main/daemon/production-launcher.ts index 6ce852bb0..10693ef79 100644 --- a/src/main/daemon/production-launcher.ts +++ b/src/main/daemon/production-launcher.ts @@ -32,27 +32,53 @@ export function createProductionLauncher(opts: ProductionLauncherOptions): Daemo function waitForReady(child: ChildProcess): Promise { return new Promise((resolve, reject) => { - const timeout = setTimeout(() => { - child.kill('SIGTERM') - reject(new Error('Daemon failed to signal readiness within timeout')) - }, READY_TIMEOUT_MS) - - child.on('message', (msg: unknown) => { - if (msg && typeof msg === 'object' && (msg as Record).type === 'ready') { + let timeout: ReturnType | undefined + let settled = false + function cleanupStartupListeners(): void { + if (timeout) { clearTimeout(timeout) + } + child.off('message', onMessage) + child.off('error', onError) + child.off('exit', onExit) + } + function fail(error: Error, killChild = false): void { + if (settled) { + return + } + settled = true + cleanupStartupListeners() + if (killChild) { + child.kill('SIGTERM') + } + reject(error) + } + function onMessage(msg: unknown): void { + if (msg && typeof msg === 'object' && (msg as Record).type === 'ready') { + if (settled) { + return + } + settled = true + // Why: the daemon is detached after readiness, so startup listeners + // must not keep the child process closure alive for the daemon lifetime. + cleanupStartupListeners() resolve() } - }) + } + function onError(err: Error): void { + fail(new Error(`Daemon process error: ${err.message}`)) + } + function onExit(code: number | null): void { + fail(new Error(`Daemon process exited prematurely with code ${code}`)) + } - child.on('error', (err) => { - clearTimeout(timeout) - reject(new Error(`Daemon process error: ${err.message}`)) - }) + timeout = setTimeout(() => { + fail(new Error('Daemon failed to signal readiness within timeout'), true) + }, READY_TIMEOUT_MS) - child.on('exit', (code) => { - clearTimeout(timeout) - reject(new Error(`Daemon process exited prematurely with code ${code}`)) - }) + child.on('message', onMessage) + child.on('error', onError) + child.on('exit', onExit) }) }