diff --git a/packaging/windows/drivers/pf-vdisplay/src/callbacks.rs b/packaging/windows/drivers/pf-vdisplay/src/callbacks.rs index 3d1788f..ac50bc5 100644 --- a/packaging/windows/drivers/pf-vdisplay/src/callbacks.rs +++ b/packaging/windows/drivers/pf-vdisplay/src/callbacks.rs @@ -37,6 +37,15 @@ pub unsafe extern "C" fn adapter_init_finished( STATUS_SUCCESS } +/// `EvtCleanupCallback` on the WDFDEVICE (E1): the device is being removed (PnP / driver unload) — drop +/// every monitor's swap-chain worker so the worker threads don't linger into teardown. IddCx-free (the +/// framework tears the monitors down with the departing device); see +/// [`crate::monitor::cleanup_for_device_removal`]. +pub unsafe extern "C" fn device_cleanup(_object: WDFOBJECT) { + dbglog!("[pf-vd] device cleanup — releasing monitors"); + crate::monitor::cleanup_for_device_removal(); +} + /// SDR mode list for an EDID monitor: EDID-serial lookup → count-then-fill `IDDCX_MONITOR_MODE`. pub unsafe extern "C" fn parse_monitor_description( p_in: *const iddcx::IDARG_IN_PARSEMONITORDESCRIPTION, diff --git a/packaging/windows/drivers/pf-vdisplay/src/entry.rs b/packaging/windows/drivers/pf-vdisplay/src/entry.rs index 4126844..c8a4c9b 100644 --- a/packaging/windows/drivers/pf-vdisplay/src/entry.rs +++ b/packaging/windows/drivers/pf-vdisplay/src/entry.rs @@ -113,6 +113,9 @@ extern "C" fn driver_add(_driver: WDFDRIVER, mut init: PWDFDEVICE_INIT) -> NTSTA dev_attr.SynchronizationScope = wdk_sys::_WDF_SYNCHRONIZATION_SCOPE::WdfSynchronizationScopeInheritFromParent; dev_attr.ContextTypeInfo = &DEVICE_CTX.0; + // Drop every monitor's swap-chain worker when the device is removed (PnP / unload), so the worker + // threads don't linger into teardown (E1 device cleanup). IddCx-free; see callbacks::device_cleanup. + dev_attr.EvtCleanupCallback = Some(callbacks::device_cleanup); // SAFETY: init configured above; dev_attr is a valid context-typed attributes block. let status = unsafe { call_unsafe_wdf_function_binding!(WdfDeviceCreate, &mut init, &mut dev_attr, &mut device) diff --git a/packaging/windows/drivers/pf-vdisplay/src/monitor.rs b/packaging/windows/drivers/pf-vdisplay/src/monitor.rs index 3917cb6..0172ff3 100644 --- a/packaging/windows/drivers/pf-vdisplay/src/monitor.rs +++ b/packaging/windows/drivers/pf-vdisplay/src/monitor.rs @@ -60,6 +60,14 @@ pub struct MonitorObject { // SAFETY: the raw IddCx monitor handle is framework-managed; access is serialized by MONITOR_MODES. unsafe impl Send for MonitorObject {} +/// All live monitors. A process-`static` (not a WDFDEVICE-context-owned allocation) BY NECESSITY: the IddCx +/// monitor/mode DDIs receive only an IddCx handle — never the WDFDEVICE or its context — so this state must +/// be reachable without one (the upstream virtual-display-rs is a process-`static` for the same reason). +/// With a single `pf_vdisplay` devnode + `UmdfHostProcessSharing=ProcessSharingDisabled` the host process +/// (and this state) die WITH the device, so it is effectively device-scoped already; a `Box` + `AtomicPtr` +/// "device-owned" variant (audit §2.5) would only add a use-after-free window — the host-gone watchdog +/// thread ([`crate::control::start_watchdog`]) races device cleanup — for no real gain. Cleanup of the +/// heavy per-monitor resources on device removal is instead done explicitly ([`cleanup_for_device_removal`]). pub static MONITOR_MODES: Mutex> = Mutex::new(Vec::new()); /// Monitor id / EDID-serial counter (unique per created monitor). static NEXT_ID: AtomicU32 = AtomicU32::new(1); @@ -305,6 +313,16 @@ pub fn create_monitor( refresh: u32, ) -> Option<(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). + if MONITOR_MODES + .lock() + .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"); + remove_monitor(session_id); + } let id = NEXT_ID.fetch_add(1, Ordering::Relaxed); let mut modes = vec![Mode { @@ -459,6 +477,27 @@ pub fn clear_all() { } } +/// `EvtCleanupCallback` (device removal, [`crate::callbacks::device_cleanup`]): drop every monitor's heavy +/// resources — the swap-chain processor workers (each RAII-joins its thread + deletes its swap-chain) — and +/// clear the list, WITHOUT `IddCxMonitorDeparture` (the framework tears the IddCx monitors down together +/// with the departing device; departing here would double-tear). Frees our worker threads promptly even +/// 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; + }; + lock.drain(..) + .map(|mut m| m.swap_chain_processor.take()) + .collect() + }; + // Drop the workers (join their threads) AFTER releasing the lock — joining under MONITOR_MODES would + // head-block the control plane (same discipline as remove_monitor / clear_all). + for processor in &mut drained { + drop(processor.take()); + } +} + /// Drop a pending entry by id (create failed before arrival). fn remove_by_id(id: u32) { if let Ok(mut lock) = MONITOR_MODES.lock() {