mirror of
https://github.com/ksyasuda/SubMiner.git
synced 2026-07-27 16:49:51 -07:00
fix(sync-ui): quit reliably after standalone sync window closes
- Re-issue post-cleanup quit via setImmediate so Electron doesn't drop it while unwinding the prevented quit - Use requestAppQuit (not app.quit) on sync window close for forced-exit fallback coverage - Settle cancelled/timed-out runs immediately when child already exited to avoid hanging on close held open by descendants
This commit is contained in:
@@ -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.
|
||||||
@@ -168,7 +168,14 @@ test('startAppLifecycle defers quit until async cleanup settles', async () => {
|
|||||||
assert.ok(releaseCleanup);
|
assert.ok(releaseCleanup);
|
||||||
(releaseCleanup as () => void)();
|
(releaseCleanup as () => void)();
|
||||||
await cleanupDone;
|
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();
|
||||||
|
await Promise.resolve();
|
||||||
|
assert.deepEqual(calls, []);
|
||||||
|
await new Promise((resolve) => setImmediate(resolve));
|
||||||
assert.deepEqual(calls, ['quitApp']);
|
assert.deepEqual(calls, ['quitApp']);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|||||||
@@ -219,7 +219,11 @@ export function startAppLifecycle(initialArgs: CliArgs, deps: AppLifecycleServic
|
|||||||
.finally(() => {
|
.finally(() => {
|
||||||
quitCleanupPending = false;
|
quitCleanupPending = false;
|
||||||
quitCleanupComplete = true;
|
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());
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|||||||
+3
-1
@@ -2247,7 +2247,9 @@ const openSyncUiWindowHandler = createOpenConfigSettingsWindowHandler({
|
|||||||
promoteSettingsWindowAboveOverlay: (window) =>
|
promoteSettingsWindowAboveOverlay: (window) =>
|
||||||
promoteSettingsWindowAboveOverlay(window as BrowserWindow),
|
promoteSettingsWindowAboveOverlay(window as BrowserWindow),
|
||||||
onClosed: () => {
|
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),
|
log: (message) => logger.error(message),
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -165,6 +165,56 @@ test('runSyncLauncher cancel kills the child and resolves as cancelled', async (
|
|||||||
assert.match(result.error ?? '', /cancel/i);
|
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<null>((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<null>((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 () => {
|
test('runSyncLauncher times out a child that never completes', async () => {
|
||||||
const { spawn, children } = makeSpawn();
|
const { spawn, children } = makeSpawn();
|
||||||
const handle = runSyncLauncher({
|
const handle = runSyncLauncher({
|
||||||
|
|||||||
@@ -75,6 +75,7 @@ export function runSyncLauncher(options: {
|
|||||||
let resultEvent: Extract<SyncProgressEvent, { type: 'result' }> | null = null;
|
let resultEvent: Extract<SyncProgressEvent, { type: 'result' }> | null = null;
|
||||||
let terminationError: string | null = null;
|
let terminationError: string | null = null;
|
||||||
let settleAfterTerminalEvent: (() => void) | null = null;
|
let settleAfterTerminalEvent: (() => void) | null = null;
|
||||||
|
let settleAfterTermination: (() => boolean) | null = null;
|
||||||
|
|
||||||
child.stdout?.on('data', (chunk) => {
|
child.stdout?.on('data', (chunk) => {
|
||||||
stdoutBuffer += chunk.toString();
|
stdoutBuffer += chunk.toString();
|
||||||
@@ -137,6 +138,11 @@ export function runSyncLauncher(options: {
|
|||||||
settleAfterTerminalEvent = () => {
|
settleAfterTerminalEvent = () => {
|
||||||
if (exitObserved) settleFromExit(exitCode);
|
if (exitObserved) settleFromExit(exitCode);
|
||||||
};
|
};
|
||||||
|
settleAfterTermination = () => {
|
||||||
|
if (!exitObserved) return false;
|
||||||
|
settleFromExit(exitCode);
|
||||||
|
return true;
|
||||||
|
};
|
||||||
// Electron descendants can retain inherited stdio pipes after the main
|
// Electron descendants can retain inherited stdio pipes after the main
|
||||||
// child exits, delaying `close` indefinitely. Once terminal NDJSON has
|
// child exits, delaying `close` indefinitely. Once terminal NDJSON has
|
||||||
// arrived, `exit` is authoritative; keep `close` as the fallback.
|
// arrived, `exit` is authoritative; keep `close` as the fallback.
|
||||||
@@ -153,6 +159,11 @@ export function runSyncLauncher(options: {
|
|||||||
const terminate = (error: string): void => {
|
const terminate = (error: string): void => {
|
||||||
if (terminationError) return;
|
if (terminationError) return;
|
||||||
terminationError = error;
|
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 = setTimeout(() => {
|
||||||
killTimer = null;
|
killTimer = null;
|
||||||
try {
|
try {
|
||||||
|
|||||||
Reference in New Issue
Block a user