From 633c790bd1369cd6079cd802472973b1d6b66a91 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Tue, 21 Jul 2026 20:44:44 -0700 Subject: [PATCH] fix(app): bound the wake/quit paths implicated in the phone-session-ended freeze (#9447) - relay-transport: waitForClose now times out (5s) so a half-open post-sleep socket can't wedge runtimeRpc.stop() - will-quit: race teardown against a 20s deadline so app.quit() always runs (Force Quit was the only escape when any teardown member never settled) - terminal-fit-restore: local restoreTerminalFit invoke gets the same 15s bound as the remote path so the held-fit modal buttons can't pin disabled --- src/main/index.ts | 10 +++- src/main/quit-teardown-deadline.test.ts | 45 +++++++++++++++ src/main/quit-teardown-deadline.ts | 21 +++++++ src/main/runtime/rpc/relay-transport.test.ts | 55 +++++++++++++++++++ src/main/runtime/rpc/relay-transport.ts | 13 ++++- .../terminal-fit-restore.test.ts | 22 ++++++++ .../terminal-pane/terminal-fit-restore.ts | 25 ++++++++- 7 files changed, 187 insertions(+), 4 deletions(-) create mode 100644 src/main/quit-teardown-deadline.test.ts create mode 100644 src/main/quit-teardown-deadline.ts diff --git a/src/main/index.ts b/src/main/index.ts index a1e50ba3b42..018b8a3cc90 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -195,6 +195,7 @@ import { createHeadlessAutomationOutputSnapshotBuffer } from './automations/head import { buildHeadlessAutomationWorktreeCreateArgs } from './automations/headless-workspace-create' import { AgentAwakeService } from './agent-awake-service' import { registerSystemResumeBroadcast } from './system-resume-broadcast' +import { settleTeardownWithinDeadline } from './quit-teardown-deadline' import { recordCoalescedCrashBreadcrumb, recordCrashBreadcrumb @@ -2478,7 +2479,14 @@ app.on('will-quit', (e) => { // Why: telemetry flush folds in before app.quit() (bounded 2s); catch defensively so a flush failure can't cancel the quit chain. // Why: normal quits keep the detached daemon for warm reattach, but a dead dev parent leaves the temp/dev profile ownerless. const daemonTeardown = isDevParentShutdownRequested() ? shutdownDaemon() : disconnectDaemon() - Promise.allSettled([daemonTeardown, rpcStopAndClear, watcherShutdown, emulatorShutdown]) + // Why: a wedged transport (half-open post-sleep socket) can leave one + // member unsettled forever and block app.quit() until Force Quit (#9447). + settleTeardownWithinDeadline([ + daemonTeardown, + rpcStopAndClear, + watcherShutdown, + emulatorShutdown + ]) .then(() => shutdownTelemetry()) .then(() => shutdownObservability()) .catch(() => { diff --git a/src/main/quit-teardown-deadline.test.ts b/src/main/quit-teardown-deadline.test.ts new file mode 100644 index 00000000000..3fa7eeb6715 --- /dev/null +++ b/src/main/quit-teardown-deadline.test.ts @@ -0,0 +1,45 @@ +import { afterEach, describe, expect, it, vi } from 'vitest' +import { + settleTeardownWithinDeadline, + WILL_QUIT_TEARDOWN_DEADLINE_MS +} from './quit-teardown-deadline' + +describe('settleTeardownWithinDeadline', () => { + afterEach(() => { + vi.useRealTimers() + }) + + it('resolves as soon as all teardowns settle, including rejections', async () => { + vi.useFakeTimers() + let resolved = false + const pending = settleTeardownWithinDeadline([ + Promise.resolve(), + Promise.reject(new Error('daemon disconnect failed')) + ]).then(() => { + resolved = true + }) + await vi.advanceTimersByTimeAsync(0) + await pending + expect(resolved).toBe(true) + }) + + it('resolves at the deadline when a teardown never settles', async () => { + vi.useFakeTimers() + let resolved = false + const pending = settleTeardownWithinDeadline([new Promise(() => {})]).then(() => { + resolved = true + }) + await vi.advanceTimersByTimeAsync(WILL_QUIT_TEARDOWN_DEADLINE_MS - 1) + expect(resolved).toBe(false) + await vi.advanceTimersByTimeAsync(1) + await pending + expect(resolved).toBe(true) + }) + + // Why: pin the magnitude so the wedge escape hatch cannot be silently + // shrunk below checkpoint-write time or grown past user patience. + it('keeps the deadline within the checkpoint-safe window', () => { + expect(WILL_QUIT_TEARDOWN_DEADLINE_MS).toBeGreaterThanOrEqual(10_000) + expect(WILL_QUIT_TEARDOWN_DEADLINE_MS).toBeLessThanOrEqual(30_000) + }) +}) diff --git a/src/main/quit-teardown-deadline.ts b/src/main/quit-teardown-deadline.ts new file mode 100644 index 00000000000..888f2aeed58 --- /dev/null +++ b/src/main/quit-teardown-deadline.ts @@ -0,0 +1,21 @@ +// Why: will-quit defers app.quit() until teardown settles. Teardown members +// are individually bounded, but a wedged transport (half-open post-sleep +// socket) can leave one unsettled forever and make Force Quit the only way +// out (#9447). Racing a deadline guarantees quit always completes. + +// Why: generous enough for daemon checkpoint writes on a slow disk; small +// enough that a wedged teardown never needs Force Quit. +export const WILL_QUIT_TEARDOWN_DEADLINE_MS = 20_000 + +export function settleTeardownWithinDeadline( + teardowns: Promise[], + deadlineMs: number = WILL_QUIT_TEARDOWN_DEADLINE_MS +): Promise { + const deadline = new Promise((resolve) => { + const timer = setTimeout(resolve, deadlineMs) + if (typeof timer.unref === 'function') { + timer.unref() + } + }) + return Promise.race([Promise.allSettled(teardowns).then(() => undefined), deadline]) +} diff --git a/src/main/runtime/rpc/relay-transport.test.ts b/src/main/runtime/rpc/relay-transport.test.ts index 432503b9a3c..e350a098f48 100644 --- a/src/main/runtime/rpc/relay-transport.test.ts +++ b/src/main/runtime/rpc/relay-transport.test.ts @@ -93,6 +93,61 @@ describe('CloudRelayTransport', () => { await vi.waitFor(() => expect(onConnectionClosed).toHaveBeenCalledWith('conn/with spaces')) }) + it('stop() resolves after the close timeout when a socket never emits close', async () => { + vi.useFakeTimers() + try { + const listeners = new Map void)[]>() + const addListener = (event: string, fn: (...args: unknown[]) => void): void => { + const existing = listeners.get(event) ?? [] + existing.push(fn) + listeners.set(event, existing) + } + const emit = (event: string, ...args: unknown[]): void => { + for (const fn of listeners.get(event) ?? []) { + fn(...args) + } + } + // Why: models a half-open post-sleep relay socket — terminate() never + // produces a 'close' event, which previously hung stop() forever. + const fakeSocket = { + readyState: 1, + OPEN: 1, + CLOSED: 3, + on: addListener, + once: addListener, + send: () => {}, + terminate: () => {} + } + const transport = new CloudRelayTransport({ + cellUrl: 'http://127.0.0.1:9', + relayHostId: 'AbCdEf0123_-xyZ9', + generation: 1, + createSocket: () => fakeSocket as unknown as WebSocketClient + }) + const opening = transport.openConnection({ + connId: 'conn-1', + connTicket: 'ticket-1', + kind: 'resume', + relayDeviceId: 'device-1', + attachDeadlineMs: 1_000 + }) + emit('open') + await opening + + let stopped = false + const stopPromise = transport.stop().then(() => { + stopped = true + }) + await vi.advanceTimersByTimeAsync(4_999) + expect(stopped).toBe(false) + await vi.advanceTimersByTimeAsync(1) + await stopPromise + expect(stopped).toBe(true) + } finally { + vi.useRealTimers() + } + }) + it('rejects non-origin cell URLs before opening a socket', () => { expect( () => diff --git a/src/main/runtime/rpc/relay-transport.ts b/src/main/runtime/rpc/relay-transport.ts index 1a34afdf849..b3bdb5bc34e 100644 --- a/src/main/runtime/rpc/relay-transport.ts +++ b/src/main/runtime/rpc/relay-transport.ts @@ -3,6 +3,9 @@ import type { RpcTransport } from './transport' import type { MobileSocketTransport, MobileSocketTransportMetadata } from './mobile-socket-wiring' const MAX_RELAY_MESSAGE_BYTES = 1024 * 1024 +// Why: terminate() normally emits 'close' within one tick; 5s covers slow +// teardown without letting a dead socket hold stop() (and app quit) hostage. +export const RELAY_SOCKET_CLOSE_TIMEOUT_MS = 5_000 type RelayMessagePayload = string | Uint8Array @@ -212,6 +215,14 @@ export class CloudRelayTransport implements RpcTransport, MobileSocketTransport if (socket.readyState === socket.CLOSED) { return Promise.resolve() } - return new Promise((resolve) => socket.once('close', resolve)) + // Why: a half-open relay socket after system sleep can never emit 'close'; + // an unbounded wait here wedges stop() and blocks app quit (#9447). + return new Promise((resolve) => { + const deadline = setTimeout(resolve, RELAY_SOCKET_CLOSE_TIMEOUT_MS) + socket.once('close', () => { + clearTimeout(deadline) + resolve() + }) + }) } } diff --git a/src/renderer/src/components/terminal-pane/terminal-fit-restore.test.ts b/src/renderer/src/components/terminal-pane/terminal-fit-restore.test.ts index 07f030faab6..8e034ca5129 100644 --- a/src/renderer/src/components/terminal-pane/terminal-fit-restore.test.ts +++ b/src/renderer/src/components/terminal-pane/terminal-fit-restore.test.ts @@ -99,6 +99,28 @@ describe('terminal-fit-restore', () => { await expect(restoreTerminalFitToDesktop('pty-local', undefined)).resolves.toBe(false) }) + it('fails a local restore whose invoke never resolves instead of hanging', async () => { + vi.useFakeTimers() + try { + vi.mocked(getRemoteRuntimeTerminalHandle).mockReturnValue(null) + // Why: models a wedged runtime/daemon after system sleep (#9447) — the + // IPC invoke stays pending forever. + restoreTerminalFit.mockReturnValue(new Promise(() => {})) + + let settled: boolean | null = null + const pending = restoreTerminalFitToDesktop('pty-local', undefined).then((restored) => { + settled = restored + }) + await vi.advanceTimersByTimeAsync(14_999) + expect(settled).toBeNull() + await vi.advanceTimersByTimeAsync(1) + await pending + expect(settled).toBe(false) + } finally { + vi.useRealTimers() + } + }) + it('treats failed remote RPC restore transport as not restored', async () => { vi.mocked(getRemoteRuntimeTerminalHandle).mockReturnValue('terminal-fail') vi.mocked(getRemoteRuntimePtyEnvironmentId).mockReturnValue('env-fail') diff --git a/src/renderer/src/components/terminal-pane/terminal-fit-restore.ts b/src/renderer/src/components/terminal-pane/terminal-fit-restore.ts index 54738f59021..f8ddfb3bcde 100644 --- a/src/renderer/src/components/terminal-pane/terminal-fit-restore.ts +++ b/src/renderer/src/components/terminal-pane/terminal-fit-restore.ts @@ -15,11 +15,30 @@ type TerminalFitRestoreSettings = Pick { // Why: terminal fit restore is best-effort when mobile/remote transports disappear. return { restored: false } } +// Why: a wedged runtime/daemon after system sleep can leave the invoke pending +// forever, which pins the held-fit modal's buttons disabled (#9447). Fail the +// restore instead so the user can retry. +const withRestoreFitTimeout = async ( + pending: Promise<{ restored: boolean }> +): Promise<{ restored: boolean }> => { + let timer: ReturnType | undefined + const timedOut = new Promise<{ restored: boolean }>((resolve) => { + timer = setTimeout(() => resolve(restoreFailedResult()), RESTORE_FIT_TIMEOUT_MS) + }) + try { + return await Promise.race([pending, timedOut]) + } finally { + clearTimeout(timer) + } +} + export async function restoreTerminalFitToDesktop( ptyId: string, settings: TerminalFitRestoreSettings @@ -33,9 +52,11 @@ export async function restoreTerminalFitToDesktop( { kind: 'environment', environmentId }, 'terminal.restoreFit', { terminal: remoteHandle }, - { timeoutMs: 15_000 } + { timeoutMs: RESTORE_FIT_TIMEOUT_MS } ).catch(restoreFailedResult) - : await window.api.runtime.restoreTerminalFit(ptyId).catch(restoreFailedResult) + : await withRestoreFitTimeout( + window.api.runtime.restoreTerminalFit(ptyId).catch(restoreFailedResult) + ) return result.restored }