From 5243acc7c5cf1a1fe3323a2c16a87cdebc1db04e Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Tue, 14 Jul 2026 00:45:05 -0700 Subject: [PATCH] fix(runtime): keep local watching alive after an idle-kill deadline miss MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An idle child that outlived the exit deadline set shutdownRequested on the shared desktop supervisor, which has no retire-and-replace path — every later subscribe rejected supervisor_disposed and the roots were cached unwatchable, silently ending local file watching for the session. The idle path owns zero records, so there is no double-watch hazard; the zombie keeps its capacity reservation until physical exit and the next subscribe gets a fresh child. --- .../ipc/parcel-watcher-process-supervisor.ts | 9 +++-- ...parcel-watcher-unsubscribe-timeout.test.ts | 37 +++++++++++++++++++ 2 files changed, 42 insertions(+), 4 deletions(-) diff --git a/src/main/ipc/parcel-watcher-process-supervisor.ts b/src/main/ipc/parcel-watcher-process-supervisor.ts index 324056b4418..3f0a534c198 100644 --- a/src/main/ipc/parcel-watcher-process-supervisor.ts +++ b/src/main/ipc/parcel-watcher-process-supervisor.ts @@ -254,11 +254,12 @@ export class WatcherProcessSupervisor { // Why: destructive Windows cleanup must await exit to release directory handles. this.canaryDir = removeWatcherCanaryDirectory(this.canaryDir) return this.terminationQueue.track( - termination.terminateIdleWatcherChild(proc, this.pendingUnsubscribes, (exited) => { + termination.terminateIdleWatcherChild(proc, this.pendingUnsubscribes, () => { + // Why: an idle child owns zero records, so a missed exit deadline has no + // double-watch hazard; the child keeps its capacity reservation until + // physical exit, and poisoning this supervisor would permanently end + // local watching — the shared singleton has no retire-and-replace path. this.terminatingChild = null - if (!exited) { - this.shutdownRequested = true - } }) ) } diff --git a/src/main/ipc/parcel-watcher-unsubscribe-timeout.test.ts b/src/main/ipc/parcel-watcher-unsubscribe-timeout.test.ts index 026578938e7..b028a13bf92 100644 --- a/src/main/ipc/parcel-watcher-unsubscribe-timeout.test.ts +++ b/src/main/ipc/parcel-watcher-unsubscribe-timeout.test.ts @@ -83,6 +83,43 @@ describe('watcher native unsubscribe timeout', () => { } }) + it('spawns a fresh child after the idle kill misses the exit deadline', async () => { + vi.useFakeTimers() + try { + const promise = subscribeViaWatcherProcess('/repo', vi.fn(), {}) + const first = currentChild() + ackSubscribe(first, 0) + const subscription = await promise + + const unsubscribe = subscription.unsubscribe().catch((error: unknown) => error) + expect(first.kill).toHaveBeenCalledTimes(1) + await vi.advanceTimersByTimeAsync(WATCHER_PROCESS_EXIT_DEADLINE_MS) + expect(first.kill).toHaveBeenLastCalledWith('SIGKILL') + // Why: removal callers still fail closed on the unproven exit. + const error = (await unsubscribe) as Error & { physicalExit?: Promise } + expect(error).toMatchObject({ + message: 'file watcher process did not exit after termination deadline', + physicalExit: expect.any(Promise) + }) + + // Why: the idle child owned zero records, so its zombie must not poison + // the shared supervisor — the next subscribe gets a fresh child. + const respawnPromise = subscribeViaWatcherProcess('/repo2', vi.fn(), {}) + expect(forkMock).toHaveBeenCalledTimes(2) + const second = currentChild() + expect(second).not.toBe(first) + expect(second.sent[0]).toMatchObject({ op: 'subscribe', dir: '/repo2' }) + ackSubscribe(second) + await expect(respawnPromise).resolves.toMatchObject({ unsubscribe: expect.any(Function) }) + + first.emit('exit', null, 'SIGKILL') + await error.physicalExit + expect(vi.getTimerCount()).toBe(0) + } finally { + vi.useRealTimers() + } + }) + it('rejects only after an unkillable child reaches its exit deadline', async () => { vi.useFakeTimers() try {