feat(windows-drivers): EvtCleanupCallback + single-identity dedup; document state ownership (E1)
EvtCleanupCallback on the WDFDEVICE (entry.rs + callbacks::device_cleanup): on device removal (PnP/unload) drop every monitor's swap-chain worker via monitor::cleanup_for_device_removal (joins threads, IddCx-free — the framework tears the monitors down with the device). Worker threads no longer linger into teardown. Single identity per session (create_monitor): a re-ADD of a still-live session_id departs the stale monitor first, so one session maps to exactly one monitor (no duplicate EDID/target). DeviceContext-owned state (audit §2.5): documented decision NOT to migrate the globals to a Box/AtomicPtr device-owned allocation. The IddCx monitor/mode DDIs receive only an IddCx handle (never the WDFDEVICE/context), so the state MUST be globally reachable (upstream virtual-display-rs is a process-static for the same reason); the globals are already module-encapsulated; and with one devnode + UmdfHostProcessSharing=ProcessSharingDisabled they die with the host process on removal anyway. A pointer variant would only add a host-gone-watchdog-race use-after-free for zero benefit. Verified: driver workspace builds clean on the RTX box (.173). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -37,6 +37,15 @@ pub unsafe extern "C" fn adapter_init_finished(
|
|||||||
STATUS_SUCCESS
|
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`.
|
/// SDR mode list for an EDID monitor: EDID-serial lookup → count-then-fill `IDDCX_MONITOR_MODE`.
|
||||||
pub unsafe extern "C" fn parse_monitor_description(
|
pub unsafe extern "C" fn parse_monitor_description(
|
||||||
p_in: *const iddcx::IDARG_IN_PARSEMONITORDESCRIPTION,
|
p_in: *const iddcx::IDARG_IN_PARSEMONITORDESCRIPTION,
|
||||||
|
|||||||
@@ -113,6 +113,9 @@ extern "C" fn driver_add(_driver: WDFDRIVER, mut init: PWDFDEVICE_INIT) -> NTSTA
|
|||||||
dev_attr.SynchronizationScope =
|
dev_attr.SynchronizationScope =
|
||||||
wdk_sys::_WDF_SYNCHRONIZATION_SCOPE::WdfSynchronizationScopeInheritFromParent;
|
wdk_sys::_WDF_SYNCHRONIZATION_SCOPE::WdfSynchronizationScopeInheritFromParent;
|
||||||
dev_attr.ContextTypeInfo = &DEVICE_CTX.0;
|
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.
|
// SAFETY: init configured above; dev_attr is a valid context-typed attributes block.
|
||||||
let status = unsafe {
|
let status = unsafe {
|
||||||
call_unsafe_wdf_function_binding!(WdfDeviceCreate, &mut init, &mut dev_attr, &mut device)
|
call_unsafe_wdf_function_binding!(WdfDeviceCreate, &mut init, &mut dev_attr, &mut device)
|
||||||
|
|||||||
@@ -60,6 +60,14 @@ pub struct MonitorObject {
|
|||||||
// SAFETY: the raw IddCx monitor handle is framework-managed; access is serialized by MONITOR_MODES.
|
// SAFETY: the raw IddCx monitor handle is framework-managed; access is serialized by MONITOR_MODES.
|
||||||
unsafe impl Send for MonitorObject {}
|
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<Vec<MonitorObject>> = Mutex::new(Vec::new());
|
pub static MONITOR_MODES: Mutex<Vec<MonitorObject>> = Mutex::new(Vec::new());
|
||||||
/// Monitor id / EDID-serial counter (unique per created monitor).
|
/// Monitor id / EDID-serial counter (unique per created monitor).
|
||||||
static NEXT_ID: AtomicU32 = AtomicU32::new(1);
|
static NEXT_ID: AtomicU32 = AtomicU32::new(1);
|
||||||
@@ -305,6 +313,16 @@ pub fn create_monitor(
|
|||||||
refresh: u32,
|
refresh: u32,
|
||||||
) -> Option<(u32, u32, i32)> {
|
) -> Option<(u32, u32, i32)> {
|
||||||
let adapter = crate::adapter::adapter()?;
|
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 id = NEXT_ID.fetch_add(1, Ordering::Relaxed);
|
||||||
|
|
||||||
let mut modes = vec![Mode {
|
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<Option<crate::swap_chain_processor::SwapChainProcessor>> = {
|
||||||
|
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).
|
/// Drop a pending entry by id (create failed before arrival).
|
||||||
fn remove_by_id(id: u32) {
|
fn remove_by_id(id: u32) {
|
||||||
if let Ok(mut lock) = MONITOR_MODES.lock() {
|
if let Ok(mut lock) = MONITOR_MODES.lock() {
|
||||||
|
|||||||
Reference in New Issue
Block a user