diff --git a/crates/pf-driver-proto/src/lib.rs b/crates/pf-driver-proto/src/lib.rs index 900c587..065edef 100644 --- a/crates/pf-driver-proto/src/lib.rs +++ b/crates/pf-driver-proto/src/lib.rs @@ -80,7 +80,14 @@ pub mod control { pub width: u32, pub height: u32, pub refresh_hz: u32, - pub _reserved: u32, + /// Host-preferred per-client monitor id (`1..=15`) — the EDID serial / IddCx `ConnectorIndex` / + /// `ContainerId` the driver names this monitor by. A given client (keyed by its cert fingerprint) + /// gets a STABLE id across reconnects, so the OS device path + EDID stay identical and Windows + /// reapplies that client's saved per-monitor config (DPI scaling). `0` = AUTO: the driver + /// allocates the lowest-free id (the original slot-based behavior — used for anonymous/TOFU and + /// GameStream sessions). Byte-compatible with the old `_reserved` (offset 20): an un-upgraded + /// driver ignores it (→ auto), which the host detects via [`AddReply::resolved_monitor_id`]. + pub preferred_monitor_id: u32, } /// `IOCTL_ADD` reply: the OS target id + the adapter LUID the IDD landed on (split low/high to @@ -91,7 +98,11 @@ pub mod control { pub adapter_luid_low: u32, pub adapter_luid_high: i32, pub target_id: u32, - pub _reserved: u32, + /// The monitor id the driver ACTUALLY used — echoes [`AddRequest::preferred_monitor_id`] when the + /// preference was honored, or the auto-allocated id otherwise. Byte-compatible with the old + /// `_reserved` (offset 12): an un-upgraded driver leaves it `0`, so the host can tell its + /// preference was ignored (stale driver) and log it instead of silently losing per-client config. + pub resolved_monitor_id: u32, } /// `IOCTL_REMOVE` input. @@ -129,11 +140,13 @@ pub mod control { assert!(offset_of!(AddRequest, width) == 8); assert!(offset_of!(AddRequest, height) == 12); assert!(offset_of!(AddRequest, refresh_hz) == 16); + assert!(offset_of!(AddRequest, preferred_monitor_id) == 20); assert!(size_of::() == 16); assert!(offset_of!(AddReply, adapter_luid_low) == 0); assert!(offset_of!(AddReply, adapter_luid_high) == 4); assert!(offset_of!(AddReply, target_id) == 8); + assert!(offset_of!(AddReply, resolved_monitor_id) == 12); assert!(size_of::() == 8); assert!(offset_of!(RemoveRequest, session_id) == 0); @@ -436,11 +449,25 @@ mod tests { width: 3840, height: 2160, refresh_hz: 120, - _reserved: 0, + preferred_monitor_id: 7, }; let bytes = bytemuck::bytes_of(&req); assert_eq!(bytes.len(), 24); assert_eq!(*bytemuck::from_bytes::(bytes), req); + // preferred_monitor_id occupies the old `_reserved` slot at offset 20 — byte-compatible. + assert_eq!(bytes[20..24], 7u32.to_le_bytes()); + + let reply = control::AddReply { + adapter_luid_low: 0x1234_5678, + adapter_luid_high: -2, + target_id: 262, + resolved_monitor_id: 7, + }; + let rbytes = bytemuck::bytes_of(&reply); + assert_eq!(rbytes.len(), 16); + assert_eq!(*bytemuck::from_bytes::(rbytes), reply); + // resolved_monitor_id occupies the old `_reserved` slot at offset 12 — byte-compatible. + assert_eq!(rbytes[12..16], 7u32.to_le_bytes()); } #[test] diff --git a/crates/punktfunk-host/src/punktfunk1.rs b/crates/punktfunk-host/src/punktfunk1.rs index aa23779..4b2c6d4 100644 --- a/crates/punktfunk-host/src/punktfunk1.rs +++ b/crates/punktfunk-host/src/punktfunk1.rs @@ -2792,6 +2792,11 @@ fn virtual_stream(ctx: SessionContext) -> Result<()> { // host-lifetime VirtualDisplayManager (§2.5). It does NO monitor work, so it must precede the IDD-push // preempt below (which reaches the manager) — otherwise `vdm()` is called before init and panics. let mut vd = crate::vdisplay::open(compositor)?; + // Per-client STABLE monitor identity (Phase 2): hand the backend the connecting client's cert + // fingerprint so a freshly CREATED virtual monitor gets this client's persistent id — Windows then + // reapplies the client's saved per-monitor config (DPI scaling) on reconnect. No-op on Linux backends + // and for anonymous/GameStream clients (no fingerprint → the driver auto-allocates). + vd.set_client_identity(endpoint::peer_fingerprint(&conn)); // IDD-push reconnect preempt (the dance now lives in the manager, Goal-1 §2.5): serialize setup so a // reconnect FLOOD can't run concurrent monitor create/teardown, STOP the prior session + WAIT for it // to release its monitor (instead of tearing a monitor out from under a still-live session), and @@ -3310,6 +3315,23 @@ fn build_pipeline_with_retry( // 30-60s to produce its first frame, and a first-connect timeout would tear down the warm // session (forcing another cold start on reconnect). A genuinely permanent failure still fails // fast via `is_permanent_build_error`; only transient "no frame yet" retries consume the budget. + // IDD-push only: HOLD one monitor lease across all build attempts. A failed attempt's capturer + // drop releases ITS lease, but this held lease keeps the shared monitor Active (refs >= 1), so the + // next attempt's `vd.create` JOINS it (refcount++) instead of finding it Lingering and tripping the + // IDD-push reconnect PREEMPT (teardown + recreate). That preempt-per-retry was the REMOVE→ADD churn + // that exhausts the IddCx monitor-slot pool and wedges ADD at 0x80070490 — one ADD per cold start + // now, not one per attempt. Non-IDD-push backends (Linux portal, WGC) don't use the refcount manager + // and aren't churn-wedge-prone, so they keep create-per-attempt (a held lease there would allocate a + // second virtual output). Dropped when this fn returns — on success the Pipeline's own lease keeps + // the monitor Active; on failure refs falls to 0 → Lingering → linger-timeout teardown. + let _retry_hold = if matches!(plan.capture, crate::session_plan::CaptureBackend::IddPush) { + Some( + vd.create(mode) + .context("acquire virtual output for the session (retry-hold lease)")?, + ) + } else { + None + }; const MAX_ATTEMPTS: u32 = 8; let mut backoff = std::time::Duration::from_millis(500); for attempt in 1..=MAX_ATTEMPTS { diff --git a/crates/punktfunk-host/src/vdisplay.rs b/crates/punktfunk-host/src/vdisplay.rs index 4da846c..d5aad54 100644 --- a/crates/punktfunk-host/src/vdisplay.rs +++ b/crates/punktfunk-host/src/vdisplay.rs @@ -58,6 +58,12 @@ pub trait VirtualDisplay: Send { /// sessions can't stomp each other's launch target. Default: no-op (backends that attach to an /// existing session / don't spawn a nested command ignore it; only gamescope's spawn path uses it). fn set_launch_command(&mut self, _cmd: Option) {} + /// Set the connecting client's cert fingerprint so the backend can give that client a STABLE virtual + /// monitor identity across reconnects — Windows then reapplies the client's saved per-monitor config + /// (notably DPI scaling). Carried on the backend instance; set once before [`create`](Self::create). + /// Default: no-op — only the Windows pf-vdisplay backend uses it (Linux compositors own their virtual + /// output identity). `None` = anonymous/unpaired/GameStream → the backend's auto (slot-based) identity. + fn set_client_identity(&mut self, _fingerprint: Option<[u8; 32]>) {} } /// Compositors punktfunk knows how to drive (plan §6). @@ -641,6 +647,9 @@ pub fn start_restore_worker() -> std::sync::Arc<()> { #[cfg(target_os = "linux")] #[path = "vdisplay/linux/gamescope.rs"] mod gamescope; +#[cfg(target_os = "windows")] +#[path = "vdisplay/windows/identity.rs"] +pub(crate) mod identity; #[cfg(target_os = "linux")] #[path = "vdisplay/linux/kwin.rs"] mod kwin; diff --git a/crates/punktfunk-host/src/vdisplay/windows/identity.rs b/crates/punktfunk-host/src/vdisplay/windows/identity.rs new file mode 100644 index 0000000..24c4201 --- /dev/null +++ b/crates/punktfunk-host/src/vdisplay/windows/identity.rs @@ -0,0 +1,172 @@ +//! Per-client → stable monitor-id map for pf-vdisplay (Phase 2: per-client display-config persistence). +//! +//! Windows keys per-monitor config — notably DPI **scaling** (`HKCU\Control Panel\Desktop\PerMonitorSettings`) +//! — on the monitor's EDID identity AND its OS device path (whose per-connector discriminator is the IddCx +//! `ConnectorIndex` → target UID). The pf-vdisplay driver seeds BOTH the EDID serial and the `ConnectorIndex` +//! from a single monitor `id`. So for Windows to REAPPLY a given client's saved scaling on reconnect, that +//! client must get the SAME `id` every time. This map assigns each client (keyed by its cert fingerprint) a +//! STABLE id and the host passes it as [`AddRequest::preferred_monitor_id`](pf_driver_proto::control::AddRequest). +//! +//! The id space is bounded to `1..=15` because the driver uses the id as the IddCx `ConnectorIndex`, which +//! must stay `< MaxMonitorsSupported` (16). When more than 15 distinct clients are remembered, the +//! LEAST-RECENTLY-USED entry is evicted and its id reused (that evicted client simply re-establishes its +//! scaling once on its next connect). The map persists to `%ProgramData%\punktfunk\pf-vdisplay-identity.json` +//! so ids — and therefore the client→config association — survive host restarts. +//! +//! Anonymous/TOFU and GameStream sessions have no fingerprint and resolve to id `0` (auto) upstream, never +//! reaching this map — they keep the driver's lowest-free slot behavior unchanged. + +use std::path::PathBuf; + +use serde::{Deserialize, Serialize}; + +/// Max stable id. The driver uses the id as the IddCx `ConnectorIndex`, which must stay +/// `< MaxMonitorsSupported` (16) — so ids run `1..=15`. +const MAX_ID: u32 = 15; + +#[derive(Serialize, Deserialize, Default)] +struct Store { + /// Monotonic most-recently-used counter (the entry with the highest `seen` is the MRU). Persisted so + /// the LRU ordering survives host restarts. + tick: u64, + entries: Vec, +} + +#[derive(Serialize, Deserialize)] +struct Entry { + /// Lower-hex client cert fingerprint (the map key). + fp: String, + /// The client's stable monitor id (`1..=15`). + id: u32, + /// MRU stamp (compared against [`Store::tick`]). + seen: u64, +} + +/// Persistent fingerprint → stable-id map (see the module docs). +pub(crate) struct MonitorIdentityMap { + path: PathBuf, + store: Store, +} + +impl MonitorIdentityMap { + /// Load the persisted map (empty on first run / unreadable / parse failure — a fresh map just + /// re-derives ids, costing a client one scaling re-set the first time). + pub(crate) fn load() -> Self { + let path = crate::gamestream::config_dir().join("pf-vdisplay-identity.json"); + let mut store = std::fs::read(&path) + .ok() + .and_then(|b| serde_json::from_slice::(&b).ok()) + .unwrap_or_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/fp would flow straight into preferred_monitor_id. Drop out-of-range + // ids and dedup by BOTH fp and id (keeping the most-recently-seen on a clash) so no two fingerprints + // can map to the same id. (The driver also rejects a live-colliding id as a backstop.) + store.entries.sort_by_key(|e| std::cmp::Reverse(e.seen)); + let mut seen_fp = std::collections::HashSet::new(); + let mut seen_id = std::collections::HashSet::new(); + store.entries.retain(|e| { + (1..=MAX_ID).contains(&e.id) && seen_fp.insert(e.fp.clone()) && seen_id.insert(e.id) + }); + Self { path, store } + } + + /// The stable id (`1..=15`) for the client fingerprint `fp`: 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, fp: [u8; 32]) -> u32 { + let key: String = fp.iter().map(|b| format!("{b:02x}")).collect(); + self.store.tick = self.store.tick.wrapping_add(1); + let now = self.store.tick; + + if let Some(e) = self.store.entries.iter_mut().find(|e| e.fp == key) { + e.seen = now; + let id = e.id; + self.persist(); + return 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(|| { + let lru = self + .store + .entries + .iter() + .enumerate() + .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 + }); + self.store.entries.push(Entry { + fp: key, + id, + seen: now, + }); + self.persist(); + id + } + + /// Persist atomically (temp file + rename). Best-effort: a write failure just means a restart may + /// re-derive an id (one scaling re-set). Not a credential, so a plain (non-ACL'd) write is fine. + fn persist(&self) { + let Ok(bytes) = serde_json::to_vec_pretty(&self.store) else { + return; + }; + if let Some(dir) = self.path.parent() { + let _ = std::fs::create_dir_all(dir); + } + let tmp = self.path.with_extension("json.tmp"); + if std::fs::write(&tmp, &bytes).is_ok() { + let _ = std::fs::rename(&tmp, &self.path); + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn fp(n: u8) -> [u8; 32] { + let mut f = [0u8; 32]; + f[0] = n; + f + } + + #[test] + fn stable_across_calls_and_distinct_per_client() { + let mut m = MonitorIdentityMap { + path: std::env::temp_dir().join(format!("pf-id-test-{}.json", std::process::id())), + store: Store::default(), + }; + let a1 = m.resolve(fp(1)); + let b = m.resolve(fp(2)); + let a2 = m.resolve(fp(1)); + assert_eq!(a1, a2, "same client → same id"); + assert_ne!(a1, b, "distinct clients → distinct ids"); + assert!((1..=MAX_ID).contains(&a1) && (1..=MAX_ID).contains(&b)); + let _ = std::fs::remove_file(&m.path); + } + + #[test] + fn lru_eviction_reuses_an_id_at_the_cap() { + let mut m = MonitorIdentityMap { + path: std::env::temp_dir().join(format!("pf-id-lru-{}.json", std::process::id())), + store: Store::default(), + }; + // Fill all 15 ids (clients 1..=15), then touch client 2 so client 1 is the LRU. + for n in 1..=15u8 { + m.resolve(fp(n)); + } + let _ = m.resolve(fp(2)); + // A 16th client evicts the LRU (client 1) and reuses its id; ids stay bounded. + let id16 = m.resolve(fp(16)); + 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); + } +} diff --git a/crates/punktfunk-host/src/vdisplay/windows/manager.rs b/crates/punktfunk-host/src/vdisplay/windows/manager.rs index 25ddc5f..018d55d 100644 --- a/crates/punktfunk-host/src/vdisplay/windows/manager.rs +++ b/crates/punktfunk-host/src/vdisplay/windows/manager.rs @@ -59,8 +59,9 @@ pub(crate) trait VdisplayDriver: Send + Sync { /// # Safety /// Issues setup-API + `DeviceIoControl` calls; runs in the caller's apartment. unsafe fn open(&self) -> Result<(OwnedHandle, u32)>; - /// ADD a virtual monitor at `mode`, pinning the IDD render GPU to `render_luid` first if `Some`. - /// Returns the REMOVE key + target id + the adapter LUID the driver actually used. + /// ADD a virtual monitor at `mode`, pinning the IDD render GPU to `render_luid` first if `Some`, and + /// requesting `preferred_monitor_id` (the host's per-client stable id; `0` = auto). Returns the REMOVE + /// key + target id + the adapter LUID the driver actually used. /// /// # Safety /// `dev` must be the live control handle from [`open`](Self::open). @@ -69,6 +70,7 @@ pub(crate) trait VdisplayDriver: Send + Sync { dev: HANDLE, mode: Mode, render_luid: Option, + preferred_monitor_id: u32, ) -> Result; /// REMOVE the monitor identified by `key`. /// @@ -134,6 +136,10 @@ pub(crate) struct VirtualDisplayManager { /// The current IDD-push session's stop flag; a new connection signals the prior one to release its /// monitor before the fresh one is created (was the `IDD_SESSION_STOP` global in `punktfunk1`). idd_session_stop: Mutex>>, + /// Persistent per-client (cert-fingerprint) → stable monitor-id map. A monitor CREATE resolves the + /// connecting client's id here, so the client keeps the same EDID serial + IddCx ConnectorIndex across + /// reconnects and Windows reapplies its saved per-monitor config (DPI scaling). See [`super::identity`]. + identity_map: Mutex, } static VDM: OnceLock = OnceLock::new(); @@ -149,6 +155,7 @@ pub(crate) fn init(driver: Box) -> &'static VirtualDisplayMa state: Mutex::new(MgrState::Idle), setup_lock: Mutex::new(()), idd_session_stop: Mutex::new(None), + identity_map: Mutex::new(super::identity::MonitorIdentityMap::load()), }) } @@ -196,30 +203,40 @@ impl VirtualDisplayManager { } /// Acquire the shared monitor for a new session: preempt-recreate under IDD-push, join a live one - /// (refcount++), reuse a lingering one, or create one. The returned [`MonitorLease`] releases the - /// refcount on drop. - pub(crate) fn acquire(&'static self, mode: Mode) -> Result { + /// (refcount++), reuse a lingering one, or create one. `client_fp` (the connecting client's cert + /// fingerprint; `None` = anonymous/GameStream) gives a freshly CREATED monitor a STABLE per-client id + /// (so Windows reapplies that client's saved per-monitor config); JOIN and lingering-reuse keep the + /// existing monitor's id. The returned [`MonitorLease`] releases the refcount on drop. + pub(crate) fn acquire( + &'static self, + mode: Mode, + client_fp: Option<[u8; 32]>, + ) -> Result { self.ensure_linger_timer(); let mut state = self.state.lock().unwrap(); let dev = self.ensure_device()?; - // IDD-push: a new connection while a monitor is live is a single-client RECONNECT (the prior - // client is gone). A REUSED IddCx swap-chain is DEAD, so joining it hands a black screen — - // PREEMPT: tear the old monitor down (its key/topology are restored) and create a fresh one. The - // old session's lease is gen-stamped, so its later drop is a no-op and can't tear down the new one. - if idd_push_mode() && matches!(*state, MgrState::Active { .. } | MgrState::Lingering { .. }) - { - if let MgrState::Active { mon, .. } | MgrState::Lingering { mon, .. } = - std::mem::replace(&mut *state, MgrState::Idle) + // IDD-push: a new connection while a monitor is LINGERING is a single-client RECONNECT (the + // prior session fully released). A REUSED IddCx swap-chain is DEAD, so reusing it hands a black + // screen — PREEMPT: tear the lingering monitor down (its key/topology are restored) and create a + // fresh one. The old session's lease is gen-stamped, so its later drop is a no-op. + // + // ONLY Lingering, NOT Active: an Active monitor still has a lease held — that's the build-retry + // path (`build_pipeline_with_retry` holds one lease across all attempts) or a concurrent session, + // NOT a reconnect. Preempting Active would tear a live session down AND churn REMOVE→ADD on every + // retry — the per-cold-start monitor churn that exhausts the IddCx slot pool and wedges ADD at + // 0x80070490. Active falls through to the JOIN path below (refcount++, no ADD). + if idd_push_mode() && matches!(*state, MgrState::Lingering { .. }) { + if let MgrState::Lingering { mon, .. } = std::mem::replace(&mut *state, MgrState::Idle) { tracing::info!( old_target = mon.target_id, - "IDD-push reconnect — preempting the prior session, recreating a fresh monitor" + "IDD-push reconnect — preempting the lingering monitor, recreating a fresh one" ); // SAFETY: `teardown` requires `dev` to be the live control handle; `dev` is the value // `ensure_device()` returned above (the device is cached in the `OnceLock` and never - // closed for the manager's lifetime). `mon` was moved out of the prior `Active`/ - // `Lingering` state by `mem::replace`, so it is exclusively owned here — no aliasing. + // closed for the manager's lifetime). `mon` was moved out of the prior `Lingering` + // state by `mem::replace`, so it is exclusively owned here — no aliasing. unsafe { self.teardown(dev, mon) }; // Let the OS finish the ASYNC monitor departure before the next ADD; a back-to-back // REMOVE→ADD races the teardown and the ADD IOCTL is rejected under reconnect churn. @@ -264,7 +281,7 @@ impl VirtualDisplayManager { // SAFETY: `create_monitor` requires `dev` to be the live control handle; `dev` is the // handle `ensure_device()` returned above (cached in the `OnceLock`, never closed for the // manager's lifetime), and we hold the `state` lock. - MgrState::Idle => unsafe { self.create_monitor(dev, mode)? }, + MgrState::Idle => unsafe { self.create_monitor(dev, mode, client_fp)? }, MgrState::Active { .. } => unreachable!("handled above"), }; let out = self.output_for(&mon); @@ -291,12 +308,26 @@ impl VirtualDisplayManager { /// /// # Safety /// `dev` must be the live control handle. - unsafe fn create_monitor(&'static self, dev: HANDLE, mode: Mode) -> Result { + unsafe fn create_monitor( + &'static self, + dev: HANDLE, + mode: Mode, + client_fp: Option<[u8; 32]>, + ) -> Result { + // Resolve the connecting client's STABLE per-client monitor id (so Windows reapplies its saved + // per-monitor config — DPI scaling — on reconnect); `None`/anonymous → 0 = the driver + // auto-allocates the lowest-free id (the original slot-based behavior). + let preferred_id = client_fp + .map(|fp| self.identity_map.lock().unwrap().resolve(fp)) + .unwrap_or(0); // SAFETY: `create_monitor`'s own `# Safety` contract guarantees `dev` is the live control // handle; we forward it unchanged to `add_monitor`, whose precondition is exactly that. // `resolve_render_pin()` returns an `Option` by value (plain `Copy`), so no borrowed // memory crosses the call. - let added = unsafe { self.driver.add_monitor(dev, mode, resolve_render_pin())? }; + let added = unsafe { + self.driver + .add_monitor(dev, mode, resolve_render_pin(), preferred_id)? + }; // Mandatory keepalive: ping inside the watchdog window or the driver tears all displays down. // The pinger reaches the singleton for both the device + the driver — no raw-handle smuggle. @@ -510,25 +541,62 @@ impl VirtualDisplayManager { let prev = self.idd_session_stop.lock().unwrap().replace(stop); if let Some(prev_stop) = prev { prev_stop.store(true, Ordering::SeqCst); - self.wait_for_monitor_released(Duration::from_secs(3)); + if !self.wait_for_monitor_released(Duration::from_secs(3)) { + // TIMEOUT: the prior session is STILL Active (a wedged/slow teardown). `acquire`'s preempt + // is now Lingering-only (so build-retries JOIN the held monitor instead of churning + // REMOVE→ADD), which means the upcoming `_retry_hold` acquire would JOIN this stuck monitor + // and reuse its DEAD IddCx swap-chain → a full-session black screen with no self-heal until + // this session disconnects. Force-preempt it HERE instead. This runs at most ONCE per + // session (we hold `setup_lock`), so — unlike preempting inside `acquire` — it does not + // reintroduce the per-retry churn. The next `acquire` then sees `Idle` and creates a fresh + // monitor; the stale session's gen-stamped lease release is a no-op. + if let Some(dev) = self.device_handle() { + let taken = { + let mut state = self.state.lock().unwrap(); + match std::mem::replace(&mut *state, MgrState::Idle) { + MgrState::Active { mon, .. } => Some(mon), + // Raced to Lingering/Idle between the wait and here — restore + nothing stuck. + other => { + *state = other; + None + } + } + }; + if let Some(mon) = taken { + tracing::warn!( + old_target = mon.target_id, + "IDD-push setup: force-preempting the stuck-Active prior monitor (its IddCx swap-chain is dead)" + ); + // SAFETY: `teardown` requires `dev` to be the live control handle; `dev` is the + // cached process-lifetime `OwnedHandle` from `device_handle()` (the `Some` checked + // above). `mon` was moved out of the `Active` state under the `state` lock, so it is + // exclusively owned here — no aliasing. + unsafe { self.teardown(dev, mon) }; + // Let the OS finish the ASYNC departure before the next ADD (mirrors the acquire() + // Lingering-preempt settle). + thread::sleep(Duration::from_millis(400)); + } + } + } } guard } /// Wait (up to `timeout`) for the active monitor to be RELEASED (the MGR is no longer `Active`). /// Used by the IDD-push reconnect preempt: after signalling the old session to stop, wait here so it - /// tears its monitor down cleanly before we acquire a fresh one. - pub(crate) fn wait_for_monitor_released(&self, timeout: Duration) { + /// tears its monitor down cleanly before we acquire a fresh one. Returns `true` if it released, `false` + /// on timeout (the prior session is still `Active` — the caller force-preempts it). + pub(crate) fn wait_for_monitor_released(&self, timeout: Duration) -> bool { let deadline = Instant::now() + timeout; loop { if !matches!(*self.state.lock().unwrap(), MgrState::Active { .. }) { - return; + return true; } if Instant::now() >= deadline { tracing::warn!( - "IDD-push preempt: prior session didn't release the monitor within {timeout:?} — proceeding" + "IDD-push preempt: prior session didn't release the monitor within {timeout:?} — force-preempting" ); - return; + return false; } thread::sleep(Duration::from_millis(25)); } diff --git a/crates/punktfunk-host/src/vdisplay/windows/pf_vdisplay.rs b/crates/punktfunk-host/src/vdisplay/windows/pf_vdisplay.rs index 99a5ad8..14efe89 100644 --- a/crates/punktfunk-host/src/vdisplay/windows/pf_vdisplay.rs +++ b/crates/punktfunk-host/src/vdisplay/windows/pf_vdisplay.rs @@ -75,6 +75,65 @@ unsafe fn ioctl(h: HANDLE, code: u32, input: &[u8], output: &mut [u8]) -> Result Ok(returned) } +/// Reap the ghost (NOT-present) "punktfunk" virtual-monitor device nodes that `IddCxMonitorDeparture` +/// leaves behind. Each departed monitor leaves a not-present "Generic Monitor (punktfunk)" PDO that keeps +/// pinning an OS VidPN target against the IddCx adapter's fixed monitor-slot budget; once ~16 accumulate, +/// `IOCTL_ADD` wedges at 0x80070490 (`ERROR_NOT_FOUND`) and every session black-screens until a manual +/// reset/reboot. Removing the not-present PDOs frees the slots — the in-process equivalent of +/// `reset-pf-vdisplay.ps1` step 2 (proven on-box). Best-effort + idempotent: only NOT-present nodes +/// (`Status != OK`) are removed, so the LIVE session's monitor (`Status OK`) is never touched; any +/// failure is logged and swallowed. Returns the number removed. +fn reap_ghost_monitors() -> u32 { + // Mirrors reset-pf-vdisplay.ps1 step 2. powershell is always present for the SYSTEM service; the + // matched tokens ('OK', 'punktfunk', the InstanceId) are locale-invariant, so this is safe on a + // non-English box (unlike a .ps1 *file* read in the machine codepage). + const REAP_PS: &str = "$ErrorActionPreference='SilentlyContinue'; \ + $g = Get-PnpDevice -Class Monitor | Where-Object { $_.Status -ne 'OK' -and $_.FriendlyName -match 'punktfunk' }; \ + $n = 0; foreach ($d in $g) { pnputil /remove-device $d.InstanceId *> $null; if ($LASTEXITCODE -eq 0) { $n++ } }; \ + Write-Output $n"; + // Resolve powershell by full path — the LocalSystem service's PATH is not guaranteed to include + // System32 — with a bare-name fallback. + let ps = std::env::var("SystemRoot") + .map(|r| format!(r"{r}\System32\WindowsPowerShell\v1.0\powershell.exe")) + .unwrap_or_else(|_| "powershell.exe".to_string()); + match std::process::Command::new(&ps) + .args([ + "-NoProfile", + "-NonInteractive", + "-ExecutionPolicy", + "Bypass", + "-Command", + REAP_PS, + ]) + .output() + { + Ok(o) => { + let n = String::from_utf8_lossy(&o.stdout) + .trim() + .parse::() + .unwrap_or(0); + if n > 0 { + tracing::warn!( + reaped = n, + "pf-vdisplay: reaped ghost (not-present) virtual-monitor nodes — IddCx slot-exhaustion prevention" + ); + } + n + } + Err(e) => { + tracing::warn!(error = %e, "pf-vdisplay: ghost-monitor reap could not spawn powershell"); + 0 + } + } +} + +/// True if `e`'s chain carries the IddCx monitor-slot-exhaustion wedge HRESULT (0x80070490, +/// `ERROR_NOT_FOUND`) — the `IOCTL_ADD` failure that ghost-PDO accumulation produces. The hex code is +/// locale-invariant (the OS message text is not), so we match on it. +fn is_slot_exhaustion_wedge(e: &anyhow::Error) -> bool { + format!("{e:#}").contains("0x80070490") +} + /// Pin the pf-vdisplay IddCx's RENDER GPU to `luid` (the analogue of Apollo's `SetRenderAdapter`). No /// output buffer. Issued on the driver handle BEFORE `IOCTL_ADD` to steer which GPU the new target /// renders on — on a multi-adapter box this stops DXGI from reparenting the virtual output onto a @@ -193,6 +252,12 @@ impl VdisplayDriver for PfVdisplayDriver { } else { tracing::warn!("pf-vdisplay IOCTL_CLEAR_ALL failed on startup (continuing)"); } + // CLEAR_ALL only departs the driver's own (in-process) monitor list; it can NOT remove the + // OS-side not-present "Generic Monitor (punktfunk)" PDOs that a previous host-run's monitor + // departures left behind. Reap those here so a fresh host start begins with a clean IddCx + // monitor-slot budget — prevents the 0x80070490 slot-exhaustion wedge from carrying across + // restarts (the reason a restart's CLEAR_ALL alone never recovered it before). + reap_ghost_monitors(); Ok(( // SAFETY: `device` is the valid handle from `open_device`, still owned here and NOT closed // on this success path (the error paths above close it and return). `from_raw_handle`'s @@ -208,6 +273,7 @@ impl VdisplayDriver for PfVdisplayDriver { dev: HANDLE, mode: Mode, render_luid: Option, + preferred_monitor_id: u32, ) -> Result { let session_id = next_session_id(); let add = control::AddRequest { @@ -215,7 +281,7 @@ impl VdisplayDriver for PfVdisplayDriver { width: mode.width, height: mode.height, refresh_hz: mode.refresh_hz, - _reserved: 0, + preferred_monitor_id, }; // SET_RENDER_ADAPTER (opt-in; pf-vdisplay IMPLEMENTS it). Non-fatal on failure: the driver reports // its real render LUID in the shared header, so the host binds correctly even if this is ignored. @@ -238,13 +304,47 @@ impl VdisplayDriver for PfVdisplayDriver { // borrows the local `AddRequest` (alive across this synchronous call) as the input bytes, and // `out` is a stack `[u8; size_of::()]` whose length bounds the kernel's write — both // buffers outlive the call. - unsafe { ioctl(dev, control::IOCTL_ADD, bytemuck::bytes_of(&add), &mut out) } - .with_context(|| { - format!( - "pf-vdisplay ADD {}x{}@{}", - mode.width, mode.height, mode.refresh_hz - ) - })?; + let add_res = unsafe { ioctl(dev, control::IOCTL_ADD, bytemuck::bytes_of(&add), &mut out) }; + let add_res = match add_res { + Err(e) if is_slot_exhaustion_wedge(&e) => { + // The IddCx monitor-slot pool is exhausted by accumulated ghost (departed-but-not-present) + // virtual-monitor PDOs → ADD failed 0x80070490. Reap the ghosts in-process and retry ONCE + // so the wedge SELF-HEALS instead of hard-failing every session until a manual reset/reboot + // (the long-standing failure mode). pnputil removal is synchronous; a brief settle lets the + // OS recompute the adapter's monitor budget before the retry. + let reaped = reap_ghost_monitors(); + tracing::warn!( + reaped, + "pf-vdisplay ADD wedged (0x80070490 ERROR_NOT_FOUND) — reaped ghost monitor nodes, retrying ADD" + ); + // pnputil removal is durable (the ghosts are gone permanently), but the OS reclaims the + // IddCx VidPN-target slots via ASYNC PnP teardown that can lag the synchronous pnputil + // return. Retry the ADD a few times (300 ms apart, NO re-reap — the ghosts are already + // removed) to ride out that variable reclaim latency rather than guess one magic settle. + // ~1.5 s worst case, only on the rare wedge path. + let mut res = Err(anyhow::anyhow!("pf-vdisplay ADD retry loop did not run")); + for _ in 0..5 { + std::thread::sleep(std::time::Duration::from_millis(300)); + // SAFETY: identical to the first IOCTL_ADD above — `dev` is the live control handle + // (`add_monitor`'s contract), and `bytemuck::bytes_of(&add)` + `&mut out` borrow locals + // that outlive this synchronous call. + res = unsafe { + ioctl(dev, control::IOCTL_ADD, bytemuck::bytes_of(&add), &mut out) + }; + if res.is_ok() { + break; + } + } + res + } + other => other, + }; + add_res.with_context(|| { + format!( + "pf-vdisplay ADD {}x{}@{}", + mode.width, mode.height, mode.refresh_hz + ) + })?; // `pod_read_unaligned` (NOT `from_bytes`): `out` is a stack `[u8; N]` with no guaranteed 4-byte // alignment, and `from_bytes` PANICS on a mismatch. This copies into an aligned `AddReply`. let reply: control::AddReply = @@ -261,6 +361,25 @@ impl VdisplayDriver for PfVdisplayDriver { reply.target_id, luid.LowPart ); + // Per-client identity diagnostic: did the driver honor the host's preferred (stable) monitor id? + // A pre-Phase-2 driver leaves resolved_monitor_id=0 (it ignored the field); a current driver echoes + // the id it actually used. A mismatch means this session fell back to an auto id, so Windows won't + // reapply this client's saved per-monitor config (scaling) until it gets its stable id back. + if preferred_monitor_id != 0 { + if reply.resolved_monitor_id == preferred_monitor_id { + tracing::info!( + monitor_id = preferred_monitor_id, + "pf-vdisplay: per-client monitor id honored (stable identity → saved config persists)" + ); + } else { + tracing::warn!( + preferred = preferred_monitor_id, + resolved = reply.resolved_monitor_id, + "pf-vdisplay: preferred monitor id NOT honored (live-id collision, or a pre-Phase-2 \ + driver) — per-client config persistence degraded to auto identity this session" + ); + } + } if let Some(pin) = render_luid { if luid.LowPart == pin.LowPart && luid.HighPart == pin.HighPart { tracing::info!("pf-vdisplay ADD render adapter matches the pinned GPU (pin took)"); @@ -309,14 +428,19 @@ impl VdisplayDriver for PfVdisplayDriver { } } -/// The Windows pf-vdisplay virtual-display backend. A marker — the lifecycle lives in the shared -/// [`VirtualDisplayManager`](super::manager::VirtualDisplayManager). -pub struct PfVdisplayDisplay; +/// The Windows pf-vdisplay virtual-display backend. Near-stateless — the lifecycle lives in the shared +/// [`VirtualDisplayManager`](super::manager::VirtualDisplayManager); it only carries the connecting +/// client's fingerprint so the manager can assign a STABLE per-client monitor id (config persistence). +pub struct PfVdisplayDisplay { + /// The connecting client's cert fingerprint (`None` = anonymous/GameStream → the manager's auto id). + /// Set by [`set_client_identity`](VirtualDisplay::set_client_identity) before `create`. + client_fp: Option<[u8; 32]>, +} impl PfVdisplayDisplay { pub fn new() -> Result { super::manager::init(Box::new(PfVdisplayDriver)).open_backend()?; - Ok(Self) + Ok(Self { client_fp: None }) } } @@ -325,8 +449,12 @@ impl VirtualDisplay for PfVdisplayDisplay { "pf-vdisplay" } + fn set_client_identity(&mut self, fingerprint: Option<[u8; 32]>) { + self.client_fp = fingerprint; + } + fn create(&mut self, mode: Mode) -> Result { - super::manager::vdm().acquire(mode) + super::manager::vdm().acquire(mode, self.client_fp) } } diff --git a/packaging/windows/drivers/pf-vdisplay/src/control.rs b/packaging/windows/drivers/pf-vdisplay/src/control.rs index 22c6bc8..544bad1 100644 --- a/packaging/windows/drivers/pf-vdisplay/src/control.rs +++ b/packaging/windows/drivers/pf-vdisplay/src/control.rs @@ -133,9 +133,13 @@ unsafe fn add(request: WDFREQUEST) { complete(request, STATUS_INVALID_PARAMETER); return; } - let Some((target_id, luid_low, luid_high)) = - crate::monitor::create_monitor(req.session_id, req.width, req.height, req.refresh_hz) - else { + let Some((monitor_id, target_id, luid_low, luid_high)) = crate::monitor::create_monitor( + req.session_id, + req.width, + req.height, + req.refresh_hz, + req.preferred_monitor_id, + ) else { complete(request, STATUS_NOT_FOUND); return; }; @@ -143,7 +147,7 @@ unsafe fn add(request: WDFREQUEST) { adapter_luid_low: luid_low, adapter_luid_high: luid_high, target_id, - _reserved: 0, + resolved_monitor_id: monitor_id, }; // SAFETY: `request` is the framework WDFREQUEST. unsafe { write_output_complete(request, &reply) }; diff --git a/packaging/windows/drivers/pf-vdisplay/src/monitor.rs b/packaging/windows/drivers/pf-vdisplay/src/monitor.rs index d20a0d5..9cd1fc7 100644 --- a/packaging/windows/drivers/pf-vdisplay/src/monitor.rs +++ b/packaging/windows/drivers/pf-vdisplay/src/monitor.rs @@ -7,7 +7,7 @@ use std::sync::Mutex; use std::time::{Duration, Instant}; -use wdk_sys::iddcx; +use wdk_sys::{WDFOBJECT, call_unsafe_wdf_function_binding, iddcx}; /// One resolution with the refresh rates it supports. #[derive(Clone)] @@ -69,10 +69,23 @@ unsafe impl Send for MonitorObject {} /// heavy per-monitor resources on device removal is instead done explicitly ([`cleanup_for_device_removal`]). pub static MONITOR_MODES: Mutex> = Mutex::new(Vec::new()); +/// Lock [`MONITOR_MODES`], recovering the guard on poison instead of failing. DEFENSIVE ONLY: this driver +/// workspace builds with `panic = "abort"` (packaging/windows/drivers/Cargo.toml), so a panic while the +/// lock is held aborts the process WITHOUT unwinding — `MutexGuard::drop` never runs, the poison flag is +/// never set, and `.lock()` can never return `Err`. The `into_inner()` arm is therefore currently +/// unreachable; it is retained to consolidate the lock pattern and to stay correct if the panic strategy +/// ever becomes `unwind` (the guarded data is a plain `Vec` with no cross-field invariant a half-completed +/// panic could corrupt, so recovering the guard is sound). NOTE: this does NOT explain the observed ADD +/// 0x80070490 wedge — that is ghost-monitor slot-budget exhaustion (the arrival-failure `WdfObjectDelete` +/// teardown above + the host-side reap), not lock poisoning. +fn lock_monitors() -> std::sync::MutexGuard<'static, Vec> { + MONITOR_MODES.lock().unwrap_or_else(|e| e.into_inner()) +} + /// True if any virtual monitor currently exists — the host-gone watchdog only reaps when there's /// something to reap (see [`crate::control::start_watchdog`]). pub fn has_monitors() -> bool { - MONITOR_MODES.lock().map(|l| !l.is_empty()).unwrap_or(false) + !lock_monitors().is_empty() } /// Depart every monitor that has existed at least `grace` — the host-gone watchdog reap @@ -85,9 +98,7 @@ pub fn reap_orphaned(grace: Duration) -> usize { Option, Option, )> = { - let Ok(mut lock) = MONITOR_MODES.lock() else { - return 0; - }; + let mut lock = lock_monitors(); let mut taken = Vec::new(); let mut i = 0; while i < lock.len() { @@ -138,7 +149,8 @@ pub fn display_info( // Compute in u64 then saturate the u32 rational numerators: the old u32 `refresh*(h+4)^2` overflows // for a large mode (e.g. 8K@240), which panics→aborts the extern-"C" mode DDI in a debug build. // Identical for every real mode; only an absurd (also now bounds-rejected) mode saturates. - let clock_rate: u64 = u64::from(refresh_rate) * u64::from(height + 4) * u64::from(height + 4) + 1000; + let clock_rate: u64 = + u64::from(refresh_rate) * u64::from(height + 4) * u64::from(height + 4) + 1000; let clock_rate_u32 = u32::try_from(clock_rate).unwrap_or(u32::MAX); let mut si = pod_init!(wdk_sys::DISPLAYCONFIG_VIDEO_SIGNAL_INFO); si.pixelRate = clock_rate; @@ -264,9 +276,7 @@ pub fn set_swap_chain_processor( object: iddcx::IDDCX_MONITOR, proc: crate::swap_chain_processor::SwapChainProcessor, ) -> Option { - let Ok(mut lock) = MONITOR_MODES.lock() else { - return Some(proc); - }; + let mut lock = lock_monitors(); if let Some(m) = lock.iter_mut().find(|m| m.object == Some(object)) { m.swap_chain_processor.replace(proc) } else { @@ -290,15 +300,17 @@ pub fn take_swap_chain_processor( .take() } -/// `IOCTL_ADD`: create + arrive a virtual monitor at `width`x`height`@`refresh`. Returns the OS -/// `(target_id, adapter_luid_low, adapter_luid_high)` for the [`AddReply`](pf_driver_proto::control::AddReply), -/// or `None` on failure (no adapter yet / IddCx error). +/// `IOCTL_ADD`: create + arrive a virtual monitor at `width`x`height`@`refresh` for `session_id`, naming it +/// by `preferred_id` (the host's per-client stable id; `0` = auto-allocate). Returns the resolved +/// `(monitor_id, target_id, adapter_luid_low, adapter_luid_high)` for the +/// [`AddReply`](pf_driver_proto::control::AddReply), or `None` on failure (no adapter yet / IddCx error). pub fn create_monitor( session_id: u64, width: u32, height: u32, refresh: u32, -) -> Option<(u32, u32, i32)> { + preferred_id: u32, +) -> Option<(u32, u32, u32, i32)> { let adapter = crate::adapter::adapter()?; // Single identity per session (E1): if the host re-ADDs a still-live `session_id` (it shouldn't), depart // the stale monitor first, so one session maps to exactly one monitor (no duplicate EDID/target lingers). @@ -307,7 +319,9 @@ pub fn create_monitor( .map(|l| l.iter().any(|m| m.session_id == session_id)) .unwrap_or(false) { - dbglog!("[pf-vd] create_monitor: session {session_id} already live — departing the stale monitor"); + dbglog!( + "[pf-vd] create_monitor: session {session_id} already live — departing the stale monitor" + ); remove_monitor(session_id); } let mut modes = vec![Mode { @@ -317,17 +331,17 @@ pub fn create_monitor( }]; modes.extend(default_modes()); - // Register the (pending) monitor so the mode DDIs can find it by EDID-serial id before arrival, under a - // REUSED id (the lowest not currently live). Reclaiming the id on REMOVE — instead of a monotonic - // counter — keeps the connector index / EDID serial / container GUID bounded, so IddCx reuses the same - // OS target slot on a fresh ADD rather than leaving a ghost monitor node behind (the slot-exhaustion - // wedge: sustained ADD/REMOVE churn eventually makes ADD fail 0x80070490 ERROR_NOT_FOUND). Allocated - // under the lock with the push so two concurrent ADDs can't pick the same id. + // Register the (pending) monitor so the mode DDIs can find it by EDID-serial id before arrival. The id + // seeds the EDID serial + IddCx ConnectorIndex + ContainerId — i.e. the monitor's OS IDENTITY. Honor the + // host's per-client `preferred_id` when it is valid + not currently live, so a given client gets a + // STABLE identity across reconnects (→ Windows reapplies its saved per-monitor DPI scaling); else fall + // back to the lowest-free id (auto — the original slot-based behavior). A bounded reused id (vs a + // monotonic counter) keeps IddCx reusing the same OS target slot rather than leaving a ghost monitor + // node behind (the slot-exhaustion wedge). Allocated under the lock with the push so two concurrent ADDs + // can't pick the same id. let id = { - let Ok(mut lock) = MONITOR_MODES.lock() else { - return None; - }; - let id = alloc_monitor_id(&lock); + let mut lock = lock_monitors(); + let id = resolve_id(&lock, preferred_id); lock.push(MonitorObject { object: None, id, @@ -379,7 +393,8 @@ pub fn create_monitor( return None; } let monitor = create_out.MonitorObject; - if let Ok(mut lock) = MONITOR_MODES.lock() { + { + let mut lock = lock_monitors(); if let Some(m) = lock.iter_mut().find(|m| m.id == id) { m.object = Some(monitor); } @@ -391,6 +406,24 @@ pub fn create_monitor( let st = unsafe { wdk_iddcx::IddCxMonitorArrival(monitor, &mut arrival_out) }; dbglog!("[pf-vd] IddCxMonitorArrival(id={id}) -> {st:#x}"); if !wdk_iddcx::nt_success(st) { + // Arrival failed on a monitor we already CREATED. It must be torn down with `WdfObjectDelete`: + // `IddCxMonitorDeparture` is only valid for an ARRIVED monitor, so departing here would be a + // no-op that LEAKS the IddCx monitor object and permanently pins its slot against the adapter's + // `MaxMonitorsSupported` budget — the leak that, asymmetric with the create-failure path just + // above (which only reclaims the id, having no object to delete), accelerates the ADD 0x80070490 + // wedge. Reclaim the id FIRST (drop the `MONITOR_MODES` entry that still holds this handle) so a + // concurrent `clear_all`/`reap_orphaned` can't grab + depart the handle we're about to delete, + // THEN delete the object — `monitor` is a local copy of the handle, valid across both. + dbglog!( + "[pf-vd] IddCxMonitorArrival(id={id}) FAILED — reclaiming the id + deleting the created monitor" + ); + remove_by_id(id); + // SAFETY: `monitor` is the just-created (not-yet-arrived) IddCx monitor handle, now owned solely + // here (its `MONITOR_MODES` entry was just removed); `WdfObjectDelete` takes a `WDFOBJECT` (a raw + // handle cast, as in the swap-chain / device-cleanup teardowns). + unsafe { + call_unsafe_wdf_function_binding!(WdfObjectDelete, monitor as WDFOBJECT); + } return None; } @@ -399,14 +432,15 @@ pub fn create_monitor( arrival_out.OsAdapterLuid.LowPart, arrival_out.OsAdapterLuid.HighPart, ); - if let Ok(mut lock) = MONITOR_MODES.lock() { + { + let mut lock = lock_monitors(); if let Some(m) = lock.iter_mut().find(|m| m.id == id) { m.target_id = target_id; m.adapter_luid_low = luid_low; m.adapter_luid_high = luid_high; } } - Some((target_id, luid_low, luid_high)) + Some((id, target_id, luid_low, luid_high)) } /// `IOCTL_REMOVE`: depart + drop the monitor for `session_id`. Returns true if one was removed. @@ -415,9 +449,7 @@ pub fn remove_monitor(session_id: u64) -> bool { // (which RAII-joins its worker thread) only AFTER the lock guard is released — joining a worker // while holding `MONITOR_MODES` would head-block the whole control plane / risk a self-deadlock. let (monitor, processor) = { - let Ok(mut lock) = MONITOR_MODES.lock() else { - return false; - }; + let mut lock = lock_monitors(); let Some(pos) = lock.iter().position(|m| m.session_id == session_id) else { return false; }; @@ -441,9 +473,7 @@ pub fn clear_all() { Option, Option, )> = { - let Ok(mut lock) = MONITOR_MODES.lock() else { - return; - }; + let mut lock = lock_monitors(); lock.drain(..) .map(|mut m| (m.object, m.swap_chain_processor.take())) .collect() @@ -467,9 +497,7 @@ pub fn clear_all() { /// though the per-devnode WUDFHost (`ProcessSharingDisabled`) would also reap them when it exits. pub fn cleanup_for_device_removal() { let mut drained: Vec> = { - let Ok(mut lock) = MONITOR_MODES.lock() else { - return; - }; + let mut lock = lock_monitors(); lock.drain(..) .map(|mut m| m.swap_chain_processor.take()) .collect() @@ -483,8 +511,20 @@ pub fn cleanup_for_device_removal() { /// Drop a pending entry by id (create failed before arrival). fn remove_by_id(id: u32) { - if let Ok(mut lock) = MONITOR_MODES.lock() { - lock.retain(|m| m.id != id); + lock_monitors().retain(|m| m.id != id); +} + +/// Resolve the id to name a new monitor by: honor the host's `preferred` per-client id when it is in the +/// valid range (`1..=15`, so the IddCx `ConnectorIndex` = id stays `< MaxMonitorsSupported` = 16) AND not +/// currently live (two live monitors MUST have distinct ids/connectors); otherwise fall back to +/// [`alloc_monitor_id`] (auto, lowest-free). NEVER auto-departs a colliding live monitor — that would tear +/// down an unrelated concurrent client — so the live-uniqueness invariant is preserved even against a host +/// bug. `preferred == 0` (anonymous/TOFU/GameStream) always falls through to auto. Caller holds `MONITOR_MODES`. +fn resolve_id(modes: &[MonitorObject], preferred: u32) -> u32 { + if (1..=15).contains(&preferred) && !modes.iter().any(|m| m.id == preferred) { + preferred + } else { + alloc_monitor_id(modes) } }