fix(host/pads): an unplugged controller actually disappears #26

2026-08-03T15:35:24Z

enricobuehler commented

2026-08-03 15:35:24 +00:00

Workstream M2 of the 2026-08-03 force-feedback sweep — closes B2.

The bug

Unplug a controller mid-session and the virtual pad it was driving outlives it. The game keeps seeing a connected, permanently idle device for the rest of the session. A single-controller session — the common case — hits this every time.

PadSlots::sweep needs two passes to retire a pad: the first pass to see the mask bit clear only arms the 300 ms devnode-churn grace (pad_slots.rs:96), and the drop lands on a later pass (:97). But sweep has exactly three callers (uhid_manager.rs:221, linux/gamepad.rs:650, windows/gamepad_windows.rs:322), all inside handle()'s GamepadEvent::State arm — and the producer emits exactly one frame per detach, because native/input.rs:819 guards the emit on the bit still being set.

So unless a sibling pad in the same manager keeps sending changed frames past the grace, the second pass never comes. Nothing periodic reaches sweep: heartbeat and pump both walk the slots without it.

Verified before fixing

This was checked by four independent readers plus three adversarial verifiers, because the finding shipped with a caveat that nobody had read the test module. All three verifiers returned confirmed_with_correction. The corrections, which are folded into this PR:

Severity is medium, not high. Clients flush held buttons and axes to neutral before sending the remove, so what heartbeat() keeps re-emitting is neutral — a phantom idle pad, not stuck input.
The leak is session-bounded. Pads is a local of the per-session input thread and each backend's Drop really does destroy the devnode.
Only half the dedup consequence is real. last_rumble is already covered by the 2.5 s abandoned-rumble watchdog; only hidout_dedup (the rich 0xCD plane, which has no watchdog) genuinely goes stale into a re-plug.
The precondition is broader than "last pad in a manager" — it is "no changed frame reaches that manager ≥300 ms later". An idle sibling only defers the reap; only a re-plug on the same index cancels it.

The fix

Split the two halves:

sweep(mask) still folds a frame's active_mask into the grace clocks, then reaps — unchanged for existing callers.
reap() is new: it drops whatever has run out, with no frame required. All three managers call it from the periodic pump they already run, so teardown completes ~300 ms after the detach instead of never.

reap deliberately cannot arm a clock — it only reads inactive_since and clears it, so a pad whose bit never went clear has nothing to run out and no amount of reaping can drop it. That invariant is what makes it safe on a hot loop, and it preserves the anti-flap guarantee: a mask that blips clear and returns still never churns a devnode.

reset_swept is extracted in the two managers with per-index sibling state, so an unplug completed on the pump clears exactly what one completed on a frame would — hidout_dedup in particular.

Tests

The existing tests are why this hid. Both hand-fed a second removal frame that production never sends, so they passed while the real path leaked. Both now drive the unplug through a pump tick.

Three new PadSlots tests pin the invariants: one frame + a reap completes the unplug; reap alone never drops a pad no frame deactivated; a glitch returning inside the grace still doesn't drop.

Proven non-vacuous — with the reap neutered, both manager tests fail with the right diagnostics (the pump tick never completed the unplug / the pump tick completed the unplug).

Verification

Run in the amd64 container:

cargo clippy -p pf-inject --all-targets --locked -- -D warnings → 0
cargo test -p pf-inject → 86 passed, 0 failed
cargo fmt --all --check → clean

Worth a reviewer's eye

This puts UI_DEV_DESTROY on the GameStream ENet control thread's 2 ms service budget for the first time. Reachable there before via handle→sweep, but now time-driven.
A mask glitch longer than 300 ms now genuinely flaps, where today it survives if no frame happens to arrive after the deadline. That is SWEEP_GRACE working as documented — deliberately not compensated for by raising the constant.
windows/gamepad_windows.rs has no test module and linux/gamepad.rs's tests need a real uinput node, so those two legs have review plus shared PadSlots coverage only. Windows and on-glass Linux verification still owed.
linux/steam_gadget.rs:366-374 has a comment naming PadSlots::sweep as its caller and bounding teardown at ~1 s — still true, but that second now lands in the feedback poll rather than in frame handling. Not updated here.

🤖 Generated with Claude Code

Workstream **M2** of the [2026-08-03 force-feedback sweep](https://claude.ai/code/artifact/aa75b7f8-736a-4723-b20f-cef5155ed120) — closes **B2**. ## The bug Unplug a controller mid-session and the virtual pad it was driving outlives it. The game keeps seeing a connected, permanently idle device for the rest of the session. A single-controller session — the common case — hits this every time. `PadSlots::sweep` needs **two** passes to retire a pad: the first pass to see the mask bit clear only *arms* the 300 ms devnode-churn grace (`pad_slots.rs:96`), and the drop lands on a later pass (`:97`). But `sweep` has exactly three callers (`uhid_manager.rs:221`, `linux/gamepad.rs:650`, `windows/gamepad_windows.rs:322`), all inside `handle()`'s `GamepadEvent::State` arm — and the producer emits exactly **one** frame per detach, because `native/input.rs:819` guards the emit on the bit still being set. So unless a sibling pad in the *same* manager keeps sending **changed** frames past the grace, the second pass never comes. Nothing periodic reaches `sweep`: `heartbeat` and `pump` both walk the slots without it. ## Verified before fixing This was checked by four independent readers plus three adversarial verifiers, because the finding shipped with a caveat that nobody had read the test module. All three verifiers returned `confirmed_with_correction`. The corrections, which are folded into this PR: - **Severity is medium, not high.** Clients flush held buttons and axes to neutral *before* sending the remove, so what `heartbeat()` keeps re-emitting is neutral — a phantom **idle** pad, not stuck input. - **The leak is session-bounded.** `Pads` is a local of the per-session input thread and each backend's `Drop` really does destroy the devnode. - **Only half the dedup consequence is real.** `last_rumble` is already covered by the 2.5 s abandoned-rumble watchdog; only `hidout_dedup` (the rich `0xCD` plane, which has no watchdog) genuinely goes stale into a re-plug. - **The precondition is broader than "last pad in a manager"** — it is "no *changed* frame reaches that manager ≥300 ms later". An idle sibling only defers the reap; only a re-plug on the same index cancels it. ## The fix Split the two halves: - `sweep(mask)` still folds a frame's `active_mask` into the grace clocks, then reaps — unchanged for existing callers. - **`reap()`** is new: it drops whatever has run out, with no frame required. All three managers call it from the periodic pump they already run, so teardown completes ~300 ms after the detach instead of never. `reap` **deliberately cannot arm a clock** — it only reads `inactive_since` and clears it, so a pad whose bit never went clear has nothing to run out and no amount of reaping can drop it. That invariant is what makes it safe on a hot loop, and it preserves the anti-flap guarantee: a mask that blips clear and returns still never churns a devnode. `reset_swept` is extracted in the two managers with per-index sibling state, so an unplug completed on the pump clears exactly what one completed on a frame would — `hidout_dedup` in particular. ## Tests **The existing tests are why this hid.** Both hand-fed a *second* removal frame that production never sends, so they passed while the real path leaked. Both now drive the unplug through a `pump` tick. Three new `PadSlots` tests pin the invariants: one frame + a reap completes the unplug; reap alone never drops a pad no frame deactivated; a glitch returning inside the grace still doesn't drop. **Proven non-vacuous** — with the reap neutered, both manager tests fail with the right diagnostics (`the pump tick never completed the unplug` / `the pump tick completed the unplug`). ## Verification Run in the amd64 container: - `cargo clippy -p pf-inject --all-targets --locked -- -D warnings` → **0** - `cargo test -p pf-inject` → **86 passed, 0 failed** - `cargo fmt --all --check` → clean ## Worth a reviewer's eye 1. This puts `UI_DEV_DESTROY` on the GameStream ENet control thread's 2 ms service budget for the first time. Reachable there before via `handle`→`sweep`, but now time-driven. 2. A mask glitch **longer** than 300 ms now genuinely flaps, where today it survives if no frame happens to arrive after the deadline. That is `SWEEP_GRACE` working as documented — deliberately not compensated for by raising the constant. 3. `windows/gamepad_windows.rs` has no test module and `linux/gamepad.rs`'s tests need a real uinput node, so those two legs have review plus shared `PadSlots` coverage only. **Windows and on-glass Linux verification still owed.** 4. `linux/steam_gadget.rs:366-374` has a comment naming `PadSlots::sweep` as its caller and bounding teardown at ~1 s — still true, but that second now lands in the feedback poll rather than in frame handling. Not updated here. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

enricobuehler added 1 commit 2026-08-03 15:35:25 +00:00

fix(host/pads): an unplugged controller actually disappears

ci / web (pull_request) Successful in 1m11s

Details

apple / swift (pull_request) Successful in 1m18s

Details

apple / screenshots (pull_request) Skipped

Details

ci / docs-site (pull_request) Successful in 1m24s

Details

ci / rust-arm64 (pull_request) Successful in 1m35s

Details

android / android (pull_request) Successful in 3m11s

Details

ci / rust (pull_request) Successful in 6m3s

Details

9979489b56

Unplug a controller mid-session and the virtual pad it was driving outlives
it: the game keeps seeing a connected, permanently idle device for the rest of
the session. The single-controller session — the common case — hits this every
time.

`PadSlots::sweep` needs two passes to retire a pad. The first pass to see the
mask bit clear only ARMS the 300 ms devnode-churn grace; the drop lands on a
later pass. But sweep runs only from a state frame, and the producer emits
exactly one frame per detach — `native/input.rs` guards the emit on the bit
still being set — so for a pad with no still-changing sibling in the same
manager, the second pass never comes. Nothing periodic reaches sweep:
`heartbeat` and `pump` walk the slots without it.

Split the two halves. `sweep` still folds a frame's mask into the grace
clocks, and `reap` — new — drops whatever has run out, with no frame needed.
Every manager now reaps on the periodic pump it already runs, so the teardown
completes ~300 ms after the detach instead of never.

`reap` deliberately cannot arm a clock: it only reads `inactive_since` and
clears it, so a pad whose bit never went clear has nothing to run out and no
amount of reaping can drop it. That is what makes it safe on a hot loop, and
it keeps the anti-flap guarantee intact — a mask that blips clear and returns
still never churns a devnode.

The two existing tests hand-fed a SECOND removal frame, which production never
sends; they passed while the real path leaked. Both now drive the unplug
through a pump tick, and PadSlots gains three tests pinning the new
invariants. Verified non-vacuous: with the reap neutered, both manager tests
fail with "the pump tick never completed the unplug".

Behaviour notes: this puts UI_DEV_DESTROY on the GameStream control thread's
budget for the first time, and a mask glitch longer than the grace now really
does flap — which is SWEEP_GRACE working as documented, so the constant stays.

Found by the 2026-08-03 force-feedback sweep (B2 — see the backlog in
punktfunk-planning design/haptics-sweep-2026-08-03.md).

enricobuehler referenced this pull request

2026-08-03 17:39:48 +00:00

chore(release): bump workspace version to 0.24.0 #29

enricobuehler added 1 commit 2026-08-03 17:50:01 +00:00

Merge remote-tracking branch 'origin/main' into worktree-haptics-m2-pad-slots

ci / docs-site (pull_request) Successful in 3m0s

Details

apple / swift (pull_request) Successful in 1m19s

Details

apple / screenshots (pull_request) Skipped

Details

android / android (pull_request) Successful in 4m42s