diff --git a/changes/sync-ui-window-close-quit.md b/changes/sync-ui-window-close-quit.md new file mode 100644 index 00000000..26726f5c --- /dev/null +++ b/changes/sync-ui-window-close-quit.md @@ -0,0 +1,5 @@ +type: fixed +area: sync + +- Fixed the app staying alive in the background after closing a standalone Sync window (`subminer sync --ui`) on macOS. The quit re-issued after async will-quit cleanup was dropped by Electron when the cleanup settled within the same tick as the will-quit dispatch; the re-quit now runs on a fresh macrotask, and the standalone Sync window close path uses the same forced-exit fallback as SIGTERM. Windows opened from the tray menu of a running app are unaffected: closing them still leaves the app running. +- Fixed cancelling or timing out a sync run hanging when the sync child had already exited without emitting a terminal result: the run waited on `close`, which a descendant holding the inherited stdio pipes can delay indefinitely, stalling the quit-time sync shutdown. Cancellation now settles the run immediately once the child has exited. diff --git a/src/core/services/app-lifecycle.test.ts b/src/core/services/app-lifecycle.test.ts index bba2d4e4..9d6eb74a 100644 --- a/src/core/services/app-lifecycle.test.ts +++ b/src/core/services/app-lifecycle.test.ts @@ -168,7 +168,14 @@ test('startAppLifecycle defers quit until async cleanup settles', async () => { assert.ok(releaseCleanup); (releaseCleanup as () => void)(); await cleanupDone; + // The re-quit must not fire in the microtask turn of the will-quit + // dispatch: Electron drops a quit issued while the prevented quit is + // still unwinding, leaving a windowless process alive. await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + assert.deepEqual(calls, []); + await new Promise((resolve) => setImmediate(resolve)); assert.deepEqual(calls, ['quitApp']); }); diff --git a/src/core/services/app-lifecycle.ts b/src/core/services/app-lifecycle.ts index e74643cd..e71fc138 100644 --- a/src/core/services/app-lifecycle.ts +++ b/src/core/services/app-lifecycle.ts @@ -219,7 +219,11 @@ export function startAppLifecycle(initialArgs: CliArgs, deps: AppLifecycleServic .finally(() => { quitCleanupPending = false; quitCleanupComplete = true; - deps.quitApp(); + // A cleanup promise that settles in a microtask would re-quit while + // Electron is still unwinding the prevented quit, and that quit call + // is silently dropped, leaving a windowless process alive. Re-issue + // the quit from a fresh macrotask instead. + setImmediate(() => deps.quitApp()); }); }); diff --git a/src/main.ts b/src/main.ts index c8226f6a..844aa648 100644 --- a/src/main.ts +++ b/src/main.ts @@ -2247,7 +2247,9 @@ const openSyncUiWindowHandler = createOpenConfigSettingsWindowHandler({ promoteSettingsWindowAboveOverlay: (window) => promoteSettingsWindowAboveOverlay(window as BrowserWindow), onClosed: () => { - if (appState.initialArgs?.syncWindow) app.quit(); + // requestAppQuit (not app.quit) so the forced-exit fallback covers a + // stalled quit; a standalone sync window is the app's only reason to live. + if (appState.initialArgs?.syncWindow) requestAppQuit(); }, log: (message) => logger.error(message), }); diff --git a/src/main/runtime/sync-launcher-client.test.ts b/src/main/runtime/sync-launcher-client.test.ts index 7e72998e..d326f71e 100644 --- a/src/main/runtime/sync-launcher-client.test.ts +++ b/src/main/runtime/sync-launcher-client.test.ts @@ -165,6 +165,56 @@ test('runSyncLauncher cancel kills the child and resolves as cancelled', async ( assert.match(result.error ?? '', /cancel/i); }); +test('runSyncLauncher cancel settles a child that already exited without a result', async () => { + const { spawn, children } = makeSpawn(); + const handle = runSyncLauncher({ + command: ['subminer'], + args: ['sync', 'media-box', '--json'], + onEvent: () => {}, + spawn, + }); + const child = children[0]!; + // The child is gone, but a descendant still holds the inherited stdio pipes, + // so `close` never arrives. + child.kill = () => { + child.killed = true; + return true; + }; + child.emit('exit', null, 'SIGTERM'); + + handle.cancel(); + + const result = await Promise.race([ + handle.done, + new Promise((resolve) => setTimeout(() => resolve(null), 25)), + ]); + assert.deepEqual(result, { ok: false, error: 'Sync cancelled.' }); + assert.equal(child.killed, false); +}); + +test('runSyncLauncher times out a child that already exited without a result', async () => { + const { spawn, children } = makeSpawn(); + const handle = runSyncLauncher({ + command: ['subminer'], + args: ['sync', 'media-box', '--check', '--json'], + onEvent: () => {}, + spawn, + timeoutMs: 1, + }); + const child = children[0]!; + child.kill = () => { + child.killed = true; + return true; + }; + child.emit('exit', null, 'SIGTERM'); + + const result = await Promise.race([ + handle.done, + new Promise((resolve) => setTimeout(() => resolve(null), 25)), + ]); + assert.deepEqual(result, { ok: false, error: 'Sync operation timed out.' }); +}); + test('runSyncLauncher times out a child that never completes', async () => { const { spawn, children } = makeSpawn(); const handle = runSyncLauncher({ diff --git a/src/main/runtime/sync-launcher-client.ts b/src/main/runtime/sync-launcher-client.ts index ef060721..9f462ed0 100644 --- a/src/main/runtime/sync-launcher-client.ts +++ b/src/main/runtime/sync-launcher-client.ts @@ -75,6 +75,7 @@ export function runSyncLauncher(options: { let resultEvent: Extract | null = null; let terminationError: string | null = null; let settleAfterTerminalEvent: (() => void) | null = null; + let settleAfterTermination: (() => boolean) | null = null; child.stdout?.on('data', (chunk) => { stdoutBuffer += chunk.toString(); @@ -137,6 +138,11 @@ export function runSyncLauncher(options: { settleAfterTerminalEvent = () => { if (exitObserved) settleFromExit(exitCode); }; + settleAfterTermination = () => { + if (!exitObserved) return false; + settleFromExit(exitCode); + return true; + }; // Electron descendants can retain inherited stdio pipes after the main // child exits, delaying `close` indefinitely. Once terminal NDJSON has // arrived, `exit` is authoritative; keep `close` as the fallback. @@ -153,6 +159,11 @@ export function runSyncLauncher(options: { const terminate = (error: string): void => { if (terminationError) return; terminationError = error; + // A child that already exited without terminal NDJSON is still waiting on + // `close`, which descendants holding inherited pipes can delay + // indefinitely. There is nothing left to signal, so settle now instead of + // leaving `done` (and the quit-time shutdown that awaits it) pending. + if (settleAfterTermination?.()) return; killTimer = setTimeout(() => { killTimer = null; try {