fix(persistence): make the renderer unload checkpoint durably flush before reporting success (#12387)

The sync before-unload checkpoint staged renderer state and then queued
store.flushPendingAsync() fire-and-forget, so reload/restart/update paths
navigated while the staged session, scrollback and UI state were still
only in memory. Quit is covered by the will-quit flush barrier; those
paths were not.

Keep staging synchronous (no sync durable writes), but record the flush
outcome and expose it on app:await-before-unload-checkpoint. Restart,
updater install and lazy-chunk recovery reload now join that write before
navigating and abort the attempt when it fails or outlives a 20s deadline.
This commit is contained in:
Brennan Benson 2026-08-03 19:18:40 -07:00 committed by GitHub
parent 9e5bd5fb84
commit 194e1a8d4d
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
12 changed files with 300 additions and 37 deletions

View File

@ -1,10 +1,11 @@
import { beforeEach, describe, expect, it, vi } from 'vitest'
const { syncHandlers } = vi.hoisted(() => ({
const { syncHandlers, invokeHandlers } = vi.hoisted(() => ({
syncHandlers: new Map<
string,
(event: { returnValue?: unknown }, args: Record<string, unknown>) => void
>()
>(),
invokeHandlers: new Map<string, () => Promise<{ ok: boolean }>>()
}))
vi.mock('electron', () => ({
@ -16,15 +17,25 @@ vi.mock('electron', () => ({
) => {
syncHandlers.set(channel, handler)
}
)
),
handle: vi.fn((channel: string, handler: () => Promise<{ ok: boolean }>) => {
invokeHandlers.set(channel, handler)
})
}
}))
import { registerRendererShutdownCheckpointHandler } from './renderer-shutdown-checkpoint'
import {
registerRendererShutdownCheckpointHandler,
SHUTDOWN_CHECKPOINT_FLUSH_DEADLINE_MS
} from './renderer-shutdown-checkpoint'
const AWAIT_CHANNEL = 'app:await-before-unload-checkpoint'
describe('registerRendererShutdownCheckpointHandler', () => {
beforeEach(() => {
syncHandlers.clear()
invokeHandlers.clear()
vi.restoreAllMocks()
})
it('stages every shutdown mutation before queueing persistence', () => {
@ -34,7 +45,7 @@ describe('registerRendererShutdownCheckpointHandler', () => {
callOrder.push(`session:${hostId ?? 'local'}`)
}),
updateUI: vi.fn(() => callOrder.push('ui')),
flushPendingAsync: vi.fn(() => {
flushPendingOrThrowAsync: vi.fn(() => {
callOrder.push('persist')
return Promise.resolve()
})
@ -62,7 +73,11 @@ describe('registerRendererShutdownCheckpointHandler', () => {
'runtime:host-1'
)
expect(store.updateUI).toHaveBeenCalledWith({ activeView: 'settings' })
expect(store.flushPendingAsync).toHaveBeenCalledTimes(1)
expect(store.flushPendingOrThrowAsync).toHaveBeenCalledTimes(1)
// Why: a live app keeps mutating state, so draining to a stable generation would livelock.
expect(store.flushPendingOrThrowAsync).toHaveBeenCalledWith(
expect.objectContaining({ drainToStableGeneration: false })
)
expect(callOrder).toEqual(['session:local', 'session:runtime:host-1', 'ui', 'persist'])
expect(event.returnValue).toEqual({ ok: true })
})
@ -73,7 +88,7 @@ describe('registerRendererShutdownCheckpointHandler', () => {
updateUI: vi.fn(() => {
throw new Error('disk full')
}),
flushPendingAsync: vi.fn(() => Promise.resolve())
flushPendingOrThrowAsync: vi.fn(() => Promise.resolve())
}
registerRendererShutdownCheckpointHandler(store as never)
@ -84,11 +99,11 @@ describe('registerRendererShutdownCheckpointHandler', () => {
expect(event.returnValue).toEqual({ ok: false })
})
it('does not queue persistence when staging is incomplete', () => {
it('does not queue persistence when staging is incomplete', async () => {
const store = {
stageWorkspaceSessionBeforeUnload: vi.fn(),
updateUI: vi.fn(),
flushPendingAsync: vi.fn(() => Promise.resolve())
flushPendingOrThrowAsync: vi.fn(() => Promise.resolve())
}
registerRendererShutdownCheckpointHandler(store as never)
@ -99,19 +114,16 @@ describe('registerRendererShutdownCheckpointHandler', () => {
const event: { returnValue?: unknown } = {}
handler?.(event, { sessions: [], ui: { activeView: 'settings' } })
expect(store.flushPendingAsync).not.toHaveBeenCalled()
expect(store.flushPendingOrThrowAsync).not.toHaveBeenCalled()
expect(event.returnValue).toEqual({ ok: false })
await expect(invokeHandlers.get(AWAIT_CHANNEL)?.()).resolves.toEqual({ ok: false })
})
it('returns before the asynchronous persistence settles', () => {
let resolve!: () => void
const pending = new Promise<void>((next) => {
resolve = next
})
it('stages synchronously without waiting on the durable write', () => {
const store = {
stageWorkspaceSessionBeforeUnload: vi.fn(),
updateUI: vi.fn(),
flushPendingAsync: vi.fn(() => pending)
flushPendingOrThrowAsync: vi.fn(() => new Promise<void>(() => {}))
}
registerRendererShutdownCheckpointHandler(store as never)
@ -120,6 +132,84 @@ describe('registerRendererShutdownCheckpointHandler', () => {
handler?.(event, { sessions: [], ui: { activeView: 'settings' } })
expect(event.returnValue).toEqual({ ok: true })
resolve()
})
it('holds the checkpoint open until the durable write settles', async () => {
let resolveFlush!: () => void
const store = {
stageWorkspaceSessionBeforeUnload: vi.fn(),
updateUI: vi.fn(),
flushPendingOrThrowAsync: vi.fn(
() =>
new Promise<void>((next) => {
resolveFlush = next
})
)
}
registerRendererShutdownCheckpointHandler(store as never)
syncHandlers.get('app:stage-before-unload-sync')?.({}, { sessions: [], ui: {} })
const checkpoint = invokeHandlers.get(AWAIT_CHANNEL)?.()
let settled: unknown = 'pending'
void checkpoint?.then((result) => {
settled = result
})
await Promise.resolve()
expect(settled).toBe('pending')
resolveFlush()
await expect(checkpoint).resolves.toEqual({ ok: true })
})
it('reports a failed durable write instead of a successful checkpoint', async () => {
const store = {
stageWorkspaceSessionBeforeUnload: vi.fn(),
updateUI: vi.fn(),
flushPendingOrThrowAsync: vi.fn(() => Promise.reject(new Error('disk full')))
}
vi.spyOn(console, 'error').mockImplementation(() => {})
registerRendererShutdownCheckpointHandler(store as never)
const event: { returnValue?: unknown } = {}
syncHandlers.get('app:stage-before-unload-sync')?.(event, { sessions: [], ui: {} })
expect(event.returnValue).toEqual({ ok: true })
await expect(invokeHandlers.get(AWAIT_CHANNEL)?.()).resolves.toEqual({ ok: false })
})
it('fails the checkpoint when the durable write outlives its deadline', async () => {
const store = {
stageWorkspaceSessionBeforeUnload: vi.fn(),
updateUI: vi.fn(),
flushPendingOrThrowAsync: vi.fn(
(_options: { signal: AbortSignal }) => new Promise<void>(() => {})
)
}
vi.spyOn(console, 'error').mockImplementation(() => {})
vi.useFakeTimers()
try {
registerRendererShutdownCheckpointHandler(store as never)
syncHandlers.get('app:stage-before-unload-sync')?.({}, { sessions: [], ui: {} })
const checkpoint = invokeHandlers.get(AWAIT_CHANNEL)?.()
await vi.advanceTimersByTimeAsync(SHUTDOWN_CHECKPOINT_FLUSH_DEADLINE_MS)
await expect(checkpoint).resolves.toEqual({ ok: false })
expect(store.flushPendingOrThrowAsync.mock.calls[0]?.[0]?.signal.aborted).toBe(true)
} finally {
vi.useRealTimers()
}
})
it('reports success before any checkpoint is staged', async () => {
const store = {
stageWorkspaceSessionBeforeUnload: vi.fn(),
updateUI: vi.fn(),
flushPendingOrThrowAsync: vi.fn(() => Promise.resolve())
}
registerRendererShutdownCheckpointHandler(store as never)
await expect(invokeHandlers.get(AWAIT_CHANNEL)?.()).resolves.toEqual({ ok: true })
})
})

View File

@ -8,7 +8,42 @@ type StageBeforeUnloadSyncArgs = {
ui: Partial<PersistedUIState>
}
export type ShutdownCheckpointResult = { ok: boolean }
/** Matches the will-quit teardown budget so a stalled disk can't strand a restart. */
export const SHUTDOWN_CHECKPOINT_FLUSH_DEADLINE_MS = 20_000
function flushStagedStateWithDeadline(store: Store): Promise<ShutdownCheckpointResult> {
const controller = new AbortController()
let timer: ReturnType<typeof setTimeout> | null = null
const deadline = new Promise<ShutdownCheckpointResult>((resolve) => {
timer = setTimeout(() => {
controller.abort()
console.error('[app] Timed out persisting staged renderer state')
resolve({ ok: false })
}, SHUTDOWN_CHECKPOINT_FLUSH_DEADLINE_MS)
})
// Why not drain to a stable generation: the staged snapshot lands in the first
// write, and a live app keeps mutating state, which would livelock the drain.
const flush = store
.flushPendingOrThrowAsync({ signal: controller.signal, drainToStableGeneration: false })
.then((): ShutdownCheckpointResult => ({ ok: true }))
.catch((error): ShutdownCheckpointResult => {
console.error('[app] Failed to persist staged renderer state:', error)
return { ok: false }
})
return Promise.race([flush, deadline]).finally(() => {
if (timer) {
clearTimeout(timer)
}
})
}
export function registerRendererShutdownCheckpointHandler(store: Store): void {
// Why: beforeunload cannot await, so the sync reply only reports staging.
// Durability is joined out-of-band by paths that are about to navigate.
let pendingCheckpoint: Promise<ShutdownCheckpointResult> = Promise.resolve({ ok: true })
ipcMain.on('app:stage-before-unload-sync', (event, args: StageBeforeUnloadSyncArgs) => {
let ok = true
try {
@ -20,11 +55,12 @@ export function registerRendererShutdownCheckpointHandler(store: Store): void {
console.error('[app] Failed to stage renderer state before unload:', error)
ok = false
}
if (ok) {
void store.flushPendingAsync().catch((error) => {
console.error('[app] Failed to persist staged renderer state:', error)
})
}
pendingCheckpoint = ok ? flushStagedStateWithDeadline(store) : Promise.resolve({ ok: false })
event.returnValue = { ok }
})
ipcMain.handle(
'app:await-before-unload-checkpoint',
(): Promise<ShutdownCheckpointResult> => pendingCheckpoint
)
}

View File

@ -7291,11 +7291,13 @@ export class Store {
return this.flushCurrentStateAsync(false, undefined, false).catch(() => {})
}
flushPendingOrThrowAsync(options: { signal?: AbortSignal } = {}): Promise<void> {
flushPendingOrThrowAsync(
options: { signal?: AbortSignal; drainToStableGeneration?: boolean } = {}
): Promise<void> {
if (this.writesFrozen || this.quitFlushStarted) {
return Promise.reject(new Error('Cannot flush while persistence is finalized'))
}
return this.flushCurrentStateAsync(false, options.signal)
return this.flushCurrentStateAsync(false, options.signal, options.drainToStableGeneration)
}
// Async twin of flushOrThrow: durable state only. Active-view and GitHub sidecars are

View File

@ -990,6 +990,9 @@ export type AppApi = {
sessions: { state: WorkspaceSessionState; hostId?: ExecutionHostId }[]
ui: Partial<PersistedUIState>
}) => void
/** Resolves once the last staged checkpoint is durably written; rejects if that
* write failed, so a reload/restart can abort instead of losing the snapshot. */
awaitBeforeUnloadCheckpoint: () => Promise<void>
/** Resolves when the daemon PTY provider and hook receiver have either
* started or failed open for the first BrowserWindow. */
awaitFirstWindowStartupServices: () => Promise<void>

View File

@ -290,6 +290,17 @@ import {
registerRendererRestartIpcRelays
} from './renderer-restart-wiring'
// Why: the sync checkpoint only stages; this joins its durable write so a
// navigating path can abort instead of losing the staged session.
async function awaitBeforeUnloadCheckpoint(): Promise<void> {
const result = (await ipcRenderer.invoke('app:await-before-unload-checkpoint')) as {
ok?: unknown
}
if (result?.ok !== true) {
throw new Error('Failed to persist renderer state before unload.')
}
}
type NativeFileDropCallback = (data: NativeFileDropPayload) => void
const nativeFileDropCallbacks: NativeFileDropCallback[] = []
@ -479,7 +490,8 @@ const api = {
restart: async (): Promise<void> => {
await prepareRendererForAppRestart(window, {
startedEventName: ORCA_APP_RESTART_STARTED_EVENT,
abortedEventName: ORCA_APP_RESTART_ABORTED_EVENT
abortedEventName: ORCA_APP_RESTART_ABORTED_EVENT,
awaitCheckpoint: awaitBeforeUnloadCheckpoint
})
try {
return await ipcRenderer.invoke('app:restart')
@ -2974,8 +2986,11 @@ const api = {
showLinuxPackage: () => ipcRenderer.invoke('updater:showLinuxPackage'),
listBuilds: (channel) => ipcRenderer.invoke('updater:listBuilds', channel),
quitAndInstall: (): Promise<void> =>
prepareAndInvokeUpdaterInstall(window, updaterQuitAbortRelay, () =>
ipcRenderer.invoke('updater:quitAndInstall')
prepareAndInvokeUpdaterInstall(
window,
updaterQuitAbortRelay,
() => ipcRenderer.invoke('updater:quitAndInstall'),
awaitBeforeUnloadCheckpoint
),
onStatus: (callback) => {

View File

@ -44,10 +44,27 @@ describe('renderer restart wiring', () => {
throw new Error('IPC failed')
})
await expect(prepareAndInvokeUpdaterInstall(eventTarget, relay, invoke)).rejects.toThrow(
'IPC failed'
)
await expect(
prepareAndInvokeUpdaterInstall(eventTarget, relay, invoke, async () => {
calls.push('checkpoint-flushed')
})
).rejects.toThrow('IPC failed')
expect(calls).toEqual(['prepared', 'marked', 'invoked', 'aborted'])
expect(calls).toEqual(['prepared', 'checkpoint-flushed', 'marked', 'invoked', 'aborted'])
})
it('never installs the update when the shutdown checkpoint fails to persist', async () => {
const eventTarget = new EventTarget()
const invoke = vi.fn(() => Promise.resolve())
const relay = { markPrepared: vi.fn(), abort: vi.fn() }
await expect(
prepareAndInvokeUpdaterInstall(eventTarget, relay, invoke, () =>
Promise.reject(new Error('Failed to persist renderer state before unload.'))
)
).rejects.toThrow('Failed to persist renderer state before unload.')
expect(invoke).not.toHaveBeenCalled()
expect(relay.markPrepared).not.toHaveBeenCalled()
})
})

View File

@ -26,11 +26,13 @@ export function registerRendererRestartIpcRelays(
export async function prepareAndInvokeUpdaterInstall(
eventTarget: EventTarget,
relay: Pick<UpdaterQuitAbortRelay, 'markPrepared' | 'abort'>,
invoke: () => Promise<void>
invoke: () => Promise<void>,
awaitCheckpoint: () => Promise<void>
): Promise<void> {
await prepareRendererForAppRestart(eventTarget, {
startedEventName: ORCA_UPDATER_QUIT_AND_INSTALL_STARTED_EVENT,
abortedEventName: ORCA_UPDATER_QUIT_AND_INSTALL_ABORTED_EVENT
abortedEventName: ORCA_UPDATER_QUIT_AND_INSTALL_ABORTED_EVENT,
awaitCheckpoint
})
relay.markPrepared()
try {

View File

@ -0,0 +1,39 @@
// @vitest-environment happy-dom
import { afterEach, describe, expect, it, vi } from 'vitest'
import { ORCA_RENDERER_UNLOAD_PREVENTED_EVENT } from '../../../shared/renderer-shutdown-events'
import { requestLazyChunkRecoveryReload } from './lazy-chunk-recovery-reload'
describe('requestLazyChunkRecoveryReload', () => {
afterEach(() => {
vi.restoreAllMocks()
})
it('refuses the reload when the staged checkpoint never reaches disk', async () => {
const reload = vi.spyOn(window.location, 'reload').mockImplementation(() => undefined)
await expect(
requestLazyChunkRecoveryReload(window, () =>
Promise.reject(new Error('Failed to persist renderer state before unload.'))
)
).resolves.toBe('checkpoint-refused')
expect(reload).not.toHaveBeenCalled()
})
it('navigates only after the checkpoint is durably written', async () => {
const order: string[] = []
vi.spyOn(window.location, 'reload').mockImplementation(() => {
order.push('reload')
// A real landed reload destroys the document; veto so the wait settles.
window.dispatchEvent(new Event(ORCA_RENDERER_UNLOAD_PREVENTED_EVENT))
})
await expect(
requestLazyChunkRecoveryReload(window, async () => {
order.push('flushed')
})
).resolves.toBe('unload-vetoed')
expect(order).toEqual(['flushed', 'reload'])
})
})

View File

@ -49,12 +49,16 @@ function waitForRefusedNavigation(win: Window): RefusedNavigationWait {
/** Resolves only if the navigation is refused; a landed reload destroys this document. */
export async function requestLazyChunkRecoveryReload(
win: Window
win: Window,
// Hosts without a preload bridge stage durably in-process; nothing to join.
awaitCheckpoint: () => Promise<void> = () =>
window.api?.app?.awaitBeforeUnloadCheckpoint?.() ?? Promise.resolve()
): Promise<LazyChunkRecoveryReloadOutcome> {
try {
await prepareRendererForAppRestart(win, {
startedEventName: ORCA_APP_RESTART_STARTED_EVENT,
abortedEventName: ORCA_APP_RESTART_ABORTED_EVENT
abortedEventName: ORCA_APP_RESTART_ABORTED_EVENT,
awaitCheckpoint
})
} catch {
// Never reload over editor buffers that could not be backed up.

View File

@ -556,6 +556,8 @@ function createWebPreloadApi(): Partial<PreloadApi> {
}
writeJson(UI_STORAGE_KEY, mergeWebUIState(readLocalWebUIState(), ui))
},
// Staging already wrote through to browser storage, so there is nothing left to join.
awaitBeforeUnloadCheckpoint: () => Promise.resolve(),
awaitFirstWindowStartupServices: () => Promise.resolve(),
recoverLegacyWorkerTerminalsForRendererStartup: () => Promise.resolve(),
startupDiagnostic: () => Promise.resolve(),

View File

@ -18,7 +18,8 @@ describe('prepareRendererForAppRestart', () => {
await expect(
prepareRendererForAppRestart(eventTarget, {
startedEventName: 'restart-started',
abortedEventName: 'restart-aborted'
abortedEventName: 'restart-aborted',
awaitCheckpoint: () => Promise.resolve()
})
).rejects.toThrow('Renderer shutdown checkpoint was not completed.')
@ -26,6 +27,53 @@ describe('prepareRendererForAppRestart', () => {
expect(checkpoint).toHaveBeenCalledTimes(1)
expect(aborted).toHaveBeenCalledTimes(1)
})
it('waits for the durable checkpoint write before the restart proceeds', async () => {
const eventTarget = new EventTarget()
const order: string[] = []
let releaseCheckpoint!: () => void
eventTarget.addEventListener('beforeunload', () => order.push('staged'))
const prepared = prepareRendererForAppRestart(eventTarget, {
startedEventName: 'restart-started',
abortedEventName: 'restart-aborted',
awaitCheckpoint: () =>
new Promise<void>((resolve) => {
order.push('awaiting-flush')
releaseCheckpoint = () => {
order.push('flushed')
resolve()
}
})
})
let settled = false
void prepared.then(() => {
settled = true
})
await Promise.resolve()
expect(settled).toBe(false)
releaseCheckpoint()
await prepared
expect(order).toEqual(['staged', 'awaiting-flush', 'flushed'])
})
it('aborts the restart when the staged state cannot be persisted', async () => {
const eventTarget = new EventTarget()
const aborted = vi.fn()
eventTarget.addEventListener('restart-aborted', aborted)
await expect(
prepareRendererForAppRestart(eventTarget, {
startedEventName: 'restart-started',
abortedEventName: 'restart-aborted',
awaitCheckpoint: () => Promise.reject(new Error('Failed to persist renderer state.'))
})
).rejects.toThrow('Failed to persist renderer state.')
expect(aborted).toHaveBeenCalledTimes(1)
})
})
describe('createUpdaterQuitAbortRelay', () => {

View File

@ -7,6 +7,8 @@ import type { UpdateStatus } from './types'
export type AppRestartPrepOptions = {
startedEventName: string
abortedEventName: string
/** Joins the durable write of the state the checkpoint staged; rejects if it failed. */
awaitCheckpoint: () => Promise<void>
}
function requestEditorHotExitBackup(eventTarget: EventTarget): Promise<void> {
@ -36,7 +38,7 @@ function requestEditorHotExitBackup(eventTarget: EventTarget): Promise<void> {
export async function prepareRendererForAppRestart(
eventTarget: EventTarget,
{ startedEventName, abortedEventName }: AppRestartPrepOptions
{ startedEventName, abortedEventName, awaitCheckpoint }: AppRestartPrepOptions
): Promise<void> {
eventTarget.dispatchEvent(new Event(startedEventName))
@ -48,6 +50,9 @@ export async function prepareRendererForAppRestart(
if (!accepted) {
throw new Error('Renderer shutdown checkpoint was not completed.')
}
// Why: the checkpoint only stages synchronously. Navigating before that
// write lands loses the session snapshot to a crash or power loss.
await awaitCheckpoint()
} catch (error) {
eventTarget.dispatchEvent(new Event(abortedEventName))
throw error