From 47d22b608200fe7028b2100c113fd0b5dff1f0c6 Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Fri, 10 Jul 2026 01:10:26 +0200 Subject: [PATCH] =?UTF-8?q?fix(host):=20admit=20exactly=20ONE=20parked=20k?= =?UTF-8?q?nock=20per=20Approve=20=E2=80=94=20stop=20crashing=20gnome-shel?= =?UTF-8?q?l,=20and=20self-heal=20dead=20sessions?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A retrying unpaired client parks one QUIC connection per knock, but the delegated-approval waiters were keyed on fingerprint alone — one console Approve resolved ALL of them. Observed live (2026-07-10): an iPad knocked 3x, one Approve admitted three full sessions, three Mutter virtual monitors were created within ~200us plus an ApplyMonitorsConfig, and gnome-shell SIGSEGV'd inside meta_monitor_manager_rebuild — dropping the box to the GDM greeter, unreachable until reboot (GDM auto-login runs once per boot) while the lingering host spammed "RemoteDesktop ... not activatable" libei errors. Four fixes, outermost symptom inward: - Knock generations (native_pairing): note_pending returns a per-knock generation; a re-knock bumps it and wakes the previous waiter, which resolves the new PairingDecision::Superseded (connection closes; the console list is unchanged). An approval records WHICH generation it admitted, so a stale waiter polling only after the pending entry is cleared still loses the tie — exactly one admission, no matter the interleaving. - TOPOLOGY_LOCK (vdisplay/mutter): every Mutter monitor mutation (pre-snapshot -> RecordVirtual -> ApplyMonitorsConfig, and the teardown Stop) is serialized process-wide. Concurrent rebuilds are what segfault the shell (second on-glass crash of this class — the teardown race is already documented in-file), and serialization also keeps wait_virtual_connector's snapshot diff from attributing a sibling's connector. Create timeout 20s -> 45s for lock queueing. - Session-env hygiene (vdisplay): when detection finds NOTHING live, clear the previous connect's XDG_CURRENT_DESKTOP/WAYLAND_DISPLAY retarget. A stale GNOME value kept mutter::is_available() true after the crash, routing explicit-backend connects into the dead session (create timeouts + libei error loops) instead of the crisp "no live graphical session" handshake error. - Opt-in recovery hook (config + punktfunk1): PUNKTFUNK_RECOVER_SESSION_CMD fires (detached via sh -c, debounced to one launch/min) when a client connects while no graphical session is live — e.g. `sudo -n systemctl restart gdm` re-runs auto-login and the client's retry lands in the recovered desktop. The handshake error tells the client to retry. Co-Authored-By: Claude Fable 5 --- crates/punktfunk-host/src/config.rs | 9 + crates/punktfunk-host/src/native_pairing.rs | 229 +++++++++++++++--- crates/punktfunk-host/src/punktfunk1.rs | 35 ++- crates/punktfunk-host/src/vdisplay.rs | 60 +++++ .../src/vdisplay/linux/mutter.rs | 32 ++- scripts/host.env.example | 9 + 6 files changed, 332 insertions(+), 42 deletions(-) diff --git a/crates/punktfunk-host/src/config.rs b/crates/punktfunk-host/src/config.rs index db5c1b30..17bf511c 100644 --- a/crates/punktfunk-host/src/config.rs +++ b/crates/punktfunk-host/src/config.rs @@ -68,6 +68,13 @@ pub struct HostConfig { /// backend (the legacy SudoVDA backend was removed), so this is currently informational — kept for the /// shipped `host.env` and as a forward seam if a second backend is ever added. pub vdisplay: Option, + /// `PUNKTFUNK_RECOVER_SESSION_CMD` — operator hook fired (debounced) when a client connects while NO + /// graphical session is live for this uid: the state a compositor crash leaves behind (gnome-shell + /// SIGSEGV → GDM greeter, whose auto-login is once-per-boot, so the box would otherwise need a walk-up + /// or reboot). Typically `sudo -n systemctl restart gdm` with a matching NOPASSWD sudoers rule, or + /// `systemctl restart display-manager` under a polkit rule — with auto-login enabled the restart brings + /// the desktop back and the client's retry lands in it. Unset/empty = disabled (the default). + pub recover_session_cmd: Option, } impl HostConfig { @@ -101,6 +108,8 @@ impl HostConfig { compositor: val("PUNKTFUNK_COMPOSITOR"), gamepad: val("PUNKTFUNK_GAMEPAD"), vdisplay: val("PUNKTFUNK_VDISPLAY"), + recover_session_cmd: val("PUNKTFUNK_RECOVER_SESSION_CMD") + .filter(|s| !s.trim().is_empty()), } } } diff --git a/crates/punktfunk-host/src/native_pairing.rs b/crates/punktfunk-host/src/native_pairing.rs index c46dd36f..765be346 100644 --- a/crates/punktfunk-host/src/native_pairing.rs +++ b/crates/punktfunk-host/src/native_pairing.rs @@ -84,12 +84,24 @@ struct Pending { /// A live parked knock is a genuine device waiting for the operator — eviction skips it unless /// every entry is parked, so a cert-rotating flood can't evict the device being onboarded (#13). parked: bool, + /// Generation of the MOST RECENT knock for this fingerprint. A re-knock bumps it (and wakes + /// waiters), so a stale parked connection resolves [`PairingDecision::Superseded`] instead of + /// being admitted alongside the newest one — one Approve must admit exactly ONE session. + /// (Observed live: a client retried 3× while parked, one console Approve admitted all three, + /// and the three concurrent Mutter virtual monitors segfaulted gnome-shell.) + knock_seq: u32, } #[derive(Default)] struct PendingState { next_id: u32, items: Vec, + /// Fingerprint → the knock generation an approval admitted, kept briefly after [`NativePairing::add`] + /// clears the pending entry. Closes the last double-admit window: a superseded waiter that only + /// polls AFTER the approval (entry gone, fingerprint paired) can't tell it lost from the entry + /// alone — this marker lets it resolve `Superseded` instead of a second `Approved`. Pruned on + /// the pending TTL and overwritten per fingerprint, so it stays a handful of tuples. + admitted: Vec<(String, u32, Instant)>, } /// A pending-approval snapshot for the management API / web console. @@ -114,6 +126,10 @@ pub enum PairingDecision { Denied, /// No decision within the wait window — reject; the device can knock again. TimedOut, + /// A NEWER knock from the same fingerprint replaced this one — close this connection; the + /// newest parked connection is the one an approval admits (a retrying client abandons its + /// older attempts, and admitting them all crashes compositors — see [`Pending::knock_seq`]). + Superseded, } /// Pending knocks older than this are dropped (the device retries; a stale entry shouldn't be @@ -353,9 +369,25 @@ impl NativePairing { return Err(e); } } - // A device that knocked and is now paired shouldn't linger in the approval list. + // A device that knocked and is now paired shouldn't linger in the approval list. Record + // WHICH knock generation this pairing admits before clearing the entry: only the waiter + // holding that generation may return `Approved`; a superseded sibling that polls after the + // clear resolves `Superseded` off this marker (exactly-one-admission — see `admitted`). { let mut pending = self.pending.lock().unwrap(); + let admitted_seq = pending + .items + .iter() + .find(|p| p.fp_hex.eq_ignore_ascii_case(fp_hex)) + .map(|p| p.knock_seq); + if let Some(seq) = admitted_seq { + pending + .admitted + .retain(|(fp, _, _)| !fp.eq_ignore_ascii_case(fp_hex)); + pending + .admitted + .push((fp_hex.to_string(), seq, Instant::now())); + } pending .items .retain(|p| !p.fp_hex.eq_ignore_ascii_case(fp_hex)); @@ -394,11 +426,16 @@ impl NativePairing { // -- Delegated approval (roadmap §8b-1) -------------------------------- - /// Drop expired pending knocks (called under the lock, mirroring [`Self::expire`]). + /// Drop expired pending knocks (called under the lock, mirroring [`Self::expire`]). The + /// admitted-generation markers share the TTL — they only matter while a superseded waiter + /// could still be parked, which is bounded by the approval wait (well under the TTL). fn expire_pending(pending: &mut PendingState) { pending .items .retain(|p| p.requested_at.elapsed() < PENDING_TTL); + pending + .admitted + .retain(|(_, _, at)| at.elapsed() < PENDING_TTL); } /// Pick the entry to evict, optionally restricted to a single source IP: the least-recently-active @@ -419,12 +456,14 @@ impl NativePairing { } /// Record an unpaired device's knock for delegated approval. Re-knocks from the same fingerprint - /// refresh the existing entry in place (same id; a connect-retry loop must not spam the list). A + /// refresh the existing entry in place (same id; a connect-retry loop must not spam the list) and + /// bump its knock generation — the returned generation is what [`Self::wait_for_decision`] admits, + /// so the NEWEST connection wins and any older parked sibling resolves `Superseded`. A /// fresh fingerprint gets a new id; the queue is bounded two ways so a flood can't crowd out a /// genuine knock (#13): a **per-source-IP cap** ([`MAX_PENDING_PER_IP`]) means one host can hold at /// most a few slots, and the global [`PENDING_CAP`] evicts the least-recently-active **non-parked** /// entry (never a live, held-open parked knock). The name is sanitized (untrusted). - pub fn note_pending(&self, name: &str, fp_hex: &str, src_ip: Option) { + pub fn note_pending(&self, name: &str, fp_hex: &str, src_ip: Option) -> u32 { let name = sanitize_device_name(name, fp_hex); let mut pending = self.pending.lock().unwrap(); Self::expire_pending(&mut pending); @@ -438,8 +477,19 @@ impl NativePairing { if p.src_ip.is_none() { p.src_ip = src_ip; } - return; + p.knock_seq = p.knock_seq.wrapping_add(1); + let seq = p.knock_seq; + drop(pending); + // Wake the previous knock's parked waiter so it sees it was superseded NOW instead of + // holding its dead connection open until the approval window lapses. + self.changed.notify_waiters(); + return seq; } + // A fresh knock lifecycle: drop any admitted-generation marker left from a previous + // pair→unpair round of this fingerprint, or it would wrongly supersede the new waiter. + pending + .admitted + .retain(|(fp, _, _)| !fp.eq_ignore_ascii_case(fp_hex)); // Per-source-IP cap: a single host can't occupy more than MAX_PENDING_PER_IP slots — evict its // own oldest entry first so it can't crowd out other devices' knocks (#13). if let Some(ip) = src_ip { @@ -471,22 +521,47 @@ impl NativePairing { requested_at: Instant::now(), src_ip, parked: false, + knock_seq: 0, }); + 0 } /// Mark/unmark the pending entry for `fp_hex` as having a live parked waiter (no-op if it's gone). - /// A parked entry is protected from eviction under load (#13). - fn set_parked(&self, fp_hex: &str, parked: bool) { + /// A parked entry is protected from eviction under load (#13). Gated on `knock_seq` so a + /// superseded waiter's exit can't unmark the flag the NEWER waiter (a bumped generation) owns. + fn set_parked(&self, fp_hex: &str, knock_seq: u32, parked: bool) { let mut pending = self.pending.lock().unwrap(); if let Some(p) = pending .items .iter_mut() - .find(|p| p.fp_hex.eq_ignore_ascii_case(fp_hex)) + .find(|p| p.fp_hex.eq_ignore_ascii_case(fp_hex) && p.knock_seq == knock_seq) { p.parked = parked; } } + /// The current knock generation for `fp_hex`, `None` when no entry is pending. A parked waiter + /// compares this against its own generation to detect it was superseded by a re-knock. + fn knock_seq_of(&self, fp_hex: &str) -> Option { + let pending = self.pending.lock().unwrap(); + pending + .items + .iter() + .find(|p| p.fp_hex.eq_ignore_ascii_case(fp_hex)) + .map(|p| p.knock_seq) + } + + /// The knock generation the approval of `fp_hex` admitted, if one was recorded (see + /// [`PendingState::admitted`]). + fn admitted_seq(&self, fp_hex: &str) -> Option { + let pending = self.pending.lock().unwrap(); + pending + .admitted + .iter() + .find(|(fp, _, _)| fp.eq_ignore_ascii_case(fp_hex)) + .map(|(_, seq, _)| *seq) + } + /// The devices currently awaiting approval (for the management API). pub fn pending(&self) -> Vec { let mut pending = self.pending.lock().unwrap(); @@ -560,29 +635,41 @@ impl NativePairing { } /// Park (async) until an operator decides on a knock identified by `fp_hex`, up to `timeout`. + /// `knock_seq` is the generation [`Self::note_pending`] returned for THIS connection's knock. /// Returns [`PairingDecision::Approved`] the instant the fingerprint is paired (console - /// approve or a concurrent PIN ceremony), [`PairingDecision::Denied`] if its pending entry is - /// dropped without pairing, or [`PairingDecision::TimedOut`] if the window lapses. Holds no - /// lock across the await. The QUIC accept path calls this right after [`Self::note_pending`] - /// to keep the knocking connection open until a human clicks Approve — so the device pairs and - /// streams with no reconnect (delegated approval, roadmap §8b-1). - pub async fn wait_for_decision(&self, fp_hex: &str, timeout: Duration) -> PairingDecision { + /// approve or a concurrent PIN ceremony), [`PairingDecision::Superseded`] the instant a newer + /// knock from the same fingerprint replaces this one (a retrying client — only the newest + /// connection is admitted; three siblings admitted at once has crashed gnome-shell live), + /// [`PairingDecision::Denied`] if its pending entry is dropped without pairing, or + /// [`PairingDecision::TimedOut`] if the window lapses. Holds no lock across the await. The + /// QUIC accept path calls this right after [`Self::note_pending`] to keep the knocking + /// connection open until a human clicks Approve — so the device pairs and streams with no + /// reconnect (delegated approval, roadmap §8b-1). + pub async fn wait_for_decision( + &self, + fp_hex: &str, + knock_seq: u32, + timeout: Duration, + ) -> PairingDecision { // Mark this knock parked so a cert-rotating flood can't evict the genuine, held-open // connection out of the pending queue while the operator decides (#13). Cleared on every - // exit path by the guard's Drop. - self.set_parked(fp_hex, true); + // exit path by the guard's Drop (generation-gated, so a superseded waiter's exit never + // unmarks the newer waiter's flag). + self.set_parked(fp_hex, knock_seq, true); struct ParkGuard<'a> { np: &'a NativePairing, fp: &'a str, + seq: u32, } impl Drop for ParkGuard<'_> { fn drop(&mut self) { - self.np.set_parked(self.fp, false); + self.np.set_parked(self.fp, self.seq, false); } } let _park = ParkGuard { np: self, fp: fp_hex, + seq: knock_seq, }; let deadline = tokio::time::Instant::now() + timeout; loop { @@ -592,16 +679,33 @@ impl NativePairing { tokio::pin!(notified); notified.as_mut().enable(); + // Superseded check FIRST: once a newer knock owns the fingerprint, this connection + // must never be admitted — not even if the approval lands before we wake. + match self.knock_seq_of(fp_hex) { + Some(cur) if cur != knock_seq => return PairingDecision::Superseded, + _ => {} + } if self.is_paired(fp_hex) { - return PairingDecision::Approved; + // Paired with the pending entry already cleared: make sure the approval admitted + // OUR generation. A superseded waiter that first polls after `add()` sees the same + // paired/no-entry state as the winner — the admitted marker breaks the tie. + match self.admitted_seq(fp_hex) { + Some(adm) if adm != knock_seq => return PairingDecision::Superseded, + _ => return PairingDecision::Approved, + } } if !self.pending_contains(fp_hex) { // Neither pending nor paired. This is almost always a denial — but it can also be // the tiny interval inside `add()` between pinning and clearing the pending entry. // Re-check `is_paired` once: because `add()` pins BEFORE it clears pending, a - // cleared-pending observation that is really an approval will now read as paired. + // cleared-pending observation that is really an approval will now read as paired — + // with the same generation tie-break as above (the admitted marker is written in + // the same critical section that clears the entry). if self.is_paired(fp_hex) { - return PairingDecision::Approved; + match self.admitted_seq(fp_hex) { + Some(adm) if adm != knock_seq => return PairingDecision::Superseded, + _ => return PairingDecision::Approved, + } } return PairingDecision::Denied; } @@ -781,19 +885,19 @@ mod tests { let np = Arc::new(NativePairing::load_with(Some(p.clone()), None, false).unwrap()); // TimedOut: a parked knock with no decision returns TimedOut; the entry survives. - np.note_pending("Knocker", "ab01", None); + let seq = np.note_pending("Knocker", "ab01", None); let d = np - .wait_for_decision("ab01", Duration::from_millis(80)) + .wait_for_decision("ab01", seq, Duration::from_millis(80)) .await; assert_eq!(d, PairingDecision::TimedOut); assert!(np.pending_contains("ab01")); // Approved: approving WHILE parked wakes the waiter with Approved. let np2 = np.clone(); - let waiter = - tokio::spawn( - async move { np2.wait_for_decision("ab01", Duration::from_secs(5)).await }, - ); + let waiter = tokio::spawn(async move { + np2.wait_for_decision("ab01", seq, Duration::from_secs(5)) + .await + }); tokio::time::sleep(Duration::from_millis(30)).await; let id = np .pending() @@ -806,12 +910,12 @@ mod tests { assert!(np.is_paired("ab01")); // Denied: denying WHILE parked wakes the waiter with Denied (not held until timeout). - np.note_pending("Knock2", "cd02", None); + let seq = np.note_pending("Knock2", "cd02", None); let np3 = np.clone(); - let waiter = - tokio::spawn( - async move { np3.wait_for_decision("cd02", Duration::from_secs(5)).await }, - ); + let waiter = tokio::spawn(async move { + np3.wait_for_decision("cd02", seq, Duration::from_secs(5)) + .await + }); tokio::time::sleep(Duration::from_millis(30)).await; let id = np .pending() @@ -823,12 +927,67 @@ mod tests { assert_eq!(waiter.await.unwrap(), PairingDecision::Denied); assert!(!np.is_paired("cd02")); - // Already paired before the call → immediate Approved (no waiting). - let d = np.wait_for_decision("ab01", Duration::from_secs(5)).await; + // Already paired before the call (the PIN-ceremony race) → immediate Approved: the ab01 + // marker admitted generation 0, which is also what a fresh coincidental waiter holds. + let d = np + .wait_for_decision("ab01", 0, Duration::from_secs(5)) + .await; assert_eq!(d, PairingDecision::Approved); let _ = std::fs::remove_file(&p); } + /// One Approve must admit exactly ONE session: a re-knock supersedes the previous parked + /// waiter (it resolves `Superseded` immediately, not at timeout), the console list keeps a + /// single entry, and a stale-generation waiter that polls only AFTER the approval still + /// resolves `Superseded` off the admitted marker. (Live failure this pins down: a client + /// knocked 3×, one Approve admitted all three, and the three concurrent Mutter virtual + /// monitors segfaulted gnome-shell.) + #[tokio::test] + async fn newest_knock_supersedes_parked_waiter() { + use std::sync::Arc; + let p = temp(); + let _ = std::fs::remove_file(&p); + let np = Arc::new(NativePairing::load_with(Some(p.clone()), None, false).unwrap()); + + let seq1 = np.note_pending("iPad Pro", "ee01", None); + let np1 = np.clone(); + let waiter1 = tokio::spawn(async move { + np1.wait_for_decision("ee01", seq1, Duration::from_secs(5)) + .await + }); + tokio::time::sleep(Duration::from_millis(30)).await; + + // The device retries: same fingerprint, new connection. The old waiter is superseded at + // once; the pending list still shows ONE entry. + let seq2 = np.note_pending("iPad Pro", "ee01", None); + assert_ne!(seq1, seq2); + assert_eq!(waiter1.await.unwrap(), PairingDecision::Superseded); + assert_eq!(np.pending().len(), 1); + + let np2 = np.clone(); + let waiter2 = tokio::spawn(async move { + np2.wait_for_decision("ee01", seq2, Duration::from_secs(5)) + .await + }); + tokio::time::sleep(Duration::from_millis(30)).await; + let id = np + .pending() + .into_iter() + .find(|x| x.fingerprint == "ee01") + .unwrap() + .id; + np.approve_pending(id, None).unwrap().unwrap(); + assert_eq!(waiter2.await.unwrap(), PairingDecision::Approved); + + // A stale-generation waiter polling only after the approval (entry cleared, fingerprint + // paired) must NOT read as a second Approved — the admitted marker resolves the tie. + let d = np + .wait_for_decision("ee01", seq1, Duration::from_millis(80)) + .await; + assert_eq!(d, PairingDecision::Superseded); + let _ = std::fs::remove_file(&p); + } + /// #9: a window can be bound to one operator-selected fingerprint, so an unrelated (attacker) /// fingerprint can neither pair nor BURN the window (it's rejected without a PIN). #[test] @@ -873,8 +1032,8 @@ mod tests { // A genuine knock from a different IP, parked (a live held-open connection), survives a flood // from many distinct IPs that fills the global cap. let legit = IpAddr::from([192, 168, 1, 50]); - np.note_pending("Living Room", "legit01", Some(legit)); - np.set_parked("legit01", true); + let seq = np.note_pending("Living Room", "legit01", Some(legit)); + np.set_parked("legit01", seq, true); for i in 0..(PENDING_CAP * 2) { let ip = IpAddr::from([10, 0, (i / 256) as u8, (i % 256) as u8]); np.note_pending("flood2", &format!("g{i:04}"), Some(ip)); diff --git a/crates/punktfunk-host/src/punktfunk1.rs b/crates/punktfunk-host/src/punktfunk1.rs index 2f62bc91..b210ea77 100644 --- a/crates/punktfunk-host/src/punktfunk1.rs +++ b/crates/punktfunk-host/src/punktfunk1.rs @@ -700,13 +700,16 @@ async fn serve_session( tracing::info!(name = %label, fingerprint = %fp_hex, "unpaired device knocked — parking connection for delegated approval in the console"); // Record the QUIC-validated source IP so the pending queue's per-source cap can stop one - // host from flooding/evicting genuine knocks (#13). - np.note_pending(&label, &fp_hex, Some(peer.ip())); + // host from flooding/evicting genuine knocks (#13). The returned knock generation makes + // this connection the ONE an approval admits — a retrying client parks a fresh + // connection per knock, and admitting every parked sibling on a single Approve spun up + // three concurrent Mutter virtual monitors and segfaulted gnome-shell (2026-07-10). + let knock_seq = np.note_pending(&label, &fp_hex, Some(peer.ip())); // Free the session slot while a human decides — a parked knock must not hold an NVENC // permit (a handful of parked knocks would otherwise block every real session). drop(permit); let decision = tokio::select! { - d = np.wait_for_decision(&fp_hex, PENDING_APPROVAL_WAIT) => d, + d = np.wait_for_decision(&fp_hex, knock_seq, PENDING_APPROVAL_WAIT) => d, // The client gave up (closed the connection) before a decision — stop waiting. _ = conn.closed() => anyhow::bail!("client disconnected before pairing approval"), }; @@ -720,6 +723,10 @@ async fn serve_session( "pairing request not approved within {PENDING_APPROVAL_WAIT:?} \ — the device can knock again" ), + PairingDecision::Superseded => anyhow::bail!( + "parked knock superseded by a newer connection from the same device — \ + only the newest is admitted on approval" + ), } // Re-acquire a session slot for the now-approved session (waits if all slots are busy, // exactly like any freshly accepted client). @@ -2424,9 +2431,25 @@ fn resolve_compositor( return Ok(Compositor::Gamescope); } let available = crate::vdisplay::available(); - let chosen = pick_compositor(pref, &available, detected).ok_or_else(|| { - anyhow!("no usable compositor (no live graphical session for this uid; set PUNKTFUNK_COMPOSITOR or start a desktop/gaming session)") - })?; + let chosen = match pick_compositor(pref, &available, detected) { + Some(c) => c, + None => { + // No live session — the state a compositor crash leaves behind (gnome-shell + // SIGSEGV → GDM greeter, whose auto-login is once-per-boot). If the operator + // configured a recovery hook, fire it (debounced) and tell the client to retry: + // its next knock lands in the recovered desktop. + if crate::vdisplay::try_recover_session() { + anyhow::bail!( + "no live graphical session for this uid — host session recovery launched \ + (PUNKTFUNK_RECOVER_SESSION_CMD); retry in a few seconds" + ); + } + anyhow::bail!( + "no usable compositor (no live graphical session for this uid; set \ + PUNKTFUNK_COMPOSITOR or start a desktop/gaming session)" + ); + } + }; if !overridden { // Point input at the same backend and resolve the gamescope sub-mode (managed where the // session infra exists, attach to a foreign gamescope, else per-session bare spawn). diff --git a/crates/punktfunk-host/src/vdisplay.rs b/crates/punktfunk-host/src/vdisplay.rs index 768417f3..8ebda102 100644 --- a/crates/punktfunk-host/src/vdisplay.rs +++ b/crates/punktfunk-host/src/vdisplay.rs @@ -712,6 +712,17 @@ pub fn apply_session_env(active: &ActiveSession) { Some(sig) => std::env::set_var("HYPRLAND_INSTANCE_SIGNATURE", sig), None => std::env::remove_var("HYPRLAND_INSTANCE_SIGNATURE"), } + // NOTHING live ⇒ every session-scoped var still in the env is a leftover from a previous + // connect's retarget, and the availability probes read them: after a gnome-shell crash + // (observed 2026-07-10: SIGSEGV → GDM greeter) a stale `XDG_CURRENT_DESKTOP=GNOME` kept + // `mutter::is_available()` true, so a client's explicit backend request routed into the dead + // session — 45 s create timeouts and a libei error loop instead of the crisp "no live + // graphical session" handshake error. Clear them so `available()` reports the truth and the + // client fails fast (and, when configured, `try_recover_session` can bring the desktop back). + if active.kind == ActiveKind::None { + std::env::remove_var("XDG_CURRENT_DESKTOP"); + std::env::remove_var("WAYLAND_DISPLAY"); + } // Topology (Stage 2): the per-compositor backends (KWin/Mutter) now read // [`effective_topology`] directly at create time — the console policy, else the legacy // `PUNKTFUNK_{KWIN,MUTTER}_VIRTUAL_PRIMARY` env, else the Auto default (exclusive on the @@ -721,6 +732,55 @@ pub fn apply_session_env(active: &ActiveSession) { #[cfg(not(target_os = "linux"))] pub fn apply_session_env(_active: &ActiveSession) {} +/// Fire the operator's session-recovery hook (`PUNKTFUNK_RECOVER_SESSION_CMD`) because a client +/// connected while NO graphical session is live for this uid — the state a compositor crash +/// leaves behind (gnome-shell SIGSEGV → GDM greeter, whose auto-login only fires once per boot, +/// so the box would otherwise sit headless until a walk-up login or a reboot). The command runs +/// detached via `sh -c` (typically a display-manager restart — see the config docs) and is +/// debounced to one launch per minute so a retrying client can't stack restarts. Returns whether +/// a recovery is underway (just launched, or launched within the debounce window), letting the +/// handshake error tell the client to simply retry. +#[cfg(target_os = "linux")] +pub fn try_recover_session() -> bool { + let Some(cmd) = crate::config::config().recover_session_cmd.clone() else { + return false; + }; + static LAST_LAUNCH: std::sync::Mutex> = std::sync::Mutex::new(None); + const DEBOUNCE: std::time::Duration = std::time::Duration::from_secs(60); + let mut last = LAST_LAUNCH.lock().unwrap_or_else(|e| e.into_inner()); + if last.is_some_and(|t| t.elapsed() < DEBOUNCE) { + return true; // a launch is already in flight — the retry lands in the recovered session + } + match std::process::Command::new("/bin/sh") + .arg("-c") + .arg(&cmd) + .stdin(std::process::Stdio::null()) + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()) + .spawn() + { + Ok(mut child) => { + *last = Some(std::time::Instant::now()); + tracing::warn!(cmd = %cmd, + "no live graphical session — launched the operator's session-recovery command"); + // Reap off-thread so the finished child never lingers as a zombie. + std::thread::spawn(move || { + let _ = child.wait(); + }); + true + } + Err(e) => { + tracing::error!(cmd = %cmd, error = %e, + "session-recovery command failed to launch"); + false + } + } +} +#[cfg(not(target_os = "linux"))] +pub fn try_recover_session() -> bool { + false +} + /// On a **mid-stream** switch to a desktop, the xdg-desktop-portal (D-Bus-activated) and the systemd /// `--user` environment can still point at the OLD session, so the host's RemoteDesktop portal opens /// against a half-stale env — it accepts events but they don't reach the compositor until a diff --git a/crates/punktfunk-host/src/vdisplay/linux/mutter.rs b/crates/punktfunk-host/src/vdisplay/linux/mutter.rs index cf0ef640..2edbe3b3 100644 --- a/crates/punktfunk-host/src/vdisplay/linux/mutter.rs +++ b/crates/punktfunk-host/src/vdisplay/linux/mutter.rs @@ -49,6 +49,18 @@ const APPLY_TEMPORARY: u32 = 1; /// Mutter cursor mode: render the cursor into the stream (matches the KWin/gamescope backends). const CURSOR_EMBEDDED: u32 = 1; +/// Serializes, process-wide, every Mutter operation that adds/removes a virtual monitor or applies +/// a monitor configuration. Each of these makes Mutter rebuild its monitor topology, and +/// *concurrent* rebuilds have segfaulted gnome-shell on-glass twice now: the teardown-side race is +/// documented at the teardown below, and on 2026-07-10 three simultaneous session setups (three +/// `RecordVirtual` calls within ~200 µs plus an `ApplyMonitorsConfig`) crashed the shell inside +/// `meta_monitor_manager_rebuild` — dropping the box to the GDM greeter until a DM restart. One +/// mutation at a time also keeps [`wait_virtual_connector`] sound: with two virtual outputs +/// appearing at once, "the connector absent from MY pre-snapshot" can name a sibling's monitor. +/// Each session runs on its own dedicated thread (see [`session_thread`]), so blocking on a std +/// mutex — including across the awaits of its single-threaded setup future — is safe. +static TOPOLOGY_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); + /// The Mutter virtual-display driver. Each [`create`](VirtualDisplay::create) spins up a /// keepalive thread owning the D-Bus sessions behind the virtual monitor. pub struct MutterDisplay { @@ -146,7 +158,10 @@ impl VirtualDisplay for MutterDisplay { }) .context("spawn Mutter virtual-output thread")?; - let node_id = match setup_rx.recv_timeout(Duration::from_secs(20)) { + // 45 s (was 20 s): setups now queue on TOPOLOGY_LOCK, so a session behind a slow sibling + // (whose guard spans up to a ~10 s stream wait + 6 s connector wait + the apply) must + // outwait it plus its own handshake before this fires. + let node_id = match setup_rx.recv_timeout(Duration::from_secs(45)) { Ok(Ok(v)) => v, Ok(Err(e)) => bail!("Mutter virtual monitor failed: {e}"), Err(_) => bail!("timed out creating the Mutter virtual monitor"), @@ -181,6 +196,11 @@ impl Drop for StopGuard { /// `scale_key`/`remembered_scale` carry the per-client persisted scale: reapplied at connect, /// and the user's in-session changes are recorded back under the key (GNOME itself can't — see /// [`identity::ScaleMap`](crate::vdisplay::identity)). +// TOPOLOGY_LOCK is deliberately held across the awaits of the setup/teardown sequences: each +// session owns this dedicated OS thread and its own single-future runtime, so the guard never +// blocks a shared executor — it blocks exactly the sibling session threads, which is the point +// (see TOPOLOGY_LOCK). +#[allow(clippy::await_holding_lock)] fn session_thread( setup_tx: Sender>, stop: Arc, @@ -201,6 +221,11 @@ fn session_thread( } }; rt.block_on(async move { + // The whole setup — pre-snapshot → RecordVirtual → ApplyMonitorsConfig — is one + // read-modify-write on Mutter's monitor state; hold TOPOLOGY_LOCK across it so concurrent + // sessions can't interleave rebuilds (gnome-shell SIGSEGV) or poison each other's + // connector diffs. Released before the keepalive park below. + let topology_guard = TOPOLOGY_LOCK.lock().unwrap_or_else(|e| e.into_inner()); // Display-management topology (Stage 2): the console policy's level, resolved to a concrete // value. `Extend` leaves the virtual output an extension (no config change); `Primary` makes // it the primary monitor but keeps the physicals as secondaries; `Exclusive` makes it the @@ -288,6 +313,8 @@ fn session_thread( } } + drop(topology_guard); + // Park, keeping `session` (and its zbus connection) alive until told to stop. Every ~5 s, // read the virtual output's logical-monitor scale and persist a change the user made (GNOME // Settings mid-stream) under the client's key — polled rather than teardown-only so a host @@ -317,6 +344,9 @@ fn session_thread( // make_virtual_primary applied an APPLY_TEMPORARY config; Mutter reverts that on its own once // the virtual output disappears and our DisplayConfig connection (in `tracked`) closes — so we // just drop it here and let the revert happen Mutter-side, never touching the layout ourselves. + // The Stop (+ the revert it triggers) is a topology mutation too — take TOPOLOGY_LOCK so a + // sibling's teardown or setup can't interleave with the rebuild it causes. + let _topology_guard = TOPOLOGY_LOCK.lock().unwrap_or_else(|e| e.into_inner()); let _ = session.rd_session.call_method("Stop", &()).await; drop(tracked); }); diff --git a/scripts/host.env.example b/scripts/host.env.example index 6e42456f..ad80e377 100644 --- a/scripts/host.env.example +++ b/scripts/host.env.example @@ -60,6 +60,15 @@ PUNKTFUNK_VIDEO_SOURCE=virtual # Lower (e.g. 3000) to reclaim kept displays sooner after an # ungraceful drop; clamped ≥1s, keep-alive ping scales with it so a # live session never false-disconnects. A deliberate quit is instant. +# Session recovery hook: fired (debounced, ≥60 s apart) when a client connects while NO graphical +# session is live for this uid — e.g. a compositor crash dropped the box to the GDM greeter, whose +# auto-login only runs once per boot, so the box would otherwise need a walk-up login or a reboot. +# Restarting the display manager re-runs auto-login and the client's retry lands in the recovered +# desktop. Runs detached via `sh -c` as the host's user, so it needs passwordless privilege for +# exactly this action (sudoers drop-in: `youruser ALL=(root) NOPASSWD: /usr/bin/systemctl restart +# gdm`, or a polkit rule + plain `systemctl restart display-manager`). Unset = disabled. +#PUNKTFUNK_RECOVER_SESSION_CMD=sudo -n systemctl restart gdm + # Full-chroma 4:4:4 (HEVC Range Extensions) — sharper text/desktop, no chroma loss. Honored only on # the punktfunk/1 native path when the client advertises 4:4:4 AND the GPU supports it (probed; else # the session stays 4:2:0). HEVC-only; independent of 10-bit. NVENC (NVIDIA) is the validated path;