diff --git a/src/main/startup/ensure-virtual-display.test.ts b/src/main/startup/ensure-virtual-display.test.ts index 17bcaf0bad2..478173cc807 100644 --- a/src/main/startup/ensure-virtual-display.test.ts +++ b/src/main/startup/ensure-virtual-display.test.ts @@ -23,15 +23,24 @@ vi.mock('fs', () => ({ })) vi.mock('electron', () => ({ app: appMock })) +const OWN_UID = 1000 +const OTHER_UID = 1001 const ORIGINAL_PLATFORM = process.platform +const ORIGINAL_GETUID = Object.getOwnPropertyDescriptor(process, 'getuid') const ORIGINAL_DISPLAY = process.env.DISPLAY function setPlatform(platform: NodeJS.Platform): void { Object.defineProperty(process, 'platform', { value: platform, configurable: true }) + // Why: Windows has no getuid, and a simulated Linux run still needs this account's uid. + Object.defineProperty(process, 'getuid', { + value: () => OWN_UID, + configurable: true, + writable: true + }) } function mockLiveXDisplay(pid = 4321): void { - statSyncMock.mockReturnValue({ isSocket: () => true }) + statSyncMock.mockReturnValue({ isSocket: () => true, uid: OWN_UID }) existsSyncMock.mockReturnValue(true) readFileSyncMock.mockReturnValue(`${pid}\n`) vi.spyOn(process, 'kill').mockImplementation(() => true) @@ -39,7 +48,7 @@ function mockLiveXDisplay(pid = 4321): void { function mockXvfbTakesDisplay(pid = 1234): void { let bound = false - statSyncMock.mockImplementation(() => ({ isSocket: () => true })) + statSyncMock.mockImplementation(() => ({ isSocket: () => true, uid: OWN_UID })) readFileSyncMock.mockImplementation(() => { if (!bound) { throw Object.assign(new Error('ENOENT'), { code: 'ENOENT' }) @@ -73,6 +82,11 @@ describe('ensureVirtualDisplayForHeadlessServe', () => { stopVirtualDisplay() vi.restoreAllMocks() setPlatform(ORIGINAL_PLATFORM) + if (ORIGINAL_GETUID) { + Object.defineProperty(process, 'getuid', ORIGINAL_GETUID) + } else { + Reflect.deleteProperty(process, 'getuid') + } if (ORIGINAL_DISPLAY === undefined) { delete process.env.DISPLAY } else { @@ -114,6 +128,9 @@ describe('ensureVirtualDisplayForHeadlessServe', () => { it('reports unsupported when Xvfb cannot be launched', async () => { setPlatform('linux') + statSyncMock.mockImplementation(() => { + throw Object.assign(new Error('ENOENT'), { code: 'ENOENT' }) + }) spawnMock.mockReturnValue({ pid: undefined, once: vi.fn(), kill: vi.fn(), killed: false }) const { ensureVirtualDisplayForHeadlessServe, @@ -141,7 +158,7 @@ describe('ensureVirtualDisplayForHeadlessServe', () => { it('leaves an externally configured stale display untouched', async () => { setPlatform('linux') process.env.DISPLAY = ':77' - statSyncMock.mockReturnValue({ isSocket: () => true }) + statSyncMock.mockReturnValue({ isSocket: () => true, uid: OWN_UID }) existsSyncMock.mockReturnValue(true) readFileSyncMock.mockImplementation(() => { throw new Error('display lock is outside this namespace') @@ -159,7 +176,7 @@ describe('ensureVirtualDisplayForHeadlessServe', () => { it('serves on an externally configured display that has no lock file', async () => { setPlatform('linux') process.env.DISPLAY = ':0' - statSyncMock.mockReturnValue({ isSocket: () => true }) + statSyncMock.mockReturnValue({ isSocket: () => true, uid: OWN_UID }) readFileSyncMock.mockImplementation(() => { throw Object.assign(new Error('ENOENT'), { code: 'ENOENT' }) }) @@ -191,7 +208,7 @@ describe('ensureVirtualDisplayForHeadlessServe', () => { it('reuses an existing virtual display only when its X server is alive', async () => { setPlatform('linux') - statSyncMock.mockReturnValue({ isSocket: () => true }) // :99 socket present + statSyncMock.mockReturnValue({ isSocket: () => true, uid: OWN_UID }) // :99 socket present existsSyncMock.mockReturnValue(true) readFileSyncMock.mockReturnValue('4321\n') // lock holds a PID const killSpy = vi.spyOn(process, 'kill').mockReturnValue(true as never) // PID alive @@ -209,7 +226,7 @@ describe('ensureVirtualDisplayForHeadlessServe', () => { setPlatform('linux') existsSyncMock.mockReturnValue(true) // lock present let bound = false - statSyncMock.mockImplementation(() => ({ isSocket: () => true })) + statSyncMock.mockImplementation(() => ({ isSocket: () => true, uid: OWN_UID })) readFileSyncMock.mockImplementation(() => (bound ? '1234\n' : '9999\n')) // The orphan lock names a dead PID; the freshly spawned Xvfb is alive. const killSpy = vi.spyOn(process, 'kill').mockImplementation((pid) => { @@ -236,12 +253,11 @@ describe('ensureVirtualDisplayForHeadlessServe', () => { killSpy.mockRestore() }) - // A root-owned stale :99 socket (crashed system Xvfb, serve running as User=orca) cannot be - // unlinked, so our Xvfb refuses to bind and exits. Trusting the surviving socket set DISPLAY to a - // dead server and Chromium died in Ozone init with SIGSEGV. + // A stale :99 socket this account cannot unlink makes our Xvfb refuse to bind and exit. Trusting + // the surviving socket set DISPLAY to a dead server and Chromium died in Ozone init with SIGSEGV. it('reports failure when a stale socket blocks the Xvfb rebind', async () => { setPlatform('linux') - statSyncMock.mockReturnValue({ isSocket: () => true }) + statSyncMock.mockReturnValue({ isSocket: () => true, uid: OWN_UID }) // Removal fails (foreign owner) and no lock ever appears, because Xvfb never took the display. rmSyncMock.mockImplementation(() => { throw Object.assign(new Error('permission denied'), { code: 'EACCES' }) @@ -271,7 +287,7 @@ describe('ensureVirtualDisplayForHeadlessServe', () => { it('accepts the display once the spawned Xvfb owns its lock', async () => { setPlatform('linux') let lockWritten = false - statSyncMock.mockImplementation(() => ({ isSocket: () => lockWritten })) + statSyncMock.mockImplementation(() => ({ isSocket: () => lockWritten, uid: OWN_UID })) rmSyncMock.mockImplementation(() => {}) readFileSyncMock.mockImplementation(() => { if (!lockWritten) { @@ -290,6 +306,150 @@ describe('ensureVirtualDisplayForHeadlessServe', () => { expect(process.env.DISPLAY).toBe(':99') }) + describe('with several accounts on one machine (#26602)', () => { + type FakeDisplay = { uid: number; lockPid: number | null; socket: boolean } + + // X artifacts per display number; each spawned Xvfb takes its display under this account. + function mockDisplays(displays: Record, alivePids: number[]): void { + const displayOf = (path: string): FakeDisplay | undefined => { + const match = /(?:\.X(\d+)-lock|X11-unix\/X(\d+))$/.exec(path) + return match ? displays[Number(match[1] ?? match[2])] : undefined + } + const enoent = () => Object.assign(new Error('ENOENT'), { code: 'ENOENT' }) + statSyncMock.mockImplementation((path: string) => { + const display = displayOf(path) + const isLock = path.endsWith('-lock') + if (!display || (isLock ? display.lockPid === null : !display.socket)) { + throw enoent() + } + return { uid: display.uid, isSocket: () => !isLock } + }) + readFileSyncMock.mockImplementation((path: string) => { + if (path === '/proc/net/unix') { + return '' + } + const display = displayOf(path) + if (!display || display.lockPid === null) { + throw enoent() + } + return `${display.lockPid}\n` + }) + vi.spyOn(process, 'kill').mockImplementation((pid) => { + if (!alivePids.includes(Number(pid))) { + throw Object.assign(new Error('ESRCH'), { code: 'ESRCH' }) + } + return true + }) + let nextPid = 5000 + spawnMock.mockImplementation((_command: string, args: string[]) => { + const pid = nextPid++ + alivePids.push(pid) + const displayNumber = Number(args[0].slice(1)) + if (!displays[displayNumber]) { + displays[displayNumber] = { uid: OWN_UID, lockPid: pid, socket: true } + } + return { pid, once: vi.fn(), kill: vi.fn(), killed: false } + }) + } + + it("starts its own display instead of attaching to another account's live :99", async () => { + setPlatform('linux') + mockDisplays({ 99: { uid: OTHER_UID, lockPid: 777, socket: true } }, [777]) + const { ensureVirtualDisplayForHeadlessServe } = await import('./ensure-virtual-display') + + expect(ensureVirtualDisplayForHeadlessServe({ isServeMode: true })).toBe(true) + expect(process.env.DISPLAY).toBe(':100') + expect(spawnMock).toHaveBeenCalledWith( + 'Xvfb', + expect.arrayContaining([':100']), + expect.any(Object) + ) + expect(rmSyncMock).not.toHaveBeenCalled() + }) + + it("skips another account's stale :99 that sticky /tmp keeps it from removing", async () => { + setPlatform('linux') + mockDisplays({ 99: { uid: OTHER_UID, lockPid: 777, socket: true } }, []) + const { ensureVirtualDisplayForHeadlessServe } = await import('./ensure-virtual-display') + + expect(ensureVirtualDisplayForHeadlessServe({ isServeMode: true })).toBe(true) + expect(process.env.DISPLAY).toBe(':100') + expect(rmSyncMock).not.toHaveBeenCalled() + }) + + it('reuses its own live display past the ones other accounts hold', async () => { + setPlatform('linux') + mockDisplays( + { + 99: { uid: OTHER_UID, lockPid: 777, socket: true }, + 100: { uid: OWN_UID, lockPid: 888, socket: true } + }, + [777, 888] + ) + const { ensureVirtualDisplayForHeadlessServe } = await import('./ensure-virtual-display') + + expect(ensureVirtualDisplayForHeadlessServe({ isServeMode: true })).toBe(true) + expect(process.env.DISPLAY).toBe(':100') + expect(spawnMock).not.toHaveBeenCalled() + }) + + it("leaves its own account's display alone while that server is still starting", async () => { + setPlatform('linux') + // :99 has its live lock but no socket yet. + mockDisplays({ 99: { uid: OWN_UID, lockPid: 888, socket: false } }, [888]) + const { ensureVirtualDisplayForHeadlessServe } = await import('./ensure-virtual-display') + + expect(ensureVirtualDisplayForHeadlessServe({ isServeMode: true })).toBe(true) + expect(process.env.DISPLAY).toBe(':100') + expect(rmSyncMock).not.toHaveBeenCalled() + }) + + it('does not accept a spawned Xvfb that died after writing its lock and socket', async () => { + setPlatform('linux') + const alive: number[] = [] + mockDisplays({}, alive) + const startXvfb = spawnMock.getMockImplementation()! + spawnMock.mockImplementation((command: string, args: string[]) => { + const child = startXvfb(command, args) + alive.splice(alive.indexOf(child.pid), 1) + return child + }) + const { ensureVirtualDisplayForHeadlessServe } = await import('./ensure-virtual-display') + + vi.useFakeTimers({ toFake: ['Date'] }) + const sleep = vi + .spyOn(Atomics, 'wait') + .mockImplementation((_array, _index, _value, timeout) => { + vi.setSystemTime(Date.now() + (timeout ?? 0)) + return 'timed-out' + }) + try { + expect(ensureVirtualDisplayForHeadlessServe({ isServeMode: true })).toBe(false) + expect(process.env.DISPLAY).toBeUndefined() + } finally { + sleep.mockRestore() + vi.useRealTimers() + } + }) + + it('moves on when another server wins the free display first', async () => { + setPlatform('linux') + const displays: Record = {} + mockDisplays(displays, [777]) + const startXvfb = spawnMock.getMockImplementation()! + spawnMock.mockImplementationOnce((command: string, args: string[]) => { + // Another account's Xvfb binds :99 between our check and our own bind. + displays[99] = { uid: OTHER_UID, lockPid: 777, socket: true } + return startXvfb(command, args) + }) + const { ensureVirtualDisplayForHeadlessServe } = await import('./ensure-virtual-display') + + expect(ensureVirtualDisplayForHeadlessServe({ isServeMode: true })).toBe(true) + expect(process.env.DISPLAY).toBe(':100') + expect(spawnMock).toHaveBeenCalledTimes(2) + }) + }) + describe('hasUsableLinuxDisplay', () => { it('accepts live local X11 and Wayland sockets', async () => { setPlatform('linux') @@ -363,7 +523,7 @@ describe('ensureVirtualDisplayForHeadlessServe', () => { // half-finished cleanup — trusting it because DISPLAY names it would accept a dead display. it('does not trust a lockless socket on its own managed display number', async () => { setPlatform('linux') - statSyncMock.mockReturnValue({ isSocket: () => true }) + statSyncMock.mockReturnValue({ isSocket: () => true, uid: OWN_UID }) readFileSyncMock.mockImplementation(() => { throw Object.assign(new Error('ENOENT'), { code: 'ENOENT' }) }) @@ -376,7 +536,7 @@ describe('ensureVirtualDisplayForHeadlessServe', () => { it('rejects an orphaned local X11 socket whose server PID is gone', async () => { setPlatform('linux') - statSyncMock.mockReturnValue({ isSocket: () => true }) + statSyncMock.mockReturnValue({ isSocket: () => true, uid: OWN_UID }) existsSyncMock.mockReturnValue(true) readFileSyncMock.mockReturnValue('9999\n') vi.spyOn(process, 'kill').mockImplementation(() => { @@ -392,7 +552,7 @@ describe('ensureVirtualDisplayForHeadlessServe', () => { // socket is an endpoint published from elsewhere (container bind mount, WSLg) — not an orphan. it('accepts a local X11 socket published without a lock file', async () => { setPlatform('linux') - statSyncMock.mockReturnValue({ isSocket: () => true }) + statSyncMock.mockReturnValue({ isSocket: () => true, uid: OWN_UID }) readFileSyncMock.mockImplementation(() => { throw Object.assign(new Error('ENOENT'), { code: 'ENOENT' }) }) @@ -400,6 +560,8 @@ describe('ensureVirtualDisplayForHeadlessServe', () => { const { hasUsableLinuxDisplay } = await import('./ensure-virtual-display') expect(hasUsableLinuxDisplay({ DISPLAY: ':0' })).toBe(true) + // Numbers Orca may pick for itself are still someone else's when DISPLAY names them. + expect(hasUsableLinuxDisplay({ DISPLAY: ':100' })).toBe(true) expect(killSpy).not.toHaveBeenCalled() }) @@ -432,7 +594,7 @@ describe('ensureVirtualDisplayForHeadlessServe', () => { it('rejects a lock that exists but cannot be read', async () => { setPlatform('linux') - statSyncMock.mockReturnValue({ isSocket: () => true }) + statSyncMock.mockReturnValue({ isSocket: () => true, uid: OWN_UID }) readFileSyncMock.mockImplementation(() => { throw Object.assign(new Error('permission denied'), { code: 'EACCES' }) }) @@ -443,7 +605,7 @@ describe('ensureVirtualDisplayForHeadlessServe', () => { it('accepts a live local X11 server owned by another user', async () => { setPlatform('linux') - statSyncMock.mockReturnValue({ isSocket: () => true }) + statSyncMock.mockReturnValue({ isSocket: () => true, uid: OWN_UID }) existsSyncMock.mockReturnValue(true) readFileSyncMock.mockReturnValue('4321\n') vi.spyOn(process, 'kill').mockImplementation(() => { diff --git a/src/main/startup/ensure-virtual-display.ts b/src/main/startup/ensure-virtual-display.ts index 7655990b90c..ba4db6db658 100644 --- a/src/main/startup/ensure-virtual-display.ts +++ b/src/main/startup/ensure-virtual-display.ts @@ -1,7 +1,16 @@ import { spawn, type ChildProcess } from 'node:child_process' -import { readFileSync, rmSync, statSync } from 'node:fs' import { isAbsolute, join } from 'node:path' import { app } from 'electron' +import { + displayOwnership, + hasAbstractXSocket, + isForeignDisplayServerAlive, + isManagedDisplayServerAlive, + isUnixSocket, + readDisplayLockPid, + removeStaleDisplayArtifacts, + xvfbSocketPath +} from './x-display-artifacts' // Why: headless `orca serve` backs browser panes with offscreen BrowserWindows. // On Linux, Electron has no display platform without an X server and segfaults @@ -11,8 +20,9 @@ import { app } from 'electron' const XVFB_STARTUP_TIMEOUT_MS = 5_000 const XVFB_POLL_INTERVAL_MS = 50 -const VIRTUAL_DISPLAY_NUMBER = 99 -const VIRTUAL_DISPLAY = `:${VIRTUAL_DISPLAY_NUMBER}` +// Why a range: accounts on one machine each need their own display (#26602); :99 stays the first choice. +const FIRST_VIRTUAL_DISPLAY_NUMBER = 99 +const VIRTUAL_DISPLAY_NUMBER_COUNT = 32 const XVFB_INSTALL_GUIDANCE = 'install an X virtual framebuffer: `xvfb` on Debian/Ubuntu, `xorg-x11-server-Xvfb` on RPM-based systems, or `xorg-server-xvfb` on Arch.' @@ -28,71 +38,11 @@ function configureHeadlessServeChromiumFlags(): void { app.commandLine.appendSwitch('disable-gpu') } -function xvfbSocketPath(displayNumber: number): string { - return `/tmp/.X11-unix/X${displayNumber}` -} - -function xDisplayLockPath(displayNumber: number): string { - return `/tmp/.X${displayNumber}-lock` -} - -// Why: a socket file can outlive the X server that made it. The X lock file holds the server PID; -// if that process is gone, the display is dead despite the socket. `missing` is a third outcome the -// two callers must treat differently — see each call site. -type DisplayLockProbe = 'alive' | 'dead' | 'missing' - -function probeDisplayLock(displayNumber: number): DisplayLockProbe { - let pid: number - try { - pid = Number.parseInt(readFileSync(xDisplayLockPath(displayNumber), 'utf8').trim(), 10) - } catch (error) { - // An unreadable lock is a lock we cannot clear: treat it as dead, not absent. - return (error as NodeJS.ErrnoException)?.code === 'ENOENT' ? 'missing' : 'dead' - } - if (!Number.isInteger(pid) || pid <= 0) { - return 'dead' - } - try { - // signal 0 probes existence without affecting the process. - process.kill(pid, 0) - return 'alive' - } catch (error) { - // EPERM means the PID exists under another uid — a root-owned X server is still live. - return typeof error === 'object' && error !== null && 'code' in error && error.code === 'EPERM' - ? 'alive' - : 'dead' - } -} - -/** - * Liveness for a display Orca did not create. An X server writes its lock beside the socket and - * both survive a crash (verified against Xvfb under SIGKILL), so a socket with no lock was never - * left by a crashed server — it is an endpoint published from elsewhere: a container bind-mounting - * only /tmp/.X11-unix, WSLg, or a foreign PID namespace. We cannot judge those, and refusing them - * blocks startup on displays that work. - */ -function isForeignDisplayServerAlive(displayNumber: number): boolean { - return probeDisplayLock(displayNumber) !== 'dead' -} - -/** - * Liveness for Orca's own VIRTUAL_DISPLAY_NUMBER. Stricter on purpose: `removeStaleDisplayArtifacts` - * unlinks the lock before the socket, so a lockless socket here is Orca's own half-finished - * teardown, not a foreign endpoint. Adopting it would resurrect the orphan-socket bug and stop the - * cleanup below from self-healing. - */ -function isManagedDisplayServerAlive(displayNumber: number): boolean { - return probeDisplayLock(displayNumber) === 'alive' -} - -function removeStaleDisplayArtifacts(displayNumber: number): void { - for (const path of [xDisplayLockPath(displayNumber), xvfbSocketPath(displayNumber)]) { - try { - rmSync(path, { force: true }) - } catch { - // Best effort; if removal fails, Xvfb startup below will surface the error. - } - } +function isManagedDisplayNumber(displayNumber: number): boolean { + return ( + displayNumber >= FIRST_VIRTUAL_DISPLAY_NUMBER && + displayNumber < FIRST_VIRTUAL_DISPLAY_NUMBER + VIRTUAL_DISPLAY_NUMBER_COUNT + ) } function sleepSync(ms: number): void { @@ -105,16 +55,36 @@ function sleepSync(ms: number): void { // crashed system Xvfb, which `User=orca` serve cannot remove) still exists after our own Xvfb // refused to bind the display. Treating that as ready sets DISPLAY to a dead server and Chromium // dies in Ozone init with SIGSEGV instead of reporting an unusable display. -function waitForDisplayReady(displayNumber: number, deadline: number): boolean { - const isReady = (): boolean => - isUnixSocket(xvfbSocketPath(displayNumber)) && isManagedDisplayServerAlive(displayNumber) +// Why our PID in the lock: another account's server may win the same free number between our +// check and our Xvfb's bind, and its live lock must not read as our display being ready. +type DisplayStart = 'ready' | 'taken' | 'timeout' + +function waitForDisplayReady( + displayNumber: number, + xvfbPid: number, + deadline: number +): DisplayStart { + const probe = (): DisplayStart | null => { + const lockPid = readDisplayLockPid(displayNumber) + if (lockPid === null) { + return null + } + if (lockPid !== xvfbPid) { + return isManagedDisplayServerAlive(displayNumber) ? 'taken' : null + } + // Why the liveness probe too: lock and socket both survive a SIGKILLed Xvfb. + return isUnixSocket(xvfbSocketPath(displayNumber)) && isManagedDisplayServerAlive(displayNumber) + ? 'ready' + : null + } while (Date.now() < deadline) { - if (isReady()) { - return true + const state = probe() + if (state) { + return state } sleepSync(XVFB_POLL_INTERVAL_MS) } - return isReady() + return probe() ?? 'timeout' } // Validate display syntax and local sockets before Chromium reaches Ozone initialization. @@ -157,34 +127,6 @@ export function missingLinuxDisplayMessage(isServeMode: boolean): string { return isServeMode ? MISSING_SERVE_DISPLAY_MESSAGE : MISSING_LINUX_DISPLAY_MESSAGE } -// Why: an X server may bind only the abstract namespace (`@/tmp/.X11-unix/X0`), which leaves no -// filesystem socket to stat. Abstract addresses are kernel-owned and vanish the moment the owner -// exits, so an entry here is proof of a live server — no lock file needed, and no stale entry is -// possible. Refusing these was a hard startup failure with no workaround. -function hasAbstractXSocket(displayNumber: number): boolean { - let table: unknown - try { - table = readFileSync('/proc/net/unix', 'utf8') - } catch { - return false - } - if (typeof table !== 'string') { - return false - } - const address = `@${xvfbSocketPath(displayNumber)}` - return table - .split('\n') - .some((line) => line.slice(line.lastIndexOf(' ') + 1).trimEnd() === address) -} - -function isUnixSocket(path: string): boolean { - try { - return statSync(path).isSocket() - } catch { - return false - } -} - function hasUsableXDisplay(value: string | undefined): boolean { const display = value?.trim() if (!display) { @@ -198,10 +140,10 @@ function hasUsableXDisplay(value: string | undefined): boolean { } const displayNumber = Number(localDisplay[1]) if (isUnixSocket(xvfbSocketPath(displayNumber))) { - // Why the managed number is never treated as foreign: Orca's own teardown unlinks the lock - // before the socket, so a lockless socket on VIRTUAL_DISPLAY_NUMBER is our own half-finished - // cleanup even when DISPLAY names it explicitly. Trusting it there would accept a dead display. - return displayNumber === VIRTUAL_DISPLAY_NUMBER + // Why :99 is never treated as foreign: Orca's own teardown unlinks the lock before the socket, + // so a lockless :99 socket is our own half-finished cleanup even when DISPLAY names it. Higher + // numbers stay foreign-checked: a container may publish them without a lock. + return displayNumber === FIRST_VIRTUAL_DISPLAY_NUMBER ? isManagedDisplayServerAlive(displayNumber) : isForeignDisplayServerAlive(displayNumber) } @@ -254,23 +196,50 @@ export function ensureVirtualDisplayForHeadlessServe(options: { isServeMode: boo 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. - if (isUnixSocket(xvfbSocketPath(VIRTUAL_DISPLAY_NUMBER))) { - if (isManagedDisplayServerAlive(VIRTUAL_DISPLAY_NUMBER)) { - process.env.DISPLAY = VIRTUAL_DISPLAY + for ( + let displayNumber = FIRST_VIRTUAL_DISPLAY_NUMBER; + isManagedDisplayNumber(displayNumber); + displayNumber += 1 + ) { + const ownership = displayOwnership(displayNumber) + if (ownership === 'foreign') { + continue + } + if (ownership === 'mine') { + // Why: reuse this account's 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. + if (isManagedDisplayServerAlive(displayNumber)) { + if (isUnixSocket(xvfbSocketPath(displayNumber))) { + process.env.DISPLAY = `:${displayNumber}` + return true + } + // Why: a live lock without its socket is a server still starting; its lock is not ours to remove. + continue + } + // Why: stale socket/lock — clean them up so Xvfb can rebind the display. + removeStaleDisplayArtifacts(displayNumber) + } + const started = startVirtualDisplay(displayNumber) + if (started === 'ready') { return true } - // Why: stale socket/lock — clean them up so Xvfb can rebind the display - // below instead of refusing to start on an "in use" number. - removeStaleDisplayArtifacts(VIRTUAL_DISPLAY_NUMBER) + if (started !== 'taken') { + return false + } } + console.warn( + `[serve] Every display from :${FIRST_VIRTUAL_DISPLAY_NUMBER} to :${FIRST_VIRTUAL_DISPLAY_NUMBER + VIRTUAL_DISPLAY_NUMBER_COUNT - 1} belongs to another X server; browser panes are unavailable. Set DISPLAY to a running X server.` + ) + return false +} +function startVirtualDisplay(displayNumber: number): DisplayStart | 'unavailable' { + const display = `:${displayNumber}` try { xvfbProcess = spawn( 'Xvfb', - [VIRTUAL_DISPLAY, '-screen', '0', '1280x1024x24', '-nolisten', 'tcp', '-terminate'], + [display, '-screen', '0', '1280x1024x24', '-nolisten', 'tcp', '-terminate'], { stdio: 'ignore', // Why: foreground Ctrl-C must not kill Xvfb before Electron disconnects. @@ -283,33 +252,39 @@ export function ensureVirtualDisplayForHeadlessServe(options: { isServeMode: boo // PATH lookup failures emit asynchronously, but a successful spawn has a PID immediately. if (xvfbProcess.pid === undefined) { xvfbProcess = null - return false + return 'unavailable' } } catch (error) { console.warn( '[serve] Could not start Xvfb:', error instanceof Error ? error.message : String(error) ) - return false + return 'unavailable' } - const ready = waitForDisplayReady(VIRTUAL_DISPLAY_NUMBER, Date.now() + XVFB_STARTUP_TIMEOUT_MS) - if (!ready) { - console.warn( - `[serve] Xvfb did not take ownership of ${VIRTUAL_DISPLAY}; browser panes are unavailable. ` + - 'A stale socket from another user can block the rebind.' - ) + const started = waitForDisplayReady( + displayNumber, + xvfbProcess.pid, + Date.now() + XVFB_STARTUP_TIMEOUT_MS + ) + if (started !== 'ready') { + if (started === 'timeout') { + console.warn( + `[serve] Xvfb did not take ownership of ${display}; browser panes are unavailable. ` + + 'A stale socket from another user can block the rebind.' + ) + } stopVirtualDisplay() - return false + return started } - process.env.DISPLAY = VIRTUAL_DISPLAY + process.env.DISPLAY = display // Why: -terminate only takes effect after Xvfb accepts its first client. process.once('exit', stopVirtualDisplay) app.once('ready', () => process.removeListener('exit', stopVirtualDisplay)) - return true + return 'ready' } export function stopVirtualDisplay(): void { diff --git a/src/main/startup/x-display-artifacts.ts b/src/main/startup/x-display-artifacts.ts new file mode 100644 index 00000000000..fd7115db5f0 --- /dev/null +++ b/src/main/startup/x-display-artifacts.ts @@ -0,0 +1,139 @@ +import { readFileSync, rmSync, statSync } from 'node:fs' + +// X server lock and socket files under /tmp, and what they say about who owns a display. + +export function xvfbSocketPath(displayNumber: number): string { + return `/tmp/.X11-unix/X${displayNumber}` +} + +export function xDisplayLockPath(displayNumber: number): string { + return `/tmp/.X${displayNumber}-lock` +} + +// Why: a socket file can outlive the X server that made it. The X lock file holds the server PID; +// if that process is gone, the display is dead despite the socket. `missing` is a third outcome the +// two callers must treat differently — see each call site. +export type DisplayLockProbe = 'alive' | 'dead' | 'missing' + +export function probeDisplayLock(displayNumber: number): DisplayLockProbe { + let pid: number + try { + pid = Number.parseInt(readFileSync(xDisplayLockPath(displayNumber), 'utf8').trim(), 10) + } catch (error) { + // An unreadable lock is a lock we cannot clear: treat it as dead, not absent. + return (error as NodeJS.ErrnoException)?.code === 'ENOENT' ? 'missing' : 'dead' + } + if (!Number.isInteger(pid) || pid <= 0) { + return 'dead' + } + try { + // signal 0 probes existence without affecting the process. + process.kill(pid, 0) + return 'alive' + } catch (error) { + // EPERM means the PID exists under another uid — a root-owned X server is still live. + return typeof error === 'object' && error !== null && 'code' in error && error.code === 'EPERM' + ? 'alive' + : 'dead' + } +} + +/** + * Liveness for a display Orca did not create. An X server writes its lock beside the socket and + * both survive a crash (verified against Xvfb under SIGKILL), so a socket with no lock was never + * left by a crashed server — it is an endpoint published from elsewhere: a container bind-mounting + * only /tmp/.X11-unix, WSLg, or a foreign PID namespace. We cannot judge those, and refusing them + * blocks startup on displays that work. + */ +export function isForeignDisplayServerAlive(displayNumber: number): boolean { + return probeDisplayLock(displayNumber) !== 'dead' +} + +/** + * Liveness for Orca's own managed display numbers. Stricter on purpose: `removeStaleDisplayArtifacts` + * unlinks the lock before the socket, so a lockless socket here is Orca's own half-finished + * teardown, not a foreign endpoint. Adopting it would resurrect the orphan-socket bug and stop the + * cleanup below from self-healing. + */ +export function isManagedDisplayServerAlive(displayNumber: number): boolean { + return probeDisplayLock(displayNumber) === 'alive' +} + +export type DisplayOwnership = 'free' | 'mine' | 'foreign' + +/** + * Whose display a number is. Another account's X server, or its leftovers that this account + * cannot unlink from sticky /tmp, must never be adopted or cleaned up: that account can stop it + * at any moment, and its display is not ours to share. + */ +export function displayOwnership(displayNumber: number): DisplayOwnership { + const uid = process.getuid?.() + let present = false + for (const path of [xDisplayLockPath(displayNumber), xvfbSocketPath(displayNumber)]) { + let owner: number + try { + owner = statSync(path).uid + } catch (error) { + if (error instanceof Error && 'code' in error && error.code === 'ENOENT') { + continue + } + return 'foreign' + } + present = true + if (uid === undefined || owner !== uid) { + return 'foreign' + } + } + if (present) { + return 'mine' + } + // A server in another mount namespace may hold only the abstract socket. + return hasAbstractXSocket(displayNumber) ? 'foreign' : 'free' +} + +export function removeStaleDisplayArtifacts(displayNumber: number): void { + for (const path of [xDisplayLockPath(displayNumber), xvfbSocketPath(displayNumber)]) { + try { + rmSync(path, { force: true }) + } catch { + // Best effort; if removal fails, Xvfb startup below will surface the error. + } + } +} + +export function readDisplayLockPid(displayNumber: number): number | null { + try { + const pid = Number.parseInt(readFileSync(xDisplayLockPath(displayNumber), 'utf8').trim(), 10) + return Number.isInteger(pid) && pid > 0 ? pid : null + } catch { + return null + } +} + +// Why: an X server may bind only the abstract namespace (`@/tmp/.X11-unix/X0`), which leaves no +// filesystem socket to stat. Abstract addresses are kernel-owned and vanish the moment the owner +// exits, so an entry here is proof of a live server — no lock file needed, and no stale entry is +// possible. Refusing these was a hard startup failure with no workaround. +export function hasAbstractXSocket(displayNumber: number): boolean { + let table: unknown + try { + table = readFileSync('/proc/net/unix', 'utf8') + } catch { + return false + } + if (typeof table !== 'string') { + return false + } + const address = `@${xvfbSocketPath(displayNumber)}` + return table + .split('\n') + .some((line) => line.slice(line.lastIndexOf(' ') + 1).trimEnd() === address) +} + +export function isUnixSocket(path: string): boolean { + try { + return statSync(path).isSocket() + } catch { + return false + } +}