From b2c03f1904f21ad4e199f2737d20df912d373b4c Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Tue, 11 Aug 2026 09:22:09 +0200 Subject: [PATCH] fix(pf-vdisplay): a managed launch blocked the shutdown restore that was meant to rescue it, and re-moding could flip the operator's own screen MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The gamescope subsystem — the crate's largest and fastest-churning area, and the one the 2026-07-28 sweep predates most of. * **`MANAGED_SESSION` was held across the ~90 s managed launch**, and the shutdown/idle restore blocks on that same lock — *after* it has already stopped our unit. So the display-manager restore never ran and the box was left with no session at all. `create_managed_session` now decides under the guard and acts outside it, re-acquiring only to store the result; `do_restore_tv_session` consumes the record in a short scope at the top. Same shape the SteamOS twin already used. * **The physical-display guard was bypassed whenever no gamescope node happened to be published.** `if physical_display_connected() { if let Some(node) = find_gamescope_node() { … } }` fell through to `set-environment SCREEN_WIDTH/HEIGHT/CUSTOM_REFRESH_RATES` + `restart` when the node was momentarily absent — gamescope restarting between titles, or built without PipeWire — flipping the operator's own screen to the client's resolution and bouncing a DM-driven login session. The guard now refuses instead of falling through, and the forced `SCREEN_*` values (which were never unset, so every later session on the box inherited them) are tracked and `unset-environment`ed on restore. * **`current_gamescope_output_size()` reported an arbitrary gamescope's `-W`/`-H`** — whichever `/proc` enumerated first — and four consumers treated it as this session's output size. It now answers only when every gamescope on the box agrees, and `None` ("cannot tell") when they differ. `heads.rs` no longer takes it at all: it reads the size off the DRM-backed argv it already selected. Its test previously passed `None`, which is why the hazard was invisible. Resource and honesty fixes: the ATTACH path armed the box's own session-unit bind drop-in and no in-process path ever removed it (now tracked and disarmed on both restore arms); `wait_for_node` never called `try_wait`, so a gamescope that died at `vkCreateDevice` was polled for the full 15 s and the error then blamed headless capture support; `do_restore_tv_session` deleted its crash-recovery state *before* the unbounded work that state records, so a grace-period expiry in that window left the DM down with nothing on disk to heal it; the SteamOS takeover's two failure arms never armed the TV restore though the session-plus twin does; the TV-session restore logged success with the `systemctl` status discarded; the `steam -shutdown` child was dropped un-reaped; and a managed session that took nothing over was never persisted, so a host crash orphaned the transient unit. Item 8.1: the unbounded `pw-dump`, `systemctl`, `loginctl` and `pkexec` calls in this subsystem now go through `proc::{status_within, output_within}` with per-call budgets. `pw-dump` is polled from three separate 45 s loops against the very daemon this file documents gamescope as head-blocking, and until now a hang there pinned the session's stream thread forever. --- .../src/vdisplay/linux/gamescope.rs | 932 ++++++++++++++---- .../src/vdisplay/linux/gamescope/discovery.rs | 111 ++- .../src/vdisplay/linux/gamescope/heads.rs | 42 +- .../src/vdisplay/linux/gamescope/splash.rs | 54 +- 4 files changed, 897 insertions(+), 242 deletions(-) diff --git a/crates/pf-vdisplay/src/vdisplay/linux/gamescope.rs b/crates/pf-vdisplay/src/vdisplay/linux/gamescope.rs index adde4869..85e91e78 100644 --- a/crates/pf-vdisplay/src/vdisplay/linux/gamescope.rs +++ b/crates/pf-vdisplay/src/vdisplay/linux/gamescope.rs @@ -37,12 +37,22 @@ pub(crate) use discovery::{ pub(crate) use heads::list_monitors; pub(crate) use splash::run as splash_run; -/// The gamescope virtual-display driver. Three modes by env, in precedence order: -/// * `PUNKTFUNK_GAMESCOPE_SESSION=` — host-MANAGE a `gamescope-session-plus` session -/// (full Steam-Deck-UI polish) headless at the CLIENT's mode; relaunch it when the mode changes. -/// * `PUNKTFUNK_GAMESCOPE_NODE=` — ATTACH to an already-running gamescope (capture + -/// inject, no lifecycle ownership). -/// * else — SPAWN a bare headless gamescope sized to the mode, running `PUNKTFUNK_GAMESCOPE_APP`. +/// The gamescope virtual-display driver. Three sub-modes, resolved **per session** by +/// [`crate::resolve_gamescope_route`] and handed over through +/// [`VirtualDisplay::set_gamescope_route`] — never read out of the process env here, because a +/// second connect resolving its own route would otherwise retarget this session between the +/// decision and its `create` (Phase 2.3): +/// * **Managed** — host-MANAGE a `gamescope-session-plus`/SteamOS session (full Steam-Deck-UI +/// polish) headless at the CLIENT's mode; relaunch it when the mode changes. +/// * **Attach** — attach to an already-running gamescope (capture + inject, no lifecycle +/// ownership). +/// * **Spawn** — a bare headless gamescope sized to the mode, running the per-session launch +/// command ([`VirtualDisplay::set_launch_command`]) and falling back to +/// `PUNKTFUNK_GAMESCOPE_APP` only when that is empty. +/// +/// `PUNKTFUNK_GAMESCOPE_{MANAGED,ATTACH,NODE,SESSION}` still exist, but purely as **operator +/// overrides** feeding that ladder — sampled once at first use in `routing::operator_gamescope`, +/// never republished. #[derive(Default)] pub struct GamescopeDisplay { /// The resolved per-session launch command (set via [`VirtualDisplay::set_launch_command`]); the @@ -142,9 +152,11 @@ const RESTORE_DEBOUNCE: Duration = Duration::from_secs(5); /// coexisting gamescopes (a kept lingering spawn + a fresh one) never parse each other's node id. static SPAWN_SEQ: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(1); -/// This spawn instance's log path, under `$XDG_RUNTIME_DIR` (per-user, tmpfs; falls back to `/tmp` -/// only if unset). Replaces the shared `/tmp/punktfunk-gamescope.log` so concurrent spawns don't -/// clobber each other's `stream available on node ID:` line. +/// This spawn instance's log path, under the resolved per-user runtime dir +/// ([`crate::session::runtime_dir`] — `$XDG_RUNTIME_DIR` when set and non-empty, else +/// `/run/user/`; **never** `/tmp`, which Phase 4.2 removed from every one of these paths as a +/// world-writable-directory defect). Replaces the shared `/tmp/punktfunk-gamescope.log` so +/// concurrent spawns don't clobber each other's `stream available on node ID:` line. fn spawn_log_path(inst: u64) -> std::path::PathBuf { let base = crate::session::runtime_dir(); std::path::Path::new(&base).join(format!("punktfunk-gamescope-{inst}.log")) @@ -170,6 +182,28 @@ const STEAMOS_SESSION_TARGET: &str = "gamescope-session.target"; /// restart the physical session. static STEAMOS_TOOK_OVER: std::sync::Mutex = std::sync::Mutex::new(false); +/// Set once this host has written the `gamescope-session-plus@` bind drop-in over the BOX's own +/// autologin template ([`write_session_plus_dropin`] from [`ensure_box_gamescope_mode`]) — i.e. +/// whether there is a drop-in left to take away. +/// +/// It needs its own flag because that path steals nothing: no autologin unit is stopped, no DM, no +/// SteamOS target, no managed session — so every one of the takeover statics stays empty and +/// [`takeover_live`] used to answer `false`. The drop-in then outlived the stream AND the host: +/// nothing in-process ever removed it, and from that point the operator's own Game Mode ran +/// punktfunk's patched gamescope inside our mount+user namespace, with `/tmp/.X11-unix` replaced +/// and the last client's HDR/refresh flags applied. Only the next host START swept it +/// ([`restore_takeover_on_startup`]), which is why it read as an anomaly "from a previous host +/// instance" — it was this one. +static SESSION_DROPIN_ARMED: std::sync::Mutex = std::sync::Mutex::new(false); + +/// Set once this host has pushed `SCREEN_WIDTH`/`SCREEN_HEIGHT`/`CUSTOM_REFRESH_RATES` into the +/// `systemd --user` manager's environment to re-mode the box's OWN game-mode session +/// ([`ensure_box_gamescope_mode`]). Those are process-global to the user manager and survive every +/// unit restart, so a stream that ended left the box's own Game Mode pinned at the last client's +/// resolution for the rest of the login — the restore has to `unset-environment` them, and may +/// only do so for values it put there itself (an operator's own `set-environment` is theirs). +static FORCED_SESSION_SCREEN_ENV: std::sync::Mutex = std::sync::Mutex::new(false); + /// Persisted takeover state (`design/gamemode-and-dedicated-sessions.md` A3): the takeover mechanics /// ([`STOPPED_AUTOLOGIN`] / [`STEAMOS_TOOK_OVER`]) are process memory, so a host **crash** mid-stream /// would strand the box out of gaming mode with no restore. Mirroring the statics to a file lets @@ -184,6 +218,16 @@ struct TakeoverState { /// `restart` of the DM). `default` so takeover files from older hosts still parse. #[serde(default)] stopped_dm: Option, + /// Whether a host-managed [`SESSION_UNIT`] was running. It steals nothing — it is the managed + /// session started BESIDE a live desktop (a client gamescope pin on a KDE box) — but it is + /// still ours to end, and [`takeover_live`] already counts it for exactly that reason. Without + /// it here, a host that crashed with such a session left the transient unit running with no + /// record anywhere: the next start read the three stealing fields as empty and cleared the + /// file, so nothing ever stopped it ("closing the app does not end the session" survives a host + /// restart). Restored as an impossible-mode marker, never as a reusable session — see + /// [`restore_takeover_on_startup`]. + #[serde(default)] + managed_session: bool, } /// Path of the persisted [`TakeoverState`], under `$XDG_RUNTIME_DIR` (per-user, 0700, tmpfs — cleared @@ -195,6 +239,10 @@ fn takeover_state_path() -> std::path::PathBuf { /// Persist the current takeover mechanics so a host crash doesn't strand the box out of gaming mode. /// Best-effort (a write failure just loses crash-restore, not correctness). +/// ⚠ Never call this while holding any of the four statics it samples — [`MANAGED_SESSION`] +/// included, which is new here. Each is taken and released in turn (one lock at a time, as +/// [`takeover_live`] does), and a caller holding one would deadlock on itself: `std::sync::Mutex` +/// is not reentrant. fn persist_takeover() { let state = TakeoverState { stopped_autologin: STOPPED_AUTOLOGIN @@ -203,8 +251,12 @@ fn persist_takeover() { .clone(), steamos: *STEAMOS_TOOK_OVER.lock().unwrap_or_else(|e| e.into_inner()), stopped_dm: STOPPED_DM.lock().unwrap_or_else(|e| e.into_inner()).clone(), + managed_session: MANAGED_SESSION + .lock() + .unwrap_or_else(|e| e.into_inner()) + .is_some(), }; - if state.stopped_autologin.is_empty() && !state.steamos && state.stopped_dm.is_none() { + if !takeover_state_is_live(&state) { clear_takeover(); return; } @@ -218,6 +270,23 @@ fn clear_takeover() { let _ = std::fs::remove_file(takeover_state_path()); } +/// Does this [`TakeoverState`] describe anything a restore would have to undo? The file-side twin +/// of [`takeover_live`], and split out pure because the same test used to be written longhand in +/// two places and drifted from the in-memory one: `persist_takeover` and +/// `restore_takeover_on_startup` both tested only the three *stealing* fields, so a state whose +/// sole content was a managed session was written as "nothing to restore" and deleted on the spot. +/// +/// It is deliberately NARROWER than [`takeover_live`]: the drop-in and the forced SCREEN_* that +/// arm the in-memory check are runtime-dir state a fresh host sweeps unconditionally +/// ([`restore_takeover_on_startup`]), so persisting them would only make the file exist for a +/// restore with nothing left to do. +fn takeover_state_is_live(state: &TakeoverState) -> bool { + !state.stopped_autologin.is_empty() + || state.steamos + || state.stopped_dm.is_some() + || state.managed_session +} + /// On host startup, restore the TV's gaming session if a previous host instance took it over and /// crashed before restoring (`design/gamemode-and-dedicated-sessions.md` A3). Loads the persisted /// [`TakeoverState`] into the statics and schedules a restore after a short reconnect grace (so a @@ -249,7 +318,7 @@ pub fn restore_takeover_on_startup() { clear_takeover(); return; }; - if state.stopped_autologin.is_empty() && !state.steamos && state.stopped_dm.is_none() { + if !takeover_state_is_live(&state) { clear_takeover(); return; } @@ -257,6 +326,7 @@ pub fn restore_takeover_on_startup() { units = ?state.stopped_autologin, steamos = state.steamos, stopped_dm = ?state.stopped_dm, + managed_session = state.managed_session, "gamescope: found a stranded takeover from a previous host instance — scheduling TV restore" ); // Assume the adopted takeover carries our runtime mask whenever it stopped units: whether it did @@ -268,6 +338,24 @@ pub fn restore_takeover_on_startup() { *STOPPED_AUTOLOGIN.lock().unwrap_or_else(|e| e.into_inner()) = state.stopped_autologin; *STEAMOS_TOOK_OVER.lock().unwrap_or_else(|e| e.into_inner()) = state.steamos; *STOPPED_DM.lock().unwrap_or_else(|e| e.into_inner()) = state.stopped_dm; + // [`SESSION_DROPIN_ARMED`] is deliberately NOT persisted and not adopted here: the sweep at the + // top of this function already removed the drop-in (from both of its homes) and reloaded + // systemd, unconditionally and before anything else — so there is nothing left for a restored + // flag to owe. + if state.managed_session { + // An ADOPTED managed session is something to STOP, never something to reuse: the mode it + // was launched at is not persisted (and a wrong one would hand the next client a session at + // someone else's resolution while reporting a same-mode reuse). The impossible 0x0/0 Hz + // marker can never satisfy `create_managed_session`'s equality test, so every route through + // it relaunches — while `takeover_live` and `do_restore_tv_session`'s nothing-was-stolen arm + // both see a session that owes the box a `stop`. + *MANAGED_SESSION.lock().unwrap_or_else(|e| e.into_inner()) = Some(SessionState { + width: 0, + height: 0, + refresh_hz: 0, + hdr: false, + }); + } // Re-baseline the session-select sentinel: after a crash-restore the launch-time baseline is // gone, and a long-existing sentinel file must not read as a fresh in-stream switch request. record_session_select_baseline(); @@ -308,11 +396,14 @@ impl VirtualDisplay for GamescopeDisplay { } fn poolable_now(&self) -> bool { - // Only a bare SPAWN is registry-poolable (its `create` reports `Owned`); managed - // (`PUNKTFUNK_GAMESCOPE_SESSION`) and attach (`PUNKTFUNK_GAMESCOPE_NODE`) report - // `SessionManaged`/`External`, so the registry must not reuse a kept spawn for them (same - // backend name). Mirrors [`crate::launch_is_nested`]; read under the env lock the - // sub-mode ladder writes these keys under. + // Only a bare SPAWN is registry-poolable (its `create` reports `Owned`); Managed and + // Attach report `SessionManaged`/`External`, so the registry must not reuse a kept spawn + // for them (same backend name). `None` is poolable because `create`'s own `None` arm falls + // through to the bare spawn — the invariant to keep is "agrees with what `create` does with + // the same route", NOT "agrees with [`crate::launch_is_nested`]", which answers `false` for + // `None`. Reads THIS session's resolved route (handed over by `set_gamescope_route`): no + // env read, and therefore no env lock — Phase 2.3 deleted both the read and the writer that + // used to publish `PUNKTFUNK_GAMESCOPE_NODE`/`_SESSION` back into the process env. matches!(self.route, None | Some(crate::GamescopeRoute::Spawn)) } @@ -383,7 +474,15 @@ impl VirtualDisplay for GamescopeDisplay { // first and exit (crashing the spawn) — so free the autologin session first. Its restore is the // A3 takeover machinery (recorded in STOPPED_AUTOLOGIN + persisted; restarted on session end via // schedule_restore_tv_session). Non-Steam launches don't conflict, so they skip this. - if self.cmd.as_deref().is_some_and(is_steam_launch) { + + // Resolve the app ONCE, HERE — before the gate, and hand the answer to [`spawn`]. The gate + // used to test `self.cmd` alone while `spawn` resolved the `PUNKTFUNK_GAMESCOPE_APP` + // fallback afterwards and recognised it as Steam (`steam_mode`), so the documented + // `PUNKTFUNK_GAMESCOPE_APP="steam -gamepadui"` route got gamescope's `--steam` mode with + // NO instance free — and then collided with the box's own autologin/desktop Steam, which + // is precisely the collision this block exists to prevent. + let app = resolved_spawn_app(self.cmd.as_deref()); + if app.as_deref().is_some_and(is_steam_launch) { // A dedicated launch NEEDS Steam's single instance — no attach degrade exists here, so // a mask-fragile-DM box without takeover privilege fails with the actionable error. stop_autologin_sessions() @@ -402,25 +501,28 @@ impl VirtualDisplay for GamescopeDisplay { mode.width, mode.height, mode.refresh_hz.max(1), - self.cmd.as_deref(), + app, &log, self.hdr, )?; - let child_pid = child.id(); - let proc = GamescopeProc { + let mut proc = GamescopeProc { child, log: log.clone(), }; // gamescope creates its PipeWire node a moment after start; poll for it (the proc is held - // alive meanwhile, and killed if we give up). Discovery reads THIS spawn's log, and the - // fallback is scoped to this spawn's process tree. - let node_id = wait_for_node(Duration::from_secs(15), &log, child_pid).ok_or_else(|| { - anyhow!( - "gamescope PipeWire node did not appear within 15s — gamescope may have failed to \ - start or headless capture is unsupported on this GPU/driver (see {})", - log.display() - ) - })?; + // alive meanwhile, and killed if we give up). Discovery reads THIS spawn's log, the + // fallback is scoped to this spawn's process tree, and it gives up early if the process is + // already gone — a `vkCreateDevice` failure exits in under a second, and the 15 s the wait + // used to spend on its corpse produced a timeout error that blamed the GPU instead. + let node_id = + wait_for_node(Duration::from_secs(15), &log, &mut proc.child).ok_or_else(|| { + anyhow!( + "gamescope published no PipeWire node within 15s (or exited first) — it may \ + have failed to start, or headless capture may be unsupported on this \ + GPU/driver; its own log says which (see {})", + log.display() + ) + })?; tracing::info!( node_id, w = mode.width, @@ -489,13 +591,18 @@ fn create_managed_session(client: &str, mode: Mode, hdr: bool) -> Result Result Result id, Err(e) => { // The takeover already happened (autologin units stopped, possibly the DM down) — arm // the restore now, or a failed launch strands the box sessionless until a host // restart. Policy-timed; a quick client retry cancels it and relaunches warm. - // MANAGED_SESSION must be released first: the scheduler reads it (orphan detection). - drop(guard); + // MANAGED_SESSION is already released (the scheduler reads it for orphan detection). schedule_restore_tv_session(); return Err(e); } @@ -581,12 +712,19 @@ fn create_managed_session(client: &str, mode: Mode, hdr: bool) -> Result Option<(Option, Option, Opt None } +/// Budget for a cheap unit/user STATE query (`systemctl is-active`, `loginctl show-user -p +/// Linger`). These answer from the manager's in-memory state, so tens of milliseconds is the normal +/// cost and anything near this bound means the manager itself is wedged — which is exactly the +/// case each caller's failure path already covers. Several sit inside 10–45 s poll loops, where an +/// unbounded one blows the whole deadline in a single tick. +const UNIT_STATE_BUDGET: Duration = Duration::from_millis(300); + +/// Budget for an enumeration or a `loginctl` write (`systemctl list-units`, `enable-linger`) — +/// heavier than a state query (it walks every loaded unit, and lingering goes through polkit) but +/// still a local round trip. +const UNIT_QUERY_BUDGET: Duration = Duration::from_secs(5); + +/// Budget for a unit LIFECYCLE verb on the user manager: `start`/`stop`/`restart`/`kill`, +/// `daemon-reload`, `mask`/`unmask`, `reset-failed`, `set-environment`, `systemd-run --user`. +/// Generous, because a stop job legitimately waits on the unit's own teardown — but bounded, +/// because every one of these runs on a stream thread or the shutdown restore, and +/// [`crate::proc`]'s module doc is about precisely that: a hung helper pins the thread whose only +/// way to end a session is to return. +const UNIT_VERB_BUDGET: Duration = Duration::from_secs(10); + +/// Budget for a SYSTEM-bus display-manager verb ([`systemctl_system`]) and for the distro's own +/// session-switch helper. Much larger than [`UNIT_VERB_BUDGET`]: stopping a DM tears down a whole +/// seat's worth of sessions, and a premature kill here would abandon a takeover half-done. The +/// bound still matters — the callers fall through to the pkexec helper on failure, so the worst a +/// timeout costs is one redundant helper call, against a wedge that would otherwise be permanent. +const DM_VERB_BUDGET: Duration = Duration::from_secs(30); + /// Run a `systemctl --user` subcommand best-effort — a failure just means the session won't change, /// which the caller's node-wait surfaces. +/// +/// Status-blind by design (every caller here treats it as fire-and-forget), but NOT time-blind: it +/// is the shared chokepoint behind daemon-reload/restart/stop/set-environment across this file, so +/// one wedged user manager used to pin the stream thread through any of them. fn systemctl_user(args: &[&str]) { - let _ = Command::new("systemctl").arg("--user").args(args).status(); + let _ = crate::proc::status_within( + Command::new("systemctl").arg("--user").args(args), + UNIT_VERB_BUDGET, + ); } /// Directory holding the per-user `gamescope` PATH-shim (tmpfs under `XDG_RUNTIME_DIR`). @@ -1058,6 +1230,47 @@ fn remove_session_plus_dropin() -> bool { removed } +/// [`remove_session_plus_dropin`] **plus** clearing [`SESSION_DROPIN_ARMED`] and the +/// `daemon-reload` the removal owes systemd — the one call every hand-back path should make, so +/// that "the box's own unit template is clean again" and "we no longer think we owe it a removal" +/// can never disagree. A removal systemd has not reloaded still applies at the unit's next start, +/// which for the box's own autologin unit means its next BOOT. +fn disarm_session_plus_dropin() { + let removed = remove_session_plus_dropin(); + *SESSION_DROPIN_ARMED + .lock() + .unwrap_or_else(|e| e.into_inner()) = false; + if removed { + systemctl_user(&["daemon-reload"]); + } +} + +/// Take back the `SCREEN_WIDTH`/`SCREEN_HEIGHT`/`CUSTOM_REFRESH_RATES` this host forced into the +/// user manager for a re-moded box session ([`FORCED_SESSION_SCREEN_ENV`]). No-op unless we set +/// them — `unset-environment` is indiscriminate, and an operator who set their own must keep it. +/// `session.rs`'s `scrub_desktop_manager_env` does exactly this for `WAYLAND_DISPLAY`/`DISPLAY`; +/// these three had no such counterpart at all, which is why they outlived every stream. +fn unset_forced_session_screen_env() { + let mut forced = FORCED_SESSION_SCREEN_ENV + .lock() + .unwrap_or_else(|e| e.into_inner()); + if !*forced { + return; + } + *forced = false; + drop(forced); + systemctl_user(&[ + "unset-environment", + "SCREEN_WIDTH", + "SCREEN_HEIGHT", + "CUSTOM_REFRESH_RATES", + ]); + tracing::info!( + "gamescope: unset the forced SCREEN_WIDTH/SCREEN_HEIGHT/CUSTOM_REFRESH_RATES — the box's \ + own game mode is back on its own resolution" + ); +} + /// Take over SteamOS's `gamescope-session.target` headless at the CLIENT's mode: write the shim + a /// drop-in carrying the mode, `daemon-reload`, then RESTART the target so `steam-launcher.service` /// brings Steam up in the fresh headless gamescope — and attach to its node. A same-mode reconnect @@ -1111,17 +1324,36 @@ fn create_managed_session_steamos(mode: Mode, hdr: bool) -> Result id, + None => { + schedule_restore_tv_session(); + bail!( + "SteamOS headless gamescope node did not appear within 30s after restarting \ + {STEAMOS_SESSION_TARGET} — check `journalctl --user -u gamescope-session.service`" + ); + } + }; // The shim is only a PATH entry — confirm the session actually took it before we trust the // capabilities the plan was already built on (a stock gamescope here means no HDR and, worse, // a silently pointerless stream). Leaves the tracked state unset on failure, so the retry does // a clean restart rather than a same-mode reuse of a session we just rejected. - verify_managed_spawn_flags(hdr)?; + if let Err(e) = verify_managed_spawn_flags(hdr) { + schedule_restore_tv_session(); + return Err(e); + } point_injector_at_eis(); // Re-acquire to record the tracked session — the same shape `create_managed_session` uses. *MANAGED_SESSION.lock().unwrap_or_else(|e| e.into_inner()) = Some(SessionState { @@ -1130,6 +1362,9 @@ fn create_managed_session_steamos(mode: Mode, hdr: bool) -> Result Result { // restart is the headless-box model (no panel ⇒ the game-mode resolution is ours to set); // on-glass it would flip the user's own screen to the client's resolution — and on a // DM-session-driven box (Nobara) the unit restart bounces the login session with it. + // + // The guard is on the DECISION, not on finding a node: a momentarily absent node (the session + // restarting, a `pw-dump` that timed out) used to fall straight THROUGH this `if` into + // `set-environment SCREEN_WIDTH/HEIGHT` + a unit restart — doing on a panel exactly what the + // paragraph above forbids, and leaving the forced SCREEN_* behind in the user manager + // afterwards. Refuse instead, the same shape the attach-only rebuild probe above uses: the + // caller's retry re-asks once the node is back. if physical_display_connected() { - if let Some(node) = find_gamescope_node() { - tracing::info!( - node, - client_w = mode.width, - client_h = mode.height, - "gamescope: box drives a physical display — attaching at its own mode (no re-mode)" - ); - return Ok(node); - } + let node = find_gamescope_node().ok_or_else(|| { + anyhow!( + "the box drives a physical display, so its game-mode session is mirrored at its \ + OWN mode — and it publishes no gamescope Video/Source node right now. Refusing to \ + re-mode it to {}x{}: that would flip the screen someone is looking at and, on a \ + DM-driven box, bounce the login session with it", + mode.width, + mode.height + ) + })?; + tracing::info!( + node, + client_w = mode.width, + client_h = mode.height, + "gamescope: box drives a physical display — attaching at its own mode (no re-mode)" + ); + return Ok(node); } let Some(unit) = running_autologin_gamescope_unit() else { // No box-owned autologin session to reconfigure (a bare/foreign gamescope): attach to @@ -1212,7 +1462,13 @@ fn ensure_box_gamescope_mode(mode: Mode, hdr: bool) -> Result { "gamescope: relaunching the box game-mode session at the client's resolution" ); // The session reads SCREEN_WIDTH/HEIGHT (+ CUSTOM_REFRESH_RATES) from the user-manager - // environment; set them and restart the box's own unit. + // environment; set them and restart the box's own unit. Record that we did: the manager keeps + // them for the rest of the login, so the restore owes them an `unset-environment` + // ([`unset_forced_session_screen_env`]) or the box's own Game Mode stays at the client's + // resolution long after the stream ended. + *FORCED_SESSION_SCREEN_ENV + .lock() + .unwrap_or_else(|e| e.into_inner()) = true; systemctl_user(&[ "set-environment", &format!("SCREEN_WIDTH={}", mode.width), @@ -1225,6 +1481,14 @@ fn ensure_box_gamescope_mode(mode: Mode, hdr: bool) -> Result { .and_then(|w| write_session_plus_dropin(&w, mode, hdr, wsi_layer_matches_our_gamescope())) { Ok(true) => { + // Record it BEFORE the restart, and persist it: from this instant the box's OWN + // autologin template carries our bind, and only a restore takes it back. Nothing else + // on this path sets a takeover static, so without this flag `takeover_live()` answered + // `false` and neither the disconnect restore nor the shutdown one ever ran — the + // drop-in outlived the stream and the host process both. + *SESSION_DROPIN_ARMED + .lock() + .unwrap_or_else(|e| e.into_inner()) = true; tracing::info!( bin = %gamescope_bin(), %unit, @@ -1234,7 +1498,15 @@ fn ensure_box_gamescope_mode(mode: Mode, hdr: bool) -> Result { ); true } - Ok(false) => false, + Ok(false) => { + // `write_session_plus_dropin` REMOVES rather than writes when there is no bind to arm, + // so this arm is also the disarm — the flag must follow it or the restore would owe a + // removal for a drop-in that is already gone. + *SESSION_DROPIN_ARMED + .lock() + .unwrap_or_else(|e| e.into_inner()) = false; + false + } // Best-effort: a box whose session already runs our binary loses nothing, and a failure // here must not block a restart that would otherwise work. Err(e) => { @@ -1271,8 +1543,7 @@ fn ensure_box_gamescope_mode(mode: Mode, hdr: bool) -> Result { // drop-in, reload, and give it one more restart without it. if bound { note_bind_hazard(&unit); - remove_session_plus_dropin(); - systemctl_user(&["daemon-reload"]); + disarm_session_plus_dropin(); // also clears the flag: there is nothing left to undo systemctl_user(&["restart", &unit]); bound = false; deadline = Instant::now() + Duration::from_secs(45); @@ -1325,19 +1596,63 @@ fn gamescope_argvs() -> Vec> { found } -/// Output (capture) resolution `-W -H ` of the running gamescope, parsed from its -/// `/proc//cmdline`. `None` if no gamescope is running or the flags aren't present — which is -/// also the final filter that separates a compositor from anything else [`gamescope_argvs`] let by. +/// Output (capture) resolution `-W -H ` of ONE gamescope, from its argv. `None` if the +/// flags aren't both present — which is also the final filter that separates a compositor from +/// anything else [`gamescope_argvs`] let by. Pure + unit-tested. +fn gamescope_output_size(argv: &[String]) -> Option<(u32, u32)> { + match ( + argv_u32(argv, &["-W", "--output-width"]), + argv_u32(argv, &["-H", "--output-height"]), + ) { + (Some(w), Some(h)) => Some((w, h)), + _ => None, + } +} + +/// The output size of "the gamescope on this box" — **only when every gamescope on it agrees**. +/// +/// This used to be `find_map` over `/proc` read_dir order: the first gamescope with a `-W`/`-H`, +/// which on the one deployment that matters is arbitrary. A Deck or Bazzite box in Game Mode +/// routinely runs TWO — the session compositor and a nested one for the game (a per-title +/// `gamescope -W 1920 -H 1080 -- %command%` launch option produces exactly that, and +/// `heads.rs`'s own test encodes the shape as normal) — plus any headless one this crate spawned. +/// Every consumer read the answer as "this session's output size", and a wrong-but-confident one +/// is worse than no answer at each of them: it scaled the libei injector's absolute coordinates to +/// the wrong picture (the pointer could not reach the right/bottom edge), and it made +/// [`ensure_box_gamescope_mode`] compare its target against a compositor it was not restarting. +/// +/// So `None` now means "cannot tell", which every caller already handles: the injector omits the +/// hint and falls back to raw client pixels, and the re-mode path declines its fast path and looks +/// again after the restart — by which time the session's own restart has left exactly one. fn current_gamescope_output_size() -> Option<(u32, u32)> { - gamescope_argvs().into_iter().find_map(|args| { - match ( - argv_u32(&args, &["-W", "--output-width"]), - argv_u32(&args, &["-H", "--output-height"]), - ) { - (Some(w), Some(h)) => Some((w, h)), - _ => None, + unanimous_output_size(&gamescope_argvs()) +} + +/// [`current_gamescope_output_size`]'s decision, over a supplied argv set: the single size every +/// gamescope carrying `-W`/`-H` agrees on, or `None` when they disagree (or none says). Pure + +/// unit-tested — the ambiguity rule is the whole point of the function, and it is not observable +/// from the `/proc`-reading half. +fn unanimous_output_size(argvs: &[Vec]) -> Option<(u32, u32)> { + let mut agreed: Option<(u32, u32)> = None; + for argv in argvs { + let Some(size) = gamescope_output_size(argv) else { + continue; + }; + match agreed { + None => agreed = Some(size), + Some(seen) if seen == size => {} + Some(seen) => { + tracing::debug!( + ?seen, + ?size, + "gamescope: two coexisting gamescopes report different output sizes — \ + answering 'unknown' rather than picking one of them" + ); + return None; + } } - }) + } + agreed } /// The numeric value following the first of `names` present in `argv`. Pure + unit-tested — it is @@ -1514,8 +1829,8 @@ fn missing_flags<'a>(expected: &'a [String], argvs: &[Vec]) -> Vec<&'a s /// The running autologin gaming-mode unit (`gamescope-session-plus@.service`), if any — the /// box's own game-mode session, which [`ensure_box_gamescope_mode`] reconfigures + restarts. fn running_autologin_gamescope_unit() -> Option { - let out = Command::new("systemctl") - .args([ + let out = crate::proc::output_within( + Command::new("systemctl").args([ "--user", "list-units", "--type=service", @@ -1523,9 +1838,10 @@ fn running_autologin_gamescope_unit() -> Option { "--no-legend", "--plain", "gamescope-session-plus@*.service", - ]) - .output() - .ok()?; + ]), + UNIT_QUERY_BUDGET, + ) + .ok()?; String::from_utf8_lossy(&out.stdout) .lines() .filter_map(|l| l.split_whitespace().next()) @@ -1541,15 +1857,21 @@ fn running_autologin_gamescope_unit() -> Option { /// reboot. SIGKILL skips that handler so the driver reclaims the context cleanly via normal process /// exit. Follow with `stop` + `reset-failed` to clear the unit's state so a relaunch is clean. fn kill_unit(unit: &str) { - let _ = Command::new("systemctl") - .args(["--user", "kill", "--signal=SIGKILL", unit]) - .status(); - let _ = Command::new("systemctl") - .args(["--user", "stop", unit]) - .status(); - let _ = Command::new("systemctl") - .args(["--user", "reset-failed", unit]) - .status(); + // All three budgeted: this runs on the disconnect restore and on the shutdown path, where the + // whole sequence has ~20 s before `native.rs` gives up and exits — three unbounded `systemctl` + // calls against a busy user manager were enough on their own to spend it. + let _ = crate::proc::status_within( + Command::new("systemctl").args(["--user", "kill", "--signal=SIGKILL", unit]), + UNIT_VERB_BUDGET, + ); + let _ = crate::proc::status_within( + Command::new("systemctl").args(["--user", "stop", unit]), + UNIT_VERB_BUDGET, + ); + let _ = crate::proc::status_within( + Command::new("systemctl").args(["--user", "reset-failed", unit]), + UNIT_VERB_BUDGET, + ); } /// Runtime-mask `unit` so the box's session supervisor cannot restart it underneath the takeover. @@ -1580,17 +1902,19 @@ fn kill_unit(unit: &str) { /// the way back is barred until reboot (`--runtime` lives in tmpfs — which is exactly why "it works /// again after a reboot"). fn mask_unit(unit: &str) { - let _ = Command::new("systemctl") - .args(["--user", "mask", "--runtime", unit]) - .status(); + let _ = crate::proc::status_within( + Command::new("systemctl").args(["--user", "mask", "--runtime", unit]), + UNIT_VERB_BUDGET, + ); } /// Undo [`mask_unit`] — every restore path must unmask before (or regardless of) restarting, or the /// box's own return-to-gaming-mode stays broken until reboot. fn unmask_unit(unit: &str) { - let _ = Command::new("systemctl") - .args(["--user", "unmask", "--runtime", unit]) - .status(); + let _ = crate::proc::status_within( + Command::new("systemctl").args(["--user", "unmask", "--runtime", unit]), + UNIT_VERB_BUDGET, + ); } /// Lift the takeover's runtime mask ([`mask_unit`]) on the box's own autologin units, so the box can @@ -1992,10 +2316,17 @@ fn user_in_group(user: &str, group: &str) -> bool { /// failure the callers are written for, so the pkexec helper (`allow_any`, no agent needed) takes /// over instead of a dialog. Field-suspect in the 0.20.0 Nobara report (intermittent disconnect + /// a screen that never comes back), where the timing is a race against the KDE agent's own death. +/// +/// Bounded by [`DM_VERB_BUDGET`] on top of that: `--no-ask-password` removes the *dialog*, not the +/// wait — a system manager mid-shutdown can still take the request and never answer. A timeout +/// reads as `false`, which is the same answer an unauthorized call already gives, so every caller +/// falls through to the pkexec helper exactly as it does today. fn systemctl_system(args: &[&str]) -> bool { let mut cmd = Command::new("systemctl"); cmd.arg("--no-ask-password").args(args); - cmd.status().map(|s| s.success()).unwrap_or(false) + crate::proc::status_within(&mut cmd, DM_VERB_BUDGET) + .map(|s| s.success()) + .unwrap_or(false) } /// Would stopping the display manager also stop US? A packaged host runs as a `systemd --user` @@ -2027,9 +2358,10 @@ fn ensure_host_survives_dm_stop() -> std::result::Result<(), String> { // user's session can do this itself; a sessionless one (the packaged unit) goes through the // helper, whose grant is scoped to the calling uid. let uid = uid_string(); - let _ = Command::new("loginctl") - .args(["--no-ask-password", "enable-linger", &uid]) - .status(); + let _ = crate::proc::status_within( + Command::new("loginctl").args(["--no-ask-password", "enable-linger", &uid]), + UNIT_QUERY_BUDGET, + ); let helper = if linger_enabled() { Ok(()) // the plain verb was enough — the helper was never needed } else { @@ -2077,13 +2409,17 @@ fn uid_string() -> String { crate::proc::current_uid().to_string() } -/// Is lingering on for this user (logind keeps the `--user` manager alive with no session)? +/// Is lingering on for this user (logind keeps the `--user` manager alive with no session)? A +/// property read off logind's in-memory state, so [`UNIT_STATE_BUDGET`] is generous; an unanswered +/// one reads as "not lingering", which refuses the takeover rather than risking the DM stop taking +/// the host down with it. fn linger_enabled() -> bool { - Command::new("loginctl") - .args(["show-user", &uid_string(), "-p", "Linger", "--value"]) - .output() - .map(|o| String::from_utf8_lossy(&o.stdout).trim() == "yes") - .unwrap_or(false) + crate::proc::output_within( + Command::new("loginctl").args(["show-user", &uid_string(), "-p", "Linger", "--value"]), + UNIT_STATE_BUDGET, + ) + .map(|o| String::from_utf8_lossy(&o.stdout).trim() == "yes") + .unwrap_or(false) } /// Stop the display manager for a takeover on a mask-fragile DM flavor. Plain `systemctl stop` on @@ -2213,11 +2549,14 @@ fn honor_session_select_switch(dm: String) { } let deadline = Instant::now() + Duration::from_secs(10); while Instant::now() < deadline { - let active = Command::new("systemctl") - .args(["is-active", &dm]) - .output() - .map(|o| String::from_utf8_lossy(&o.stdout).trim() == "active") - .unwrap_or(false); + // Budgeted: this is a 10 s loop, and a single unbounded `is-active` against a system + // manager that is itself mid-restart would consume the whole window in one tick. + let active = crate::proc::output_within( + Command::new("systemctl").args(["is-active", &dm]), + UNIT_STATE_BUDGET, + ) + .map(|o| String::from_utf8_lossy(&o.stdout).trim() == "active") + .unwrap_or(false); if active { break; } @@ -2227,7 +2566,13 @@ fn honor_session_select_switch(dm: String) { // Absent/failing helper degrades to a plain DM restore — the box lands back in game mode on // glass and the stream follows that instead (no black screen either way). if std::path::Path::new(OS_SESSION_SELECT).exists() { - match Command::new(OS_SESSION_SELECT).arg("desktop").status() { + // [`DM_VERB_BUDGET`] rather than a unit budget: the helper self-pkexecs and rewrites the + // DM's autologin config, which is DM-verb-shaped work — and it runs on the stream thread + // that is honoring the user's in-stream switch. + match crate::proc::status_within( + Command::new(OS_SESSION_SELECT).arg("desktop"), + DM_VERB_BUDGET, + ) { Ok(s) if s.success() => { // The relogin only fires when the CURRENT (game-mode) login session exits: wait // for its autologin unit to come up, then stop it. Never mask here — the mask is @@ -2293,8 +2638,8 @@ fn honor_session_select_switch(dm: String) { /// **fails the takeover** — the error tells the caller to degrade to ATTACH (mirror the box's /// own session) rather than destabilize the seat. fn stop_autologin_sessions() -> Result<()> { - let Ok(out) = Command::new("systemctl") - .args([ + let Ok(out) = crate::proc::output_within( + Command::new("systemctl").args([ "--user", "list-units", "--type=service", @@ -2302,9 +2647,9 @@ fn stop_autologin_sessions() -> Result<()> { "--no-legend", "--plain", "gamescope-session-plus@*.service", - ]) - .output() - else { + ]), + UNIT_QUERY_BUDGET, + ) else { return Ok(()); }; // `(unit, ACTIVE state)` — the `--plain` columns are UNIT LOAD ACTIVE SUB DESCRIPTION. @@ -2453,11 +2798,20 @@ fn free_desktop_steam() -> Result<()> { pid, "freeing Steam: a desktop-session Steam holds the single instance — sending `steam -shutdown`" ); - let _ = Command::new("steam") - .arg("-shutdown") - .stdout(Stdio::null()) - .stderr(Stdio::null()) - .spawn(); + // REAPED, not fire-and-forget. `let _ = …spawn()` dropped the `Child` immediately, and + // `Child`'s Drop explicitly does not wait — so this helper became a zombie held for the host's + // whole lifetime. Nothing downstream collected it: the loop below polls the TARGET Steam's pid, + // and `pid_running` documents the very state this call site was manufacturing ("a zombie keeps + // its /proc entry but has already released the Steam instance"). The helper only forwards over + // Steam's single-instance IPC and exits, so the budget is short; the real waiting is the + // `STEAM_SHUTDOWN_WAIT` loop, which is about the OTHER process. + let _ = crate::proc::status_within( + Command::new("steam") + .arg("-shutdown") + .stdout(Stdio::null()) + .stderr(Stdio::null()), + UNIT_QUERY_BUDGET, + ); let deadline = Instant::now() + STEAM_SHUTDOWN_WAIT; while Instant::now() < deadline { if !pid_running(pid) { @@ -2591,8 +2945,9 @@ pub fn schedule_restore_tv_session() { } /// Is anything of the box's own session ours right now — an autologin unit we stopped, a stopped -/// display manager, a SteamOS target we re-pointed, or a managed session we launched beside a live -/// desktop? The precondition for every restore path. +/// display manager, a SteamOS target we re-pointed, a bind drop-in we armed over the box's own +/// session template, a resolution we pinned into its user manager, or a managed session we +/// launched beside a live desktop? The precondition for every restore path. fn takeover_live() -> bool { // ONE lock at a time. In a `||` chain every `.lock()` temporary lives to the end of the // statement, so this used to hold all four simultaneously — putting it in the lock-order graph @@ -2606,9 +2961,20 @@ fn takeover_live() -> bool { .lock() .unwrap_or_else(|e| e.into_inner()) .is_some(); + // The ATTACH re-mode path steals nothing, so none of the three above records it — but it DID + // rewrite the box's own `gamescope-session-plus@` template and pin the user manager's + // SCREEN_*/CUSTOM_REFRESH_RATES at the client's mode, and only a restore takes those back. + let dropin = *SESSION_DROPIN_ARMED + .lock() + .unwrap_or_else(|e| e.into_inner()); + let screen_env = *FORCED_SESSION_SCREEN_ENV + .lock() + .unwrap_or_else(|e| e.into_inner()); autologin || steamos || dm + || dropin + || screen_env // A managed session that took nothing over (started beside a live desktop — e.g. a client // gamescope pin on a KDE box) still owns the transient SESSION_UNIT: without this arm it // was ORPHANED forever after disconnect ("closing the app does not end the session", @@ -2683,7 +3049,6 @@ fn do_restore_tv_session() { return; } *took = false; - clear_takeover(); // A3: takeover undone — drop the persisted crash-restore marker *MANAGED_SESSION.lock().unwrap_or_else(|e| e.into_inner()) = None; remove_steamos_dropin(); systemctl_user(&["daemon-reload"]); @@ -2699,12 +3064,29 @@ fn do_restore_tv_session() { "gamescope (SteamOS): a desktop session is active — removed the headless \ override, not restarting the gaming session" ); + clear_takeover(); // A3: takeover undone — and only now that it actually is return; } - systemctl_user(&["restart", STEAMOS_SESSION_TARGET]); - tracing::info!( - "gamescope (SteamOS): restored the physical gaming session (removed headless override)" - ); + // Checked, not `systemctl_user`'d: that helper is status-blind (`let _ = …status()`), + // so this used to log an unconditional success over a restart that may never have + // happened — on the one box whose panel goes dark until it does. + match crate::proc::status_within( + Command::new("systemctl").args(["--user", "restart", STEAMOS_SESSION_TARGET]), + UNIT_VERB_BUDGET, + ) { + Ok(s) if s.success() => tracing::info!( + "gamescope (SteamOS): restored the physical gaming session (removed headless \ + override)" + ), + other => tracing::error!( + status = ?other, + "gamescope (SteamOS): could not restart {STEAMOS_SESSION_TARGET} — the Deck's \ + panel stays dark until someone runs \ + `systemctl --user restart {STEAMOS_SESSION_TARGET}` (the headless override is \ + already removed, so that restart is all it needs)" + ), + } + clear_takeover(); // A3: consumed — after the restart, not before it return; } } @@ -2716,27 +3098,51 @@ fn do_restore_tv_session() { lift_autologin_mask(); let units = std::mem::take(&mut *STOPPED_AUTOLOGIN.lock().unwrap_or_else(|e| e.into_inner())); let dm = std::mem::take(&mut *STOPPED_DM.lock().unwrap_or_else(|e| e.into_inner())); + // Consume the managed-session record HERE, in a scope of its own, and never hold that lock + // across the work below. `create_managed_session` can be inside `launch_session` holding it for + // up to ~90 s (two 45 s node-poll deadlines plus `systemd-run` round trips), and this function + // is the shutdown path: blocking on it AFTER `stop_session` had already killed the unit meant + // the display-manager restore below was never reached before `native.rs`'s 20 s + // `SHUTDOWN_RESTORE_GRACE` expired and the process `exit(0)`ed — a box with its DM stopped, no + // graphical session, and (with `clear_takeover` where it used to be) nothing on disk to heal it. + let managed_was_running = MANAGED_SESSION + .lock() + .unwrap_or_else(|e| e.into_inner()) + .take() + .is_some(); if units.is_empty() && dm.is_none() { // Nothing was stolen — but a managed session that started BESIDE a live desktop (client // gamescope pin on a KDE box) still owns the transient unit; stop it so it doesn't run // orphaned forever after the disconnect. No-op when the unit isn't running. - if MANAGED_SESSION - .lock() - .unwrap_or_else(|e| e.into_inner()) - .take() - .is_some() - { + if managed_was_running { stop_session(SESSION_UNIT); tracing::info!( "gamescope: stopped the idle managed session (nothing was taken over — no box \ session to restore)" ); } + // …and the ATTACH re-mode path steals nothing either, yet leaves the two most durable + // marks this backend can make on a box: a bind drop-in over its OWN autologin template and + // a user manager pinned at the client's resolution. Both used to survive the stream, the + // host process, and (for the `$HOME` drop-in) the reboot. Undoing them does NOT restart + // the box's session: the live one keeps the client's mode until it next starts, which is + // the non-destructive direction — bouncing the user's own Game Mode on a disconnect is the + // very thing the physical-display guard in `ensure_box_gamescope_mode` refuses to do. + disarm_session_plus_dropin(); + unset_forced_session_screen_env(); + clear_takeover(); return; } - clear_takeover(); // A3: takeover consumed — drop the persisted crash-restore marker stop_session(SESSION_UNIT); // our gamescope/Steam session, so Steam is free for the autologin - *MANAGED_SESSION.lock().unwrap_or_else(|e| e.into_inner()) = None; + + // Hand the box back its OWN gamescope and its own resolution BEFORE any of the branches below, + // every one of which can return: our bind drop-in exists to serve a punktfunk stream, and + // leaving it would silently put the patched build (plus our HDR/cursor flags) under the user's + // ordinary game mode — exactly the "sits beside the distro package" rule this whole design + // rests on. It used to sit after the desktop-active and DM returns, so those two paths leaked + // it. + disarm_session_plus_dropin(); + unset_forced_session_screen_env(); // Only bring the gaming autologin BACK if the box is still meant to be in gaming mode. If the // user switched to a desktop session (KDE/GNOME/wlroots/Hyprland) in the meantime, don't yank // them back to gaming — leave the desktop alone. (We still stopped our idle managed session @@ -2752,6 +3158,7 @@ fn do_restore_tv_session() { tracing::info!( "gamescope: a desktop session is active — not restoring the TV gaming session" ); + clear_takeover(); // A3: consumed — the units/DM records are already drained into locals return; } // DM-stop takeover ([`dm_plan`] — every flavor stops a DM that drove a live gaming session): @@ -2782,23 +3189,38 @@ fn do_restore_tv_session() { `systemctl reset-failed {dm} && systemctl restart {dm}` as root" ), } + // LAST, not first. The persisted marker is the ONLY thing that heals a box whose DM is + // down after this process dies, and every step above it is unbounded work on a shutdown + // path with a 20 s grace (`native.rs`'s `SHUTDOWN_RESTORE_GRACE`, then `exit(0)` — which + // runs no destructors). Deleting it before the restart had been issued meant an expiry + // anywhere in between left the box dark with nothing on disk saying so. + clear_takeover(); return; } - // Hand the box back its OWN gamescope before restarting its session: our bind drop-in exists - // to serve a punktfunk stream, and leaving it would silently put the patched build (plus our - // HDR/cursor flags) under the user's ordinary game mode — exactly the "sits beside the distro - // package" rule this whole design rests on. - remove_session_plus_dropin(); - systemctl_user(&["daemon-reload"]); for unit in units { - let _ = Command::new("systemctl") - .args(["--user", "start", &unit]) - .status(); - tracing::info!( - unit, - "restored the TV's autologin gaming session (debounce elapsed, no client)" - ); + // Checked, not discarded: this call and the SteamOS `restart` above were the two places + // that logged an unconditional success over a thrown-away exit status. A `--user start` + // fails for reasons an operator can act on (the unit is masked, its start limit tripped), + // and the DM branch thirty lines up already shows the shape — say what happened. + match crate::proc::status_within( + Command::new("systemctl").args(["--user", "start", &unit]), + UNIT_VERB_BUDGET, + ) { + Ok(s) if s.success() => tracing::info!( + unit, + "restored the TV's autologin gaming session (debounce elapsed, no client)" + ), + other => tracing::error!( + unit, + status = ?other, + "could not restart the TV's autologin gaming session — the box is left out of \ + game mode until someone runs `systemctl --user start {unit}` (a masked unit or a \ + tripped start limit are the usual causes: \ + `systemctl --user unmask --runtime {unit} && systemctl --user reset-failed {unit}`)" + ), + } } + clear_takeover(); // A3: consumed — and only now, with the restarts actually issued } /// Host-lifetime worker that fires a pending [`schedule_restore_tv_session`] once its debounce @@ -2844,6 +3266,14 @@ fn point_injector_at_eis() { // the injector can't learn the output geometry from the protocol — the hint lets // it scale normalized client positions correctly even when the client streams at // a different resolution than the session runs (foreign attach, supersample). + // + // The two halves of this file are resolved from different sources — the socket is the + // newest CONNECTABLE `gamescope-*-ei`, the size comes off `/proc` — so on a box running + // two gamescopes they could describe two different compositors. That is why the size + // probe now answers `None` unless every gamescope agrees: omitting the hint makes + // `pf-inject` fall back to raw client pixels, whereas a hint taken from the nested + // game's `-W 1280 -H 800` while the socket belongs to the 1920x1080 session put the + // pointer's reachable area at two thirds of the picture. let size = current_gamescope_output_size(); let body = match size { Some((w, h)) => format!("{sock}\n{w}x{h}"), @@ -3608,11 +4038,10 @@ fn launch_session(client: &str, unit_name: &str, mode: Mode, hdr: bool) -> Resul if !wsi_ok { cmd.arg("--setenv=ENABLE_GAMESCOPE_WSI=0"); } - let status = cmd - // Same headless-must-not-attach rule as [`spawn`]: the transient unit inherits the - // user manager env, which can carry a (possibly stale) desktop DISPLAY/WAYLAND_DISPLAY - // that would abort gamescope at startup. - .arg("--property=UnsetEnvironment=DISPLAY WAYLAND_DISPLAY") + // Same headless-must-not-attach rule as [`spawn`]: the transient unit inherits the + // user manager env, which can carry a (possibly stale) desktop DISPLAY/WAYLAND_DISPLAY + // that would abort gamescope at startup. + cmd.arg("--property=UnsetEnvironment=DISPLAY WAYLAND_DISPLAY") .arg("--setenv=BACKEND=headless") .arg(format!("--setenv=SCREEN_WIDTH={}", mode.width)) .arg(format!("--setenv=SCREEN_HEIGHT={}", mode.height)) @@ -3632,12 +4061,15 @@ fn launch_session(client: &str, unit_name: &str, mode: Mode, hdr: bool) -> Resul .arg(format!("--setenv=CUSTOM_REFRESH_RATES={offered}")) .arg("--") .arg(SESSION_PLUS_BIN) - .arg(client) - .status() - .context( - "launch gamescope-session-plus via `systemd-run --user` (is the user systemd \ - manager up with XDG_RUNTIME_DIR + DBUS_SESSION_BUS_ADDRESS set?)", - )?; + .arg(client); + // Budgeted: without `--wait`, `systemd-run` returns as soon as the manager has accepted the + // transient unit, so the only way this takes seconds is a user manager that is not + // answering — the exact case the caller's error text is written for, and one that used to + // pin the connecting client's thread indefinitely instead. + let status = crate::proc::status_within(&mut cmd, UNIT_VERB_BUDGET).context( + "launch gamescope-session-plus via `systemd-run --user` (is the user systemd \ + manager up with XDG_RUNTIME_DIR + DBUS_SESSION_BUS_ADDRESS set?)", + )?; if !status.success() { anyhow::bail!( "`systemd-run --user` failed to start the gamescope session (exit {status})" @@ -3696,9 +4128,10 @@ fn launch_session(client: &str, unit_name: &str, mode: Mode, hdr: bool) -> Resul // and the NVIDIA driver reclaims that context asynchronously — an instant relaunch pays // the reclaim serialization on top of device init and misses the 5 s window again. std::thread::sleep(Duration::from_millis(1500)); - let _ = Command::new("systemctl") - .args(["--user", "reset-failed", unit_name]) - .status(); + let _ = crate::proc::status_within( + Command::new("systemctl").args(["--user", "reset-failed", unit_name]), + UNIT_VERB_BUDGET, + ); start_unit(bind.as_ref())?; } std::thread::sleep(Duration::from_millis(500)); @@ -3708,11 +4141,16 @@ fn launch_session(client: &str, unit_name: &str, mode: Mode, hdr: bool) -> Resul /// Is the unit currently starting or up (`activating` / `active` — also `deactivating`: let a stop /// finish; the next poll tick sees the settled state)? Unknown/unreachable states report `true` so a /// systemctl hiccup can't trigger a relaunch storm. +/// +/// Budgeted at [`UNIT_STATE_BUDGET`], and this is the site where that matters most: it is polled +/// every 500 ms inside `launch_session`'s 45 s node-wait, so one unbounded `is-active` against a +/// wedged user manager blew the entire deadline in a single tick — and a timeout reads as `true`, +/// which is already the documented "don't storm" answer. fn unit_starting_or_active(unit: &str) -> bool { - let Ok(out) = Command::new("systemctl") - .args(["--user", "is-active", unit]) - .output() - else { + let Ok(out) = crate::proc::output_within( + Command::new("systemctl").args(["--user", "is-active", unit]), + UNIT_STATE_BUDGET, + ) else { return true; }; matches!( @@ -3902,9 +4340,30 @@ fn refresh_rate_args(session_hz: u32) -> Vec { ] } -/// Spawn `gamescope --backend headless -W w -H h -r hz -- `. The app comes from -/// `PUNKTFUNK_GAMESCOPE_APP` (default a no-op that just keeps gamescope alive — set it to a real -/// game/GL app for actual content, e.g. `steam -gamepadui` for the SteamOS-like session). +/// What a bare SPAWN will actually run, resolved ONCE per `create`: the per-session launch command +/// ([`VirtualDisplay::set_launch_command`]) wins; else the process-global `PUNKTFUNK_GAMESCOPE_APP` +/// (the documented manual fallback); else `None`, meaning [`spawn`]'s no-op keep-alive. Each level +/// is taken only if non-empty, so a blank per-session command transparently falls through to the +/// env. +/// +/// Split out of [`spawn`] because `create` has to gate the Steam single-instance free on the SAME +/// answer `spawn` later turns into `--steam`. It did not: it tested only the per-session command, +/// so the env fallback reached `spawn`, was recognised as Steam there, and got the integration +/// flag with none of the instance work — a nested second Steam that sees the box's and exits, +/// taking the spawn down with it. It also removes the second env read from the spawn path: one +/// [`crate::with_env_lock`] acquisition per create instead of two. +fn resolved_spawn_app(cmd: Option<&str>) -> Option { + cmd.map(str::to_string) + .filter(|s| !s.trim().is_empty()) + // Read the env fallback under the shared env lock so it can't race a concurrent session's + // `set_var` of the same key (security-review 2026-06-28 #7). + .or_else(|| crate::with_env_lock(|| std::env::var("PUNKTFUNK_GAMESCOPE_APP").ok())) + .filter(|s| !s.trim().is_empty()) +} + +/// Spawn `gamescope --backend headless -W w -H h -r hz -- `. `app` is this session's resolved +/// launch command ([`resolved_spawn_app`] — the per-session `set_launch_command`, else the +/// `PUNKTFUNK_GAMESCOPE_APP` fallback); `None` runs a no-op that just keeps gamescope alive. /// stdout/stderr go to `log` (this spawn's per-instance log, A5). The app is launched through a tiny /// shell wrapper that relays gamescope's `LIBEI_SOCKET` (set for its children) to [`ei_socket_file`] /// so the input injector can connect to gamescope's EIS server from outside — and (unless @@ -3916,21 +4375,10 @@ fn spawn( w: u32, h: u32, hz: u32, - cmd: Option<&str>, + app: Option, log: &std::path::Path, hdr: bool, ) -> Result { - // A non-empty per-session command (set via `set_launch_command`) wins; else the - // `PUNKTFUNK_GAMESCOPE_APP` env var (the documented manual fallback); else a no-op that keeps - // gamescope alive. Each level is taken only if non-empty, so a blank per-session cmd transparently - // falls through to the env (matching the pre-fix behaviour). - let app = cmd - .map(str::to_string) - .filter(|s| !s.trim().is_empty()) - // Read the env fallback under the shared env lock so it can't race a concurrent session's - // `set_var` of the same key (security-review 2026-06-28 #7). - .or_else(|| crate::with_env_lock(|| std::env::var("PUNKTFUNK_GAMESCOPE_APP").ok())) - .filter(|s| !s.trim().is_empty()); // A real app was requested (vs. the `sleep infinity` keep-alive) — used to scope the game-only // cursor-grab flag below. let game_launch = app.is_some(); @@ -4040,14 +4488,110 @@ impl Drop for GamescopeProc { mod tests { use super::{ cgroup_is_punktfunk_owned, cgroup_under_user_manager, connected_connector_under, - display_manager_unit_under, dm_plan, dm_survives_masked_unit, game_hz, hdr_args, - is_steam_launch, mask_unit, missing_flags, mode_mismatch, nested_wrapper_script, plan_bind, - release_autologin_mask, script_hardcodes_gamescope, sentinel_advanced, - shape_dedicated_command, switch_ends_mask_window, unmask_unit, xwayland_refusal_marker, - BindOff, BindPlan, DmHelperError, SessionBind, AUTOLOGIN_MASKED, DISTRO_GAMESCOPE_PATH, - STOPPED_AUTOLOGIN, X11_SOCKET_DIR, + display_manager_unit_under, dm_plan, dm_survives_masked_unit, game_hz, + gamescope_output_size, hdr_args, is_steam_launch, mask_unit, missing_flags, mode_mismatch, + nested_wrapper_script, plan_bind, release_autologin_mask, script_hardcodes_gamescope, + sentinel_advanced, shape_dedicated_command, switch_ends_mask_window, + takeover_state_is_live, unanimous_output_size, unmask_unit, xwayland_refusal_marker, + BindOff, BindPlan, DmHelperError, SessionBind, TakeoverState, AUTOLOGIN_MASKED, + DISTRO_GAMESCOPE_PATH, STOPPED_AUTOLOGIN, X11_SOCKET_DIR, }; + fn argv(s: &str) -> Vec { + s.split_whitespace().map(String::from).collect() + } + + /// The output-size probe answers for the box, and three consumers read it as "this session's + /// size" — the libei injector's coordinate scale among them. A box running two gamescopes (a + /// Game Mode session plus a nested per-title one, the shape `heads.rs` pins as normal) has no + /// single answer, and the old `find_map` returned whichever `/proc` `read_dir` yielded first. + /// "Cannot tell" is a state every consumer already handles; a confident wrong number is not. + #[test] + fn the_output_size_probe_refuses_to_pick_between_disagreeing_gamescopes() { + let session = argv("/usr/bin/gamescope -W 1920 -H 1080 --prefer-output HDMI-A-1"); + let nested = argv("gamescope --backend wayland -W 1280 -H 800"); + // One compositor, or several that agree — a plain answer. + assert_eq!( + unanimous_output_size(std::slice::from_ref(&session)), + Some((1920, 1080)) + ); + assert_eq!( + unanimous_output_size(&[session.clone(), session.clone()]), + Some((1920, 1080)) + ); + // Disagreement is unknown, not a coin flip — in either enumeration order. + assert_eq!( + unanimous_output_size(&[session.clone(), nested.clone()]), + None + ); + assert_eq!(unanimous_output_size(&[nested, session]), None); + // Nothing to go on at all. + assert_eq!(unanimous_output_size(&[]), None); + assert_eq!(unanimous_output_size(&[argv("gamescope --steam")]), None); + } + + /// `-W`/`-H` must be read as a pair off ONE argv, and the long spellings count: a half-answer + /// would otherwise be published as a monitor row (`heads_under`) or a pointer scale. + #[test] + fn output_size_needs_both_flags_from_the_same_argv() { + assert_eq!( + gamescope_output_size(&argv("gamescope -W 2560 -H 1440")), + Some((2560, 1440)) + ); + assert_eq!( + gamescope_output_size(&argv("gamescope --output-width 800 --output-height 600")), + Some((800, 600)) + ); + assert_eq!(gamescope_output_size(&argv("gamescope -W 2560")), None); + assert_eq!(gamescope_output_size(&argv("gamescope -H 1440")), None); + // The NESTED size (`-w`/`-h`) is a different thing and must never stand in for the output. + assert_eq!( + gamescope_output_size(&argv("gamescope -w 1280 -h 800")), + None + ); + } + + /// A persisted takeover exists so a host CRASH can heal the box. The file used to be written + /// (and, on the next start, adopted) only when one of the three *stealing* fields was set — so + /// a managed session that took nothing over, the one arm `takeover_live` grew specifically + /// because such a session was "ORPHANED forever after disconnect", was recorded as nothing at + /// all and its transient unit outlived the host. + #[test] + fn a_managed_session_alone_is_a_takeover_worth_persisting() { + let nothing = TakeoverState::default(); + assert!(!takeover_state_is_live(¬hing)); + let managed = TakeoverState { + managed_session: true, + ..Default::default() + }; + assert!(takeover_state_is_live(&managed)); + // …and the three original fields keep their meaning, each on its own. + assert!(takeover_state_is_live(&TakeoverState { + stopped_autologin: vec!["gamescope-session-plus@steam.service".into()], + ..Default::default() + })); + assert!(takeover_state_is_live(&TakeoverState { + steamos: true, + ..Default::default() + })); + assert!(takeover_state_is_live(&TakeoverState { + stopped_dm: Some("sddm.service".into()), + ..Default::default() + })); + } + + /// An older host's takeover file has neither new field; it must still parse (the box it + /// describes is mid-takeover, and refusing the file is refusing the restore). + #[test] + fn an_older_takeover_file_still_parses() { + let old = + r#"{"stopped_autologin":["gamescope-session-plus@steam.service"],"steamos":false}"#; + let state: TakeoverState = serde_json::from_str(old).expect("older file parses"); + assert_eq!(state.stopped_autologin.len(), 1); + assert!(!state.managed_session, "absent field defaults to false"); + assert!(takeover_state_is_live(&state)); + } + /// The HDR spawn flags are what make a nested game render HDR at all — and their absence is /// indistinguishable, on-glass, from a capture negotiation failure. Both flags are required: /// `--hdr-enabled` alone does nothing on the HEADLESS backend, whose connector hardcodes diff --git a/crates/pf-vdisplay/src/vdisplay/linux/gamescope/discovery.rs b/crates/pf-vdisplay/src/vdisplay/linux/gamescope/discovery.rs index 1a70fb31..0222fae7 100644 --- a/crates/pf-vdisplay/src/vdisplay/linux/gamescope/discovery.rs +++ b/crates/pf-vdisplay/src/vdisplay/linux/gamescope/discovery.rs @@ -5,10 +5,18 @@ use super::*; -/// Wait for gamescope to report its PipeWire node. Authoritative source: gamescope's own log -/// line `stream available on node ID: N` (its node carries `node.name=gamescope` on TWO objects -/// — the adapter and the inner stream — and only the advertised id is the correct capture -/// target). Falls back to `pw-dump` discovery if the log line doesn't show. +/// Budget for a `pw-dump` snapshot. Two facts make an unbounded one the worst call in this file: +/// it is polled every 300–500 ms from three separate 45 s loops, and it talks to the very daemon +/// this module documents gamescope as head-blocking below [`MIN_GAMESCOPE`] — so the failure mode +/// is not "slow", it is "never returns", on the session's own stream thread. Two seconds is far +/// above a populated graph's real cost; every caller already has a "couldn't ask" path. +const PW_DUMP_BUDGET: Duration = Duration::from_secs(2); + +/// Budget for a `gamescope --version` probe. It loads the binary and prints a banner — no Vulkan +/// device, no daemon — so anything approaching this bound is a binary that cannot run at all, +/// which is exactly what a `None`/`false` answer means to each caller. +const VERSION_PROBE_BUDGET: Duration = Duration::from_secs(2); + /// B2 (game-exit detection): confirm a **dedicated** gamescope session's game has exited. gamescope is /// a single-app compositor — it exits when its nested app exits — so once capture is lost, THIS /// session's `node_id` not reappearing within a short confirmation window means the game quit (vs. a @@ -159,16 +167,51 @@ pub(super) fn poll_managed_node(timeout: Duration) -> Option { } } +/// Wait for a freshly spawned gamescope to report its PipeWire node. Authoritative source: +/// gamescope's own log line `stream available on node ID: N` (its node carries +/// `node.name=gamescope` on TWO objects — the adapter and the inner stream — and only the +/// advertised id is the correct capture target). Falls back, at the deadline, to `pw-dump` +/// discovery SCOPED to this spawn's process tree (`child`'s pid, A5), so a coexisting gamescope's +/// node is never mistaken for ours. +/// +/// Takes the `Child` rather than a bare pid so it can **stop early when gamescope is already +/// dead**. A gamescope that fails `vkCreateDevice` exits in under a second, and polling its corpse +/// for the full 15 s bought nothing except a caller error that blamed the wrong thing ("headless +/// capture is unsupported on this GPU/driver"). `try_wait` turns that into an immediate `None` +/// while the log — which the caller names in the same error — still holds the real reason. pub(super) fn wait_for_node( timeout: Duration, log: &std::path::Path, - child_pid: u32, + child: &mut Child, ) -> Option { + let child_pid = child.id(); let deadline = Instant::now() + timeout; loop { if let Some(id) = node_from_log(log) { return Some(id); } + // Check for a node FIRST, then for death: a gamescope that published its node and then + // exited in the same tick still gives us the id, and the caller's own liveness handling + // (the keepalive `Child`, `kept_display_alive`) owns what happens next. + match child.try_wait() { + // Still running — keep waiting. + Ok(None) => {} + // Exited. One last scoped look (the node line may have been written between the two + // reads above), then give up rather than poll a corpse to the deadline. + Ok(Some(status)) => { + tracing::warn!( + pid = child_pid, + %status, + log = %log.display(), + "gamescope: the spawned process exited before publishing a PipeWire node — \ + not waiting out the rest of the budget" + ); + return node_from_log(log).or_else(|| find_gamescope_node_scoped(Some(child_pid))); + } + // `try_wait` itself failed (the child was reaped elsewhere, ECHILD): fall back to the + // old behaviour rather than inventing a death. + Err(_) => {} + } if Instant::now() >= deadline { // Last-resort fallback scoped to THIS spawn's process tree (A5), so a coexisting gamescope's // node isn't picked by mistake. @@ -197,7 +240,10 @@ fn node_from_log(log: &std::path::Path) -> Option { /// keep-alive reuse liveness probe ([`GamescopeDisplay::kept_display_alive`]): a kept gamescope node /// vanishes when its nested game exits, so a missing id means "recreate, don't reuse the corpse". pub(super) fn gamescope_node_present(node_id: u32) -> bool { - let Ok(out) = Command::new("pw-dump").arg(node_id.to_string()).output() else { + let Ok(out) = crate::proc::output_within( + Command::new("pw-dump").arg(node_id.to_string()), + PW_DUMP_BUDGET, + ) else { // pw-dump unavailable → don't block reuse (mark_failed is the backstop on a genuinely dead node). return true; }; @@ -229,7 +275,7 @@ pub(super) fn find_gamescope_node() -> Option { /// belong to OUR gamescope's process tree, so a coexisting foreign / other-session gamescope node is /// never mistaken for ours). `None` = any gamescope node (the managed/attach paths, single-session). fn find_gamescope_node_scoped(scope: Option) -> Option { - let out = Command::new("pw-dump").output().ok()?; + let out = crate::proc::output_within(&mut Command::new("pw-dump"), PW_DUMP_BUDGET).ok()?; let dump: serde_json::Value = serde_json::from_slice(&out.stdout).ok()?; let nodes = dump.as_array()?; let node_props = |obj: &serde_json::Value| -> Option<(u32, String, String, Option)> { @@ -302,7 +348,12 @@ fn find_gamescope_node_scoped(scope: Option) -> Option { /// most recently created (the live session). Returns the bare socket *name* (the injector /// resolves it against `XDG_RUNTIME_DIR`, matching libei's own `LIBEI_SOCKET` semantics). pub(super) fn find_gamescope_eis_socket() -> Option { - let runtime = std::env::var("XDG_RUNTIME_DIR").ok()?; + // Under the shared env lock: `session::apply_session_env` `set_var`s XDG_RUNTIME_DIR from the + // connect thread, and glibc's setenv/getenv pair is a data race the crate's own `lib.rs` + // documents as UB. The lock is not reentrant, so this must stay a read taken HERE and not + // hoisted into a caller — the only caller, `point_injector_at_eis`, holds nothing (its + // `ei_socket_file()` takes and releases the same lock separately). + let runtime = crate::with_env_lock(|| std::env::var("XDG_RUNTIME_DIR").ok())?; let mut live: Vec<(std::time::SystemTime, String)> = Vec::new(); for entry in std::fs::read_dir(&runtime).ok()?.flatten() { let name = entry.file_name().to_string_lossy().into_owned(); @@ -328,11 +379,12 @@ pub(super) fn find_gamescope_eis_socket() -> Option { /// not require any particular desktop to be running. Quiet (no version warning — that's for the /// create path); just checks the binary executes. pub(crate) fn is_available() -> bool { - std::process::Command::new(gamescope_bin()) - .arg("--version") - .output() - .map(|o| o.status.success()) - .unwrap_or(false) + crate::proc::output_within( + Command::new(gamescope_bin()).arg("--version"), + VERSION_PROBE_BUDGET, + ) + .map(|o| o.status.success()) + .unwrap_or(false) } /// The gamescope binary this host spawns, resolved ONCE per process: @@ -400,14 +452,20 @@ fn which_in_path(name: &str) -> Option { /// /// Monotonic, so one probe answers every capability: /// * `1` — 10-bit BT.2020/PQ capture formats ([`gamescope_hdr_capable`]); -/// * `2` — …and `--pipewire-composite-cursor` ([`gamescope_can_composite_cursor`]). +/// * `2` — …and `--pipewire-composite-cursor` ([`gamescope_can_composite_cursor`]); +/// * `3` — …and `--custom-refresh-rates` ([`gamescope_can_offer_refresh_rates`]); +/// * `4` — …and `--pipewire-composite-external-overlay` +/// ([`gamescope_can_composite_external_overlay`]). /// /// When upstream takes the functional patches this becomes a plain version floor, exactly like /// [`MIN_GAMESCOPE_OVERLAY`]. fn gamescope_patch_level() -> u32 { static LEVEL: std::sync::OnceLock = std::sync::OnceLock::new(); *LEVEL.get_or_init(|| { - let Ok(out) = Command::new(gamescope_bin()).arg("--version").output() else { + let Ok(out) = crate::proc::output_within( + Command::new(gamescope_bin()).arg("--version"), + VERSION_PROBE_BUDGET, + ) else { return 0; }; // The banner goes to stderr on some builds, stdout on others (same as the version gate). @@ -530,7 +588,8 @@ fn parse_patch_level(banner: &str) -> u32 { /// WSI-layer check has to compare TWO binaries — ours and the distro's — and a `None` there means /// "leave the layer alone", not "assume old". pub(super) fn gamescope_version_of(bin: &std::path::Path) -> Option<(u32, u32, u32)> { - let out = Command::new(bin).arg("--version").output().ok()?; + let out = crate::proc::output_within(Command::new(bin).arg("--version"), VERSION_PROBE_BUDGET) + .ok()?; // Same stdout/stderr split as the version gate: builds disagree on where the banner goes. let text = format!( "{}{}", @@ -549,8 +608,15 @@ const MIN_GAMESCOPE: (u32, u32, u32) = (3, 16, 22); /// the overlay-window paint (gated on the consumer negotiating `gamescope_focus_appid == 0`, which /// we do by never advertising that property — see the capturer's EnumFormat builders) first ships /// in 3.16.23 (gamescope commits `ccd62074` + `f8b33d38`). Below this the overlay is *never* in the -/// node, so it cannot appear in the stream no matter what the host does. The cursor and -/// external-overlay / notification layers are excluded on *every* version (handled host-side). +/// node, so it cannot appear in the stream no matter what the host does. +/// +/// On a **stock** gamescope the cursor and external-overlay / notification layers are excluded from +/// `paint_pipewire` on every version, and the host handles the cursor itself. punktfunk's own build +/// puts both back: `--pipewire-composite-cursor` at patch level 2+ +/// ([`gamescope_can_composite_cursor`], which is what suppresses the host-side blend) and +/// `--pipewire-composite-external-overlay` at 4+ ([`gamescope_can_composite_external_overlay`]) — +/// see [`gamescope_patch_level`]. So "the overlay is missing from the stream" is a question about +/// which flags reached the running compositor, not about host-side compositing. const MIN_GAMESCOPE_OVERLAY: (u32, u32, u32) = (3, 16, 23); /// Best-effort: warn if the installed gamescope is older than [`MIN_GAMESCOPE`] (capture is @@ -558,10 +624,11 @@ const MIN_GAMESCOPE_OVERLAY: (u32, u32, u32) = (3, 16, 23); /// the stream). Parsing failures are silent (don't block a possibly-fine custom build) — this is a /// diagnostic, not a gate. Returns the parsed version when it could read one. pub(super) fn check_gamescope_version() -> Option<(u32, u32, u32)> { - let out = Command::new(gamescope_bin()) - .arg("--version") - .output() - .ok()?; + let out = crate::proc::output_within( + Command::new(gamescope_bin()).arg("--version"), + VERSION_PROBE_BUDGET, + ) + .ok()?; // gamescope prints the version banner to stderr on some builds, stdout on others. let text = format!( "{}{}", diff --git a/crates/pf-vdisplay/src/vdisplay/linux/gamescope/heads.rs b/crates/pf-vdisplay/src/vdisplay/linux/gamescope/heads.rs index 22bc667c..2b73300b 100644 --- a/crates/pf-vdisplay/src/vdisplay/linux/gamescope/heads.rs +++ b/crates/pf-vdisplay/src/vdisplay/linux/gamescope/heads.rs @@ -34,24 +34,28 @@ pub(crate) fn list_monitors() -> anyhow::Result> { Ok(heads_under( Path::new("/sys/class/drm"), &super::gamescope_argvs(), - super::current_gamescope_output_size(), )) } /// [`list_monitors`] against an arbitrary sysfs root and a supplied argv set — the unit-testable -/// core. `output_size` is gamescope's own `-W`/`-H`, which OUTRANKS the EDID's preferred timing -/// because it is the size the capture node actually produces. -fn heads_under( - base: &Path, - argvs: &[Vec], - output_size: Option<(u32, u32)>, -) -> Vec { +/// core. +/// +/// The head's size comes from the `-W`/`-H` of the argv selected HERE, which OUTRANKS the EDID's +/// preferred timing because it is the size the capture node actually produces. It used to arrive as +/// a parameter filled by a scan over ALL gamescopes on the box — including the nested child this +/// function had just deliberately rejected, and any headless one the crate spawned itself. On a +/// Deck driving eDP-1 at 1280x800 with a game nested at `-W 1920 -H 1080`, the panel was listed as +/// 1920x1080, and `mirror::create` publishes that row verbatim as the `preferred_mode` the stream +/// negotiates against — a mode the composited node never produces, and one `check_mirrorable` waves +/// through because it only rejects `0x0`. +fn heads_under(base: &Path, argvs: &[Vec]) -> Vec { // A gamescope that isn't on DRM has no head of its own. Any DRM-backed one qualifies the box: // a Deck streaming from Game Mode often has a second, nested gamescope running the game inside // the session one, and that child must not disqualify its parent. let Some(argv) = argvs.iter().find(|a| drives_drm(a)) else { return Vec::new(); }; + let output_size = super::gamescope_output_size(argv); let connected = connected_connectors(base); if connected.is_empty() { return Vec::new(); @@ -342,7 +346,6 @@ mod tests { let heads = heads_under( &base, &[argv("/usr/bin/gamescope --prefer-output HDMI-A-1 --steam")], - None, ); assert_eq!(heads.len(), 1); assert_eq!(heads[0].connector, "HDMI-A-1"); @@ -366,12 +369,12 @@ mod tests { "gamescope --backend sdl", ] { assert!( - heads_under(&base, &[argv(a)], None).is_empty(), + heads_under(&base, &[argv(a)]).is_empty(), "expected no heads for {a:?}" ); } // No gamescope at all is the same answer, not an error. - assert!(heads_under(&base, &[], None).is_empty()); + assert!(heads_under(&base, &[]).is_empty()); std::fs::remove_dir_all(&base).unwrap(); } @@ -384,12 +387,15 @@ mod tests { &base, &[ argv("gamescope --backend wayland -W 1280 -H 800"), - argv("/usr/bin/gamescope --prefer-output *,eDP-1 --steam"), + argv("/usr/bin/gamescope --prefer-output *,eDP-1 -W 2560 -H 1440 --steam"), ], - None, ); assert_eq!(heads.len(), 1); assert_eq!(heads[0].connector, "eDP-1"); + // …and the size comes from the DRM PARENT, not from the nested child listed first. Reading + // it off any-gamescope-on-the-box is what published a 1280x800 panel as the mirror's + // preferred mode on a box where the game happened to be nested at a different size. + assert_eq!((heads[0].width, heads[0].height), (2560, 1440)); std::fs::remove_dir_all(&base).unwrap(); } @@ -404,7 +410,7 @@ mod tests { ("card1-HDMI-A-1", "connected\n", "enabled\n"), ], ); - let heads = heads_under(&base, &[argv("gamescope --prefer-output *,eDP-1")], None); + let heads = heads_under(&base, &[argv("gamescope --prefer-output *,eDP-1")]); assert_eq!(heads.len(), 1); assert_eq!(heads[0].connector, "eDP-1"); std::fs::remove_dir_all(&base).unwrap(); @@ -421,7 +427,7 @@ mod tests { ("card1-HDMI-A-1", "connected\n", "enabled\n"), ], ); - let heads = heads_under(&base, &[argv("gamescope --steam")], None); + let heads = heads_under(&base, &[argv("gamescope --steam")]); assert_eq!( heads .iter() @@ -440,7 +446,7 @@ mod tests { "unplugged", &[("card1-HDMI-A-1", "disconnected\n", "disabled\n")], ); - assert!(heads_under(&base, &[argv("gamescope --steam")], None).is_empty()); + assert!(heads_under(&base, &[argv("gamescope --steam")]).is_empty()); std::fs::remove_dir_all(&base).unwrap(); } @@ -452,7 +458,6 @@ mod tests { let heads = heads_under( &base, &[argv("gamescope -W 2560 -H 1440 --prefer-output HDMI-A-1")], - Some((2560, 1440)), ); assert_eq!((heads[0].width, heads[0].height), (2560, 1440)); std::fs::remove_dir_all(&base).unwrap(); @@ -511,7 +516,6 @@ mod tests { &[argv( "gamescope --nested-refresh 30 --prefer-output HDMI-A-1", )], - None, ); assert_eq!(heads[0].refresh_mhz, 60_000); assert_eq!(heads[0].mode_label(), "1920x1080@60"); @@ -539,7 +543,7 @@ mod tests { "3840x2160\n1920x1080\n", ) .unwrap(); - let heads = heads_under(&base, &[argv("gamescope --steam")], None); + let heads = heads_under(&base, &[argv("gamescope --steam")]); assert_eq!((heads[0].width, heads[0].height), (3840, 2160)); std::fs::remove_dir_all(&base).unwrap(); } diff --git a/crates/pf-vdisplay/src/vdisplay/linux/gamescope/splash.rs b/crates/pf-vdisplay/src/vdisplay/linux/gamescope/splash.rs index d99fe239..4275ca68 100644 --- a/crates/pf-vdisplay/src/vdisplay/linux/gamescope/splash.rs +++ b/crates/pf-vdisplay/src/vdisplay/linux/gamescope/splash.rs @@ -143,17 +143,57 @@ pub(crate) fn run() -> Result<()> { } } +/// How long the splash waits for the session's X server before giving up. +const CONNECT_BUDGET: Duration = Duration::from_secs(10); + /// Connect to the session's `DISPLAY`, retrying briefly — gamescope sets the variable before /// exec'ing the nested command, but a slow Xwayland under cold driver init gets a grace window. +/// +/// The retry runs on a worker thread and the budget is enforced by `recv_timeout` rather than by +/// re-checking a deadline between attempts. The difference is the whole point: `x11rb::connect` +/// has no timeout of its own, so against an Xwayland that ACCEPTED the socket and then never +/// answered the setup handshake it blocks indefinitely — and a deadline consulted only in the +/// `Err` arm is never reached at all. That is the failure this module exists to prevent, from the +/// inside: no painting client, no composite, no PipeWire buffers, and the capture dies on its 10 s +/// first-frame timeout having never logged "gamescope splash: mapped", so the diagnosis points +/// anywhere but here. +/// +/// A worker still stuck in `connect` is abandoned rather than joined; it is one thread in a +/// process whose whole job is this window, and the alternative is the hang. fn connect_with_retry() -> Result<(RustConnection, usize)> { - let deadline = std::time::Instant::now() + Duration::from_secs(10); - loop { - match x11rb::connect(None) { - Ok(ok) => return Ok(ok), - Err(e) if std::time::Instant::now() >= deadline => { - return Err(e).context("gamescope splash: could not connect to the session DISPLAY") + let (tx, rx) = std::sync::mpsc::channel(); + std::thread::Builder::new() + .name("pf-splash-x11-connect".into()) + .spawn(move || { + let deadline = std::time::Instant::now() + CONNECT_BUDGET; + loop { + match x11rb::connect(None) { + Ok(ok) => { + let _ = tx.send(Ok(ok)); + return; + } + Err(e) if std::time::Instant::now() >= deadline => { + let _ = tx.send(Err(e)); + return; + } + Err(_) => std::thread::sleep(Duration::from_millis(200)), + } } - Err(_) => std::thread::sleep(Duration::from_millis(200)), + }) + .context("gamescope splash: could not start the X connect thread")?; + // A little past the worker's own deadline, so a connect that merely finished slowly still wins + // and only a genuinely blocked one trips this. + match rx.recv_timeout(CONNECT_BUDGET + Duration::from_secs(1)) { + Ok(Ok(conn)) => Ok(conn), + Ok(Err(e)) => Err(e).context("gamescope splash: could not connect to the session DISPLAY"), + Err(_) => { + tracing::warn!( + secs = CONNECT_BUDGET.as_secs(), + "gamescope splash: the session's X server accepted no connection and never \ + answered — giving up. Nothing will paint in this gamescope, so it will composite \ + nothing and the capture will starve; the gamescope log is where the reason is." + ); + anyhow::bail!("gamescope splash: connecting to the session DISPLAY did not return") } } }