From 3d9b32908492f3582a020d291065ad9e11837e9b Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Fri, 17 Jul 2026 10:47:47 +0200 Subject: [PATCH] =?UTF-8?q?fix(host):=20name=20the=20lid-closed/no-frames?= =?UTF-8?q?=20failure=20=E2=80=94=20display-write=20decode,=20console-sess?= =?UTF-8?q?ion=20guard,=20driver-truth=20attach=20diagnostics?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Field report (Windows laptop, lid closed, Tailscale): v0.12.0's activation fix works — the pf-vdisplay target activates in ~200ms — but the session still dies at the first-frame gate: 'driver_status=1 but no frame published within 4s'. Triage showed three independent blind spots; this names all of them at their source instead of guessing downstream: - pf-win-display: decode ChangeDisplaySettingsExW failures (-1 FAILED — a display write rejected, the wrong/remote-session signature — vs -2 BADMODE, which the old 'mode not advertised?' text conflated), and WARN on every non-zero SetDisplayConfig rc in the CCD isolate even when verification passes vacuously (the lid-closed case: nothing else active, so the INFO swallowed rc=0x5 ERROR_ACCESS_DENIED while the load-bearing COMMIT_MODES → ASSIGN_SWAPCHAIN re-commit silently never applied). Access-denied rcs get the remedy appended (console session / installed service). - host: console-session guard (interactive::console_session_mismatch) — a host outside the active console session (a hand-launched host after an RDP round-trip) fails every display write, reads the wrong session's GDI view, and its SendInput compose kicks go nowhere. Named ERROR at vdisplay acquire + appended to the first-frame timeout, instead of the misleading generic failure. (The idd_push diagnosis half of this landed in 9a36ea21; this commit adds the proto helpers + session guard it references, healing the windows-cfg build.) - proto + driver: while OPENED, driver_status_detail now carries a live packed word (bit31 live-marker | offered 15-bit | mismatch-dropped 16-bit) maintained by the publisher, so the host's first-frame timeout can tell apart: never-attached (no swap-chain worker ran), attached-but-DWM-composed- zero-frames (undamaged/powered-off desktop, kicks blocked on the secure desktop), and composed-but-every-frame-mismatched (ring sized from a stale/ foreign-session GDI mode). Zero layout change, old drivers read as 'no detail'; unit-tested pack/unpack in pf-driver-proto. Verified on winbox: cargo check + clippy -p punktfunk-host -p pf-win-display -p pf-driver-proto EXIT 0, drivers ws cargo check -p pf-vdisplay EXIT 0 (Version_Number=10.0.26100.0), cargo fmt --all --check clean; pf-driver-proto tests 13/13 pass locally. Co-Authored-By: Claude Fable 5 --- crates/pf-driver-proto/src/lib.rs | 61 ++++++++++++++++++ crates/pf-win-display/src/win_display.rs | 64 +++++++++++++++++-- .../src/vdisplay/windows/manager.rs | 16 +++++ .../punktfunk-host/src/windows/interactive.rs | 21 ++++++ .../pf-vdisplay/src/frame_transport.rs | 36 ++++++++++- 5 files changed, 190 insertions(+), 8 deletions(-) diff --git a/crates/pf-driver-proto/src/lib.rs b/crates/pf-driver-proto/src/lib.rs index 283f585f..a4b4cd91 100644 --- a/crates/pf-driver-proto/src/lib.rs +++ b/crates/pf-driver-proto/src/lib.rs @@ -379,6 +379,46 @@ pub mod frame { /// `design/idd-push-security.md`); `driver_status_detail` carries the target id the ring claims. pub const DRV_STATUS_BIND_FAIL: u32 = 4; + /// While `driver_status` is [`DRV_STATUS_OPENED`], `driver_status_detail` carries a LIVE + /// diagnostic word maintained by the driver's publisher (best-effort plain writes — the same + /// visibility contract as `driver_status` itself). Layout: + /// + /// - bit 31 (this constant): stamped at attach by a detail-capable driver, so the host can + /// tell "pre-detail driver, no information" (field = 0) from "zero frames offered". + /// - bits 30..16: surfaces the swap-chain worker OFFERED to the ring (15-bit, saturating) — + /// every DWM compose that reached `publish()`, whatever its outcome. + /// - bits 15..0: publishes DROPPED for a descriptor mismatch (16-bit, saturating) — the + /// surface's size/format didn't match the ring's. + /// + /// The host's wait-for-attach reads this on its first-frame timeout to NAME the failure + /// instead of guessing (the lid-closed field report was undiagnosable without it): + /// `offered == 0` → DWM never composed the display (powered-off / undamaged desktop, compose + /// kicks blocked); `offered > 0` with `seq` still 0 → every compose was dropped mismatched + /// (the host sized the ring from a stale or foreign-session GDI mode). + pub const OPENED_DETAIL_LIVE: u32 = 0x8000_0000; + + /// Pack the live OPENED diagnostic word (see [`OPENED_DETAIL_LIVE`]); both counters saturate. + #[must_use] + pub const fn pack_opened_detail(offered: u32, mismatched: u32) -> u32 { + let o = if offered > 0x7FFF { 0x7FFF } else { offered }; + let m = if mismatched > 0xFFFF { + 0xFFFF + } else { + mismatched + }; + OPENED_DETAIL_LIVE | (o << 16) | m + } + + /// Unpack a live OPENED diagnostic word → `(offered, mismatched)`; `None` when the driver + /// never stamped [`OPENED_DETAIL_LIVE`] (a pre-detail driver — the field carries nothing). + #[must_use] + pub const fn unpack_opened_detail(detail: u32) -> Option<(u32, u32)> { + if detail & OPENED_DETAIL_LIVE == 0 { + return None; + } + Some(((detail >> 16) & 0x7FFF, detail & 0xFFFF)) + } + /// The shared metadata header (host-created, mapped by both sides). Atomic fields (`magic`, `latest`, /// `generation`) are accessed via each side's own atomic view over the mapping; this is the layout. #[repr(C)] @@ -758,6 +798,27 @@ mod tests { assert_eq!(t.pack(), (7u64 << 40) | (42u64 << 8) | 3u64); } + #[test] + fn opened_detail_roundtrips_and_saturates() { + use frame::{pack_opened_detail, unpack_opened_detail, OPENED_DETAIL_LIVE}; + // Zero counters still stamp LIVE — "attached, nothing offered yet" is information. + assert_eq!(pack_opened_detail(0, 0), OPENED_DETAIL_LIVE); + assert_eq!(unpack_opened_detail(pack_opened_detail(0, 0)), Some((0, 0))); + // Roundtrip within range. + assert_eq!( + unpack_opened_detail(pack_opened_detail(1234, 567)), + Some((1234, 567)) + ); + // Saturation at each counter's width (15-bit offered, 16-bit mismatched). + assert_eq!( + unpack_opened_detail(pack_opened_detail(u32::MAX, u32::MAX)), + Some((0x7FFF, 0xFFFF)) + ); + // A pre-detail driver's field (any value without the LIVE bit) carries no information. + assert_eq!(unpack_opened_detail(0), None); + assert_eq!(unpack_opened_detail(0x7FFF_FFFF), None); + } + #[test] fn shared_header_is_pod_and_64_bytes() { let mut h = frame::SharedHeader::zeroed(); diff --git a/crates/pf-win-display/src/win_display.rs b/crates/pf-win-display/src/win_display.rs index c8218501..c3dac18b 100644 --- a/crates/pf-win-display/src/win_display.rs +++ b/crates/pf-win-display/src/win_display.rs @@ -429,10 +429,11 @@ pub fn set_active_mode(gdi_name: &str, mode: Mode) { if test != DISP_CHANGE_SUCCESSFUL { tracing::warn!( result = test.0, - "{gdi_name}: driver rejected {}x{}@{} (mode not advertised?) — leaving OS default", + "{gdi_name}: mode-set {}x{}@{} rejected ({}) — leaving OS default", mode.width, mode.height, - chosen_hz + chosen_hz, + disp_change_reason(test.0) ); return; } @@ -458,14 +459,48 @@ pub fn set_active_mode(gdi_name: &str, mode: Mode) { } else { tracing::warn!( result = apply.0, - "{gdi_name}: failed to apply {}x{}@{}", + "{gdi_name}: failed to apply {}x{}@{} ({})", mode.width, mode.height, - chosen_hz + chosen_hz, + disp_change_reason(apply.0) ); } } +/// Human decode for a failed `ChangeDisplaySettingsExW` result. The two codes worth telling apart +/// in a field log: `BADMODE` (the display's mode list genuinely lacks the mode) vs `FAILED` (the +/// write itself was rejected — on a healthy driver that is the signature of a host process without +/// console-session access, e.g. one trapped in a disconnected RDP session). An earlier revision +/// printed "mode not advertised?" for BOTH, which sent a lid-closed field report chasing the wrong +/// cause. +fn disp_change_reason(rc: i32) -> &'static str { + match rc { + -1 => { + "DISP_CHANGE_FAILED: the display write was rejected — a host without console-session \ + access (disconnected RDP session / non-console session) fails ALL display writes \ + this way" + } + -2 => "DISP_CHANGE_BADMODE: the display does not advertise this mode", + -3 => "DISP_CHANGE_NOTUPDATED: registry write failed", + -4 => "DISP_CHANGE_BADFLAGS", + -5 => "DISP_CHANGE_BADPARAM", + _ => "unrecognized DISP_CHANGE code", + } +} + +/// Appended to `SetDisplayConfig` failure logs when rc is `ERROR_ACCESS_DENIED` (0x5) — per MS +/// docs "the caller does not have access to the console session", the field signature of a host +/// running in a disconnected RDP / non-console session. Every other rc gets no hint. +fn sdc_access_denied_hint(rc: i32) -> &'static str { + if rc == 5 { + " (ERROR_ACCESS_DENIED: the host has no console-session access — disconnected RDP \ + session? run via the installed service so it tracks the console session)" + } else { + "" + } +} + /// Saved active display topology, for restoring on teardown. // pub so vdisplay::pf_vdisplay's Monitor can hold the same saved-topology type. pub type SavedConfig = (Vec, Vec); @@ -688,6 +723,17 @@ pub unsafe fn isolate_displays_ccd(keep_target_ids: &[u32]) -> Option Option, quit: Option>, ) -> Result { + // Console-session guard: a host outside the ACTIVE console session cannot drive the display + // it is about to create — every SetDisplayConfig/CDS write fails ERROR_ACCESS_DENIED, GDI + // reads describe the wrong session's displays, and SendInput compose kicks go nowhere; the + // session then dies far downstream as "no frame published within 4s" (the lid-closed field + // report). Name the real problem up front, once per acquire. Non-fatal: the OS-side + // persistence-DB activation can still succeed, so the attempt proceeds. + if let Some((own, console)) = crate::interactive::console_session_mismatch() { + tracing::error!( + own_session = own, + console_session = console, + "punktfunk-host is NOT in the active console session — display activation, \ + mode-set and capture will fail (disconnected RDP session?). Reconnect the \ + console (`tscon {own} /dest:console`) or run the host via the installed \ + service, which follows the console session" + ); + } self.ensure_linger_timer(); let slot = slot_id_for(client_fp, (mode.width, mode.height)); let mut inner = self.state.lock().unwrap(); diff --git a/crates/punktfunk-host/src/windows/interactive.rs b/crates/punktfunk-host/src/windows/interactive.rs index 69298a68..412e826a 100644 --- a/crates/punktfunk-host/src/windows/interactive.rs +++ b/crates/punktfunk-host/src/windows/interactive.rs @@ -30,6 +30,27 @@ use windows::Win32::System::Threading::{ CreateProcessAsUserW, CREATE_UNICODE_ENVIRONMENT, PROCESS_INFORMATION, STARTUPINFOW, }; +/// `Some((own_session, console_session))` when this process is NOT in the active console session — +/// the state in which every display write (`SetDisplayConfig` / `ChangeDisplaySettingsExW`) fails +/// `ERROR_ACCESS_DENIED`, GDI reads describe the wrong session's displays, and `SendInput` compose +/// kicks go nowhere. Field signature: a hand-launched host whose session later went remote (an RDP +/// round-trip) — the installed service relaunches the host when the console session moves, a +/// hand-launched one stays trapped. `None` in the console session, and `None` when the check +/// itself can't answer (no console session attached — boot / session transition — or the session +/// query failed): the guard exists to NAME a known-bad state, so an unknown state stays quiet. +pub fn console_session_mismatch() -> Option<(u32, u32)> { + use windows::Win32::System::RemoteDesktop::ProcessIdToSessionId; + use windows::Win32::System::Threading::GetCurrentProcessId; + let mut own: u32 = 0; + // SAFETY: `own` is a live local out-param for this synchronous call; no pointer escapes it. + if unsafe { ProcessIdToSessionId(GetCurrentProcessId(), &mut own) }.is_err() { + return None; + } + // SAFETY: takes no arguments and returns the console session id by value. + let console = unsafe { WTSGetActiveConsoleSessionId() }; + (console != 0xFFFF_FFFF && own != console).then_some((own, console)) +} + /// Spawn `cmdline` in the active console session, under the logged-in user's token, on the /// interactive desktop (`winsta0\default`). Returns the new process id. /// diff --git a/packaging/windows/drivers/pf-vdisplay/src/frame_transport.rs b/packaging/windows/drivers/pf-vdisplay/src/frame_transport.rs index d5e1fb2d..8ab4eebf 100644 --- a/packaging/windows/drivers/pf-vdisplay/src/frame_transport.rs +++ b/packaging/windows/drivers/pf-vdisplay/src/frame_transport.rs @@ -31,7 +31,7 @@ use std::time::Instant; use pf_driver_proto::control::SetFrameChannelRequest; use pf_driver_proto::frame::{ AttachReject, DRV_STATUS_BIND_FAIL, DRV_STATUS_NO_DEVICE1, DRV_STATUS_OPENED, - DRV_STATUS_TEX_FAIL, FrameToken, RING_LEN, SharedHeader, check_attach, + DRV_STATUS_TEX_FAIL, FrameToken, RING_LEN, SharedHeader, check_attach, pack_opened_detail, }; use windows::Win32::Foundation::{CloseHandle, HANDLE}; use windows::Win32::Graphics::Direct3D11::{ @@ -286,6 +286,12 @@ pub struct FramePublisher { /// Set when a surface is dropped for a descriptor mismatch (a game mode-set the display), cleared on a /// matched publish — throttles the drop log to once per mismatch episode (game-capture bug GB1). mismatch_logged: bool, + /// Live diagnostic counters mirrored into `SharedHeader::driver_status_detail` after every + /// `publish()` (see proto `pack_opened_detail`): surfaces OFFERED to the ring, and how many of + /// those were DROPPED for a descriptor mismatch. What lets the host's first-frame timeout tell + /// "DWM never composed" from "every compose mismatched the ring". + offered: u32, + mismatch_drops: u32, /// The slot of the most recent successful publish + when it happened — what [`Self::harvest_into`] /// reads when this publisher is superseded. `None` until the first publish. last_published: Option<(u32, Instant)>, @@ -486,8 +492,13 @@ impl FramePublisher { } } - // SAFETY: `header` is the mapped host header; the status field lives within it. + // Stamp the LIVE diagnostic word BEFORE the status flip, so a host that reads OPENED can + // trust the detail field is ours (zero counters = "attached, nothing offered yet" — the + // host's wait-for-attach uses this to tell a never-composed display from a pre-detail + // driver). Plain best-effort writes, same contract as `driver_status` itself. + // SAFETY: `header` is the mapped host header; the status/detail fields live within it. unsafe { + (*header).driver_status_detail = pack_opened_detail(0, 0); (*header).driver_status = DRV_STATUS_OPENED; } dbglog!( @@ -505,12 +516,26 @@ impl FramePublisher { ring_format: unsafe { (*header).dxgi_format }, generation: header_gen, mismatch_logged: false, + offered: 0, + mismatch_drops: 0, last_published: None, render_luid_low, render_luid_high, }) } + /// Mirror the live diagnostic counters into the header's detail word (proto + /// `pack_opened_detail`) — read by the host's first-frame timeout to name a no-frames failure. + #[inline] + fn write_opened_detail(&self) { + // SAFETY: `self.header` stays mapped for the publisher's lifetime (unmapped only in Drop); + // `driver_status_detail` is a plain in-bounds u32 field — a best-effort diagnostic write. + unsafe { + (*self.header).driver_status_detail = + pack_opened_detail(self.offered, self.mismatch_drops); + } + } + #[inline] fn latest_cell(&self) -> &AtomicU64 { // SAFETY: `self.header` stays mapped for the publisher's lifetime (unmapped only in Drop); the @@ -595,7 +620,13 @@ impl FramePublisher { // report the ACTUAL descriptor once per episode so a repro shows exactly what changed. // SAFETY: `self.header` stays mapped for the publisher's lifetime; width/height are plain u32 fields. let (rw, rh) = unsafe { ((*self.header).width, (*self.header).height) }; + // Live diagnostics: count every surface offered (and, below, every mismatch drop) into the + // header's detail word — what lets the host's first-frame timeout tell "DWM never composed" + // from "every compose mismatched the ring". Written once per call, after the outcome is known. + self.offered = self.offered.saturating_add(1); if desc.Format.0 as u32 != self.ring_format || desc.Width != rw || desc.Height != rh { + self.mismatch_drops = self.mismatch_drops.saturating_add(1); + self.write_opened_detail(); if !self.mismatch_logged { self.mismatch_logged = true; dbglog!( @@ -611,6 +642,7 @@ impl FramePublisher { return PublishOutcome::DescMismatch; } self.mismatch_logged = false; + self.write_opened_detail(); let start = self.next; for attempt in 0..ring_len { let slot = (start + attempt) % ring_len;