diff --git a/docs/windows-host-goal1-plan.md b/docs/windows-host-goal1-plan.md index 844d7e2..03c2bb6 100644 --- a/docs/windows-host-goal1-plan.md +++ b/docs/windows-host-goal1-plan.md @@ -6,6 +6,11 @@ NVENC + IDD-push on-glass) and Goal-1 rewires its session/config/dispatch flow **preserve behavior**, compile + box-verify on its own, and be committed before the next starts. The plan's own §14 makes the §1 preservation checklist a mandatory per-module assert contract; honour it. +> **Status (2026-06-25):** all six staged stages **and** §2.5 (the ownership-model rewrite) are **DONE** — +> each is code + box-`cargo check --features nvenc` + (where it touches the deployed path) on-glass +> validated. Work lives on branch **`windows-host-goal1`** (off `main`, **not merged**). What's left is +> small and non-blocking — see [Remaining (next session)](#remaining-next-session) at the end. + ## Why staged (not one big rewrite) `main` is at parity and shipping. A monolithic rewrite would put the validated host in a broken @@ -126,6 +131,63 @@ follow-on; this delivers the folder confinement the stage is about). Done LAST, Verify: Linux `cargo check`/`clippy`/`fmt` clean; all 36 `#[path]` targets exist; no internal `#[path]`/`include!`/file-child-`mod` in any moved file; **box `cargo check --features nvenc` clean**. +**§2.5 — ownership-model rewrite: `VirtualDisplayManager` + `MonitorLease`. ✅ DONE (3 steps; code + box + on-glass reconnect-leak validated).** +The natural home for the deferrals above (Stage 4's `SessionFactory`/`Session::drop`/`vdm.lease`; Stage 5 +tightening 2's HDR/release → `VirtualLease`). A 5-agent map first established two facts that shaped the work: +**`CURRENT_MON_GEN` was WRITE-ONLY** (its only reader, `idd_push::my_gen`, was set-but-never-read — the +"per-frame monitor-gen bail" the docs describe was never wired; per-frame staleness is the *separate* ring +`FrameToken.generation`), so the design's "carry the monitor gen through `WinCaptureTarget`" was unnecessary; +and the two Windows backends (`sudovda` + `pf_vdisplay`) **duplicated the Idle/Active/Lingering refcount +state machine verbatim** (differing only in IOCTL proto + REMOVE key). User-approved shape: **one OnceLock +singleton `VirtualDisplayManager`**, not a threaded `Arc`. + +- **Step 1 (`1520201`)** — delete the dead/write-only code: `CURRENT_MON_GEN`/`my_gen`, `IDD_PERSIST`/ + `open_or_reuse`/`IddReuseHandle` (~150 lines). +- **Step 2 (`d9b8b88`)** — new `vdisplay/windows/manager.rs`: the two duplicated `MGR: Mutex` globals + collapse into one OnceLock `VirtualDisplayManager` { `Box`, `Arc` device + (typed — kills the raw-`isize` cross-thread smuggle **and** fixes a latent control-handle leak), + `Mutex`, `AtomicU64` gen }. `sudovda`/`pf_vdisplay` shrink to thin `VdisplayDriver` impls + (`open`/`add_monitor`/`remove_monitor`/`ping`) + thin `VirtualDisplay` wrappers; the IOCTL surface is the + only backend-specific code left. `MonitorKey = Guid(GUID) | Session(u64)`. `MON_GEN` + + `wait_for_monitor_released` move onto the manager; `MonitorLease::drop → vdm().release(gen)` preserves the + stale-lease no-op verbatim. +- **Step 3 (`fe61597`)** — the last two globals (`IDD_SETUP_LOCK`/`IDD_SESSION_STOP`) move onto the manager + behind `vdm().begin_idd_setup(stop)`; `punktfunk1` no longer reaches into vdisplay internals for the preempt. + +Net: `CURRENT_MON_GEN` / `MON_GEN` / two `MGR` / `IDD_PERSIST` / `IDD_SETUP_LOCK` / `IDD_SESSION_STOP` — +**all gone**, replaced by one encapsulated, typed manager. Behavior-preserving (the state machine is the +canonical `sudovda` copy routed through the driver seam). + +**On-glass reconnect-leak test ✅ (`683c81b`)** — it earned its keep: the box *compile* was clean, but the +first deploy **panicked** (`VirtualDisplayManager used before a backend initialised it`) because +`begin_idd_setup` called `vdm()` **before** `vdisplay::open` constructs the backend that runs +`manager::init()` (the old globals needed no init, so the ordering only broke once it became a manager +method). Fixed by opening the backend first — it does no monitor work, so the preempt-before-monitor-creation +semantics are preserved. After the fix: **0 panics**, the new `manager` module owns the lifecycle +(`vdisplay::manager: virtual-display monitor removed`), create == removed (net 0, **bounded**), **0 leaked +active monitors** across many reconnects; an A/B vs the shipping binary confirmed §2.5 is behaviour-equivalent. +Verified live on the **IDD-push zero-copy path** (`new_fps ~200` @5120×1440@240, **0 DDA fallbacks**). + +## Remaining (next session) + +Small, non-blocking follow-ons — the layered architecture is in place: + +1. **`EncoderCaps` (Stage 5 tightening 3)** — query optional encoder capabilities behind a small trait + instead of the default no-ops; additive, low-risk. The last seam-trait tightening. +2. **Optional `crate::*::windows::` namespace collapse** — Stage 6 confined the platform files into + `windows/`/`linux/` folders via `#[path]` (flat module names, zero reference churn); the deeper rename to + real `crate::capture::windows::` paths is optional cleanup, not required. +3. **Merge `windows-host-goal1` → `main`** — the branch is off `main` and **not merged**; local `main` is + also ~20 commits ahead of `origin/main` with unpushed audit/Stage work. Land both when ready (the + [Work-on-main] habit otherwise applies). +4. **(driver — NOT the host refactor) pf-vdisplay slot reclaim** — surfaced on-glass: sustained ADD/REMOVE + churn wedges the driver (`ADD → 0x80070490 ERROR_NOT_FOUND`) because it doesn't reclaim IddCx monitor + slots on REMOVE (ghost monitor nodes accumulate, `target_id`s climb). Recovery today is + `packaging/windows/reset-pf-vdisplay.ps1`; the real fix lives in the driver WIP + (`packaging/windows/drivers/pf-vdisplay/src/{control,adapter}.rs`). Dev-iteration helpers + `reset-pf-vdisplay.ps1` + `redeploy-pf-vdisplay.ps1` are committed under `packaging/windows/` (validated + live). + ## Guardrails (mandatory, plan §14) - Each stage is its own commit; box-verify before moving on. diff --git a/docs/windows-host-rewrite.md b/docs/windows-host-rewrite.md index 5e956d4..b586800 100644 --- a/docs/windows-host-rewrite.md +++ b/docs/windows-host-rewrite.md @@ -229,6 +229,12 @@ path readable, and removes the capture/encode backend-disagreement bug class. It ### 2.5 Ownership model — delete the global statics +> **✅ IMPLEMENTED (2026-06-25, branch `windows-host-goal1`).** Landed as 3 steps + an on-glass +> reconnect-leak test — see [`windows-host-goal1-plan.md`](windows-host-goal1-plan.md) §2.5 for the +> commits + results. One deviation from the sketch below: the 5-agent map found **`CURRENT_MON_GEN` was +> write-only** (the per-frame monitor-gen bail was never wired), so the "generation carried through +> `WinCaptureTarget`" item was unnecessary and dropped; the gen lives on the manager + lease only. + Today the lifecycle is smeared across `IDD_PERSIST` + `open_or_reuse` (dead code), `CURRENT_MON_GEN` (read per-frame), `IDD_SETUP_LOCK`/`IDD_SESSION_STOP` (the preempt dance), `MGR: Mutex`, and on the driver side `ADAPTER`/`MONITOR_MODES`/`NEXT_ID`/`WATCHDOG_*`/`DEVICE_POOL`. Replace with: