From 4916c090796a09531a4e48eb7205c7ce3f87a604 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Wed, 28 Oct 2026 06:18:54 -0700 Subject: [PATCH] fix(terminal): a WSL bridge pane keeps its session when its distro is booted again before the check (#35050) * fix(terminal): a WSL bridge pane keeps its session when its distro is booted again before the check A bridge exit that the running list calls 'running' is now host loss when the distro's boot id changed since the pane spawned (something booted it again first), and an unanswered list is asked once more before the exit stays an exit. Boot ids are read once per distro boot, only into a distro the background-work gate confirms running, so the read never boots one. Co-Authored-By: Claude * fix(terminal): an exit drops the distro's boot id at once, and a new incarnation takes the current one Reviews of #35050: a pane spawned during another pane's exit verdict, or the same session respawned into a rebooted distro, kept the old boot's id, so its own later exit read as 'rebooted' and its tab never closed. A burst of exits now shares one read of the current id, and the verdict log names the exit code. Co-Authored-By: Claude * fix(terminal): a stale bridge exit drops the boot id, and the respawn that beat it re-reads its own Co-Authored-By: Claude * test(terminal): wait on the exit, not the clock, in the bridge relaunch suite Co-Authored-By: Claude --------- Co-authored-by: Claude --- .../daemon/daemon-pty-adapter-test-harness.ts | 5 +- ...y-adapter-wsl-distro-loss-relaunch.test.ts | 139 +++++++++++++++++ ...daemon-pty-adapter-wsl-distro-loss.test.ts | 65 +++++++- src/main/daemon/daemon-pty-adapter.ts | 22 ++- src/main/daemon/daemon-pty-runtime-state.ts | 6 + src/main/daemon/daemon-pty-spawn-request.ts | 16 ++ src/main/daemon/daemon-pty-spawn-result.ts | 22 +-- .../daemon-wsl-bridge-boot-identity.test.ts | 144 ++++++++++++++++++ .../daemon/daemon-wsl-bridge-boot-identity.ts | 116 ++++++++++++++ .../daemon/daemon-wsl-bridge-exit.test.ts | 66 +++++++- src/main/daemon/daemon-wsl-bridge-exit.ts | 65 ++++++-- src/main/wsl/wsl-distro-boot-id.test.ts | 36 +++++ src/main/wsl/wsl-distro-boot-id.ts | 28 ++++ 13 files changed, 688 insertions(+), 42 deletions(-) create mode 100644 src/main/daemon/daemon-pty-adapter-wsl-distro-loss-relaunch.test.ts create mode 100644 src/main/daemon/daemon-wsl-bridge-boot-identity.test.ts create mode 100644 src/main/daemon/daemon-wsl-bridge-boot-identity.ts create mode 100644 src/main/wsl/wsl-distro-boot-id.test.ts create mode 100644 src/main/wsl/wsl-distro-boot-id.ts diff --git a/src/main/daemon/daemon-pty-adapter-test-harness.ts b/src/main/daemon/daemon-pty-adapter-test-harness.ts index 55a372516ed..9e793465284 100644 --- a/src/main/daemon/daemon-pty-adapter-test-harness.ts +++ b/src/main/daemon/daemon-pty-adapter-test-harness.ts @@ -82,7 +82,10 @@ export async function waitFor(predicate: () => boolean, timeoutMs = 2000): Promi /** Boots a real daemon server over a temp socket plus a lazily-connecting adapter. */ export async function startDaemonAdapterHarness( spawnSubprocess: SpawnSubprocess, - adapterOptions: Pick = {} + adapterOptions: Pick< + DaemonPtyAdapterOptions, + 'readRunningWslDistros' | 'historyPath' | 'wslBridgeBootIdentity' + > = {} ): Promise { const dir = createTestDir() const socketPath = getDaemonSocketPath(dir) diff --git a/src/main/daemon/daemon-pty-adapter-wsl-distro-loss-relaunch.test.ts b/src/main/daemon/daemon-pty-adapter-wsl-distro-loss-relaunch.test.ts new file mode 100644 index 00000000000..13b244b4319 --- /dev/null +++ b/src/main/daemon/daemon-pty-adapter-wsl-distro-loss-relaunch.test.ts @@ -0,0 +1,139 @@ +// 5e: a bridge pane whose distro was terminated must cold-restore its own session after a relaunch. +import './mock-descendant-sweep' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { join } from 'node:path' +import { rmSync } from 'node:fs' +import type { WslRunningDistrosRead } from '../wsl-background-distro-gate' +import { DaemonPtyAdapter } from './daemon-pty-adapter' +import type { DaemonPtyAdapterOptions } from './daemon-pty-runtime-state' +import { DaemonServer } from './daemon-server' +import { WslBridgeBootIdentity } from './daemon-wsl-bridge-boot-identity' +import { createMockSubprocess, startDaemonAdapterHarness } from './daemon-pty-adapter-test-harness' + +/** Drives the adapter's own periodic save, which has no public trigger. */ +class SavingDaemonPtyAdapter extends DaemonPtyAdapter { + savePeriodically(sessionIds: string[]): Promise { + return this.checkpointSessions(sessionIds, { final: false }) + } + + /** What a spawn in `\\wsl.localhost\Ubuntu\...` records, without faking win32. */ + runsInUbuntu(sessionId: string): void { + this.noteSpawnWslDistro(sessionId, 'Ubuntu', false) + } +} + +const PANE = 'wt-1::/project@@bridge' +const OTHER = 'wt-2::/other@@native' +const WORKTREES = new Set(['wt-1::/project', 'wt-2::/other']) + +describe('a WSL bridge pane whose distro was terminated, after a relaunch', () => { + let dir: string + let socketPath: string + let tokenPath: string + let server: DaemonServer + let historyDir: string + let lastSubprocess: ReturnType + const spawnSubprocess = () => { + lastSubprocess = createMockSubprocess() + return lastSubprocess + } + + beforeEach(async () => { + const harness = await startDaemonAdapterHarness(spawnSubprocess) + harness.adapter.dispose() + dir = harness.dir + socketPath = harness.socketPath + tokenPath = harness.tokenPath + server = harness.server + historyDir = join(dir, 'history') + }) + afterEach(async () => { + await server?.shutdown() + rmSync(dir, { recursive: true, force: true }) + }) + + const launch = ( + options: Pick = {} + ) => new SavingDaemonPtyAdapter({ socketPath, tokenPath, historyPath: historyDir, ...options }) + + /** A bridge pane with saved history, beside a native pane that keeps the daemon running. */ + async function openBridgePane(app: SavingDaemonPtyAdapter) { + await app.spawn({ cols: 80, rows: 24, cwd: '/other', sessionId: OTHER }) + await app.spawn({ cols: 80, rows: 24, cwd: '/project', sessionId: PANE }) + app.runsInUbuntu(PANE) + const bridge = lastSubprocess + bridge._simulateData('user@host:~/project$ echo KEPT\r\nKEPT\r\nuser@host:~/project$ ') + await app.savePeriodically([PANE]) + return bridge + } + + async function relaunchAndRestore(): Promise { + const app = launch() + await app.reconcileOnStartup(WORKTREES) + const restored = await app.spawn({ cols: 80, rows: 24, cwd: '/project', sessionId: PANE }) + await app.disconnectOnly() + return Boolean(restored.coldRestore) + } + + it('restores when the distro stopped while Orca was quit and the daemon kept running', async () => { + const app = launch() + const bridge = await openBridgePane(app) + await app.disconnectOnly() + + // The daemon reaps the session as its process exits. + bridge._simulateExit(1) + + await expect(relaunchAndRestore()).resolves.toBe(true) + }) + + it('restores when the distro stopped while Orca was quit and the daemon exited with it', async () => { + const app = launch() + await app.spawn({ cols: 80, rows: 24, cwd: '/project', sessionId: PANE }) + lastSubprocess._simulateData('user@host:~/project$ echo KEPT\r\nKEPT\r\n') + await app.savePeriodically([PANE]) + await app.disconnectOnly() + + lastSubprocess._simulateExit(1) + // Its last session gone and no client, the daemon exits; the relaunch starts a new one. + await server.shutdown() + server = new DaemonServer({ socketPath, tokenPath, spawnSubprocess }) + await server.start() + + await expect(relaunchAndRestore()).resolves.toBe(true) + }) + + it.each([ + [ + 'something booted the distro again before the running list answered', + [{ status: 'confirmed', distros: ['Ubuntu'] }], + ['boot-1', 'boot-2'] + ], + [ + 'the first running list could not answer', + [{ status: 'unavailable' }, { status: 'confirmed', distros: [] }], + ['boot-1'] + ] + ] as const)('restores when %s while Orca ran', async (_name, answers, ids) => { + const reads: WslRunningDistrosRead[] = answers.map((answer) => ({ + ...answer, + ...('distros' in answer ? { distros: [...answer.distros] } : {}) + })) + const bootIds = [...ids] + const app = launch({ + readRunningWslDistros: async () => (reads.length > 1 ? reads.shift()! : reads[0]!), + wslBridgeBootIdentity: new WslBridgeBootIdentity({ + isRunning: async () => true, + readBootId: async () => (bootIds.length > 1 ? bootIds.shift()! : bootIds[0]!) + }) + }) + const exits: unknown[] = [] + app.onExit((exit) => exits.push(exit)) + const bridge = await openBridgePane(app) + + bridge._simulateExit(1) + await vi.waitFor(() => expect(exits).toEqual([expect.objectContaining({ hostLost: true })])) + await app.disconnectOnly() + + await expect(relaunchAndRestore()).resolves.toBe(true) + }) +}) diff --git a/src/main/daemon/daemon-pty-adapter-wsl-distro-loss.test.ts b/src/main/daemon/daemon-pty-adapter-wsl-distro-loss.test.ts index 77515fa7652..270c8978a26 100644 --- a/src/main/daemon/daemon-pty-adapter-wsl-distro-loss.test.ts +++ b/src/main/daemon/daemon-pty-adapter-wsl-distro-loss.test.ts @@ -7,6 +7,7 @@ import type { WslRunningDistrosRead } from '../wsl-background-distro-gate' import type { DaemonPtyAdapter } from './daemon-pty-adapter' import type { DaemonServer } from './daemon-server' import { HistoryManager } from './history-manager' +import { WslBridgeBootIdentity } from './daemon-wsl-bridge-boot-identity' import { createMockSubprocess, startDaemonAdapterHarness, @@ -23,12 +24,17 @@ describe('a WSL bridge pane exit', () => { let subprocesses: ReturnType[] let running: () => Promise const readRunning = vi.fn(() => running()) + // The distro's boot id each read returns; never a real wsl.exe. + let bootIds: string[] + const readBootId = vi.fn(async () => (bootIds.length > 1 ? bootIds.shift()! : bootIds[0]!)) beforeEach(async () => { historyDir = mkdtempSync(join(tmpdir(), 'orca-distro-loss-history-')) subprocesses = [] running = async () => ({ status: 'confirmed', distros: ['Ubuntu'] }) readRunning.mockClear() + bootIds = ['boot-1'] + readBootId.mockClear() const harness = await startDaemonAdapterHarness( () => { const subprocess = createMockSubprocess() @@ -37,7 +43,11 @@ describe('a WSL bridge pane exit', () => { }, { readRunningWslDistros: readRunning, - historyPath: historyDir + historyPath: historyDir, + wslBridgeBootIdentity: new WslBridgeBootIdentity({ + isRunning: async () => true, + readBootId + }) } ) dir = harness.dir @@ -55,9 +65,11 @@ describe('a WSL bridge pane exit', () => { async function spawnBridgePane(): Promise { const { id } = await adapter.spawn({ cols: 80, rows: 24 }) // What a spawn in `\\wsl.localhost\Ubuntu\...` records. - // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the adapter's protected map, which a WSL spawn fills; set here without faking win32. - const internals = adapter as unknown as { wslDistrosBySessionId: Map } - internals.wslDistrosBySessionId.set(id, 'Ubuntu') + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the adapter's protected spawn bookkeeping, which a WSL spawn runs; called here without faking win32. + const internals = adapter as unknown as { + noteSpawnWslDistro: (id: string, distro: string, clear: boolean) => void + } + internals.noteSpawnWslDistro(id, 'Ubuntu', false) return id } @@ -98,13 +110,15 @@ describe('a WSL bridge pane exit', () => { await waitFor(() => exits.length > 0) expect(info).toHaveBeenLastCalledWith( expect.stringMatching( - new RegExp(`^\\[pty\\] WSL bridge exit: ${verdict} \\(distro Ubuntu, probe \\d+ms\\)$`) + new RegExp( + `^\\[pty\\] WSL bridge exit: ${verdict} \\(distro Ubuntu, code 1, probe \\d+ms\\)$` + ) ) ) } }) - it('stays a normal exit while its distro runs, so `exit` still closes the tab', async () => { + it('stays a normal exit while its distro runs on the same boot, so `exit` still closes the tab', async () => { const exits = collectExits() const id = await spawnBridgePane() @@ -115,6 +129,45 @@ describe('a WSL bridge pane exit', () => { expect(exits[0]).not.toHaveProperty('hostLost') }) + // 5e hardening (a): something booted the distro again before the running list answered. + it('is host loss when the distro runs again under a new boot id', async () => { + bootIds = ['boot-1', 'boot-2'] + const exits = collectExits() + const id = await spawnBridgePane() + + subprocesses[0]!._simulateExit(1) + + await waitFor(() => exits.length > 0) + expect(exits).toEqual([{ id, code: -1, incarnationId: expect.any(String), hostLost: true }]) + expect(readBootId).toHaveBeenCalledTimes(2) + }) + + // #35050 review: a pane opened during another's exit verdict took the old boot's id. + it("lets a pane opened during another pane's exit verdict close on its own `exit`", async () => { + bootIds = ['boot-1', 'boot-2'] + let answer: (read: WslRunningDistrosRead) => void = () => {} + running = () => + new Promise((resolve) => { + answer = resolve + }) + const exits = collectExits() + const first = await spawnBridgePane() + await waitFor(() => readBootId.mock.calls.length === 1) + + subprocesses[0]!._simulateExit(1) + await waitFor(() => readRunning.mock.calls.length === 1) + const second = await spawnBridgePane() + answer({ status: 'confirmed', distros: ['Ubuntu'] }) + await waitFor(() => exits.length === 1) + expect(exits[0]).toMatchObject({ id: first, code: -1, hostLost: true }) + + running = async () => ({ status: 'confirmed', distros: ['Ubuntu'] }) + subprocesses[1]!._simulateExit(1) + await waitFor(() => exits.length === 2) + expect(exits[1]).toMatchObject({ id: second, code: 1 }) + expect(exits[1]).not.toHaveProperty('hostLost') + }) + it('stays a normal exit when the running list cannot answer', async () => { running = async () => ({ status: 'unavailable' }) const exits = collectExits() diff --git a/src/main/daemon/daemon-pty-adapter.ts b/src/main/daemon/daemon-pty-adapter.ts index 3b302f4e89d..dad2fed546d 100644 --- a/src/main/daemon/daemon-pty-adapter.ts +++ b/src/main/daemon/daemon-pty-adapter.ts @@ -6,7 +6,7 @@ import { import { FENCED_TERMINATION_PROTOCOL_VERSION } from './daemon-protocol-version' import { emitPtyListeners, createPtyExitPayload } from './daemon-pty-listener-emission' import type { DaemonPtyRouterExitEvent } from './daemon-pty-router-events' -import { checkWslBridgeDistro, WSL_DISTRO_LOSS_EXIT_CODE } from './daemon-wsl-bridge-exit' +import { classifyWslBridgeExit, WSL_DISTRO_LOSS_EXIT_CODE } from './daemon-wsl-bridge-exit' import { isProvenProcessExit } from '../../shared/terminal-exit-cause' import { DaemonPtyDaemonRecovery } from './daemon-pty-daemon-recovery' import type { DaemonPtyAdapterOptions } from './daemon-pty-runtime-state' @@ -211,14 +211,21 @@ export class DaemonPtyAdapter extends DaemonPtyDaemonRecovery implements IPtyPro ) operation.exitsBySessionId.set(event.sessionId, exits) } + const wslDistro = this.wslDistrosBySessionId.get(event.sessionId) // Retain raced exits for pending spawns; only this adapter's published incarnation may clear state or notify. if ( currentIncarnationId !== undefined && event.payload.incarnationId !== currentIncarnationId ) { + // A stale exit may still be a reboot's; the respawn that beat it re-reads its boot id. + if (wslDistro) { + this.wslBridgeBootIdentity.noteStaleExit(event.sessionId, wslDistro) + } return } - const wslDistro = this.wslDistrosBySessionId.get(event.sessionId) + if (wslDistro) { + this.wslBridgeBootIdentity.noteExit(wslDistro) + } if (wslDistro && isProvenProcessExit(event.payload.code)) { const exitAt = performance.now() void this.finishWslBridgeExit(event.sessionId, event.payload, wslDistro, exitAt) @@ -243,17 +250,20 @@ export class DaemonPtyAdapter extends DaemonPtyDaemonRecovery implements IPtyPro wslDistro: string, exitAt: number ): Promise { - const verdict = await checkWslBridgeDistro(wslDistro, exitAt, this.readRunningWslDistros) - // Only 'lost' keeps the tab; a reboot (or slow wsl.exe) racing the probe reads as an exit. + const verdict = await classifyWslBridgeExit(wslDistro, exitAt, this.readRunningWslDistros, () => + this.wslBridgeBootIdentity.rebootedSinceSpawn(sessionId, wslDistro) + ) + // Lost, or booted again since the spawn, keeps the tab; an unanswered check reads as an exit. + // The code lets a verify run show which codes a terminate yields (only 1 is on record). console.info( - `[pty] WSL bridge exit: ${verdict} (distro ${wslDistro}, probe ${Math.round(performance.now() - exitAt)}ms)` + `[pty] WSL bridge exit: ${verdict} (distro ${wslDistro}, code ${exit.code}, probe ${Math.round(performance.now() - exitAt)}ms)` ) // A spawn that replaced this incarnation meanwhile owns the session now. const current = this.sessionIncarnations.get(sessionId) if (current !== undefined && current !== exit.incarnationId) { return } - if (verdict !== 'lost') { + if (verdict !== 'lost' && verdict !== 'rebooted') { this.finishExit(sessionId, exit) return } diff --git a/src/main/daemon/daemon-pty-runtime-state.ts b/src/main/daemon/daemon-pty-runtime-state.ts index a847d8a8559..7e31dd49b20 100644 --- a/src/main/daemon/daemon-pty-runtime-state.ts +++ b/src/main/daemon/daemon-pty-runtime-state.ts @@ -32,6 +32,7 @@ import { HistoryReader } from './history-reader' import type { PtyBackgroundStreamEvent } from '../providers/pty-provider-contract' import type { DaemonPtyRouterDataEvent, DaemonPtyRouterExitEvent } from './daemon-pty-router-events' import type { ReadRunningWslDistros } from './daemon-wsl-bridge-exit' +import { WslBridgeBootIdentity } from './daemon-wsl-bridge-boot-identity' export type PendingDaemonSpawnOperation = { exitsBySessionId: Map @@ -62,6 +63,8 @@ export type DaemonPtyAdapterOptions = { respawn?: (reason: DaemonRespawnReason) => Promise void)> /** Test seam; production reads the shared, coalesced `wsl --list --running`. */ readRunningWslDistros?: ReadRunningWslDistros + /** Test seam; production reads each distro's boot id through the background-work gate. */ + wslBridgeBootIdentity?: WslBridgeBootIdentity } export type DaemonRespawnReason = @@ -83,6 +86,7 @@ export type DaemonIdleRetirementResult = export abstract class DaemonPtyRuntimeState { protected readonly readRunningWslDistros: ReadRunningWslDistros | undefined + protected readonly wslBridgeBootIdentity: WslBridgeBootIdentity readonly protocolVersion: number protected socketPath: string protected tokenPath: string @@ -209,6 +213,7 @@ export abstract class DaemonPtyRuntimeState { } this.initialCwds.delete(sessionId) this.wslDistrosBySessionId.delete(sessionId) + this.wslBridgeBootIdentity.forget(sessionId) this.wslPaneKeysBySessionId.delete(sessionId) this.sessionIncarnations.delete(sessionId) } @@ -239,6 +244,7 @@ export abstract class DaemonPtyRuntimeState { this.historyReader = opts.historyPath ? new HistoryReader(opts.historyPath) : null this.respawnFn = opts.respawn ?? null this.readRunningWslDistros = opts.readRunningWslDistros + this.wslBridgeBootIdentity = opts.wslBridgeBootIdentity ?? new WslBridgeBootIdentity() this.runtimeDir = opts.runtimeDir ?? opts.profileScope ?? null this.packagedAppVersion = opts.packagedAppVersion ?? null this.supportsCheckpoints = this.protocolVersion >= 4 diff --git a/src/main/daemon/daemon-pty-spawn-request.ts b/src/main/daemon/daemon-pty-spawn-request.ts index ea5abb25641..fc7271d5110 100644 --- a/src/main/daemon/daemon-pty-spawn-request.ts +++ b/src/main/daemon/daemon-pty-spawn-request.ts @@ -224,4 +224,20 @@ export abstract class DaemonPtySpawnRequest extends DaemonPtyRuntimeState { ? { ...result, historySeeded: false } : result } + + /** Records a spawn result's WSL distro (and its boot id), or clears it when `clear`. */ + protected noteSpawnWslDistro( + sessionId: string, + wslDistro: string | undefined, + clear: boolean, + incarnationId?: string + ): void { + if (wslDistro) { + this.wslDistrosBySessionId.set(sessionId, wslDistro) + this.wslBridgeBootIdentity.noteSpawn(sessionId, wslDistro, incarnationId) + } else if (clear) { + this.wslDistrosBySessionId.delete(sessionId) + this.wslBridgeBootIdentity.forget(sessionId) + } + } } diff --git a/src/main/daemon/daemon-pty-spawn-result.ts b/src/main/daemon/daemon-pty-spawn-result.ts index 167edd5f628..b390f94c749 100644 --- a/src/main/daemon/daemon-pty-spawn-result.ts +++ b/src/main/daemon/daemon-pty-spawn-result.ts @@ -88,11 +88,12 @@ export abstract class DaemonPtySpawnResult extends DaemonPtySpawnRequest { // Null overrides caller WSL preference; undefined preserves compatibility with older daemons. wslDistro = providerWslDistro ?? undefined context.wslDistro = wslDistro - if (wslDistro) { - this.wslDistrosBySessionId.set(sessionId, wslDistro) - } else if (providerWslDistro === null || result.isNew) { - this.wslDistrosBySessionId.delete(sessionId) - } + this.noteSpawnWslDistro( + sessionId, + wslDistro, + providerWslDistro === null || result.isNew, + result.incarnationId + ) if (wslDistro && opts.env?.ORCA_PANE_KEY) { this.wslPaneKeysBySessionId.set(sessionId, opts.env.ORCA_PANE_KEY) } @@ -160,11 +161,12 @@ export abstract class DaemonPtySpawnResult extends DaemonPtySpawnRequest { providerWslDistro = result.wslDistro === undefined ? wslDistro : result.wslDistro wslDistro = providerWslDistro ?? undefined context.wslDistro = wslDistro - if (wslDistro) { - this.wslDistrosBySessionId.set(sessionId, wslDistro) - } else if (providerWslDistro === null || result.isNew) { - this.wslDistrosBySessionId.delete(sessionId) - } + this.noteSpawnWslDistro( + sessionId, + wslDistro, + providerWslDistro === null || result.isNew, + result.incarnationId + ) pid = typeof result.pid === 'number' && result.pid > 0 ? result.pid : null this.initialCwds.set(sessionId, effectiveCwd) } diff --git a/src/main/daemon/daemon-wsl-bridge-boot-identity.test.ts b/src/main/daemon/daemon-wsl-bridge-boot-identity.test.ts new file mode 100644 index 00000000000..808fe624913 --- /dev/null +++ b/src/main/daemon/daemon-wsl-bridge-boot-identity.test.ts @@ -0,0 +1,144 @@ +import { describe, expect, it, vi } from 'vitest' +import { WslBridgeBootIdentity } from './daemon-wsl-bridge-boot-identity' + +function identity(options: { running?: boolean; ids?: (string | null)[] } = {}) { + const ids = [...(options.ids ?? ['boot-1'])] + const isRunning = vi.fn(async () => options.running ?? true) + const readBootId = vi.fn(async () => (ids.length > 1 ? ids.shift()! : (ids[0] ?? null))) + return { boot: new WslBridgeBootIdentity({ isRunning, readBootId }), isRunning, readBootId } +} + +describe('WslBridgeBootIdentity', () => { + it('reads each distro once while any of its panes lives', async () => { + const { boot, readBootId } = identity() + boot.noteSpawn('a', 'Ubuntu') + boot.noteSpawn('b', 'ubuntu') + boot.noteSpawn('a', 'Ubuntu') + await vi.waitFor(() => expect(readBootId).toHaveBeenCalledOnce()) + }) + + it('calls an exit rebooted only when the distro booted again since the spawn', async () => { + const same = identity({ ids: ['boot-1'] }) + same.boot.noteSpawn('a', 'Ubuntu') + await expect(same.boot.rebootedSinceSpawn('a', 'Ubuntu')).resolves.toBe(false) + + const again = identity({ ids: ['boot-1', 'boot-2'] }) + again.boot.noteSpawn('a', 'Ubuntu') + await expect(again.boot.rebootedSinceSpawn('a', 'Ubuntu')).resolves.toBe(true) + }) + + it('falls back to an exit when either id is unreadable, or the pane was never noted', async () => { + for (const ids of [ + [null, 'boot-2'], + ['boot-1', null] + ]) { + const { boot } = identity({ ids }) + boot.noteSpawn('a', 'Ubuntu') + await expect(boot.rebootedSinceSpawn('a', 'Ubuntu')).resolves.toBe(false) + } + await expect(identity().boot.rebootedSinceSpawn('unknown', 'Ubuntu')).resolves.toBe(false) + }) + + it('never reads a distro the background-work gate does not confirm running (E9)', async () => { + const { boot, readBootId } = identity({ running: false }) + boot.noteSpawn('a', 'Ubuntu') + await expect(boot.rebootedSinceSpawn('a', 'Ubuntu')).resolves.toBe(false) + expect(readBootId).not.toHaveBeenCalled() + }) + + it('reads again for a spawn after an exit, or after its last pane went away', async () => { + const { boot, readBootId } = identity({ ids: ['boot-1', 'boot-2', 'boot-3', 'boot-4'] }) + boot.noteSpawn('a', 'Ubuntu') + boot.noteSpawn('b', 'Ubuntu') + boot.noteExit('Ubuntu') + boot.noteSpawn('c', 'Ubuntu') + await vi.waitFor(() => expect(readBootId).toHaveBeenCalledTimes(2)) + boot.forget('a') + boot.forget('b') + boot.forget('c') + boot.noteSpawn('d', 'Ubuntu') + await vi.waitFor(() => expect(readBootId).toHaveBeenCalledTimes(3)) + }) + + // Reviewers of #35050: a spawn during an earlier exit's verdict took the old boot's id. + it("gives a pane spawned during another's exit verdict the new boot, so its own exit is an exit", async () => { + const world = { boot: 'boot-1' } + const readBootId = vi.fn(async () => world.boot) + const boot = new WslBridgeBootIdentity({ isRunning: async () => true, readBootId }) + boot.noteSpawn('a', 'Ubuntu', 'inc-a') + await vi.waitFor(() => expect(readBootId).toHaveBeenCalledOnce()) + // The distro is terminated and booted again; A's exit arrives, and C opens before A's verdict. + world.boot = 'boot-2' + boot.noteExit('Ubuntu') + boot.noteSpawn('c', 'Ubuntu', 'inc-c') + await expect(boot.rebootedSinceSpawn('a', 'Ubuntu')).resolves.toBe(true) + boot.forget('a') + + boot.noteExit('Ubuntu') + await expect(boot.rebootedSinceSpawn('c', 'Ubuntu')).resolves.toBe(false) + }) + + it('gives a respawn of the same session into a rebooted distro the new boot id', async () => { + const world = { boot: 'boot-1' } + const readBootId = vi.fn(async () => world.boot) + const boot = new WslBridgeBootIdentity({ isRunning: async () => true, readBootId }) + boot.noteSpawn('a', 'Ubuntu', 'inc-1') + await vi.waitFor(() => expect(readBootId).toHaveBeenCalledOnce()) + world.boot = 'boot-2' + boot.noteExit('Ubuntu') + boot.noteSpawn('a', 'Ubuntu', 'inc-2') + + boot.noteExit('Ubuntu') + await expect(boot.rebootedSinceSpawn('a', 'Ubuntu')).resolves.toBe(false) + }) + + // #35050 review, case 1b: a respawn into a rebooted distro beat the dead process's exit event. + it('re-reads the boot id of a respawn that beat its old incarnation’s exit', async () => { + const world = { boot: 'boot-1' } + const readBootId = vi.fn(async () => world.boot) + const boot = new WslBridgeBootIdentity({ isRunning: async () => true, readBootId }) + boot.noteSpawn('a', 'Ubuntu', 'inc-1') + await vi.waitFor(() => expect(readBootId).toHaveBeenCalledOnce()) + world.boot = 'boot-2' + // The respawn lands first and takes the cached (old) id; then the stale exit arrives. + boot.noteSpawn('a', 'Ubuntu', 'inc-2') + boot.noteStaleExit('a', 'Ubuntu') + boot.noteSpawn('b', 'Ubuntu', 'inc-b') + + boot.noteExit('Ubuntu') + await expect(boot.rebootedSinceSpawn('a', 'Ubuntu')).resolves.toBe(false) + await expect(boot.rebootedSinceSpawn('b', 'Ubuntu')).resolves.toBe(false) + }) + + it('keeps the id across a reattach of the same incarnation', async () => { + const { boot, readBootId } = identity({ ids: ['boot-1', 'boot-2'] }) + boot.noteSpawn('a', 'Ubuntu', 'inc-1') + boot.noteExit('Ubuntu') + boot.noteSpawn('a', 'Ubuntu', 'inc-1') + await vi.waitFor(() => expect(readBootId).toHaveBeenCalledOnce()) + }) + + it('shares one read of the current id across a burst of exits', async () => { + const { boot, readBootId } = identity({ ids: ['boot-1', 'boot-2'] }) + for (const id of ['a', 'b', 'c']) { + boot.noteSpawn(id, 'Ubuntu') + } + await vi.waitFor(() => expect(readBootId).toHaveBeenCalledOnce()) + boot.noteExit('Ubuntu') + await expect( + Promise.all(['a', 'b', 'c'].map((id) => boot.rebootedSinceSpawn(id, 'Ubuntu'))) + ).resolves.toEqual([true, true, true]) + expect(readBootId).toHaveBeenCalledTimes(2) + }) + + it('treats a failing read as unreadable', async () => { + const boot = new WslBridgeBootIdentity({ + isRunning: async () => true, + readBootId: async () => { + throw new Error('wsl.exe failed') + } + }) + boot.noteSpawn('a', 'Ubuntu') + await expect(boot.rebootedSinceSpawn('a', 'Ubuntu')).resolves.toBe(false) + }) +}) diff --git a/src/main/daemon/daemon-wsl-bridge-boot-identity.ts b/src/main/daemon/daemon-wsl-bridge-boot-identity.ts new file mode 100644 index 00000000000..683f408f7f4 --- /dev/null +++ b/src/main/daemon/daemon-wsl-bridge-boot-identity.ts @@ -0,0 +1,116 @@ +/** + * Each WSL bridge pane remembers its distro's boot id from its spawn, so an exit the running list + * calls 'running' can still be told apart as the distro having been terminated and booted again. + * Reads go through the background-work gate (E9): only into a distro a fresh list says is running. + */ +import { isWslDistroRunningForBackgroundWork } from '../wsl-background-distro-gate' +import { readWslDistroBootId } from '../wsl/wsl-distro-boot-id' + +export type WslBridgeBootIdentityDeps = { + isRunning: (distro: string) => Promise + /** Null when the id could not be read. */ + readBootId: (distro: string) => Promise +} + +const PRODUCTION_DEPS: WslBridgeBootIdentityDeps = { + isRunning: isWslDistroRunningForBackgroundWork, + readBootId: readWslDistroBootId +} + +function distroKey(distro: string): string { + return distro.trim().toLowerCase() +} + +export class WslBridgeBootIdentity { + // One read per distro boot, reused by later spawns; any bridge exit of the distro drops it. + private readonly byDistro = new Map>() + // A burst of exits (a terminate exits every pane) shares one read of the current id. + private readonly currentReads = new Map>() + private readonly bySession = new Map< + string, + { distro: string; incarnationId: string | undefined; bootId: Promise } + >() + + constructor(private readonly deps: WslBridgeBootIdentityDeps = PRODUCTION_DEPS) {} + + /** A spawn or reattach of `sessionId`; a new incarnation takes the distro's current boot id. */ + noteSpawn(sessionId: string, distro: string, incarnationId?: string): void { + const key = distroKey(distro) + const current = this.bySession.get(sessionId) + if ( + current?.distro === key && + incarnationId !== undefined && + current.incarnationId === incarnationId + ) { + return + } + this.bySession.set(sessionId, { distro: key, incarnationId, bootId: this.distroBootId(distro) }) + } + + /** + * Synchronous, as the exit arrives: it may be the first of a reboot's, so a spawn during its + * verdict must not inherit the old boot's id. + */ + noteExit(distro: string): void { + this.byDistro.delete(distroKey(distro)) + } + + /** + * An exit of an incarnation the session already replaced: the respawn that beat it took the + * cached id, which may be the old boot's, so the current incarnation reads it again. + */ + noteStaleExit(sessionId: string, distro: string): void { + this.noteExit(distro) + const current = this.bySession.get(sessionId) + if (current?.distro === distroKey(distro)) { + current.bootId = this.distroBootId(distro) + } + } + + /** After a bridge exit the running list answered 'running': has the distro booted again since? */ + async rebootedSinceSpawn(sessionId: string, distro: string): Promise { + const spawned = this.bySession.get(sessionId) + if (!spawned) { + return false + } + const [before, now] = await Promise.all([spawned.bootId, this.readCurrent(distro)]) + // An unreadable id on either side proves nothing: the exit stays an exit. + return before !== null && now !== null && before !== now + } + + forget(sessionId: string): void { + const spawned = this.bySession.get(sessionId) + this.bySession.delete(sessionId) + if (spawned && ![...this.bySession.values()].some(({ distro }) => distro === spawned.distro)) { + this.byDistro.delete(spawned.distro) + } + } + + private distroBootId(distro: string): Promise { + const key = distroKey(distro) + let bootId = this.byDistro.get(key) + if (!bootId) { + bootId = this.read(distro) + this.byDistro.set(key, bootId) + } + return bootId + } + + private readCurrent(distro: string): Promise { + const key = distroKey(distro) + let reading = this.currentReads.get(key) + if (!reading) { + reading = this.read(distro).finally(() => this.currentReads.delete(key)) + this.currentReads.set(key, reading) + } + return reading + } + + private async read(distro: string): Promise { + try { + return (await this.deps.isRunning(distro)) ? await this.deps.readBootId(distro) : null + } catch { + return null + } + } +} diff --git a/src/main/daemon/daemon-wsl-bridge-exit.test.ts b/src/main/daemon/daemon-wsl-bridge-exit.test.ts index f945ce30b40..b690b1e2557 100644 --- a/src/main/daemon/daemon-wsl-bridge-exit.test.ts +++ b/src/main/daemon/daemon-wsl-bridge-exit.test.ts @@ -4,7 +4,7 @@ import { _resetRunningWslDistroCacheForTests, resolveRunningWslDistros } from '../wsl-running-distro-cache' -import { checkWslBridgeDistro } from './daemon-wsl-bridge-exit' +import { checkWslBridgeDistro, classifyWslBridgeExit } from './daemon-wsl-bridge-exit' afterEach(() => { _resetRunningWslDistroCacheForTests() @@ -42,9 +42,35 @@ describe('checkWslBridgeDistro', () => { }) it('treats a hung check as unanswered, so the exit stays an exit', async () => { - await expect(checkWslBridgeDistro('Ubuntu', 0, () => new Promise(() => {}), 10)).resolves.toBe( - 'unavailable' - ) + await expect( + checkWslBridgeDistro('Ubuntu', 0, () => new Promise(() => {}), { firstMs: 10, totalMs: 30 }) + ).resolves.toBe('unavailable') + }) + + // 5e hardening (b): a slow or failing first read is asked once more before the exit stays an exit. + it('retries an unanswered read once, past the failure backoff, within the total budget', async () => { + const reads: { bypassFailureBackoff?: boolean }[] = [] + const answers: WslRunningDistrosRead[] = [ + { status: 'unavailable' }, + { status: 'confirmed', distros: [] } + ] + await expect( + checkWslBridgeDistro('Ubuntu', 0, async (options) => { + reads.push({ bypassFailureBackoff: options.bypassFailureBackoff }) + return answers.shift() ?? { status: 'unavailable' } + }) + ).resolves.toBe('lost') + expect(reads).toEqual([{ bypassFailureBackoff: undefined }, { bypassFailureBackoff: true }]) + }) + + it('gives the retry only what is left of the total budget after a hung first read', async () => { + const reads = vi.fn(() => new Promise(() => {})) + const startedAt = performance.now() + await expect( + checkWslBridgeDistro('Ubuntu', 0, reads, { firstMs: 20, totalMs: 60 }) + ).resolves.toBe('unavailable') + expect(reads).toHaveBeenCalledTimes(2) + expect(performance.now() - startedAt).toBeLessThan(500) }) it('treats a failed check as unanswered', async () => { @@ -77,3 +103,35 @@ describe('checkWslBridgeDistro', () => { await expect(Promise.all(checks)).resolves.toEqual(['running', 'running']) }) }) + +describe('classifyWslBridgeExit', () => { + const running = async (): Promise => ({ + status: 'confirmed', + distros: ['Ubuntu'] + }) + + it('calls a running distro that booted again since the spawn rebooted, not an exit', async () => { + await expect(classifyWslBridgeExit('Ubuntu', 0, running, async () => true)).resolves.toBe( + 'rebooted' + ) + await expect(classifyWslBridgeExit('Ubuntu', 0, running, async () => false)).resolves.toBe( + 'running' + ) + }) + + it('asks about the boot id only when the running list says running', async () => { + const rebooted = vi.fn(async () => true) + await expect( + classifyWslBridgeExit( + 'Ubuntu', + 0, + async () => ({ status: 'confirmed', distros: [] }), + rebooted + ) + ).resolves.toBe('lost') + await expect( + classifyWslBridgeExit('Ubuntu', 0, async () => ({ status: 'unavailable' }), rebooted) + ).resolves.toBe('unavailable') + expect(rebooted).not.toHaveBeenCalled() + }) +}) diff --git a/src/main/daemon/daemon-wsl-bridge-exit.ts b/src/main/daemon/daemon-wsl-bridge-exit.ts index b769b2506b1..e8aa57e8f23 100644 --- a/src/main/daemon/daemon-wsl-bridge-exit.ts +++ b/src/main/daemon/daemon-wsl-bridge-exit.ts @@ -2,7 +2,8 @@ * A WSL bridge pane's process exit, when its whole distro is gone (`wsl --terminate`, a shutdown * or a crash), is host loss, not the shell exiting: like a daemon crash, the pane keeps its tab * and history and cold-restores later. `wsl.exe` reports such an exit as an ordinary code (1), - * so only the running list tells it from a shell's own `exit`. + * so only the running list, and the distro's boot id when something booted it again first, tells + * it from a shell's own `exit`. */ import { readConfirmedRunningWslDistros, @@ -13,10 +14,13 @@ import { export const WSL_DISTRO_LOSS_EXIT_CODE = -1 // Every bridge exit, a user's own `exit` too, waits on this before its tab reacts: keep it short. -const WSL_EXIT_CHECK_TIMEOUT_MS = 3_000 +const WSL_EXIT_CHECK_FIRST_MS = 3_000 +// One retry past the failure backoff when the first read can't answer; a wedged wsl.exe costs this. +const WSL_EXIT_CHECK_TOTAL_MS = 10_000 export type ReadRunningWslDistros = (options: { startedAfter?: number + bypassFailureBackoff?: boolean }) => Promise /** What the running list said about the distro after a bridge exit. */ @@ -26,33 +30,64 @@ export type WslBridgeExitVerdict = 'lost' | 'running' | 'unavailable' * Lost only when a running list confirmed after the exit lacks the distro. The probe is the shared, * coalesced `wsl --list --running` (it never boots): a terminate that exits many panes costs one * spawn, and a probe already in flight from before the exit (which may predate the distro's boot) - * gets one fresh probe after it. Unavailable or late means unchanged: the exit stays an exit. + * gets one fresh probe after it. A read that can't answer is retried once; still unavailable or + * late means unchanged: the exit stays an exit. */ export async function checkWslBridgeDistro( distro: string, exitAt: number, readRunning: ReadRunningWslDistros = readConfirmedRunningWslDistros, - timeoutMs = WSL_EXIT_CHECK_TIMEOUT_MS + { firstMs = WSL_EXIT_CHECK_FIRST_MS, totalMs = WSL_EXIT_CHECK_TOTAL_MS } = {} ): Promise { + const startedAt = performance.now() + let running = await readWithin(() => readRunning({ startedAfter: exitAt }), firstMs) + if (running.status !== 'confirmed') { + const remainingMs = totalMs - (performance.now() - startedAt) + running = + remainingMs > 0 + ? await readWithin( + () => readRunning({ startedAfter: exitAt, bypassFailureBackoff: true }), + remainingMs + ) + : running + } + if (running.status !== 'confirmed') { + return 'unavailable' + } + const key = distro.trim().toLowerCase() + return running.distros.some((candidate) => candidate.trim().toLowerCase() === key) + ? 'running' + : 'lost' +} + +async function readWithin( + read: () => Promise, + timeoutMs: number +): Promise { let timer: ReturnType | undefined const timedOut = new Promise((resolve) => { timer = setTimeout(() => resolve({ status: 'unavailable' }), timeoutMs) }) try { - const running = await Promise.race([ - readRunning({ startedAfter: exitAt }).catch((): WslRunningDistrosRead => ({ - status: 'unavailable' - })), + return await Promise.race([ + read().catch((): WslRunningDistrosRead => ({ status: 'unavailable' })), timedOut ]) - if (running.status !== 'confirmed') { - return 'unavailable' - } - const key = distro.trim().toLowerCase() - return running.distros.some((candidate) => candidate.trim().toLowerCase() === key) - ? 'running' - : 'lost' } finally { clearTimeout(timer) } } + +/** + * The verdict for a bridge exit. A distro that answers 'running' may have been booted again by + * something else before the list ran; a new boot id since the pane's spawn still makes it loss. + */ +export async function classifyWslBridgeExit( + distro: string, + exitAt: number, + readRunning: ReadRunningWslDistros | undefined, + rebootedSinceSpawn: () => Promise +): Promise { + const verdict = await checkWslBridgeDistro(distro, exitAt, readRunning) + return verdict === 'running' && (await rebootedSinceSpawn()) ? 'rebooted' : verdict +} diff --git a/src/main/wsl/wsl-distro-boot-id.test.ts b/src/main/wsl/wsl-distro-boot-id.test.ts new file mode 100644 index 00000000000..58d8b468dea --- /dev/null +++ b/src/main/wsl/wsl-distro-boot-id.test.ts @@ -0,0 +1,36 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const runProcess = vi.hoisted(() => vi.fn()) +vi.mock('@orca/process-host', () => ({ runProcess })) +vi.mock('./wsl-executable-path', () => ({ resolveWslExecutablePath: () => 'wsl.exe' })) +vi.mock('../wsl-interop-spawn-directory', () => ({ resolveWslInteropSpawnCwd: () => 'C:\\' })) + +import { readWslDistroBootId } from './wsl-distro-boot-id' + +const BOOT_ID = '0e3a7f2c-1b4d-4c5e-9f60-7a8b9c0d1e2f' +const answer = (result: Partial<{ code: number; stdout: string; timedOut: boolean }>) => + runProcess.mockResolvedValueOnce({ code: 0, stdout: '', timedOut: false, ...result }) + +beforeEach(() => runProcess.mockReset()) + +describe('readWslDistroBootId', () => { + it('reads the boot id with --exec and no shell, so no rc file runs', async () => { + answer({ stdout: `${BOOT_ID.toUpperCase()}\n` }) + await expect(readWslDistroBootId('Ubuntu')).resolves.toBe(BOOT_ID) + expect(runProcess).toHaveBeenCalledWith( + expect.objectContaining({ + program: 'wsl.exe', + args: ['-d', 'Ubuntu', '--exec', 'cat', '/proc/sys/kernel/random/boot_id'] + }) + ) + }) + + it.each([ + ['a failed read', { code: 1, stdout: BOOT_ID }], + ['a timeout', { timedOut: true, stdout: BOOT_ID }], + ['output that is not a boot id', { stdout: `Welcome to Ubuntu\n${BOOT_ID}` }] + ])('answers null for %s', async (_name, result) => { + answer(result) + await expect(readWslDistroBootId('Ubuntu')).resolves.toBeNull() + }) +}) diff --git a/src/main/wsl/wsl-distro-boot-id.ts b/src/main/wsl/wsl-distro-boot-id.ts new file mode 100644 index 00000000000..4851a5c7bae --- /dev/null +++ b/src/main/wsl/wsl-distro-boot-id.ts @@ -0,0 +1,28 @@ +import { runProcess } from '@orca/process-host' +import { buildWslExecArgs } from '../../shared/wsl-login-shell-command' +import { resolveWslExecutablePath } from './wsl-executable-path' +import { resolveWslInteropSpawnCwd } from '../wsl-interop-spawn-directory' + +const BOOT_ID_TIMEOUT_MS = 5_000 +const BOOT_ID = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i + +/** + * The distro kernel's boot id, which changes each time the distro starts. `wsl.exe --exec` boots a + * stopped distro, so callers must have just confirmed it runs. Null when it can't be read. + */ +export async function readWslDistroBootId(distro: string): Promise { + const result = await runProcess({ + program: resolveWslExecutablePath(), + // --exec with no shell: no rc files, so stdout is the file alone. + args: buildWslExecArgs(distro, ['cat', '/proc/sys/kernel/random/boot_id']), + env: { ...process.env, WSL_UTF8: '1' }, + cwd: resolveWslInteropSpawnCwd(), + timeoutMs: BOOT_ID_TIMEOUT_MS, + maxOutputBytes: 256 + }) + if (result.timedOut || result.code !== 0) { + return null + } + const bootId = result.stdout.trim() + return BOOT_ID.test(bootId) ? bootId.toLowerCase() : null +}