diff --git a/src/cli/runtime/launch-serve-replaced-app-restart.test.ts b/src/cli/runtime/launch-serve-replaced-app-restart.test.ts new file mode 100644 index 00000000000..76ba0d5b3b7 --- /dev/null +++ b/src/cli/runtime/launch-serve-replaced-app-restart.test.ts @@ -0,0 +1,249 @@ +import { EventEmitter } from 'node:events' +import { chmod, mkdir, mkdtemp, rename, rm, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type * as ReplacedAppRestart from './serve-replaced-app-restart' +import type * as ServeInstallHold from './serve-install-hold' +import { getServeUpdateHandoffPath } from '../../shared/serve-update-handoff' +import { resetSupervisedServeStop } from './serve-supervised-stop' +import { readServeUpdateHandoff } from './serve-update-supervisor' + +const { spawnMock, restartMock } = vi.hoisted(() => ({ + spawnMock: vi.fn(), + restartMock: vi.fn() +})) + +vi.mock('child_process', () => ({ spawn: spawnMock, spawnSync: vi.fn() })) +vi.mock('./serve-replaced-app-restart', async (importOriginal) => ({ + ...(await importOriginal()), + restartThisServeSupervisorIfReplaced: restartMock +})) + +// Why: the real check lists processes for the installer, which the spawn mock cannot answer. +vi.mock('./serve-install-hold', async (importOriginal) => ({ + ...(await importOriginal()), + readServeInstallEvidence: async () => ({ kind: 'clear' }) +})) + +import { serveOrcaApp } from './launch' + +class FakeChild extends EventEmitter { + kill = vi.fn() + constructor(public pid: number) { + super() + } +} + +const OLD = '1.0.51' +const NEW = '1.0.61' +const ENV_KEYS = [ + 'XPC_SERVICE_NAME', + 'ELECTRON_RUN_AS_NODE', + 'ORCA_SERVE_RUNTIME', + 'ORCA_USER_DATA_PATH', + 'ORCA_APP_EXECUTABLE' +] as const + +function overrideProperty(key: 'platform' | 'execPath', value: string): () => void { + const descriptor = Object.getOwnPropertyDescriptor(process, key) + Object.defineProperty(process, key, { configurable: true, value }) + return () => { + if (descriptor) { + Object.defineProperty(process, key, descriptor) + } + } +} + +async function installApp(dir: string, version: string): Promise { + await mkdir(join(dir, 'Contents', 'MacOS'), { recursive: true }) + await writeFile(join(dir, 'Contents', 'MacOS', 'Orca'), '') + await chmod(join(dir, 'Contents', 'MacOS', 'Orca'), 0o755) + await writeFile( + join(dir, 'Contents', 'Info.plist'), + `CFBundleShortVersionString${version}` + ) +} + +describe('serveOrcaApp restarts from a replaced app before any server starts', () => { + let root: string + let app: string + let handoffPath: string + const restore: (() => void)[] = [] + const savedEnv: Record = {} + const requested = (servingPid: number) => ({ + schemaVersion: 1 as const, + phase: 'install-requested' as const, + fromVersion: OLD, + targetVersion: NEW, + servingPid + }) + + beforeEach(async () => { + root = await mkdtemp(join(tmpdir(), 'orca-serve-replaced-app-')) + app = join(root, 'Orca.app') + // The update is already on disk unless a test swaps it in. + await installApp(app, NEW) + for (const key of ENV_KEYS) { + savedEnv[key] = process.env[key] + } + process.env.ELECTRON_RUN_AS_NODE = '1' + process.env.ORCA_SERVE_RUNTIME = 'electron' + process.env.ORCA_USER_DATA_PATH = join(root, 'profile') + delete process.env.ORCA_APP_EXECUTABLE + delete process.env.XPC_SERVICE_NAME + await mkdir(join(root, 'profile'), { recursive: true }) + handoffPath = getServeUpdateHandoffPath(join(root, 'profile')) + restore.push(overrideProperty('platform', 'darwin')) + restore.push(overrideProperty('execPath', join(app, 'Contents', 'MacOS', 'Orca'))) + vi.spyOn(process.stderr, 'write').mockImplementation(() => true) + }) + + afterEach(async () => { + for (const undo of restore.splice(0).toReversed()) { + undo() + } + for (const key of ENV_KEYS) { + if (savedEnv[key] === undefined) { + delete process.env[key] + } else { + process.env[key] = savedEnv[key] + } + } + resetSupervisedServeStop() + vi.restoreAllMocks() + restartMock.mockReset() + spawnMock.mockReset() + await rm(root, { recursive: true, force: true }) + }) + + /** Each restart check must precede its own spawn: none may start a server unchecked. */ + function expectCheckedBeforeEverySpawn(spawns: number): void { + expect(spawnMock).toHaveBeenCalledTimes(spawns) + expect(restartMock).toHaveBeenCalledTimes(spawns) + for (let index = 0; index < spawns; index++) { + expect(restartMock.mock.invocationCallOrder[index]).toBeLessThan( + spawnMock.mock.invocationCallOrder[index] + ) + } + } + + async function exitForUpdate(): Promise<{ code: Promise; replacement: FakeChild }> { + const old = new FakeChild(4101) + const replacement = new FakeChild(4102) + spawnMock.mockReturnValueOnce(old).mockReturnValueOnce(replacement) + const code = serveOrcaApp({ json: true }) + await vi.waitFor(() => expect(spawnMock).toHaveBeenCalledOnce()) + await writeFile(handoffPath, JSON.stringify(requested(old.pid))) + old.emit('exit', 0, null) + return { code, replacement } + } + + it('checks before the first server and before the update replacement, keeping the handoff', async () => { + let stateAtSecondCheck: unknown + restartMock.mockImplementation(() => { + if (spawnMock.mock.calls.length === 1) { + stateAtSecondCheck = readServeUpdateHandoff(handoffPath) + } + }) + const { code, replacement } = await exitForUpdate() + await vi.waitFor(() => expect(spawnMock).toHaveBeenCalledTimes(2)) + await expect(stateAtSecondCheck).resolves.toMatchObject({ phase: 'install-requested' }) + replacement.emit('exit', 0, null) + await expect(code).resolves.toBe(1) + expectCheckedBeforeEverySpawn(2) + }) + + it('neither restarts nor spawns a replacement once a stop is pending', async () => { + resetSupervisedServeStop(async () => false) + const { code } = await exitForUpdate() + await expect(code).resolves.toBe(0) + expectCheckedBeforeEverySpawn(1) + }) + + it('checks before the server a resumed update starts', async () => { + await writeFile(handoffPath, JSON.stringify(requested(4101))) + const replacement = new FakeChild(4102) + spawnMock.mockReturnValueOnce(replacement) + const code = serveOrcaApp({ json: true }) + await vi.waitFor(() => expect(spawnMock).toHaveBeenCalledOnce()) + replacement.emit('exit', 0, null) + await code + expectCheckedBeforeEverySpawn(1) + }) + + it('does not restart when serving an executable other than its own', async () => { + process.env.ORCA_APP_EXECUTABLE = join(app, 'Contents', 'MacOS', 'Orca') + restore.push(overrideProperty('execPath', join(root, 'node'))) + const { code, replacement } = await exitForUpdate() + await vi.waitFor(() => expect(spawnMock).toHaveBeenCalledTimes(2)) + replacement.emit('exit', 0, null) + await expect(code).resolves.toBe(1) + expect(restartMock).not.toHaveBeenCalled() + }) + + // The real decision, for a supervisor that started on the old app before the install swapped it. + async function withRealRestart() { + const actual = await vi.importActual('./serve-replaced-app-restart') + restartMock.mockImplementation(actual.restartThisServeSupervisorIfReplaced) + // Throwing models a runtime without exec, so the supervisor falls back to spawning here. + return vi.spyOn(process, 'execve').mockImplementation(() => { + throw new Error('exec recorded') + }) + } + + async function swapInstall(version: string): Promise { + await installApp(join(root, 'Update.app'), version) + await rename(app, join(root, 'Previous.app')) + await rename(join(root, 'Update.app'), app) + } + + async function resumeAcrossInstall(swapDuringWait: boolean) { + const execve = await withRealRestart() + if (swapDuringWait) { + await installApp(app, OLD) + } + await writeFile(handoffPath, JSON.stringify(requested(4101))) + const replacement = new FakeChild(4102) + spawnMock.mockReturnValueOnce(replacement) + const code = serveOrcaApp({ json: true }) + if (swapDuringWait) { + await swapInstall(NEW) + } + await vi.waitFor(() => expect(spawnMock).toHaveBeenCalledOnce()) + replacement.emit('message', { type: 'orca:serve-ready', version: NEW, runtimeId: 'new' }) + await vi.waitFor(async () => expect(await readServeUpdateHandoff(handoffPath)).toBeNull()) + replacement.emit('exit', 0, null) + await expect(code).resolves.toBe(0) + return execve + } + + it('restarts a resumed supervisor whose executable the install replaced while it waited', async () => { + const execve = await resumeAcrossInstall(true) + expect(execve).toHaveBeenCalledOnce() + expect(execve.mock.calls[0][0]).toBe(process.execPath) + expect(execve.mock.invocationCallOrder[0]).toBeLessThan(spawnMock.mock.invocationCallOrder[0]) + }) + + it('resumes without restarting when already on the installed executable, as after a restart', async () => { + expect(await resumeAcrossInstall(false)).not.toHaveBeenCalled() + }) + + // A later attempt of the service loop (here after a crash; also after a park) has no handoff. + it('restarts before a supervised service starts its next server from a replaced app', async () => { + const execve = await withRealRestart() + const crashed = new FakeChild(4101) + const next = new FakeChild(4102) + spawnMock.mockReturnValueOnce(crashed).mockReturnValueOnce(next) + const service = serveOrcaApp({ supervise: true }) + await vi.waitFor(() => expect(spawnMock).toHaveBeenCalledOnce()) + await swapInstall(NEW) + crashed.emit('exit', 1, null) + await vi.waitFor(() => expect(spawnMock).toHaveBeenCalledTimes(2), { timeout: 10_000 }) + expect(execve).toHaveBeenCalledOnce() + expect(execve.mock.invocationCallOrder[0]).toBeLessThan(spawnMock.mock.invocationCallOrder[1]) + process.emit('SIGTERM', 'SIGTERM') + next.emit('exit', null, 'SIGTERM') + await expect(service).resolves.toBe(0) + }, 20_000) +}) diff --git a/src/cli/runtime/launch.ts b/src/cli/runtime/launch.ts index a47d78513d3..f7d6476bbd5 100644 --- a/src/cli/runtime/launch.ts +++ b/src/cli/runtime/launch.ts @@ -42,6 +42,10 @@ import { relocateThisServeSupervisor, SERVE_SUPERVISOR_APP_ROOT_ENV } from './serve-supervisor-relocation' +import { + noteServeSupervisorStart, + restartThisServeSupervisorIfReplaced +} from './serve-replaced-app-restart' import { serverOnlyServeMessage } from './serve-app-launcher' import { isSupervisedSpawnHeld } from './serve-supervised-stop' import { openServeBundlePark } from '../../shared/serve-bundle-park' @@ -131,6 +135,7 @@ function spawnDetached(command: string, args: string[], options: SpawnOptions): export function serveOrcaApp(args: ServeOrcaAppArgs = {}): Promise { // Why first: as a container or PrivatePIDs init, nothing would reap orphaned descendants. reexecThisServeUnderInitReaper() + noteServeSupervisorStart() const executable = resolveForegroundOrcaExecutable() // Why one value: the selector, orcad and Electron must all serve the profile the caller chose. const userDataPath = resolveLaunchUserDataPath() @@ -255,9 +260,18 @@ function spawnElectronServe( : 'inherit', env: childEnv } - const spawnServeChild: ServeSpawnChild = shouldRedactSupervisedStdout(args.supervise === true) + const spawnServer: ServeSpawnChild = shouldRedactSupervisedStdout(args.supervise === true) ? redactingSpawn(spawnProcess) : spawnProcess + // Why in the spawn: every server start (first, resumed, after an install or a park) passes here, + // after its stop and park gates. Only our own executable: an update never deletes an override. + const spawnServeChild: ServeSpawnChild = + handoffPath && executable === process.execPath + ? (childSpec) => { + restartThisServeSupervisorIfReplaced(resolveAppRoot()) + return spawnServer(childSpec) + } + : spawnServer const interruptedHandoff = handoffPath ? readServeUpdateHandoffSync(handoffPath) : null if (interruptedHandoff?.phase === 'install-requested') { // Why: the node-mode CLI is not an NSRunningApplication, so it can retain launchd ownership while ShipIt swaps the app. diff --git a/src/cli/runtime/serve-replaced-app-restart.ts b/src/cli/runtime/serve-replaced-app-restart.ts new file mode 100644 index 00000000000..2d3195577dc --- /dev/null +++ b/src/cli/runtime/serve-replaced-app-restart.ts @@ -0,0 +1,92 @@ +/** + * macOS charges the permissions of every process a launchd-started supervisor spawns to that + * supervisor, so once an update deletes the app it runs from, no server may start under it. Before + * each server starts, a supervisor whose executable was replaced re-runs itself in place (same pid) + * from the installed app. + */ +import { accessSync, constants, existsSync, statSync } from 'node:fs' +import { join } from 'node:path' +import { + execveThisProcess, + releaseServeSupervisorSnapshot, + SERVE_SUPERVISOR_APP_ROOT_ENV, + type SupervisorRelocationDeps +} from './serve-supervisor-relocation' + +/** `dev:ino` of a file, which changes when an update replaces the app; null if unreadable. */ +function fileIdentity(path: string): string | null { + try { + const stats = statSync(path) + return `${stats.dev}:${stats.ino}` + } catch { + return null + } +} + +function isExecutable(path: string): boolean { + try { + accessSync(path, constants.X_OK) + return true + } catch { + return false + } +} + +// Why process state: the first server may start long after an update wait or a park. +let startedFrom: string | null = null + +export function noteServeSupervisorStart(): void { + startedFrom = fileIdentity(process.execPath) +} + +export type ReplacedAppRestartDeps = Pick< + SupervisorRelocationDeps, + 'env' | 'execPath' | 'argv' | 'appRoot' | 'pid' | 'execve' | 'log' +> & { execArgv: string[]; startedFrom: string | null } + +/** Returns only when it did not restart. */ +export function restartServeSupervisorIfReplaced(deps: ReplacedAppRestartDeps): void { + const installed = fileIdentity(deps.execPath) + // Why: after a restart the start identity is the installed one, so this never loops. + if (!deps.startedFrom || !installed || installed === deps.startedFrom) { + return + } + const env = { ...deps.env } + const relocated = Boolean(env[SERVE_SUPERVISOR_APP_ROOT_ENV]) + delete env[SERVE_SUPERVISOR_APP_ROOT_ENV] + // Why the app's CLI: a relocated supervisor must copy the new release aside, not rerun the old copy. + const cliEntry = relocated ? join(deps.appRoot, 'out', 'cli', 'index.js') : deps.argv[1] + // Why preflight: a failed process.execve aborts the process natively instead of throwing. + if (!isExecutable(deps.execPath) || !existsSync(cliEntry)) { + deps.log('[serve] Keeping the old executable; the installed app is not ready to run.') + return + } + if (relocated) { + releaseServeSupervisorSnapshot(deps.argv[1], deps.pid) + } + try { + deps.execve( + deps.execPath, + [deps.execPath, ...deps.execArgv, cliEntry, ...deps.argv.slice(2)], + env + ) + } catch (error) { + deps.log( + `[serve] Keeping the old executable; could not restart from the installed app: ${error instanceof Error ? error.message : String(error)}` + ) + } +} + +export function restartThisServeSupervisorIfReplaced(appRoot: string): void { + restartServeSupervisorIfReplaced({ + env: process.env, + execPath: process.execPath, + execArgv: process.execArgv, + argv: process.argv, + appRoot, + pid: process.pid, + execve: execveThisProcess, + log: (line) => process.stderr.write(`${line}\n`), + startedFrom + }) +} diff --git a/src/cli/runtime/serve-supervisor-relocation.test.ts b/src/cli/runtime/serve-supervisor-relocation.test.ts index ff929f050ce..66a9c17e6a8 100644 --- a/src/cli/runtime/serve-supervisor-relocation.test.ts +++ b/src/cli/runtime/serve-supervisor-relocation.test.ts @@ -10,7 +10,7 @@ import { symlink, writeFile } from 'node:fs/promises' -import { existsSync } from 'node:fs' +import { existsSync, statSync } from 'node:fs' import { tmpdir } from 'node:os' import { join } from 'node:path' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' @@ -20,6 +20,10 @@ import { serveSupervisorSnapshotRoot, type SupervisorRelocationDeps } from './serve-supervisor-relocation' +import { + restartServeSupervisorIfReplaced, + type ReplacedAppRestartDeps +} from './serve-replaced-app-restart' describe('relocateServeSupervisor', () => { let root: string @@ -203,4 +207,103 @@ describe('relocateServeSupervisor', () => { await relocateServeSupervisor(deps({ pid: 300, isProcessAlive: (pid) => pid === 300 })) expect(await releaseCopies()).toHaveLength(1) }) + + describe('restartServeSupervisorIfReplaced', () => { + const log = vi.fn() + beforeEach(async () => { + await mkdir(join(root, 'Orca.app', 'Contents', 'MacOS'), { recursive: true }) + await writeFile(deps().execPath, '') + await chmod(deps().execPath, 0o755) + }) + const restart = ( + env: NodeJS.ProcessEnv, + argv: string[], + overrides: Partial = {} + ) => { + const d: ReplacedAppRestartDeps = { + env, + execPath: deps().execPath, + execArgv: [], + argv, + appRoot, + pid: 4242, + execve: vi.fn(), + log, + startedFrom: 'old', + ...overrides + } + restartServeSupervisorIfReplaced(d) + return { execve: vi.mocked(d.execve), execPath: d.execPath } + } + const appCli = () => join(appRoot, 'out', 'cli', 'index.js') + + it('re-runs plain serve with the same arguments on the replaced executable', () => { + const argv = ['/ignored', appCli(), 'serve', '--json'] + const { execve, execPath } = restart({ ELECTRON_RUN_AS_NODE: '1' }, argv) + expect(execve).toHaveBeenCalledWith(execPath, [execPath, ...argv.slice(1)], { + ELECTRON_RUN_AS_NODE: '1' + }) + }) + + it('does not restart when the executable is the one it started from, as after a restart', () => { + const argv = ['/ignored', appCli(), 'serve'] + const installed = statSync(deps().execPath) + const startedFrom = `${installed.dev}:${installed.ino}` + expect(restart({}, argv, { startedFrom }).execve).not.toHaveBeenCalled() + expect(restart({}, argv, { startedFrom: null }).execve).not.toHaveBeenCalled() + const gone = join(root, 'gone') + expect(restart({}, argv, { execPath: gone }).execve).not.toHaveBeenCalled() + }) + + // Why: a failed process.execve aborts natively, so nothing may reach it that cannot run. + it('keeps running and logs when the installed app cannot run yet', async () => { + const argv = ['/ignored', appCli(), 'serve'] + await chmod(deps().execPath, 0o644) + expect(restart({}, argv).execve).not.toHaveBeenCalled() + await chmod(deps().execPath, 0o755) + expect( + restart({}, ['/ignored', join(root, 'gone.js'), 'serve']).execve + ).not.toHaveBeenCalled() + expect(log).toHaveBeenCalledWith(expect.stringContaining('not ready to run')) + }) + + it('logs instead of throwing on a runtime that cannot replace its own process', () => { + const execve = vi.fn(() => { + throw new Error('this runtime cannot replace its own process') + }) + expect(() => restart({}, ['/ignored', appCli(), 'serve'], { execve })).not.toThrow() + expect(log).toHaveBeenCalledWith(expect.stringContaining('cannot replace')) + }) + + it('re-runs a relocated supervisor from the app, which copies the new release aside', async () => { + const first = deps() + await relocateServeSupervisor(first) + const [, relocatedArgs, relocatedEnv] = vi.mocked(first.execve).mock.calls[0] + await writeFile(join(appRoot, 'out', 'package.json'), '{"version":"1.1.0","commit":"bbb"}') + + const { execve, execPath } = restart(relocatedEnv, relocatedArgs) + const [, args, env] = execve.mock.calls[0] + expect(args).toEqual([execPath, appCli(), 'serve', '--supervise']) + expect(env).not.toHaveProperty(SERVE_SUPERVISOR_APP_ROOT_ENV) + + const again = deps({ env, argv: args }) + await expect(relocateServeSupervisor(again)).resolves.toBe('relocated') + expect(vi.mocked(again.execve).mock.calls[0][1][1]).not.toBe(relocatedArgs[1]) + }) + + it('releases the copy it leaves, so updates under one live pid keep a single copy', async () => { + const isProcessAlive = (pid: number) => pid === 4242 + let env: NodeJS.ProcessEnv = { ELECTRON_RUN_AS_NODE: '1' } + let argv = deps().argv + for (const version of ['1.0.0', '1.1.0', '1.2.0']) { + await writeFile(join(appRoot, 'out', 'package.json'), `{"version":"${version}"}`) + const relocation = deps({ env, argv, isProcessAlive }) + await expect(relocateServeSupervisor(relocation)).resolves.toBe('relocated') + const [, relocatedArgs, relocatedEnv] = vi.mocked(relocation.execve).mock.calls[0] + expect(await releaseCopies()).toHaveLength(1) + const { execve } = restart(relocatedEnv, relocatedArgs) + ;[, argv, env] = execve.mock.calls[0] + } + }) + }) }) diff --git a/src/cli/runtime/serve-supervisor-relocation.ts b/src/cli/runtime/serve-supervisor-relocation.ts index 8259cfbaeb9..1daba10cb29 100644 --- a/src/cli/runtime/serve-supervisor-relocation.ts +++ b/src/cli/runtime/serve-supervisor-relocation.ts @@ -5,7 +5,7 @@ * starts from the app, so it updates as before. */ import { createHash } from 'node:crypto' -import { constants, readFileSync, statSync } from 'node:fs' +import { constants, readFileSync, rmSync, statSync } from 'node:fs' import { cp, mkdir, readdir, rename, rm, stat, writeFile } from 'node:fs/promises' import { basename, dirname, isAbsolute, join, relative, resolve } from 'node:path' import { cloneMacTree } from '../../main/macos-tree-clone' @@ -203,6 +203,19 @@ export async function relocateServeSupervisor( return 'relocated' } +/** Releases `pid`'s claim on the copy it runs `cliEntry` from, so cleanup can remove that copy. */ +export function releaseServeSupervisorSnapshot(cliEntry: string, pid: number): void { + const snapshotDir = resolve(cliEntry, '..', '..', '..', '..') + rmSync(join(usersDir(dirname(snapshotDir), basename(snapshotDir)), String(pid)), { force: true }) +} + +export function execveThisProcess(file: string, args: string[], env: NodeJS.ProcessEnv): void { + if (!process.execve) { + throw new Error('this runtime cannot replace its own process') + } + process.execve(file, args, env) +} + export function relocateThisServeSupervisor( appRoot: string, userDataPath: string @@ -215,12 +228,7 @@ export function relocateThisServeSupervisor( userDataPath, pid: process.pid, isProcessAlive, - execve: (file, args, env) => { - if (!process.execve) { - throw new Error('this runtime cannot replace its own process') - } - process.execve(file, args, env) - }, + execve: execveThisProcess, log: (line) => process.stderr.write(`${line}\n`) }) }