diff --git a/crates/pf-vdisplay/src/vdisplay/identity.rs b/crates/pf-vdisplay/src/vdisplay/identity.rs index 6115e8ae..e0cfda51 100644 --- a/crates/pf-vdisplay/src/vdisplay/identity.rs +++ b/crates/pf-vdisplay/src/vdisplay/identity.rs @@ -21,6 +21,7 @@ //! Persisted to `/display-identity.json` (migrated from the legacy Windows //! `pf-vdisplay-identity.json`) so ids — and the client→config association — survive host restarts. +use std::collections::BTreeSet; use std::path::PathBuf; use std::sync::{Mutex, OnceLock}; @@ -78,12 +79,38 @@ impl DisplayIdentityMap { pub(crate) fn load() -> Self { let dir = pf_paths::config_dir(); let path = dir.join(FILE); - let bytes = std::fs::read(&path) - .or_else(|_| std::fs::read(dir.join(LEGACY_FILE))) - .ok(); - let mut store = bytes - .and_then(|b| serde_json::from_slice::(&b).ok()) - .unwrap_or_default(); + let (from, bytes) = match std::fs::read(&path) { + Ok(b) => (path.clone(), Some(b)), + Err(_) => { + let legacy = dir.join(LEGACY_FILE); + match std::fs::read(&legacy) { + Ok(b) => (legacy, Some(b)), + // No file at all is the ordinary first-run case — not worth a word. + Err(_) => (path.clone(), None), + } + } + }; + let mut store = match bytes { + Some(b) => match serde_json::from_slice::(&b) { + Ok(s) => s, + Err(e) => { + // An UNPARSEABLE map used to be swallowed into `Default::default()`, and the very + // next `resolve` persisted that empty store OVER the file — silently discarding + // every client's Windows EDID serial / KWin `Virtual-punktfunk-` and the + // per-display DPI the OS keyed to them. Say so, and move the file aside so the + // damage is recoverable by hand (same treatment `display-presets.json` gets). + tracing::warn!( + path = %from.display(), + error = %e, + "display-identity map is unreadable — starting a fresh one; \ + the old file is kept as .bad (every client re-derives its display id once)" + ); + let _ = std::fs::rename(&from, from.with_extension("json.bad")); + Store::default() + } + }, + None => Store::default(), + }; // SANITIZE a hand-edited / corrupt / cross-version file before trusting it: resolve()'s // found-entry branch returns the stored id verbatim, so an out-of-range id (0 = the "auto" // sentinel, or > MAX_ID) or a duplicate id/key would flow straight into the display identity. @@ -100,7 +127,17 @@ impl DisplayIdentityMap { /// The stable id (`1..=15`) for the client `key` ([`identity_key`]): its remembered id, or a /// freshly assigned one (lowest free, else LRU-evict at the cap). Bumps the entry to MRU and persists. - pub(crate) fn resolve(&mut self, key: &str) -> u32 { + /// + /// `live` is the set of ids that currently drive a REAL display (the Windows manager's slot keys + /// / the Linux pool's `identity_slot`s). An id in it is never evicted, and when every eviction + /// candidate is live this **refuses** (`None`) rather than handing the newcomer an id that is + /// already someone else's monitor. That is not hypothetical: the id keys the Windows manager's + /// slot map, whose plain-JOIN branch attaches an arriving session to whatever monitor the slot + /// already holds — so evicting a live id handed client B client A's streaming monitor, capture + /// target and all. Refusing costs the newcomer its stable identity (upstream falls back to the + /// shared/auto slot: `resolve_slot` → `None`, `slot_id_for` → `0`); evicting cost a live client + /// its session. + pub(crate) fn resolve(&mut self, key: &str, live: &BTreeSet) -> Option { self.store.tick = self.store.tick.wrapping_add(1); let now = self.store.tick; @@ -108,32 +145,43 @@ impl DisplayIdentityMap { e.seen = now; let id = e.id; self.persist(); - return id; + return Some(id); } - // New client: prefer the lowest free id in 1..=MAX_ID; if all are taken, evict the LRU entry and - // reuse its id (the evicted client re-establishes its scaling once on its next connect). - let id = (1..=MAX_ID) - .find(|i| !self.store.entries.iter().any(|e| e.id == *i)) - .unwrap_or_else(|| { + // New client: prefer the lowest free id in 1..=MAX_ID; if all are taken, evict the + // least-recently-seen entry that is NOT live and reuse its id (that client re-establishes its + // scaling once on its next connect). + let id = match (1..=MAX_ID).find(|i| !self.store.entries.iter().any(|e| e.id == *i)) { + Some(free) => free, + None => { let lru = self .store .entries .iter() .enumerate() + .filter(|(_, e)| !live.contains(&e.id)) .min_by_key(|(_, e)| e.seen) - .map(|(i, _)| i) - .expect("entries are non-empty whenever every id 1..=MAX_ID is taken"); - let evicted = self.store.entries.remove(lru); - evicted.id - }); + .map(|(i, _)| i); + let Some(lru) = lru else { + tracing::warn!( + cap = MAX_ID, + live = live.len(), + "display identity map is full and every id is driving a live display — \ + this client gets the shared/auto display identity (no persisted per-client \ + scaling) rather than displacing a live one" + ); + return None; + }; + self.store.entries.remove(lru).id + } + }; self.store.entries.push(Entry { key: key.to_string(), id, seen: now, }); self.persist(); - id + Some(id) } /// Persist atomically (temp file + rename). Best-effort: a write failure just means a restart may @@ -168,7 +216,8 @@ pub(crate) fn global() -> &'static Mutex { /// Resolve the connecting client's stable slot id per the `identity` policy. When no policy is /// configured, `default` applies — **PerClient on Windows / Shared on Linux**, preserving each /// platform's historical behavior (Windows always keyed monitors per-client; Linux used one shared -/// output name). `None` ⇒ shared / anonymous → the backend uses its base name / auto slot. +/// output name). `None` ⇒ shared / anonymous (or the map [refused](DisplayIdentityMap::resolve) an +/// id because every one is live) → the backend uses its base name / auto slot. pub(crate) fn resolve_slot( fp: Option<[u8; 32]>, mode: (u32, u32), @@ -185,12 +234,40 @@ pub(crate) fn resolve_slot( Identity::PerClientMode => true, }; let fp = fp?; - Some( - global() - .lock() - .unwrap() - .resolve(&identity_key(fp, mode, per_client_mode)), - ) + // Sample the live ids BEFORE taking the map lock, never under it: the sources below take the + // Windows manager's `state` lock / the Linux pool lock, and this map is reached from inside a + // backend `create` — a lock order of (display owner → identity map) in both directions would be + // a deadlock. One direction only, and the map lock stays a leaf. + let live = live_slot_ids(); + global() + .lock() + .unwrap() + .resolve(&identity_key(fp, mode, per_client_mode), &live) +} + +/// The identity slots currently driving a REAL display — the eviction guard for +/// [`DisplayIdentityMap::resolve`]. Windows reads the manager's slot map (the key IS the identity +/// slot); Linux reads the registry pool's per-entry `identity_slot`. Both include KEPT +/// (lingering/pinned) displays on purpose: a kept display is a live compositor/driver resource whose +/// owner is expected back, and the whole point of the id is that the reconnect finds it again. +/// Anonymous (`0`) is not an identity and never blocks an assignment. +fn live_slot_ids() -> BTreeSet { + #[cfg(target_os = "windows")] + { + crate::manager::snapshot() + .into_iter() + .map(|i| i.slot_id) + .filter(|s| *s != 0) + .collect() + } + #[cfg(target_os = "linux")] + { + crate::registry::live_identity_slots() + } + #[cfg(not(any(target_os = "windows", target_os = "linux")))] + { + BTreeSet::new() + } } // --------------------------------------------------------------------------------------- @@ -306,24 +383,31 @@ mod tests { } } + /// Nothing is streaming — the ordinary case, where the live set never constrains anything. + fn nothing_live() -> BTreeSet { + BTreeSet::new() + } + #[test] fn stable_across_calls_and_distinct_per_client() { let mut m = temp_map("stable"); - let a1 = m.resolve(&identity_key(fp(1), (1920, 1080), false)); - let b = m.resolve(&identity_key(fp(2), (1920, 1080), false)); - let a2 = m.resolve(&identity_key(fp(1), (1280, 720), false)); // per-client: mode ignored + let a1 = m.resolve(&identity_key(fp(1), (1920, 1080), false), ¬hing_live()); + let b = m.resolve(&identity_key(fp(2), (1920, 1080), false), ¬hing_live()); + // per-client: mode ignored + let a2 = m.resolve(&identity_key(fp(1), (1280, 720), false), ¬hing_live()); assert_eq!(a1, a2, "same client → same id (per-client ignores mode)"); assert_ne!(a1, b, "distinct clients → distinct ids"); - assert!((1..=MAX_ID).contains(&a1) && (1..=MAX_ID).contains(&b)); + assert!(a1.is_some_and(|i| (1..=MAX_ID).contains(&i))); + assert!(b.is_some_and(|i| (1..=MAX_ID).contains(&i))); let _ = std::fs::remove_file(&m.path); } #[test] fn per_client_mode_splits_by_resolution() { let mut m = temp_map("permode"); - let hd = m.resolve(&identity_key(fp(1), (1920, 1080), true)); - let uhd = m.resolve(&identity_key(fp(1), (3840, 2160), true)); - let hd2 = m.resolve(&identity_key(fp(1), (1920, 1080), true)); + let hd = m.resolve(&identity_key(fp(1), (1920, 1080), true), ¬hing_live()); + let uhd = m.resolve(&identity_key(fp(1), (3840, 2160), true), ¬hing_live()); + let hd2 = m.resolve(&identity_key(fp(1), (1920, 1080), true), ¬hing_live()); assert_ne!(hd, uhd, "same client, different resolution → different id"); assert_eq!(hd, hd2, "same client + resolution → same id"); let _ = std::fs::remove_file(&m.path); @@ -333,16 +417,72 @@ mod tests { fn lru_eviction_reuses_an_id_at_the_cap() { let mut m = temp_map("lru"); for n in 1..=15u8 { - m.resolve(&identity_key(fp(n), (1920, 1080), false)); + m.resolve(&identity_key(fp(n), (1920, 1080), false), ¬hing_live()); } - let _ = m.resolve(&identity_key(fp(2), (1920, 1080), false)); // touch 2 so 1 is LRU - let id16 = m.resolve(&identity_key(fp(16), (1920, 1080), false)); + // touch 2 so 1 is LRU + let _ = m.resolve(&identity_key(fp(2), (1920, 1080), false), ¬hing_live()); + let id16 = m + .resolve(&identity_key(fp(16), (1920, 1080), false), ¬hing_live()) + .expect("nothing is live → the LRU id is free to take"); assert!((1..=MAX_ID).contains(&id16)); assert_eq!(m.store.entries.len(), 15, "cap holds at 15 entries"); assert!(m.store.entries.iter().all(|e| (1..=MAX_ID).contains(&e.id))); let _ = std::fs::remove_file(&m.path); } + /// 10.2: the LRU victim is chosen among ids that are NOT driving a display. Handing the LRU id + /// to a newcomer while its owner streams is what let the Windows manager's plain-JOIN branch + /// attach the newcomer to the live client's monitor. + #[test] + fn lru_eviction_never_takes_a_live_id() { + let mut m = temp_map("lru-live"); + let mut ids = Vec::new(); + for n in 1..=15u8 { + ids.push( + m.resolve(&identity_key(fp(n), (1920, 1080), false), ¬hing_live()) + .unwrap(), + ); + } + // fp(1) is the least-recently-seen — and it is the one that is streaming. + let lru_id = ids[0]; + let live: BTreeSet = [lru_id].into_iter().collect(); + let id16 = m + .resolve(&identity_key(fp(16), (1920, 1080), false), &live) + .expect("14 idle ids remain — one of them is the victim"); + assert_ne!(id16, lru_id, "must not take the id of a live display"); + assert_eq!(id16, ids[1], "the next-least-recently-seen IDLE id instead"); + // The live client's mapping is untouched, so its reconnect still finds its own display. + assert_eq!( + m.resolve(&identity_key(fp(1), (1920, 1080), false), &live), + Some(lru_id) + ); + let _ = std::fs::remove_file(&m.path); + } + + /// Fail-closed at the extreme: every id live ⇒ refuse, rather than displace a streaming client. + /// The caller degrades to the shared/auto identity (`resolve_slot` → `None`, `slot_id_for` → 0). + #[test] + fn refuses_rather_than_evicting_when_every_id_is_live() { + let mut m = temp_map("lru-all-live"); + let mut live = BTreeSet::new(); + for n in 1..=15u8 { + live.insert( + m.resolve(&identity_key(fp(n), (1920, 1080), false), &BTreeSet::new()) + .unwrap(), + ); + } + assert_eq!( + m.resolve(&identity_key(fp(16), (1920, 1080), false), &live), + None + ); + assert_eq!(m.store.entries.len(), 15, "nothing was evicted"); + // A KNOWN client is still resolved even when everything is live — it owns that id already. + assert!(m + .resolve(&identity_key(fp(3), (1920, 1080), false), &live) + .is_some()); + let _ = std::fs::remove_file(&m.path); + } + #[test] fn key_composition() { assert_eq!(identity_key(fp(0xab), (1920, 1080), false).len(), 64); // hex fp only diff --git a/crates/pf-vdisplay/src/vdisplay/registry.rs b/crates/pf-vdisplay/src/vdisplay/registry.rs index 604ac623..82aa6f57 100644 --- a/crates/pf-vdisplay/src/vdisplay/registry.rs +++ b/crates/pf-vdisplay/src/vdisplay/registry.rs @@ -108,7 +108,17 @@ pub fn acquire( let _ = (quit, supersedes); vd.create(mode) }; - if out.is_ok() { + // A `Created` event means a display came into EXISTENCE. A keep-alive reuse does not: the + // console was already told about that display when it was created (and at the mode it was + // created with), so emitting again double-counted it — and, since the pool emits exactly one + // `Released` when it finally goes away, left a phantom display in the console's live view for + // the rest of the host's life. `reused_gen` is the pool's own answer to "did this already + // exist"; every non-pooling platform creates on every acquire. + #[cfg(target_os = "linux")] + let created = matches!(&out, Ok(o) if o.reused_gen.is_none()); + #[cfg(not(target_os = "linux"))] + let created = out.is_ok(); + if created { crate::emit_display_event(crate::DisplayEvent::Created { backend: backend.to_string(), width: mode.width, @@ -175,6 +185,11 @@ pub fn release(slot: Option) -> usize { let _ = slot; 0 }; + // Linux emits from every teardown site the pool owns (lease drop, linger expiry, the A2 dead- + // display teardown, this mgmt release, the A4 invalidation), so emitting here as well would + // double-count the one path it already covers. The Windows manager has no such hook — this + // endpoint is the only teardown the console can learn about — so it emits here. + #[cfg(not(target_os = "linux"))] if released > 0 { crate::emit_display_event(crate::DisplayEvent::Released { count: released as u32, @@ -220,28 +235,42 @@ pub fn invalidate_backend(backend: &str) { let _ = backend; } -// --------------------------------------------------------------------------------------------- -// Linux keep-alive pool -// --------------------------------------------------------------------------------------------- - +/// The identity slots currently driving a pooled display — the eviction guard the identity map +/// consults before it hands a newly-arriving client an id (see +/// [`identity::live_slot_ids`](crate::identity)). KEPT (lingering/pinned) entries count: their +/// compositor output still exists and their owner is expected back. Linux-only — Windows reads the +/// manager's slot map instead, and no other platform pools displays. #[cfg(target_os = "linux")] -mod linux { - use std::sync::atomic::{AtomicBool, AtomicU64, Ordering}; - use std::sync::{Arc, Mutex, Once, OnceLock}; - use std::time::{Duration, Instant}; +pub(crate) fn live_identity_slots() -> std::collections::BTreeSet { + linux::live_identity_slots() +} - use anyhow::Result; +// --------------------------------------------------------------------------------------------- +// Pool core — the platform-NEUTRAL half of the Linux keep-alive pool +// --------------------------------------------------------------------------------------------- + +/// The pool's pure machinery: the entry record, the group/reuse/budget predicates, the expiry sweep +/// and the `/display/state` assembly. None of it touches an OS API or a `cfg`-gated type — only +/// `mod linux` (which owns the global pool, the backend calls and the `VirtualOutput`'s Linux-only +/// fields) does. Split out so the rules the sweep findings live in are unit-testable on any host +/// this crate builds on, instead of only on the one CI leg that compiles `mod linux`. +/// +/// Off Linux nothing calls it, hence the conditional `dead_code` allowance — kept conditional so a +/// genuinely dead helper is still reported on the platform that runs this code. +#[cfg_attr(not(target_os = "linux"), allow(dead_code))] +mod pool { + use std::time::Instant; use super::DisplayInfo; - use crate::lifecycle::{self, Release}; - use crate::policy::{self, Layout, Linger}; - use crate::{Mode, VirtualDisplay, VirtualOutput}; + use crate::lifecycle; + use crate::policy::{Layout, Linger}; + use crate::Mode; /// One pooled display: the lifecycle state + the backend's REAL keepalive (kept alive here so the /// compositor output — and thus its PipeWire `node_id` — survives past the session), plus the /// capture coordinates a reconnecting session needs. - struct Entry { - life: lifecycle::State, + pub(super) struct Entry { + pub(super) life: lifecycle::State, /// The backend's keepalive (KWin Wayland conn / Mutter D-Bus session / gamescope child). Its /// `Drop` releases the compositor output — so it is dropped only on teardown/expiry. // NEVER READ, ON PURPOSE — `dead_code` is right that nothing loads this field and wrong @@ -249,45 +278,701 @@ mod linux { // `Drop` is what releases the compositor output. Deleting it as "unused" would release // every pooled display the instant it was created. #[allow(dead_code)] - keepalive: Box, - node_id: u32, - preferred_mode: Option<(u32, u32, u32)>, - mode: Mode, - backend: &'static str, + pub(super) keepalive: Box, + pub(super) node_id: u32, + pub(super) preferred_mode: Option<(u32, u32, u32)>, + pub(super) mode: Mode, + pub(super) backend: &'static str, /// The identity slot the backend resolved for this display (KWin per-slot naming; `None` for /// shared/anonymous or a backend with no per-client identity) — keys the group arrangement + /// the `/display/state` slot. Captured at create; kept across a keep-alive reuse. - identity_slot: Option, + pub(super) identity_slot: Option, /// The topology-restore action for this display's GROUP (design §6.1): re-enable the physical /// outputs an `exclusive` topology disabled. At most ONE entry per group carries it (the first /// exclusive session); on teardown it hands off to a surviving sibling, and only runs when the /// group's last member drops. `None` for extend/primary and non-first / non-exclusive members. - topology_restore: Option, + pub(super) topology_restore: Option, /// The launch command this display was created with (`design/gamemode-and-dedicated-sessions.md` /// A2): keep-alive reuse requires an exact match, so a kept spawn running game A never serves a /// session launching game B. `None` = a plain desktop / no nested command. - launch: Option, + pub(super) launch: Option, /// The session epoch at creation (A4). Reuse requires an epoch match; the linger timer reaps /// entries whose epoch is stale (their compositor instance was replaced under them). - epoch: u64, - /// Generation stamp: a [`DisplayLease`] only releases if its gen still matches (a stale lease + pub(super) epoch: u64, + /// Generation stamp: a `DisplayLease` only releases if its gen still matches (a stale lease /// — its entry was reused + re-stamped — is a no-op). - gen: u64, + pub(super) gen: u64, /// The out-of-band-cursor mode this display was CREATED with (Phase B): metadata-pointer /// (cursor-channel session) vs compositor-embedded. Reuse requires an exact match — a kept /// embedded display has no cursor metadata for a channel session to forward, and a kept /// metadata display would leave a channel-less session with no pointer in its frames. - hw_cursor: bool, + pub(super) hw_cursor: bool, /// The stream colourimetry this display was BROUGHT UP for: 10-bit BT.2020/PQ (HDR) vs /// 8-bit SDR. Reuse requires an exact match — a kept SDR gamescope was spawned without /// `--hdr-enabled`, so an HDR session reusing it would get a game with no HDR surfaces /// under a stream that negotiated PQ over an SDR composite (wrong, and not obviously /// broken); the reverse would try to negotiate 8-bit off a PQ composite. - hdr: bool, + pub(super) hdr: bool, } /// A per-group topology-restore action (see [`Entry::topology_restore`]). - type Restore = Box; + pub(super) type Restore = Box; + + /// The display **group** a backend+display belongs to (design §6.1). The desktop compositors + /// (KWin/Mutter/wlroots) put every managed output on ONE desktop → one group per backend. A + /// gamescope **spawn** is an independent nested session per client (no shared desktop), so each + /// gamescope display is its OWN group — never auto-rowed against, or topology-/restore-grouped with, + /// another gamescope session. + pub(super) fn group_key(backend: &str, gen: u64) -> String { + if backend == "gamescope" { + format!("gamescope#{gen}") + } else { + backend.to_string() + } + } + + /// Is the pooled entry `(e_backend, e_gen)` a member of the group the display `(backend, gen)` + /// belongs to — the ONE definition of membership, shared by the restore hand-off, the + /// first-in-group probe and the layout collection. + /// + /// `supersedes` names the display a mid-stream mode switch is REPLACING (create-before-drop): it + /// is still in the pool, and still Active, but it is leaving and its successor takes its place — + /// so it is not a sibling of its own replacement. Counting it made the newcomer both defer to it + /// for group topology and auto-row *past* it, walking the display one width to the right on every + /// resize. + /// + /// Membership deliberately says nothing about lifecycle: a kept (Lingering/Pinned) entry still + /// owns a real compositor output occupying real desktop space, so it counts for placement. Only + /// the first-in-group probe adds a liveness term, because "may I establish this group's topology" + /// is a question about live *sessions*, not about outputs. + pub(super) fn in_group( + e_backend: &str, + e_gen: u64, + backend: &str, + gen: u64, + supersedes: Option, + ) -> bool { + Some(e_gen) != supersedes && group_key(e_backend, e_gen) == group_key(backend, gen) + } + + /// Hand off a torn-down display's topology restore (design §6.1 — per-group restore): if a + /// same-[group](in_group) sibling survives in `remaining`, MOVE the restore onto it (a later teardown + /// runs it); if the group is now empty, RETURN the action so the caller runs it (before dropping the + /// reclaimed display's keepalive, so the physical is re-enabled while our output still exists — + /// the compositor never sees zero outputs). `None` in → `None` out. + /// + /// `backend`+`gen` identify the DEPARTING display, and both are needed: keyed on the backend name + /// alone, one gamescope spawn's restore floated onto an unrelated client's spawn — where it would + /// run when THAT session ended and never when its own did. + pub(super) fn hand_off_restore( + remaining: &mut [Entry], + backend: &'static str, + gen: u64, + restore: Option, + ) -> Option { + let action = restore?; + // At most one restore per group, so any surviving sibling has `None` to receive it. + match remaining + .iter_mut() + .find(|e| in_group(e.backend, e.gen, backend, gen, None)) + { + Some(sibling) => { + sibling.topology_restore = Some(action); + None + } + None => Some(action), // group empty → run it now + } + } + + /// Does a pooled entry's session `epoch` still match the current one for reuse / expiry purposes? + /// The session epoch tracks the box's **active-session (desktop) compositor** instance (KWin / + /// Mutter / wlroots) — whose PipeWire node dies with the compositor, so a stale-epoch kept output + /// is a corpse. A **gamescope** spawn is the exact opposite: an independent nested session (its own + /// group), whose node lives with its own child process, wholly unrelated to whatever desktop / + /// game-mode compositor the epoch tracks. So gamescope entries are EXEMPT from the epoch — a desktop + /// switch, or a game-mode gamescope restart, must never invalidate a kept dedicated game session + /// (review findings #2/#5/#6/#7/#10). Their liveness is the `kept_display_alive` node probe + the B2 + /// game-exit path + `mark_failed`, not the epoch. + pub(super) fn epoch_matches(backend: &str, entry_epoch: u64, cur_epoch: u64) -> bool { + backend == "gamescope" || entry_epoch == cur_epoch + } + + /// Is the pool already at the host's `max_displays` budget, so this acquire must NOT create + /// another display (design §5.3 / `windows-parallel-virtual-displays.md` §2.5 — fail closed)? + /// + /// Counts KEPT entries as well as Active ones, exactly as the Windows admission budget counts the + /// manager's lingering/pinned slots: a kept display is a real compositor output holding real + /// resources, and the operator's ceiling is a ceiling on displays, not on sessions. The display + /// this acquire SUPERSEDES doesn't count — its replacement takes its place, so a mid-stream mode + /// switch at the budget must not be refused. + pub(super) fn at_display_budget(entries: &[Entry], supersedes: Option, max: u32) -> bool { + let n = entries.iter().filter(|e| Some(e.gen) != supersedes).count(); + n as u64 >= max as u64 + } + + /// Remove entries whose linger deadline has passed, returning them so the caller drops (tears + /// them down) *after* releasing the lock — a backend keepalive `Drop` (Mutter D-Bus Stop) can + /// block, and holding the pool lock across it would stall every other acquire/release. Each + /// expired entry's topology restore is [handed off](hand_off_restore) to a surviving group sibling, + /// or collected into the returned `restores` when its group empties (run before the entries drop). + pub(super) fn take_expired( + entries: &mut Vec, + now: Instant, + cur_epoch: u64, + ) -> (Vec, Vec) { + let mut expired = Vec::new(); + let mut restores = Vec::new(); + // A4 backstop: also reap a KEPT (non-Active) DESKTOP display whose session epoch is stale — its + // compositor instance was replaced (a Game↔Desktop switch / same-kind restart), so its node id + // now means nothing. gamescope spawns are exempt (`epoch_matches` — independent nested sessions). + // An Active entry is left to its own session's capture-loss rebuild (which, under the bumped + // epoch, won't reuse it); `invalidate_backend` clears a whole desktop backend on a known switch. + let mut i = 0; + while i < entries.len() { + let dead_epoch = !epoch_matches(entries[i].backend, entries[i].epoch, cur_epoch) + && !matches!(entries[i].life, lifecycle::State::Active { .. }); + if entries[i].life.poll_expiry(now) || dead_epoch { + let mut e = entries.remove(i); + let (backend, gen) = (e.backend, e.gen); + if let Some(r) = hand_off_restore(entries, backend, gen, e.topology_restore.take()) + { + restores.push(r); + } + expired.push(e); + } else { + i += 1; + } + } + (expired, restores) + } + + /// The linger a releasing session actually gets. A deliberate quit (`force_immediate` — the + /// client closed with the quit code, a user "stop") downgrades a linger WINDOW to an immediate + /// teardown; a bare disconnect honors the policy. `keep_alive = forever` (the gaming-rig + /// preset) OUTRANKS the quit: its promise is "the screen stays alive", so a deliberate quit + /// still pins — only an explicit `/display/release` frees it. + pub(super) fn effective_linger(force_immediate: bool, policy: Linger) -> Linger { + match (force_immediate, policy) { + (true, Linger::Forever) => Linger::Forever, + (true, _) => Linger::Immediate, + (false, l) => l, + } + } + + /// One live/kept display, flattened out of the pool under the lock — so the group + arrangement + /// math (which calls the layout engine) runs OUTSIDE the lock. + pub(super) struct Row { + pub(super) gen: u64, + pub(super) backend: &'static str, + pub(super) mode: Mode, + pub(super) identity_slot: Option, + pub(super) state: &'static str, + pub(super) expires_in_ms: Option, + pub(super) sessions: u32, + } + + /// The desktop position for a display just appended to its group (design §6.2): the group's + /// `existing` members (each with its acquire `gen`) plus `new` last, ordered by `gen`, arranged by + /// the pure [`layout`](crate::layout) engine, taking the new member's placement. Pure — so the + /// append-in-acquire-order + auto-row/manual arrangement is unit-tested independent of the + /// pool/global. + pub(super) fn position_for_new( + mut existing: Vec<(u64, crate::layout::Member)>, + new: crate::layout::Member, + layout_policy: &Layout, + ) -> crate::layout::Placement { + existing.sort_by_key(|(g, _)| *g); + let mut members: Vec = + existing.into_iter().map(|(_, m)| m).collect(); + members.push(new); + *crate::layout::arrange(&members, layout_policy) + .last() + .expect("members is non-empty (just pushed `new`)") + } + + /// Bring the group-key → **id** map up to date for the currently-live `keys`: every key already + /// known keeps the id it had, every new one takes `next` (bumped), and a key whose group is gone + /// is dropped. Pure over its `known`/`next` state — the caller owns the process-lifetime copy. + /// + /// Ids used to be the index into the sorted key list, which meant a new group could RENUMBER an + /// untouched one: with one KWin desktop at group 1, a gamescope spawn at gen 3 sorts ahead of + /// `"kwin"` and silently moved the unchanged desktop to group 2 on the next `/display/state` poll. + /// A monotonic counter, remembered per key, cannot do that. Pruning to the live keys is what keeps + /// the map bounded: the per-spawn keys (`gamescope#`, one per dedicated session) would + /// otherwise accumulate for the host's lifetime — an id is retired with its group. + pub(super) fn assign_group_ids( + known: &mut std::collections::BTreeMap, + next: &mut u32, + keys: &[String], + ) { + known.retain(|k, _| keys.iter().any(|live| live == k)); + for k in keys { + if !known.contains_key(k) { + known.insert(k.clone(), *next); + *next += 1; + } + } + } + + /// Group the flattened rows into the mgmt `/display/state` view (design §6.1/§6.2) by + /// [`group_key`], ordered by acquire (`gen`), with each member's position from the pure + /// [`layout`](crate::layout) engine. `ids` maps each group key to its reported group id (see + /// [`group_ids`]). Pure — no I/O, no global — so the grouping / ordering / position assignment is + /// unit-tested against synthetic rows. + pub(super) fn assemble_displays( + rows: Vec, + layout_policy: &Layout, + topology: &str, + ids: &std::collections::BTreeMap, + ) -> Vec { + use crate::layout::{self, Member}; + + let mut keys: Vec = rows.iter().map(|r| group_key(r.backend, r.gen)).collect(); + keys.sort(); + keys.dedup(); + + let mut out: Vec = Vec::new(); + for key in keys.iter() { + // This group's members in acquire order (gen ascending) → display_index + arrangement. + let mut idx: Vec = rows + .iter() + .enumerate() + .filter(|(_, row)| &group_key(row.backend, row.gen) == key) + .map(|(i, _)| i) + .collect(); + idx.sort_by_key(|&i| rows[i].gen); + let members: Vec = idx + .iter() + .map(|&i| Member { + identity_slot: rows[i].identity_slot, + width: rows[i].mode.width as i32, + }) + .collect(); + let places = layout::arrange(&members, layout_policy); + for (ord, &i) in idx.iter().enumerate() { + let row = &rows[i]; + let p = places[ord]; + out.push(DisplayInfo { + slot: row.gen, + backend: row.backend.to_string(), + mode: (row.mode.width, row.mode.height, row.mode.refresh_hz), + state: row.state.to_string(), + expires_in_ms: row.expires_in_ms, + sessions: row.sessions, + client: None, + // A key with no id can't happen (the caller derives `ids` from these same rows); + // 0 is the honest "ungrouped" answer rather than a panic in a mgmt read. + group: ids.get(key).copied().unwrap_or(0), + display_index: ord as u32, + position: (p.x, p.y), + identity_slot: row.identity_slot, + topology: topology.to_string(), + }); + } + } + out + } + + #[cfg(test)] + mod tests { + use super::*; + use crate::policy::{Layout, LayoutMode, Position}; + use std::collections::BTreeMap; + use std::sync::atomic::{AtomicBool, Ordering}; + use std::sync::Arc; + + /// A minimal pool entry for the pure teardown/restore tests (dummy keepalive; the + /// `hand_off_restore` logic only reads `backend` + `gen` + `topology_restore`). + fn test_entry(backend: &'static str, gen: u64, restore: Option) -> Entry { + Entry { + life: lifecycle::State::default(), + keepalive: Box::new(()), + node_id: 0, + preferred_mode: None, + mode: Mode { + width: 1920, + height: 1080, + refresh_hz: 60, + }, + backend, + identity_slot: None, + topology_restore: restore, + launch: None, + epoch: 0, + gen, + hw_cursor: false, + hdr: false, + } + } + + /// A restore closure that flips `flag` when run — so a test can assert exactly WHEN it fires. + fn flag_restore(flag: &Arc) -> Restore { + let f = flag.clone(); + Box::new(move || f.store(true, Ordering::SeqCst)) + } + + /// Group ids for a set of rows, as `snapshot` derives them — starting from an EMPTY map, so + /// the tests are independent of each other (the real caller threads one process-lifetime map). + fn ids_for(rows: &[Row]) -> BTreeMap { + let mut known = BTreeMap::new(); + let mut next = 1; + ids_into(&mut known, &mut next, rows); + known + } + + /// `ids_for` against a CARRIED map — for the stability test, which needs the same state + /// across two assemblies. + fn ids_into(known: &mut BTreeMap, next: &mut u32, rows: &[Row]) { + let mut keys: Vec = rows.iter().map(|r| group_key(r.backend, r.gen)).collect(); + keys.sort(); + keys.dedup(); + assign_group_ids(known, next, &keys); + } + + #[test] + fn deliberate_quit_skips_the_linger_window_but_never_a_pin() { + use std::time::Duration; + // Quit downgrades a linger window (and a no-linger policy stays immediate)… + assert_eq!( + effective_linger(true, Linger::For(Duration::from_secs(10))), + Linger::Immediate + ); + assert_eq!(effective_linger(true, Linger::Immediate), Linger::Immediate); + // …but never a pin: keep_alive=forever (gaming-rig) promises the screen stays alive. + assert_eq!(effective_linger(true, Linger::Forever), Linger::Forever); + // A bare disconnect honors the policy untouched. + assert_eq!( + effective_linger(false, Linger::For(Duration::from_secs(10))), + Linger::For(Duration::from_secs(10)) + ); + assert_eq!(effective_linger(false, Linger::Forever), Linger::Forever); + } + + #[test] + fn topology_restore_floats_to_a_sibling_then_runs_on_the_last_teardown() { + let ran = Arc::new(AtomicBool::new(false)); + // Two KWin displays in one group; the first (gen 1) carries the group's restore. + let mut pool = vec![ + test_entry("kwin", 1, Some(flag_restore(&ran))), + test_entry("kwin", 2, None), + ]; + + // Tear down the restore-carrier while its sibling is still alive → transfer, don't run. + let mut e1 = pool.remove(0); + let out = hand_off_restore(&mut pool, "kwin", 1, e1.topology_restore.take()); + assert!(out.is_none(), "transferred, not run"); + assert!(!ran.load(Ordering::SeqCst)); + // The restore floated onto the surviving sibling. + assert!(pool[0].topology_restore.is_some()); + + // Tear down the last member → group empty → the restore is returned to run. + let mut e2 = pool.remove(0); + let out = hand_off_restore(&mut pool, "kwin", 2, e2.topology_restore.take()); + let action = out.expect("group empty → run the restore"); + assert!(!ran.load(Ordering::SeqCst), "not run yet"); + action(); + assert!(ran.load(Ordering::SeqCst), "runs on the last drop"); + } + + #[test] + fn single_session_topology_restore_runs_on_its_own_teardown() { + // The validated single-display case: one exclusive session → restore runs at its teardown. + let ran = Arc::new(AtomicBool::new(false)); + let mut pool = vec![test_entry("kwin", 1, Some(flag_restore(&ran)))]; + let mut e = pool.remove(0); + let action = hand_off_restore(&mut pool, "kwin", 1, e.topology_restore.take()) + .expect("last (only) member → run"); + action(); + assert!(ran.load(Ordering::SeqCst)); + } + + #[test] + fn tearing_down_a_non_carrier_first_leaves_the_restore_for_last() { + let ran = Arc::new(AtomicBool::new(false)); + // gen 2 carries the restore; gen 1 does not (a later exclusive session found the physical + // already disabled). + let mut pool = vec![ + test_entry("kwin", 1, None), + test_entry("kwin", 2, Some(flag_restore(&ran))), + ]; + // Tear down the non-carrier first → nothing to hand off, carrier untouched. + let mut e1 = pool.remove(0); + assert!(hand_off_restore(&mut pool, "kwin", 1, e1.topology_restore.take()).is_none()); + // The carrier (gen 2) still holds the group's restore. + assert!(pool[0].topology_restore.is_some()); + // Now the carrier (last member) → run. + let mut e2 = pool.remove(0); + hand_off_restore(&mut pool, "kwin", 2, e2.topology_restore.take()) + .expect("last member → run")(); + assert!(ran.load(Ordering::SeqCst)); + } + + #[test] + fn restore_never_floats_across_backends() { + // group = backend: a KWin restore must not land on a Mutter display (a different desktop). + let ran = Arc::new(AtomicBool::new(false)); + let mut pool = vec![test_entry("mutter", 2, None)]; + let out = hand_off_restore(&mut pool, "kwin", 1, Some(flag_restore(&ran))); + assert!(out.is_some(), "no same-backend sibling → return to run"); + assert!( + pool[0].topology_restore.is_none(), + "restore must not cross into another backend's group" + ); + } + + /// Each gamescope **spawn** is its own group (`group_key`), so a departing spawn's restore + /// must not float onto another client's spawn — where it would run at THAT session's end and + /// never at its own. Keyed on the backend name alone (the old rule), it did. + #[test] + fn restore_never_floats_between_gamescope_spawns() { + let ran = Arc::new(AtomicBool::new(false)); + let mut pool = vec![test_entry("gamescope", 2, None)]; + let out = hand_off_restore(&mut pool, "gamescope", 1, Some(flag_restore(&ran))); + assert!(out.is_some(), "another client's spawn is not a sibling"); + assert!(pool[0].topology_restore.is_none()); + } + + /// The one definition of group membership, exercised on the three cases the two old + /// hand-rolled ones disagreed about. + #[test] + fn group_membership_splits_spawns_and_excludes_the_superseded() { + // Same desktop backend → same group. + assert!(in_group("kwin", 1, "kwin", 2, None)); + assert!(!in_group("mutter", 1, "kwin", 2, None)); + // Distinct gamescope spawns are distinct groups; a spawn is in its own. + assert!(!in_group("gamescope", 1, "gamescope", 2, None)); + assert!(in_group("gamescope", 7, "gamescope", 7, None)); + // The display this acquire replaces is not a sibling of its replacement. + assert!(!in_group("kwin", 1, "kwin", 2, Some(1))); + assert!(in_group("kwin", 3, "kwin", 2, Some(1))); + } + + #[test] + fn the_display_budget_counts_kept_entries_but_not_the_superseded_one() { + let pool = vec![ + test_entry("kwin", 1, None), + test_entry("kwin", 2, None), + test_entry("kwin", 3, None), + ]; + assert!( + !at_display_budget(&pool, None, 4), + "3 < 4 → room for one more" + ); + assert!(at_display_budget(&pool, None, 3), "3 >= 3 → at the ceiling"); + // A mid-stream mode switch replaces gen 2 rather than adding, so it still fits. + assert!(!at_display_budget(&pool, Some(2), 3)); + // …but only for an entry that is actually in the pool. + assert!(at_display_budget(&pool, Some(99), 3)); + assert!( + at_display_budget(&[], None, 0), + "a zero budget admits nothing" + ); + } + + fn row(gen: u64, backend: &'static str, w: u32, slot: Option) -> Row { + Row { + gen, + backend, + mode: Mode { + width: w, + height: 1080, + refresh_hz: 60, + }, + identity_slot: slot, + state: "active", + expires_in_ms: None, + sessions: 1, + } + } + + #[test] + fn groups_by_backend_and_auto_rows_in_acquire_order() { + // Two KWin displays (acquired gen 5 then gen 2 — deliberately out of vec order) + a Mutter one. + let rows = vec![ + row(5, "kwin", 2560, Some(1)), + row(2, "kwin", 1920, Some(7)), + row(9, "mutter", 3840, None), + ]; + let ids = ids_for(&rows); + let out = assemble_displays(rows, &Layout::default(), "exclusive", &ids); + + let kwin: Vec<&DisplayInfo> = out.iter().filter(|d| d.backend == "kwin").collect(); + assert_eq!(kwin.len(), 2); + assert_eq!(kwin[0].slot, 2); // lower gen (earlier acquire) sorts to index 0 + assert_eq!(kwin[0].display_index, 0); + assert_eq!(kwin[0].position, (0, 0)); + assert_eq!(kwin[1].slot, 5); + assert_eq!(kwin[1].display_index, 1); + assert_eq!(kwin[1].position, (1920, 0)); // auto-row: after the 1920px gen-2 display + assert_eq!(kwin[0].topology, "exclusive"); + + // A distinct backend is a distinct group. + let mutter = out.iter().find(|d| d.backend == "mutter").unwrap(); + assert_ne!(mutter.group, kwin[0].group); + assert_eq!(mutter.display_index, 0); + assert_eq!(mutter.position, (0, 0)); + } + + /// 10.7: a group id is a property of the GROUP, not of where its key happens to sort. A + /// gamescope spawn (`gamescope#3` < `kwin`) must not renumber the untouched desktop. + #[test] + fn a_new_group_never_renumbers_an_existing_one() { + let mut known = BTreeMap::new(); + let mut next = 1; + let desktop = vec![row(1, "kwin", 1920, None)]; + ids_into(&mut known, &mut next, &desktop); + let before = assemble_displays(desktop, &Layout::default(), "extend", &known); + let kwin_group = before[0].group; + + // The SAME map, one poll later, with a gamescope spawn alongside. + let both = vec![row(1, "kwin", 1920, None), row(3, "gamescope", 1280, None)]; + ids_into(&mut known, &mut next, &both); + let after = assemble_displays(both, &Layout::default(), "extend", &known); + let kwin_after = after.iter().find(|d| d.backend == "kwin").unwrap(); + let gs = after.iter().find(|d| d.backend == "gamescope").unwrap(); + assert_eq!( + kwin_after.group, kwin_group, + "the untouched desktop keeps its group id" + ); + assert_ne!(gs.group, kwin_group); + } + + #[test] + fn position_for_new_appends_right_in_acquire_order() { + use crate::layout::{Member, Placement}; + let m = |slot, w| Member { + identity_slot: slot, + width: w, + }; + // Existing group (given out of gen order): gen 8 @ 1920 acquired AFTER gen 3 @ 2560. + let existing = vec![(8, m(Some(2), 1920)), (3, m(Some(1), 2560))]; + // A new 1280-wide display appends to the right of 2560 + 1920. + let pos = position_for_new(existing, m(Some(5), 1280), &Layout::default()); + assert_eq!(pos, Placement { x: 4480, y: 0 }); + // First-of-group lands at the origin (so the registry skips the apply). + let first = position_for_new(vec![], m(None, 3840), &Layout::default()); + assert_eq!(first, Placement { x: 0, y: 0 }); + } + + #[test] + fn position_for_new_honors_a_manual_pin() { + use crate::layout::{Member, Placement}; + let mut positions = BTreeMap::new(); + positions.insert("5".to_string(), Position { x: 100, y: 200 }); + let layout = Layout { + mode: LayoutMode::Manual, + positions, + }; + let new = Member { + identity_slot: Some(5), + width: 1280, + }; + let pos = position_for_new(vec![(1, new)], new, &layout); + assert_eq!(pos, Placement { x: 100, y: 200 }); + } + + #[test] + fn gamescope_spawns_are_separate_groups() { + // Two independent gamescope spawns must NOT share a group or auto-row against each other. + let rows = vec![ + row(1, "gamescope", 1920, None), + row(2, "gamescope", 1280, None), + ]; + let ids = ids_for(&rows); + let out = assemble_displays(rows, &Layout::default(), "extend", &ids); + assert_eq!(out.len(), 2); + assert_ne!(out[0].group, out[1].group, "distinct groups"); + // Each is display 0 of its own group, at the origin (not auto-rowed against the other). + assert_eq!(out[0].display_index, 0); + assert_eq!(out[1].display_index, 0); + assert_eq!(out[0].position, (0, 0)); + assert_eq!(out[1].position, (0, 0)); + } + + #[test] + fn manual_layout_keys_positions_by_identity_slot() { + // Client 7 arranged to the LEFT of client 1 (reversed vs. auto-row). + let rows = vec![row(1, "kwin", 2560, Some(1)), row(2, "kwin", 1920, Some(7))]; + let mut positions = BTreeMap::new(); + positions.insert("1".to_string(), Position { x: 1920, y: 0 }); + positions.insert("7".to_string(), Position { x: 0, y: 0 }); + let layout = Layout { + mode: LayoutMode::Manual, + positions, + }; + let ids = ids_for(&rows); + let out = assemble_displays(rows, &layout, "extend", &ids); + let by_slot = |s: u32| out.iter().find(|d| d.identity_slot == Some(s)).unwrap(); + assert_eq!(by_slot(1).position, (1920, 0)); + assert_eq!(by_slot(7).position, (0, 0)); + } + + /// The expiry sweep is the linger timer's whole job, and it is pure over `(entries, now, + /// epoch)` — so the A4 stale-epoch backstop and the Active exemption are pinnable here. + #[test] + fn expiry_reaps_deadlines_and_stale_epoch_corpses_but_never_an_active_entry() { + use std::time::Duration; + let t0 = Instant::now(); + let mut es = Vec::new(); + // gen 1: lingering, deadline passed. + let mut e1 = test_entry("kwin", 1, None); + e1.life = lifecycle::State::Lingering { + until: t0 - Duration::from_millis(1), + }; + es.push(e1); + // gen 2: lingering, deadline in the future, current epoch → survives. + let mut e2 = test_entry("kwin", 2, None); + e2.life = lifecycle::State::Lingering { + until: t0 + Duration::from_secs(60), + }; + e2.epoch = 5; + es.push(e2); + // gen 3: pinned, but from a DEAD epoch → reaped (its compositor is gone). + let mut e3 = test_entry("kwin", 3, None); + e3.life = lifecycle::State::Pinned; + e3.epoch = 4; + es.push(e3); + // gen 4: ACTIVE from a dead epoch → left to its own session's rebuild. + let mut e4 = test_entry("kwin", 4, None); + e4.life = lifecycle::State::Active { refs: 1 }; + e4.epoch = 4; + es.push(e4); + // gen 5: a gamescope spawn from a "dead" epoch — exempt (independent nested session). + let mut e5 = test_entry("gamescope", 5, None); + e5.life = lifecycle::State::Pinned; + e5.epoch = 1; + es.push(e5); + + let (expired, restores) = take_expired(&mut es, t0, 5); + assert!(restores.is_empty()); + let gone: Vec = expired.iter().map(|e| e.gen).collect(); + assert_eq!(gone, vec![1, 3]); + let left: Vec = es.iter().map(|e| e.gen).collect(); + assert_eq!(left, vec![2, 4, 5]); + } + } +} + +// --------------------------------------------------------------------------------------------- +// Linux keep-alive pool +// --------------------------------------------------------------------------------------------- + +#[cfg(target_os = "linux")] +mod linux { + use std::sync::atomic::{AtomicBool, AtomicU64, Ordering}; + use std::sync::{Arc, Mutex, OnceLock}; + use std::time::{Duration, Instant}; + + use anyhow::Result; + + use super::pool::{ + assemble_displays, assign_group_ids, at_display_budget, effective_linger, epoch_matches, + group_key, hand_off_restore, in_group, position_for_new, take_expired, Entry, Restore, Row, + }; + use super::DisplayInfo; + use crate::lifecycle::{self, Release}; + use crate::policy::{self, Layout, Linger}; + use crate::{Mode, VirtualDisplay, VirtualOutput}; /// The result of the keep-alive reuse lookup (A2 validated reuse): a live kept display was reused, /// a dead one was pulled out (recreate), or nothing matched. @@ -301,27 +986,6 @@ mod linux { Miss, } - /// Hand off a torn-down display's topology restore (design §6.1 — per-group restore): if a - /// same-group (backend) sibling survives in `remaining`, MOVE the restore onto it (a later teardown - /// runs it); if the group is now empty, RETURN the action so the caller runs it (before dropping the - /// reclaimed display's keepalive, so the physical is re-enabled while our output still exists — - /// the compositor never sees zero outputs). `None` in → `None` out. - fn hand_off_restore( - remaining: &mut [Entry], - backend: &'static str, - restore: Option, - ) -> Option { - let action = restore?; - // At most one restore per group, so any surviving sibling has `None` to receive it. - match remaining.iter_mut().find(|e| e.backend == backend) { - Some(sibling) => { - sibling.topology_restore = Some(action); - None - } - None => Some(action), // group empty → run it now - } - } - struct Reg { entries: Mutex>, gen: AtomicU64, @@ -336,17 +1000,14 @@ mod linux { }) } - /// Does a pooled entry's session `epoch` still match the current one for reuse / expiry purposes? - /// The session epoch tracks the box's **active-session (desktop) compositor** instance (KWin / - /// Mutter / wlroots) — whose PipeWire node dies with the compositor, so a stale-epoch kept output - /// is a corpse. A **gamescope** spawn is the exact opposite: an independent nested session (its own - /// group), whose node lives with its own child process, wholly unrelated to whatever desktop / - /// game-mode compositor the epoch tracks. So gamescope entries are EXEMPT from the epoch — a desktop - /// switch, or a game-mode gamescope restart, must never invalidate a kept dedicated game session - /// (review findings #2/#5/#6/#7/#10). Their liveness is the `kept_display_alive` node probe + the B2 - /// game-exit path + `mark_failed`, not the epoch. - fn epoch_matches(backend: &str, entry_epoch: u64, cur_epoch: u64) -> bool { - backend == "gamescope" || entry_epoch == cur_epoch + /// The identity slots of the displays currently in the pool (see + /// [`super::live_identity_slots`]). Takes the pool lock only — never call it while holding it. + pub(super) fn live_identity_slots() -> std::collections::BTreeSet { + let Some(r) = REG.get() else { + return Default::default(); + }; + let es = r.entries.lock().unwrap(); + es.iter().filter_map(|e| e.identity_slot).collect() } /// The linger resolution for Linux: the console policy's `keep_alive` when configured, else @@ -358,63 +1019,60 @@ mod linux { .unwrap_or(Linger::Immediate) } - /// Remove entries whose linger deadline has passed, returning them so the caller drops (tears - /// them down) *after* releasing the lock — a backend keepalive `Drop` (Mutter D-Bus Stop) can - /// block, and holding the pool lock across it would stall every other acquire/release. Each - /// expired entry's topology restore is [handed off](hand_off_restore) to a surviving group sibling, - /// or collected into the returned `restores` when its group empties (run before the entries drop). - fn take_expired(entries: &mut Vec, now: Instant) -> (Vec, Vec) { - let mut expired = Vec::new(); - let mut restores = Vec::new(); - // A4 backstop: also reap a KEPT (non-Active) DESKTOP display whose session epoch is stale — its - // compositor instance was replaced (a Game↔Desktop switch / same-kind restart), so its node id - // now means nothing. gamescope spawns are exempt (`epoch_matches` — independent nested sessions). - // An Active entry is left to its own session's capture-loss rebuild (which, under the bumped - // epoch, won't reuse it); `invalidate_backend` clears a whole desktop backend on a known switch. - let cur_epoch = crate::session_epoch(); - let mut i = 0; - while i < entries.len() { - let dead_epoch = !epoch_matches(entries[i].backend, entries[i].epoch, cur_epoch) - && !matches!(entries[i].life, lifecycle::State::Active { .. }); - if entries[i].life.poll_expiry(now) || dead_epoch { - let mut e = entries.remove(i); - let backend = e.backend; - if let Some(r) = hand_off_restore(entries, backend, e.topology_restore.take()) { - restores.push(r); + /// Start the background reaper (lingering displays past their deadline) — once, but only once it + /// has actually STARTED. + /// + /// This used to be a `Once` around a `spawn()` whose `Result` was discarded, so a single failed + /// spawn (EAGAIN under thread pressure, an RLIMIT_NPROC ceiling) consumed the `Once` and left + /// every kept display un-reaped for the process lifetime — silently, with no later acquire ever + /// retrying. The flag is set only on success, so a failure is loud and the next acquire tries + /// again; the mutex makes the check-and-spawn atomic against concurrent acquires. + fn ensure_timer() { + static STARTED: Mutex = Mutex::new(false); + let mut started = STARTED.lock().unwrap_or_else(|e| e.into_inner()); + if *started { + return; + } + match std::thread::Builder::new() + .name("vdisplay-linger".into()) + .spawn(|| loop { + std::thread::sleep(Duration::from_millis(500)); + let (expired, restores) = { + let mut es = reg().entries.lock().unwrap(); + take_expired(&mut es, Instant::now(), crate::session_epoch()) + }; + // Re-enable physicals (group emptied) BEFORE dropping the outputs — outside the lock. + for restore in restores { + restore(); } - expired.push(e); - } else { - i += 1; - } + let reaped = expired.len(); + for e in expired { + tracing::info!( + backend = e.backend, + "virtual display: linger expired — torn down" + ); + drop(e); // outside the lock + } + emit_released(reaped); + }) { + Ok(_) => *started = true, + Err(e) => tracing::error!( + error = %e, + "virtual display: could not start the keep-alive linger reaper — kept displays \ + will not expire until a later session retries" + ), } - (expired, restores) } - /// Background thread (started once): reap lingering displays past their deadline. - fn ensure_timer() { - static ONCE: Once = Once::new(); - ONCE.call_once(|| { - let _ = std::thread::Builder::new() - .name("vdisplay-linger".into()) - .spawn(|| loop { - std::thread::sleep(Duration::from_millis(500)); - let (expired, restores) = { - let mut es = reg().entries.lock().unwrap(); - take_expired(&mut es, Instant::now()) - }; - // Re-enable physicals (group emptied) BEFORE dropping the outputs — outside the lock. - for restore in restores { - restore(); - } - for e in expired { - tracing::info!( - backend = e.backend, - "virtual display: linger expired — torn down" - ); - drop(e); // outside the lock - } - }); - }); + /// Tell the console `n` managed displays just went away (design §7 — the SSE display view). + /// Every Linux teardown path funnels through here: the lease drop, the linger reaper, the A2 + /// dead-display teardown, the mgmt release / mode-switch retire and the A4 backend invalidation. + /// Before, only `/display/release` emitted, so a display that lingered out — or that a + /// Game↔Desktop switch invalidated — stayed in the console's live view forever. + fn emit_released(n: usize) { + if n > 0 { + crate::emit_display_event(crate::DisplayEvent::Released { count: n as u32 }); + } } /// Build the session-facing [`VirtualOutput`]: the kept node + a fresh gen-stamped lease. Only @@ -458,12 +1116,14 @@ mod linux { // Reap expired first (run any group restores + drop outside the lock). let (expired, restores) = { let mut es = r.entries.lock().unwrap(); - take_expired(&mut es, Instant::now()) + take_expired(&mut es, Instant::now(), cur_epoch) }; for restore in restores { restore(); } + let reaped = expired.len(); drop(expired); + emit_released(reaped); // Reuse: a kept (lingering/pinned) display of the same backend + mode + launch + epoch. A // reconnecting session re-attaches a fresh PipeWire consumer to the still-live `node_id`. Gated @@ -529,11 +1189,9 @@ mod linux { Some(idx) => { // Dead kept display: remove it, hand off its group restore, create fresh. let mut dead = es.remove(idx); - let restore = hand_off_restore( - &mut es, - dead.backend, - dead.topology_restore.take(), - ); + let (b, g) = (dead.backend, dead.gen); + let restore = + hand_off_restore(&mut es, b, g, dead.topology_restore.take()); ReuseOutcome::Dead(dead, restore) } None => ReuseOutcome::Miss, // adopted/removed by another thread @@ -552,27 +1210,59 @@ mod linux { "virtual display: kept display was dead — recreating (validated reuse)" ); drop(dead); + emit_released(1); } ReuseOutcome::Miss => {} } } } - // Tell the backend whether it's the FIRST display of its group (no same-backend sibling live, - // §6.1) — so a topology-establishing backend (Mutter exclusive) extends into an already-exclusive - // desktop rather than re-clobbering the first session's virtual. Best-effort (a concurrent create - // is a narrow race); single-session is always `first == true` → today's behavior. - // Siblings that don't demote the newcomer: - // * the display this acquire SUPERSEDES (mode switch, create-before-drop) — still Active - // here because the old lease drops only after the new pipeline is up, but it's leaving, - // and deferring to it loses the group's Primary/Exclusive topology on every resize; - // * kept (Lingering/Pinned) entries — no session owns them, so there is no live desktop to - // clobber; a new session next to an unclaimed leftover should still establish topology. + // The new display's generation stamp, taken BEFORE the group questions below: it is this + // display's identity for the rest of the acquire, and `group_key` needs it — a gamescope + // spawn's group IS its gen. A gen burned by a failed create is harmless (they are opaque + // and monotonic, never an index). + let gen = r.gen.fetch_add(1, Ordering::Relaxed); + + // The operator's `max_displays` ceiling (design §5.3). Windows enforces it twice — in + // admission and in the manager — while the Linux pool had NO ceiling at all: because the + // reuse key includes the client-supplied mode, a client that reconnects at a different + // resolution misses reuse and mints a fresh display, so a handful of reconnects could row + // out an unbounded number of KWin outputs across the desktop. Refuse here, at the one place + // a Linux display is created, and fail closed: a display we are not allowed to create is an + // honest error to the connecting session, never a silent 17th monitor. + // + // Gated on `poolable_now()` for the same reason the reuse lookup is: a gamescope + // attach/managed acquire produces a display the registry does not own and never counts, so + // it must not be refused against a ceiling it does not consume either. + let max = policy::prefs().get().effective().max_displays; + let budget_used = if vd.poolable_now() { + let es = r.entries.lock().unwrap(); + at_display_budget(es.as_slice(), supersedes, max).then(|| es.len()) + } else { + None + }; + if let Some(used) = budget_used { + anyhow::bail!( + "host display budget exhausted: {used} display(s) live/kept, max_displays = {max}" + ); + } + + // Tell the backend whether it's the FIRST display of its group (no live sibling in the same + // §6.1 group) — so a topology-establishing backend (Mutter exclusive) extends into an + // already-exclusive desktop rather than re-clobbering the first session's virtual. + // Best-effort (a concurrent create is a narrow race); single-session is always `first == + // true` → today's behavior. Group membership is [`in_group`]'s single definition, so the + // display this acquire SUPERSEDES is excluded (still Active here — the old lease drops only + // after the new pipeline is up — but it is leaving, and deferring to it loses the group's + // Primary/Exclusive topology on every resize) and a second gamescope spawn is correctly its + // own group rather than an extension of another client's. Only the liveness term is local to + // this question: kept (Lingering/Pinned) entries have no session owning them, so there is no + // live desktop to clobber and a new session next to an unclaimed leftover should still + // establish topology. let first_in_group = { let es = r.entries.lock().unwrap(); !es.iter().any(|e| { - e.backend == backend - && Some(e.gen) != supersedes + in_group(e.backend, e.gen, backend, gen, supersedes) && matches!(e.life, lifecycle::State::Active { .. }) }) }; @@ -611,7 +1301,6 @@ mod linux { // lifted into the group so it runs once when the group's last member drops (§6.1), not at this // session's teardown. `None` for non-exclusive / non-first / backends whose topology auto-reverts. let topology_restore = vd.take_topology_restore(); - let gen = r.gen.fetch_add(1, Ordering::Relaxed); let mut life = lifecycle::State::default(); life.acquire(); // Idle → Active{refs:1} (Acquire::Create) let entry = Entry { @@ -631,9 +1320,9 @@ mod linux { }; // Compute this new display's position in its group (design §6.2) BEFORE pushing, then push - // under the same lock: the group is the same-backend entries; the new one appends last - // (rightmost under auto-row). `position_for_new` is pure; the lock is held only across it - // (I/O-free) — the backend apply is below, outside the lock. + // under the same lock: the new one appends last (rightmost under auto-row). + // `position_for_new` is pure; the lock is held only across it (I/O-free) — the backend apply + // is below, outside the lock. let position = { use crate::layout::Member; let layout_policy = policy::prefs() @@ -641,12 +1330,13 @@ mod linux { .map(|e| e.layout) .unwrap_or_default(); let mut es = r.entries.lock().unwrap(); - // Same-group members (design §6.1): same backend for a shared desktop, but each gamescope - // spawn is its own group, so a new gamescope never auto-rows against another. - let new_group = group_key(backend, gen); + // Same-group members ([`in_group`], design §6.1): one group per desktop backend, each + // gamescope spawn its own — and NOT the display this acquire supersedes, or a mid-stream + // resize would auto-row the replacement past its own predecessor and walk the display one + // width to the right on every mode switch. let existing: Vec<(u64, Member)> = es .iter() - .filter(|e| group_key(e.backend, e.gen) == new_group) + .filter(|e| in_group(e.backend, e.gen, backend, gen, supersedes)) .map(|e| { ( e.gen, @@ -677,19 +1367,6 @@ mod linux { Ok(out) } - /// The linger a releasing session actually gets. A deliberate quit (`force_immediate` — the - /// client closed with the quit code, a user "stop") downgrades a linger WINDOW to an immediate - /// teardown; a bare disconnect honors the policy. `keep_alive = forever` (the gaming-rig - /// preset) OUTRANKS the quit: its promise is "the screen stays alive", so a deliberate quit - /// still pins — only an explicit `/display/release` frees it. - fn effective_linger(force_immediate: bool, policy: Linger) -> Linger { - match (force_immediate, policy) { - (true, Linger::Forever) => Linger::Forever, - (true, _) => Linger::Immediate, - (false, l) => l, - } - } - /// The [`DisplayLease`] `Drop` path: release the session's hold on the pooled display. The /// lifecycle machine decides linger / pin / teardown; a torn-down entry's keepalive drops *after* /// the lock is released. @@ -702,14 +1379,22 @@ mod linux { return; // stale lease (entry reused + re-stamped, or already gone) — no-op }; match es[idx].life.release(Instant::now(), linger) { - Release::Teardown | Release::Noop => { + Release::Teardown => { let mut e = es.remove(idx); - let backend = e.backend; + let (backend, g) = (e.backend, e.gen); // Per-group restore (§6.1): hand the physical re-enable to a surviving sibling, or run // it now if this was the group's last member. - let restore = hand_off_restore(&mut es, backend, e.topology_restore.take()); + let restore = hand_off_restore(&mut es, backend, g, e.topology_restore.take()); (Some(e), restore) } + // A release against a slot with NO live hold — a stale or duplicate lease drop. The + // machine's contract for it is "do nothing", and it was wired to the teardown arm: + // the one outcome that means the caller has no claim on this display would have torn + // the display down. Unreachable today (a lease's gen is unique per acquire and the + // lookup above is by gen, so a stale lease misses the pool entirely and returns + // early), which is exactly why it has to be right by construction rather than by + // luck — matching the Windows manager's own Noop arm. + Release::Noop => (None, None), Release::Linger => { tracing::info!( backend = es[idx].backend, @@ -745,21 +1430,10 @@ mod linux { ); } drop(e); // outside the lock — the keepalive Drop may block + emit_released(1); } } - /// One live/kept display, flattened out of the pool under the lock — so the group + arrangement - /// math (which calls the layout engine) runs OUTSIDE the lock. - struct Row { - gen: u64, - backend: &'static str, - mode: Mode, - identity_slot: Option, - state: &'static str, - expires_in_ms: Option, - sessions: u32, - } - pub(super) fn snapshot() -> Vec { let Some(r) = REG.get() else { return Vec::new(); @@ -800,96 +1474,22 @@ mod linux { .configured_effective() .map(|e| e.layout) .unwrap_or_default(); - - assemble_displays(rows, &layout_policy, &topology) - } - - /// The desktop position for a display just appended to its group (design §6.2): the group's - /// `existing` members (each with its acquire `gen`) plus `new` last, ordered by `gen`, arranged by - /// the pure [`layout`] engine, taking the new member's placement. Pure — so the append-in-acquire- - /// order + auto-row/manual arrangement is unit-tested independent of the pool/global. - fn position_for_new( - mut existing: Vec<(u64, crate::layout::Member)>, - new: crate::layout::Member, - layout_policy: &Layout, - ) -> crate::layout::Placement { - existing.sort_by_key(|(g, _)| *g); - let mut members: Vec = - existing.into_iter().map(|(_, m)| m).collect(); - members.push(new); - *crate::layout::arrange(&members, layout_policy) - .last() - .expect("members is non-empty (just pushed `new`)") - } - - /// The display **group** a backend+display belongs to (design §6.1). The desktop compositors - /// (KWin/Mutter/wlroots) put every managed output on ONE desktop → one group per backend. A - /// gamescope **spawn** is an independent nested session per client (no shared desktop), so each - /// gamescope display is its OWN group — never auto-rowed against, or topology-/restore-grouped with, - /// another gamescope session. - fn group_key(backend: &str, gen: u64) -> String { - if backend == "gamescope" { - format!("gamescope#{gen}") - } else { - backend.to_string() - } - } - - /// Group the flattened rows into the mgmt `/display/state` view (design §6.1/§6.2) by - /// [`group_key`], ordered by acquire (`gen`), with each member's position from the pure [`layout`] - /// engine. Pure — no I/O, no global — so the grouping / ordering / position assignment is - /// unit-tested against synthetic rows. - fn assemble_displays( - rows: Vec, - layout_policy: &Layout, - topology: &str, - ) -> Vec { - use crate::layout::{self, Member}; - - // Small stable group ids by sorted group key — deterministic; in practice a host runs one live - // desktop backend → group 1 (with each gamescope spawn its own group). + // Stable per-group ids, carried across polls (see `assign_group_ids`) — a new group must + // never renumber an existing one under the console. The state is process-lifetime and this + // is the only thing that touches it, so it lives here rather than in the pure core. let mut keys: Vec = rows.iter().map(|r| group_key(r.backend, r.gen)).collect(); keys.sort(); keys.dedup(); + static GROUP_IDS: Mutex, u32)>> = + Mutex::new(None); + let ids = { + let mut g = GROUP_IDS.lock().unwrap_or_else(|e| e.into_inner()); + let (known, next) = g.get_or_insert_with(|| (Default::default(), 1)); + assign_group_ids(known, next, &keys); + known.clone() + }; - let mut out: Vec = Vec::new(); - for (gi, key) in keys.iter().enumerate() { - // This group's members in acquire order (gen ascending) → display_index + arrangement. - let mut idx: Vec = rows - .iter() - .enumerate() - .filter(|(_, row)| &group_key(row.backend, row.gen) == key) - .map(|(i, _)| i) - .collect(); - idx.sort_by_key(|&i| rows[i].gen); - let members: Vec = idx - .iter() - .map(|&i| Member { - identity_slot: rows[i].identity_slot, - width: rows[i].mode.width as i32, - }) - .collect(); - let places = layout::arrange(&members, layout_policy); - for (ord, &i) in idx.iter().enumerate() { - let row = &rows[i]; - let p = places[ord]; - out.push(DisplayInfo { - slot: row.gen, - backend: row.backend.to_string(), - mode: (row.mode.width, row.mode.height, row.mode.refresh_hz), - state: row.state.to_string(), - expires_in_ms: row.expires_in_ms, - sessions: row.sessions, - client: None, - group: gi as u32 + 1, - display_index: ord as u32, - position: (p.x, p.y), - identity_slot: row.identity_slot, - topology: topology.to_string(), - }); - } - } - out + assemble_displays(rows, &layout_policy, &topology, &ids) } pub(super) fn force_release(slot: Option) -> usize { @@ -917,9 +1517,9 @@ mod linux { let selected = slot.is_none_or(|s| es[i].gen == s); if selected && es[i].life.force_release() { let mut e = es.remove(i); - let backend = e.backend; + let (backend, g) = (e.backend, e.gen); let restore = e.topology_restore.take(); - if let Some(rst) = hand_off_restore(&mut es, backend, restore) { + if let Some(rst) = hand_off_restore(&mut es, backend, g, restore) { restores.push(rst); } out.push(e); @@ -938,6 +1538,7 @@ mod linux { tracing::info!(backend = e.backend, "virtual display {why}"); drop(e); } + emit_released(n); n } @@ -951,8 +1552,8 @@ mod linux { return; // already gone — the subsequent stale-gen lease drop no-ops too }; let mut e = es.remove(idx); - let backend = e.backend; - let restore = hand_off_restore(&mut es, backend, e.topology_restore.take()); + let (backend, g) = (e.backend, e.gen); + let restore = hand_off_restore(&mut es, backend, g, e.topology_restore.take()); (e, restore) }; if let Some(rst) = restore { @@ -963,6 +1564,7 @@ mod linux { "virtual display: reused kept display was dead on first frame — torn down (A2 mark_failed)" ); drop(torn); // keepalive Drop outside the lock (may block) + emit_released(1); } /// A4 — invalidate every kept display of `backend` (its compositor instance is gone). Removes them @@ -979,8 +1581,8 @@ mod linux { while i < es.len() { if es[i].backend == backend { let mut e = es.remove(i); - let b = e.backend; - if let Some(rst) = hand_off_restore(&mut es, b, e.topology_restore.take()) { + let (b, g) = (e.backend, e.gen); + if let Some(rst) = hand_off_restore(&mut es, b, g, e.topology_restore.take()) { restores.push(rst); } out.push(e); @@ -1001,9 +1603,11 @@ mod linux { count = removed.len(), "virtual displays invalidated — compositor instance gone (A4 session switch)" ); + let n = removed.len(); for e in removed { drop(e); // outside the lock } + emit_released(n); } /// The session's refcount handle — the `keepalive` the capturer holds. `Drop` releases the @@ -1021,245 +1625,4 @@ mod linux { release(self.gen, self.quit.load(Ordering::SeqCst)); } } - - #[cfg(test)] - mod tests { - use super::*; - use crate::policy::{Layout, LayoutMode, Position}; - use std::collections::BTreeMap; - use std::sync::atomic::{AtomicBool, Ordering}; - use std::sync::Arc; - - /// A minimal pool entry for the pure teardown/restore tests (dummy keepalive; the - /// `hand_off_restore` logic only reads `backend` + `topology_restore`). - fn test_entry(backend: &'static str, gen: u64, restore: Option) -> Entry { - Entry { - life: lifecycle::State::default(), - keepalive: Box::new(()), - node_id: 0, - preferred_mode: None, - mode: Mode { - width: 1920, - height: 1080, - refresh_hz: 60, - }, - backend, - identity_slot: None, - topology_restore: restore, - launch: None, - epoch: 0, - gen, - hw_cursor: false, - hdr: false, - } - } - - /// A restore closure that flips `flag` when run — so a test can assert exactly WHEN it fires. - fn flag_restore(flag: &Arc) -> Restore { - let f = flag.clone(); - Box::new(move || f.store(true, Ordering::SeqCst)) - } - - #[test] - fn deliberate_quit_skips_the_linger_window_but_never_a_pin() { - use std::time::Duration; - // Quit downgrades a linger window (and a no-linger policy stays immediate)… - assert_eq!( - effective_linger(true, Linger::For(Duration::from_secs(10))), - Linger::Immediate - ); - assert_eq!(effective_linger(true, Linger::Immediate), Linger::Immediate); - // …but never a pin: keep_alive=forever (gaming-rig) promises the screen stays alive. - assert_eq!(effective_linger(true, Linger::Forever), Linger::Forever); - // A bare disconnect honors the policy untouched. - assert_eq!( - effective_linger(false, Linger::For(Duration::from_secs(10))), - Linger::For(Duration::from_secs(10)) - ); - assert_eq!(effective_linger(false, Linger::Forever), Linger::Forever); - } - - #[test] - fn topology_restore_floats_to_a_sibling_then_runs_on_the_last_teardown() { - let ran = Arc::new(AtomicBool::new(false)); - // Two KWin displays in one group; the first (gen 1) carries the group's restore. - let mut pool = vec![ - test_entry("kwin", 1, Some(flag_restore(&ran))), - test_entry("kwin", 2, None), - ]; - - // Tear down the restore-carrier while its sibling is still alive → transfer, don't run. - let mut e1 = pool.remove(0); - let out = hand_off_restore(&mut pool, "kwin", e1.topology_restore.take()); - assert!(out.is_none(), "transferred, not run"); - assert!(!ran.load(Ordering::SeqCst)); - // The restore floated onto the surviving sibling. - assert!(pool[0].topology_restore.is_some()); - - // Tear down the last member → group empty → the restore is returned to run. - let mut e2 = pool.remove(0); - let out = hand_off_restore(&mut pool, "kwin", e2.topology_restore.take()); - let action = out.expect("group empty → run the restore"); - assert!(!ran.load(Ordering::SeqCst), "not run yet"); - action(); - assert!(ran.load(Ordering::SeqCst), "runs on the last drop"); - } - - #[test] - fn single_session_topology_restore_runs_on_its_own_teardown() { - // The validated single-display case: one exclusive session → restore runs at its teardown. - let ran = Arc::new(AtomicBool::new(false)); - let mut pool = vec![test_entry("kwin", 1, Some(flag_restore(&ran)))]; - let mut e = pool.remove(0); - let action = hand_off_restore(&mut pool, "kwin", e.topology_restore.take()) - .expect("last (only) member → run"); - action(); - assert!(ran.load(Ordering::SeqCst)); - } - - #[test] - fn tearing_down_a_non_carrier_first_leaves_the_restore_for_last() { - let ran = Arc::new(AtomicBool::new(false)); - // gen 2 carries the restore; gen 1 does not (a later exclusive session found the physical - // already disabled). - let mut pool = vec![ - test_entry("kwin", 1, None), - test_entry("kwin", 2, Some(flag_restore(&ran))), - ]; - // Tear down the non-carrier first → nothing to hand off, carrier untouched. - let mut e1 = pool.remove(0); - assert!(hand_off_restore(&mut pool, "kwin", e1.topology_restore.take()).is_none()); - // The carrier (gen 2) still holds the group's restore. - assert!(pool[0].topology_restore.is_some()); - // Now the carrier (last member) → run. - let mut e2 = pool.remove(0); - hand_off_restore(&mut pool, "kwin", e2.topology_restore.take()) - .expect("last member → run")(); - assert!(ran.load(Ordering::SeqCst)); - } - - #[test] - fn restore_never_floats_across_backends() { - // group = backend: a KWin restore must not land on a Mutter display (a different desktop). - let ran = Arc::new(AtomicBool::new(false)); - let mut pool = vec![test_entry("mutter", 2, None)]; - let out = hand_off_restore(&mut pool, "kwin", Some(flag_restore(&ran))); - assert!(out.is_some(), "no same-backend sibling → return to run"); - assert!( - pool[0].topology_restore.is_none(), - "restore must not cross into another backend's group" - ); - } - - fn row(gen: u64, backend: &'static str, w: u32, slot: Option) -> Row { - Row { - gen, - backend, - mode: Mode { - width: w, - height: 1080, - refresh_hz: 60, - }, - identity_slot: slot, - state: "active", - expires_in_ms: None, - sessions: 1, - } - } - - #[test] - fn groups_by_backend_and_auto_rows_in_acquire_order() { - // Two KWin displays (acquired gen 5 then gen 2 — deliberately out of vec order) + a Mutter one. - let rows = vec![ - row(5, "kwin", 2560, Some(1)), - row(2, "kwin", 1920, Some(7)), - row(9, "mutter", 3840, None), - ]; - let out = assemble_displays(rows, &Layout::default(), "exclusive"); - - let kwin: Vec<&DisplayInfo> = out.iter().filter(|d| d.backend == "kwin").collect(); - assert_eq!(kwin.len(), 2); - assert_eq!(kwin[0].slot, 2); // lower gen (earlier acquire) sorts to index 0 - assert_eq!(kwin[0].display_index, 0); - assert_eq!(kwin[0].position, (0, 0)); - assert_eq!(kwin[1].slot, 5); - assert_eq!(kwin[1].display_index, 1); - assert_eq!(kwin[1].position, (1920, 0)); // auto-row: after the 1920px gen-2 display - assert_eq!(kwin[0].topology, "exclusive"); - - // A distinct backend is a distinct group. - let mutter = out.iter().find(|d| d.backend == "mutter").unwrap(); - assert_ne!(mutter.group, kwin[0].group); - assert_eq!(mutter.display_index, 0); - assert_eq!(mutter.position, (0, 0)); - } - - #[test] - fn position_for_new_appends_right_in_acquire_order() { - use crate::layout::{Member, Placement}; - let m = |slot, w| Member { - identity_slot: slot, - width: w, - }; - // Existing group (given out of gen order): gen 8 @ 1920 acquired AFTER gen 3 @ 2560. - let existing = vec![(8, m(Some(2), 1920)), (3, m(Some(1), 2560))]; - // A new 1280-wide display appends to the right of 2560 + 1920. - let pos = position_for_new(existing, m(Some(5), 1280), &Layout::default()); - assert_eq!(pos, Placement { x: 4480, y: 0 }); - // First-of-group lands at the origin (so the registry skips the apply). - let first = position_for_new(vec![], m(None, 3840), &Layout::default()); - assert_eq!(first, Placement { x: 0, y: 0 }); - } - - #[test] - fn position_for_new_honors_a_manual_pin() { - use crate::layout::{Member, Placement}; - let mut positions = BTreeMap::new(); - positions.insert("5".to_string(), Position { x: 100, y: 200 }); - let layout = Layout { - mode: LayoutMode::Manual, - positions, - }; - let new = Member { - identity_slot: Some(5), - width: 1280, - }; - let pos = position_for_new(vec![(1, new)], new, &layout); - assert_eq!(pos, Placement { x: 100, y: 200 }); - } - - #[test] - fn gamescope_spawns_are_separate_groups() { - // Two independent gamescope spawns must NOT share a group or auto-row against each other. - let rows = vec![ - row(1, "gamescope", 1920, None), - row(2, "gamescope", 1280, None), - ]; - let out = assemble_displays(rows, &Layout::default(), "extend"); - assert_eq!(out.len(), 2); - assert_ne!(out[0].group, out[1].group, "distinct groups"); - // Each is display 0 of its own group, at the origin (not auto-rowed against the other). - assert_eq!(out[0].display_index, 0); - assert_eq!(out[1].display_index, 0); - assert_eq!(out[0].position, (0, 0)); - assert_eq!(out[1].position, (0, 0)); - } - - #[test] - fn manual_layout_keys_positions_by_identity_slot() { - // Client 7 arranged to the LEFT of client 1 (reversed vs. auto-row). - let rows = vec![row(1, "kwin", 2560, Some(1)), row(2, "kwin", 1920, Some(7))]; - let mut positions = BTreeMap::new(); - positions.insert("1".to_string(), Position { x: 1920, y: 0 }); - positions.insert("7".to_string(), Position { x: 0, y: 0 }); - let layout = Layout { - mode: LayoutMode::Manual, - positions, - }; - let out = assemble_displays(rows, &layout, "extend"); - let by_slot = |s: u32| out.iter().find(|d| d.identity_slot == Some(s)).unwrap(); - assert_eq!(by_slot(1).position, (1920, 0)); - assert_eq!(by_slot(7).position, (0, 0)); - } - } } diff --git a/crates/pf-vdisplay/src/vdisplay/session.rs b/crates/pf-vdisplay/src/vdisplay/session.rs index e99b6f88..d08ac2a1 100644 --- a/crates/pf-vdisplay/src/vdisplay/session.rs +++ b/crates/pf-vdisplay/src/vdisplay/session.rs @@ -58,22 +58,21 @@ pub fn observe_session_instance(active: &ActiveSession) { let changed = { let mut last = LAST_INSTANCE.lock().unwrap_or_else(|e| e.into_inner()); let prev = *last; - *last = Some(cur); + // A `None` scan result is NOT an observation (see [`classify_instance_change`]), so it must + // not become the baseline either: recording it would make the NEXT poll — the one that sees + // the still-running desktop again — read as `None → DesktopKde`, i.e. a fresh instance, and + // bump the epoch out from under every pooled display. Leave the baseline on the last REAL + // instance and a transient miss is fully inert, in both directions. + if cur.0 != ActiveKind::None { + *last = Some(cur); + } prev }; if let Some(prev) = changed { - // Only a **desktop** compositor (KWin / Mutter / wlroots) instance change bumps the epoch + - // invalidates its kept displays — its PipeWire node dies with the compositor. A **gamescope** - // session (`ActiveKind::Gaming`) is NOT the epoch's subject: the box's game-mode / managed - // gamescope isn't pooled, and dedicated **spawns** are independent nested sessions whose nodes - // outlive any active-session change. So a game-mode gamescope restart, a Gaming↔Gaming winning-PID - // flap (e.g. B1 stopping the autologin before a dedicated spawn), or a coexisting-gamescope set - // change must NOT bump/invalidate — that would tear down a live/kept dedicated session (review - // findings #6/#7/#10). Gate the whole action on a desktop kind being involved. - if prev != cur && (is_desktop_kind(prev.0) || is_desktop_kind(cur.0)) { + if let InstanceChange::NewInstance { invalidate } = classify_instance_change(prev, cur) { // Invalidate only the OLD backend, and only if it was a desktop compositor (never gamescope). - if is_desktop_kind(prev.0) { - if let Some(old) = compositor_for_kind(prev.0) { + if let Some(old_kind) = invalidate { + if let Some(old) = compositor_for_kind(old_kind) { registry::invalidate_backend(old.id()); } // The dead desktop's socket vars may still sit in the systemd --user manager env @@ -95,6 +94,54 @@ pub fn observe_session_instance(active: &ActiveSession) { } } +/// What a `prev` → `cur` observation means for the session epoch — the pure core of +/// [`observe_session_instance`], so the (surprisingly load-bearing) rules below are unit-tested +/// without the process-global baseline. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +enum InstanceChange { + /// The same instance, or a change the epoch does not track — do nothing. + Nothing, + /// A new compositor instance: bump the epoch. `invalidate` names the OUTGOING desktop + /// compositor whose kept displays must be dropped (its PipeWire nodes died with it); `None` + /// when the outgoing session was gamescope / nothing, which owns no pooled displays. + NewInstance { invalidate: Option }, +} + +/// The epoch's rules, in one place: +/// +/// * A `cur` of [`ActiveKind::None`] is **never** a change. `detect_active_session` answers `None` +/// both for "no graphical session is running" and for a scan that simply saw nothing — its whole +/// probe hangs off `if let Ok(entries) = std::fs::read_dir("/proc")`, and every per-PID rung +/// (`metadata`, `match_name`) can lose a race with a re-exec. Treating that as "the desktop +/// changed" ran `registry::invalidate_backend`, which removes pool entries in ANY lifecycle state +/// — Active ones included — so one unlucky `/proc` read tore down displays that were mid-stream, +/// and scrubbed the live session's socket vars out of the systemd `--user` manager on the way. +/// A real logout is picked up by the NEXT real observation (a different kind, or the same kind at +/// a new PID), which is the evidence-carrying end of the same transition. +/// * Only a **desktop** compositor (KWin / Mutter / wlroots) instance change counts. A **gamescope** +/// session ([`ActiveKind::Gaming`]) is not the epoch's subject: the box's game-mode / managed +/// gamescope isn't pooled, and dedicated **spawns** are independent nested sessions whose nodes +/// outlive any active-session change. So a game-mode gamescope restart, a Gaming↔Gaming +/// winning-PID flap (e.g. B1 stopping the autologin before a dedicated spawn), or a +/// coexisting-gamescope set change must NOT bump/invalidate — that would tear down a live/kept +/// dedicated session (review findings #6/#7/#10). +/// * A same-kind PID change IS a change: a fresh KWin's node-id space is unrelated to the dead +/// one's (A4). +fn classify_instance_change( + prev: (ActiveKind, Option), + cur: (ActiveKind, Option), +) -> InstanceChange { + if cur.0 == ActiveKind::None + || prev == cur + || !(is_desktop_kind(prev.0) || is_desktop_kind(cur.0)) + { + return InstanceChange::Nothing; + } + InstanceChange::NewInstance { + invalidate: is_desktop_kind(prev.0).then_some(prev.0), + } +} + /// Counterpart to [`settle_desktop_portal`]'s `import-environment`: drop the desktop session's /// socket vars from the systemd `--user` manager env once that desktop instance is GONE. They /// persist in the manager otherwise, and every later user unit inherits them — including @@ -698,6 +745,90 @@ pub fn settle_desktop_portal(chosen: Compositor) { #[cfg(not(target_os = "linux"))] pub fn settle_desktop_portal(_chosen: Compositor) {} +/// The epoch rules are platform-neutral (they are pure over [`ActiveKind`] + PID), so — unlike the +/// `/proc`-and-socket tests below — these run on every host this crate builds on. +#[cfg(test)] +mod instance_change_tests { + use super::*; + + /// The 10.9 regression: a scan that answered `None` while KDE was in fact still up used to + /// satisfy `is_desktop_kind(prev)` and run the full invalidate — which drops pool entries in + /// ANY state, live streaming ones included. + #[test] + fn a_none_observation_is_never_a_change() { + for prev in [ + (ActiveKind::DesktopKde, Some(42)), + (ActiveKind::DesktopGnome, Some(7)), + (ActiveKind::Gaming, Some(9)), + (ActiveKind::None, None), + ] { + assert_eq!( + classify_instance_change(prev, (ActiveKind::None, None)), + InstanceChange::Nothing, + "a None scan result must not invalidate {prev:?}" + ); + } + } + + #[test] + fn a_desktop_swap_invalidates_the_outgoing_desktop() { + assert_eq!( + classify_instance_change( + (ActiveKind::DesktopKde, Some(1)), + (ActiveKind::DesktopGnome, Some(2)) + ), + InstanceChange::NewInstance { + invalidate: Some(ActiveKind::DesktopKde) + } + ); + // Desktop → gamescope (Game Mode): the dead KWin's kept displays go with it. + assert_eq!( + classify_instance_change( + (ActiveKind::DesktopKde, Some(1)), + (ActiveKind::Gaming, Some(2)) + ), + InstanceChange::NewInstance { + invalidate: Some(ActiveKind::DesktopKde) + } + ); + // gamescope → desktop: a new epoch, but gamescope owns no pooled entries to invalidate. + assert_eq!( + classify_instance_change( + (ActiveKind::Gaming, Some(1)), + (ActiveKind::DesktopKde, Some(2)) + ), + InstanceChange::NewInstance { invalidate: None } + ); + } + + #[test] + fn a_same_kind_restart_is_a_new_instance_but_a_gamescope_flap_is_not() { + // A fresh KWin (new PID) has an unrelated node-id space — A4. + assert_eq!( + classify_instance_change( + (ActiveKind::DesktopKde, Some(1)), + (ActiveKind::DesktopKde, Some(2)) + ), + InstanceChange::NewInstance { + invalidate: Some(ActiveKind::DesktopKde) + } + ); + // The same instance re-detected: inert. + assert_eq!( + classify_instance_change( + (ActiveKind::DesktopKde, Some(1)), + (ActiveKind::DesktopKde, Some(1)) + ), + InstanceChange::Nothing + ); + // Gaming↔Gaming winning-PID flap: never the epoch's business (findings #6/#7/#10). + assert_eq!( + classify_instance_change((ActiveKind::Gaming, Some(1)), (ActiveKind::Gaming, Some(2))), + InstanceChange::Nothing + ); + } +} + #[cfg(all(test, target_os = "linux"))] mod tests { use super::*;