diff --git a/src/hopper/BridgeLauncher.ts b/src/hopper/BridgeLauncher.ts index 76c4776770..bbcac1bdba 100644 --- a/src/hopper/BridgeLauncher.ts +++ b/src/hopper/BridgeLauncher.ts @@ -19,8 +19,13 @@ import { ArtifactResourceScope } from "../artifacts/ArtifactResourceScope.js"; import { AnalysisCleanupLeaseGroup } from "../artifacts/AnalysisCleanupLease.js"; import { AnalysisError } from "../domain/analysisErrorBase.js"; import { cleanupOwnedProcessGroup } from "../process/ProcessOwnership.js"; +import type { + OwnedProcessGroup, + ProcessCleanupResult, +} from "../process/ProcessOwnership.js"; import { execFileOutput } from "../process/ExecFileOutput.js"; import { + type ProviderProcessHandle, type ProviderProcessLaunch, spawnOwnedProviderProcess, } from "../process/ProviderProcess.js"; @@ -49,6 +54,12 @@ import { retryLinuxHopperPublisherCleanup } from "../application/LinuxHopperPubl const execFileAsync = promisify(execFile); const HOPPER_BACKGROUND_STARTUP_MS = 5_000; +/** + * Bound for the Linux demo helper's orderly shutdown during teardown. + * The helper stops Hopper and Xvfb with SIGTERM (up to 2s each in + * stop_process); Xvfb removes its /tmp/.X-lock on that clean exit. + */ +const LINUX_DEMO_ORDERLY_SHUTDOWN_GRACE_MS = 10_000; const bindRetainedArtifactCleanup = ( error: AnalysisError, @@ -148,6 +159,11 @@ export interface HopperApplicationLauncherDependencies { readonly helperPath: string; readonly signal?: AbortSignal; }) => Promise; + /** + * How long teardown waits for the Linux demo helper's orderly shutdown + * (stdin EOF → SIGTERM to Hopper and Xvfb) before the process-group kill. + */ + readonly demoOrderlyShutdownGraceMs?: number; } /** @@ -386,7 +402,14 @@ export class HopperApplicationLauncher implements BridgeLauncher { }); if (started === null) return err(hopperCancelledBySignal(signal)); launched = true; - const cleanup = () => cleanupOwnedProcessGroup(started.ownership); + const cleanup = () => + cleanupHopperLauncherSession({ + helper: linuxDemo === undefined ? undefined : started.process, + ownership: started.ownership, + orderlyShutdownGraceMs: + this.dependencies.demoOrderlyShutdownGraceMs ?? + LINUX_DEMO_ORDERLY_SHUTDOWN_GRACE_MS, + }); const ownedLauncher = { ...(this.options.preparedImage === undefined ? {} @@ -656,6 +679,60 @@ const hopperLaunchFailure = ( ? new HopperCancelledError(signal.reason) : new HopperStartError({ cause }); +/** + * Shut down a launched Hopper session, asking the Linux demo helper to stop + * orderly before the process-group kill. + * + * The helper treats stdin EOF as its shutdown signal (OwnerLifetime): its + * run_launch finally block then stops Hopper and Xvfb with SIGTERM, and Xvfb + * removes its /tmp/.X-lock on that clean exit. SIGKILLing the group first + * orphans the lock, and later startups count each orphan as an + * address_collision. Anything still alive after the grace period is reaped by + * the token-verified group cleanup, so a stuck helper cannot hang teardown. + */ +const cleanupHopperLauncherSession = async (input: { + readonly helper: ProviderProcessHandle | undefined; + readonly ownership: OwnedProcessGroup; + readonly orderlyShutdownGraceMs: number; +}): Promise => { + const helper = input.helper; + if ( + helper !== undefined && + helper.exitCode === null && + helper.signalCode === null + ) { + try { + helper.stdin?.end(); + } catch { + // Already gone; the group cleanup below covers the remainder. + } + await waitForHelperExit(helper, input.orderlyShutdownGraceMs); + } + return cleanupOwnedProcessGroup(input.ownership); +}; + +/** Wait for the helper's exit, giving up after the grace period. */ +const waitForHelperExit = ( + helper: ProviderProcessHandle, + timeoutMs: number, +): Promise => + new Promise((resolve) => { + if (helper.exitCode !== null || helper.signalCode !== null) + return resolve(); + const timer = setTimeout( + () => { + helper.off("exit", onExit); + resolve(); + }, + Math.max(0, timeoutMs), + ); + const onExit = () => { + clearTimeout(timer); + resolve(); + }; + helper.once("exit", onExit); + }); + /** Select the publisher-verified demo adapter from the caller's explicit launch mode. */ export const usesLinuxDemo = ( options: HopperApplicationLauncherOptions, diff --git a/tests/boundary/providers/hopper/bridgeLauncher.test.ts b/tests/boundary/providers/hopper/bridgeLauncher.test.ts index 5d2611ecc6..7d48d22baf 100644 --- a/tests/boundary/providers/hopper/bridgeLauncher.test.ts +++ b/tests/boundary/providers/hopper/bridgeLauncher.test.ts @@ -547,3 +547,217 @@ it.skipIf(process.platform !== "linux")( expect(selectPrivateDisplay).not.toHaveBeenCalled(); }, ); + +it.skipIf(process.platform !== "linux")( + "asks the demo helper to shut down orderly so its X lock is removed", + async () => { + const directory = await createTestTempDirectory("rea-hopper-orderly-"); + const lockPath = join(directory, ".X199-lock"); + const markerPath = join(directory, "orderly-shutdown"); + const launcher = new HopperApplicationLauncher( + { + ...options(process.execPath), + launchMode: "verified_linux_demo", + environment: { REA_FIXTURE_LOCK_DIR: directory }, + demoHelperPath: fileURLToPath( + new URL( + "../../../fixtures/hopperOrderlyShutdown.py", + import.meta.url, + ), + ), + }, + { + verifyLinuxLauncher: async () => + ok({ + sha256: verifiedSha256, + version: "fixture", + primaryFingerprint: "0".repeat(40), + signingFingerprint: "1".repeat(40), + source: "signed_package_cache", + }), + selectPrivateDisplay: readyDirectDisplay, + }, + ); + const launched = await launcher.launch({ + directory, + socketPath: join(directory, "bridge.sock"), + token: "fixture-token", + runId: randomUUID(), + }); + if (!launched.ok) throw launched.error; + const supervisor = new ProviderProcessSupervisor(launched.value); + try { + const deadline = Date.now() + 5_000; + for (;;) { + try { + await access(lockPath); + break; + } catch { + if (Date.now() >= deadline) + throw new Error("lock file never appeared"); + await new Promise((resolve) => setTimeout(resolve, 50)); + } + } + expect(await supervisor.stop()).toMatchObject({ + status: "verified-cleanup", + }); + // The helper shut down orderly: it removed its own lock before exiting. + await expect(access(lockPath)).rejects.toMatchObject({ code: "ENOENT" }); + await expect(access(markerPath)).resolves.toBeUndefined(); + } finally { + await supervisor.stop(); + await launched.value.releaseLease?.(); + } + }, +); + +it.skipIf(process.platform !== "linux")( + "still cleans up when the demo helper ignores the orderly shutdown request", + async () => { + const directory = await createTestTempDirectory( + "rea-hopper-orderly-ignored-", + ); + const lockPath = join(directory, ".X199-lock"); + const launcher = new HopperApplicationLauncher( + { + ...options(process.execPath), + launchMode: "verified_linux_demo", + environment: { + REA_FIXTURE_LOCK_DIR: directory, + REA_FIXTURE_IGNORE_SHUTDOWN: "1", + }, + demoHelperPath: fileURLToPath( + new URL( + "../../../fixtures/hopperOrderlyShutdown.py", + import.meta.url, + ), + ), + }, + { + verifyLinuxLauncher: async () => + ok({ + sha256: verifiedSha256, + version: "fixture", + primaryFingerprint: "0".repeat(40), + signingFingerprint: "1".repeat(40), + source: "signed_package_cache", + }), + selectPrivateDisplay: readyDirectDisplay, + demoOrderlyShutdownGraceMs: 500, + }, + ); + const launched = await launcher.launch({ + directory, + socketPath: join(directory, "bridge.sock"), + token: "fixture-token", + runId: randomUUID(), + }); + if (!launched.ok) throw launched.error; + const supervisor = new ProviderProcessSupervisor(launched.value); + try { + // The fixture really started: its lock is on disk before we ask it to + // shut down, so an early fixture failure cannot fake a passing test. + const deadline = Date.now() + 5_000; + for (;;) { + try { + await access(lockPath); + break; + } catch { + if (Date.now() >= deadline) + throw new Error("lock file never appeared"); + await new Promise((resolve) => setTimeout(resolve, 50)); + } + } + const started = Date.now(); + expect(await supervisor.stop()).toMatchObject({ + status: "verified-cleanup", + }); + // Bounded by the configured 500 ms grace, not a hardcoded 10 s wait: + // the group kill must follow the ignored shutdown request promptly. + expect(Date.now() - started).toBeLessThan(5_000); + // The fallback path ran: the helper was reaped by the group SIGKILL, + // and its lock survives because the orderly path never ran. + expect(launched.value.process.signalCode).toBe("SIGKILL"); + await expect(access(lockPath)).resolves.toBeUndefined(); + } finally { + await supervisor.stop(); + await launched.value.releaseLease?.(); + } + }, +); + +it.skipIf(process.platform !== "linux")( + "does not wait out the orderly grace when the demo helper already died from a signal", + async () => { + const directory = await createTestTempDirectory("rea-hopper-signal-dead-"); + const lockPath = join(directory, ".X199-lock"); + const launcher = new HopperApplicationLauncher( + { + ...options(process.execPath), + launchMode: "verified_linux_demo", + environment: { + REA_FIXTURE_LOCK_DIR: directory, + REA_FIXTURE_SIGNAL_DEATH: "1", + }, + demoHelperPath: fileURLToPath( + new URL( + "../../../fixtures/hopperOrderlyShutdown.py", + import.meta.url, + ), + ), + }, + { + verifyLinuxLauncher: async () => + ok({ + sha256: verifiedSha256, + version: "fixture", + primaryFingerprint: "0".repeat(40), + signingFingerprint: "1".repeat(40), + source: "signed_package_cache", + }), + selectPrivateDisplay: readyDirectDisplay, + }, + ); + const launched = await launcher.launch({ + directory, + socketPath: join(directory, "bridge.sock"), + token: "fixture-token", + runId: randomUUID(), + }); + if (!launched.ok) throw launched.error; + const supervisor = new ProviderProcessSupervisor(launched.value); + try { + // The fixture wrote its lock, then killed itself with SIGKILL before + // the launcher asked for cleanup. + const lockDeadline = Date.now() + 5_000; + for (;;) { + try { + await access(lockPath); + break; + } catch { + if (Date.now() >= lockDeadline) + throw new Error("lock file never appeared"); + await new Promise((resolve) => setTimeout(resolve, 50)); + } + } + const exitDeadline = Date.now() + 5_000; + for (;;) { + if (launched.value.process.signalCode !== null) break; + if (Date.now() >= exitDeadline) + throw new Error("signal exit never observed"); + await new Promise((resolve) => setTimeout(resolve, 50)); + } + const started = Date.now(); + expect(await supervisor.stop()).toMatchObject({ + status: "verified-cleanup", + }); + // No grace wait: the helper was already gone, so cleanup must not sit + // out the 10 s orderly shutdown window. + expect(Date.now() - started).toBeLessThan(5_000); + expect(launched.value.process.signalCode).toBe("SIGKILL"); + } finally { + await supervisor.stop(); + await launched.value.releaseLease?.(); + } + }, +); diff --git a/tests/fixtures/hopperOrderlyShutdown.py b/tests/fixtures/hopperOrderlyShutdown.py new file mode 100644 index 0000000000..1edfbc6592 --- /dev/null +++ b/tests/fixtures/hopperOrderlyShutdown.py @@ -0,0 +1,46 @@ +#!/usr/bin/env python3 +"""Demo-helper stand-in: shuts down orderly when the owner pipe closes. + +Mirrors the production helper contract: the launcher owns this process group +and asks for an orderly shutdown by closing stdin (OwnerLifetime). The +production run_launch finally block then stops Hopper and Xvfb with SIGTERM; +here the fixture removes its fake X lock file instead, which is what Xvfb +does for /tmp/.X-lock on a clean exit. +""" + +import importlib.util +import os +import signal +import sys +import time +from pathlib import Path + +spec = importlib.util.spec_from_file_location( + "hopper_demo_x11", Path(__file__).resolve().parents[2] / "scripts/hopper-demo-x11.py" +) +assert spec is not None and spec.loader is not None +helper = importlib.util.module_from_spec(spec) +spec.loader.exec_module(helper) +assert "--owner-stdin" in sys.argv + +lock_dir = Path(os.environ["REA_FIXTURE_LOCK_DIR"]) +lock = lock_dir / ".X199-lock" +# Xvfb writes its PID right-justified in a 10-character field. +lock.write_text(f"{os.getpid():10d}\n") + +if os.environ.get("REA_FIXTURE_IGNORE_SHUTDOWN") == "1": + while True: + time.sleep(60) + +if os.environ.get("REA_FIXTURE_SIGNAL_DEATH") == "1": + # Die from a signal before the launcher asks for cleanup: teardown must + # notice the signal exit instead of waiting out the orderly grace period. + os.kill(os.getpid(), signal.SIGKILL) + +owner = helper.OwnerLifetime(True) +while owner.alive(): + time.sleep(0.05) +# Orderly shutdown path: remove our own lock, like Xvfb does on SIGTERM. +lock.unlink(missing_ok=True) +(lock_dir / "orderly-shutdown").write_text("ok") +sys.exit(0)