From 7e7ca31adf1e5dfabb4da1e7d1801bfa34a2d3fc Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sun, 30 Aug 2026 22:58:23 -0700 Subject: [PATCH] fix(linux): fail serve when no display is available --- src/main/index.ts | 5 ++ .../startup/ensure-virtual-display.test.ts | 48 +++++++------------ src/main/startup/ensure-virtual-display.ts | 22 +++------ ...single-instance-lock-exit.electron.test.ts | 48 +++++++++++-------- 4 files changed, 57 insertions(+), 66 deletions(-) diff --git a/src/main/index.ts b/src/main/index.ts index ea68f82dc9c..0f46961129a 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -1015,6 +1015,11 @@ if (hasSingleInstanceLock) { } // Why: headless serve's offscreen BrowserWindows need an X display (Xvfb) on Linux; the result gates whether the offscreen backend is installed. headlessBrowserDisplayAvailable = ensureVirtualDisplayForHeadlessServe({ isServeMode }) + // Why: continuing without Xvfb lets Ozone initialize without a display and SIGSEGV (#17615). + if (isServeMode && !headlessBrowserDisplayAvailable) { + process.stderr.write(`${MISSING_LINUX_DISPLAY_MESSAGE}\n`) + app.exit(1) + } } ipcMain.handle('app:awaitFirstWindowStartupServices', async () => { diff --git a/src/main/startup/ensure-virtual-display.test.ts b/src/main/startup/ensure-virtual-display.test.ts index bfd25c5097a..7350d96a56e 100644 --- a/src/main/startup/ensure-virtual-display.test.ts +++ b/src/main/startup/ensure-virtual-display.test.ts @@ -1,28 +1,20 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -const { - spawnMock, - spawnSyncMock, - existsSyncMock, - readFileSyncMock, - rmSyncMock, - statSyncMock, - appMock -} = vi.hoisted(() => ({ - spawnMock: vi.fn(), - spawnSyncMock: vi.fn(), - existsSyncMock: vi.fn(), - readFileSyncMock: vi.fn(), - rmSyncMock: vi.fn(), - statSyncMock: vi.fn(), - appMock: { - disableHardwareAcceleration: vi.fn(), - commandLine: { appendSwitch: vi.fn(), getSwitchValue: vi.fn() }, - once: vi.fn() - } -})) +const { spawnMock, existsSyncMock, readFileSyncMock, rmSyncMock, statSyncMock, appMock } = + vi.hoisted(() => ({ + spawnMock: vi.fn(), + existsSyncMock: vi.fn(), + readFileSyncMock: vi.fn(), + rmSyncMock: vi.fn(), + statSyncMock: vi.fn(), + appMock: { + disableHardwareAcceleration: vi.fn(), + commandLine: { appendSwitch: vi.fn(), getSwitchValue: vi.fn() }, + once: vi.fn() + } + })) -vi.mock('child_process', () => ({ spawn: spawnMock, spawnSync: spawnSyncMock })) +vi.mock('child_process', () => ({ spawn: spawnMock })) vi.mock('fs', () => ({ existsSync: existsSyncMock, readFileSync: readFileSyncMock, @@ -48,7 +40,6 @@ function mockLiveXDisplay(pid = 4321): void { describe('ensureVirtualDisplayForHeadlessServe', () => { beforeEach(() => { spawnMock.mockReset() - spawnSyncMock.mockReset() existsSyncMock.mockReset() readFileSyncMock.mockReset() rmSyncMock.mockReset() @@ -105,14 +96,14 @@ describe('ensureVirtualDisplayForHeadlessServe', () => { expect(appMock.commandLine.appendSwitch).toHaveBeenCalledWith('disable-gpu') }) - it('reports unsupported (no spawn) when Xvfb is not installed', async () => { + it('reports unsupported when Xvfb cannot be launched', async () => { setPlatform('linux') - spawnSyncMock.mockReturnValue({ status: 1 }) // `which Xvfb` fails + spawnMock.mockReturnValue({ pid: undefined, once: vi.fn(), kill: vi.fn(), killed: false }) const { ensureVirtualDisplayForHeadlessServe, MISSING_LINUX_DISPLAY_MESSAGE } = await import('./ensure-virtual-display') expect(ensureVirtualDisplayForHeadlessServe({ isServeMode: true })).toBe(false) - expect(spawnMock).not.toHaveBeenCalled() + expect(spawnMock).toHaveBeenCalledWith('Xvfb', expect.any(Array), expect.any(Object)) expect(MISSING_LINUX_DISPLAY_MESSAGE).toContain('endpoint is unavailable') expect(MISSING_LINUX_DISPLAY_MESSAGE).toContain('XDG_RUNTIME_DIR') expect(MISSING_LINUX_DISPLAY_MESSAGE).toContain('`xvfb` on Debian/Ubuntu') @@ -130,7 +121,6 @@ describe('ensureVirtualDisplayForHeadlessServe', () => { const { ensureVirtualDisplayForHeadlessServe } = await import('./ensure-virtual-display') expect(ensureVirtualDisplayForHeadlessServe({ isServeMode: true })).toBe(false) - expect(spawnSyncMock).not.toHaveBeenCalled() expect(spawnMock).not.toHaveBeenCalled() expect(rmSyncMock).not.toHaveBeenCalled() expect(process.env.DISPLAY).toBe(':77') @@ -138,7 +128,6 @@ describe('ensureVirtualDisplayForHeadlessServe', () => { it('reuses an existing virtual display only when its X server is alive', async () => { setPlatform('linux') - spawnSyncMock.mockReturnValue({ status: 0 }) existsSyncMock.mockReturnValue(true) // :99 socket + lock present readFileSyncMock.mockReturnValue('4321\n') // lock holds a PID const killSpy = vi.spyOn(process, 'kill').mockReturnValue(true as never) // PID alive @@ -154,14 +143,13 @@ describe('ensureVirtualDisplayForHeadlessServe', () => { it('treats a stale socket (dead server) as no display and starts a fresh Xvfb', async () => { setPlatform('linux') - spawnSyncMock.mockReturnValue({ status: 0 }) existsSyncMock.mockReturnValue(true) // orphan socket + lock present readFileSyncMock.mockReturnValue('9999\n') // PID is gone: process.kill throws ESRCH. const killSpy = vi.spyOn(process, 'kill').mockImplementation(() => { throw new Error('ESRCH') }) - spawnMock.mockReturnValue({ once: vi.fn(), kill: vi.fn(), killed: false }) + spawnMock.mockReturnValue({ pid: 1234, once: vi.fn(), kill: vi.fn(), killed: false }) const { ensureVirtualDisplayForHeadlessServe } = await import('./ensure-virtual-display') expect(ensureVirtualDisplayForHeadlessServe({ isServeMode: true })).toBe(true) diff --git a/src/main/startup/ensure-virtual-display.ts b/src/main/startup/ensure-virtual-display.ts index b04ec4e4228..1fe425a11e1 100644 --- a/src/main/startup/ensure-virtual-display.ts +++ b/src/main/startup/ensure-virtual-display.ts @@ -1,4 +1,4 @@ -import { spawn, spawnSync, type ChildProcess } from 'node:child_process' +import { spawn, type ChildProcess } from 'node:child_process' import { existsSync, readFileSync, rmSync, statSync } from 'node:fs' import { isAbsolute, join } from 'node:path' import { app } from 'electron' @@ -69,13 +69,6 @@ function removeStaleDisplayArtifacts(displayNumber: number): void { } } -function hasXvfbBinary(): boolean { - // Why: spawnSync `which` is cheap and avoids spawning Xvfb only to fail; a - // clear up-front warning beats a cryptic ENOENT mid-startup. - const result = spawnSync('which', ['Xvfb'], { stdio: 'ignore' }) - return result.status === 0 -} - function sleepSync(ms: number): void { // Why: this runs in the synchronous pre-whenReady startup path, so block // without spinning the CPU or spawning a process. @@ -188,14 +181,6 @@ export function ensureVirtualDisplayForHeadlessServe(options: { isServeMode: boo return false } - if (!hasXvfbBinary()) { - console.warn( - '[serve] Xvfb not found; browser panes are unavailable on this headless Linux host. ' + - `${XVFB_INSTALL_GUIDANCE} Set DISPLAY to enable them with an existing X server.` - ) - return false - } - // Why: reuse an existing display ONLY if a live X server actually backs it. // A crashed prior run can leave an orphan socket; trusting it by path alone // would advertise browser support that then fails at tab creation. @@ -222,6 +207,11 @@ export function ensureVirtualDisplayForHeadlessServe(options: { isServeMode: boo xvfbProcess.once('error', (error) => { console.warn('[serve] Xvfb failed to start:', error instanceof Error ? error.message : error) }) + // PATH lookup failures emit asynchronously, but a successful spawn has a PID immediately. + if (xvfbProcess.pid === undefined) { + xvfbProcess = null + return false + } } catch (error) { console.warn( '[serve] Could not start Xvfb:', diff --git a/src/main/startup/single-instance-lock-exit.electron.test.ts b/src/main/startup/single-instance-lock-exit.electron.test.ts index 7cf61a4af0f..027a5202479 100644 --- a/src/main/startup/single-instance-lock-exit.electron.test.ts +++ b/src/main/startup/single-instance-lock-exit.electron.test.ts @@ -6,11 +6,8 @@ import { join } from 'node:path' import { afterAll, describe, expect, it } from 'vitest' import { SINGLE_INSTANCE_ALREADY_RUNNING_EXIT_CODE } from './single-instance-lock' -// Why #11935: the lock-loss gate runs before Electron `ready`, where `app.quit()` is deferred, so a -// duplicate headless `orca serve` kept executing the rest of startup, reached Linux Ozone/X11 init -// with no display, died with SIGSEGV, and systemd restarted it until the leaked AppImage FUSE mounts -// hit the kernel's 1000-mount ceiling. This runs the gate's own termination statement, lifted out of -// `src/main/index.ts`, under the real Electron binary. +// Why: `app.quit()` is deferred before Electron `ready`, so fatal startup gates must use the +// synchronous `app.exit()`. Run their shipped termination statements under the real binary. // // Why not a live lock race: Chromium's Linux ProcessSingleton only answers a second process once the // browser IO thread is up, which needs `ready` and therefore a display. On a display-less CI runner @@ -19,10 +16,10 @@ import { SINGLE_INSTANCE_ALREADY_RUNNING_EXIT_CODE } from './single-instance-loc // only a real process can settle is what the loser does next, which is what this file pins. const electronBinary = createRequire(import.meta.url)('electron') as string -const LOCK_LOST = 'LOCK_LOST' +const GATE_ENTERED = 'GATE_ENTERED' const CONTINUED_INTO_STARTUP = 'CONTINUED_INTO_STARTUP' const REACHED_TAIL = 'REACHED_TAIL' -const MARKER_ENV = 'ORCA_LOCK_FIXTURE_MARKER' +const MARKER_ENV = 'ORCA_PRE_READY_EXIT_FIXTURE_MARKER' const fixtureRoots: string[] = [] @@ -32,10 +29,10 @@ afterAll(() => { } }) -/** The `app.*` call the shipped lock-loss gate executes, so a revert to `app.quit()` fails here. */ -function readLockLossTermination(): string { +/** Read the `app.*` termination statement from a pre-ready gate in the shipped entrypoint. */ +function readPreReadyTermination(gate: string): string { const source = readFileSync(join(process.cwd(), 'src/main/index.ts'), 'utf8') - const start = source.indexOf('if (!hasSingleInstanceLock) {') + const start = source.indexOf(gate) expect(start).toBeGreaterThanOrEqual(0) const end = source.indexOf('\n}', start) expect(end).toBeGreaterThan(start) @@ -56,7 +53,7 @@ function buildFixtureMain(termination: string): string { `const marker = process.env.${MARKER_ENV}`, `const mark = (name) => appendFileSync(marker, name + '\\n')`, `const SINGLE_INSTANCE_ALREADY_RUNNING_EXIT_CODE = ${SINGLE_INSTANCE_ALREADY_RUNNING_EXIT_CODE}`, - `mark('${LOCK_LOST}')`, + `mark('${GATE_ENTERED}')`, termination, `mark('${CONTINUED_INTO_STARTUP}')`, // Why: stand in for the rest of `src/main/index.ts`, which on the reported host was display init. @@ -67,15 +64,15 @@ function buildFixtureMain(termination: string): string { type FixtureRun = { status: number | null; markers: string[] } -function runLockLossGate(termination: string): FixtureRun { - const root = mkdtempSync(join(tmpdir(), 'orca-lock-loss-')) +function runPreReadyGate(termination: string): FixtureRun { + const root = mkdtempSync(join(tmpdir(), 'orca-pre-ready-exit-')) fixtureRoots.push(root) const dir = join(root, 'fixture') const marker = join(root, 'markers.log') mkdirSync(dir, { recursive: true }) writeFileSync( join(dir, 'package.json'), - '{ "name": "orca-lock-loss-fixture", "main": "main.js" }' + '{ "name": "orca-pre-ready-exit-fixture", "main": "main.js" }' ) writeFileSync(join(dir, 'main.js'), buildFixtureMain(termination)) writeFileSync(marker, '') @@ -93,23 +90,34 @@ function runLockLossGate(termination: string): FixtureRun { } } -describe('#11935 pre-ready lock-loss termination under real Electron', () => { +describe('pre-ready termination under real Electron', () => { it('stops the duplicate launch before any further startup runs, with the already-running code', () => { - const termination = readLockLossTermination() + const termination = readPreReadyTermination('if (!hasSingleInstanceLock) {') // Why: an empty slice would let the fixture fall through to its own exit and pass vacuously. expect(termination).not.toBe('') - const run = runLockLossGate(termination) + const run = runPreReadyGate(termination) - expect(run.markers).toEqual([LOCK_LOST]) + expect(run.markers).toEqual([GATE_ENTERED]) expect(run.status).toBe(SINGLE_INSTANCE_ALREADY_RUNNING_EXIT_CODE) }, 90_000) + it('#17615 stops serve when display setup fails instead of entering Chromium startup', () => { + const termination = readPreReadyTermination( + 'if (isServeMode && !headlessBrowserDisplayAvailable) {' + ) + + const run = runPreReadyGate(termination) + + expect(run.markers).toEqual([GATE_ENTERED]) + expect(run.status).toBe(1) + }, 90_000) + it('reproduces the deferred graceful quit that let the doomed launch keep booting', () => { - const run = runLockLossGate('app.quit()') + const run = runPreReadyGate('app.quit()') // Why: pins the Electron semantic the fix rests on — pre-`ready` `quit()` schedules, it does not stop. - expect(run.markers).toEqual([LOCK_LOST, CONTINUED_INTO_STARTUP, REACHED_TAIL]) + expect(run.markers).toEqual([GATE_ENTERED, CONTINUED_INTO_STARTUP, REACHED_TAIL]) expect(run.status).not.toBe(SINGLE_INSTANCE_ALREADY_RUNNING_EXIT_CODE) }, 90_000) })