fix(hidpp): guard message listener lifetimes

[AprilNEA]

Summary

• dispatch HID++ message listeners outside the pending-message and listener-map locks
• add a RAII MessageListenerGuard API that unregisters listeners on drop
• migrate internal receiver, feature, pairing, and gesture listeners to the guard
• add a regression test for listener removal during dispatch

Validation

• cargo fmt --check
• cargo test -p openlogi-hidpp
• cargo test -p openlogi-hid
• cargo test -p openlogi-hid pairing::tests after rebasing onto PR fix(pairing): keep session pause balanced #298
• cargo clippy -p openlogi-hidpp --all-targets -- -D warnings
• cargo clippy -p openlogi-hid --all-targets -- -D warnings
• pre-push cargo clippy --workspace --all-targets -- -D warnings

Stacked on #298.

[greptile-apps]

Greptile Summary

This PR fixes a latent deadlock in the HID++ message dispatch loop where a listener that tried to add or remove another listener during dispatch would deadlock on the message_listeners mutex. The fix snapshots the listener map into a Vec<Arc<...>> before releasing the lock, then dispatches outside it; a new MessageListenerGuard RAII type wraps the raw handle and removes the listener on drop.

• Dispatch loop (channel.rs): pending-message matching and listener dispatch are now in separate, non-overlapping critical sections; listeners are cloned into a Vec so the map lock is not held during callbacks.
• MessageListenerGuard: backed by a Weak reference to the listener map, so guard drop is safe regardless of whether the channel or the guard outlives the other; all eight call sites are migrated from the raw u32 handle + manual Drop/remove_msg_listener pattern.
• ListenerDropGuard (receiver/mod.rs) is deleted; Arc<MessageListenerGuard> is used directly in the Clone receiver structs to preserve the "remove on last clone drop" semantics.

Confidence Score: 5/5

Safe to merge; the deadlock fix is correctly scoped and all eight listener call sites are fully migrated.

The dispatch loop change is minimal and correct — two non-overlapping lock windows replace one that covered both matching and dispatch. The Weak-backed RAII guard handles all lifetime orderings cleanly, and the regression test directly exercises the previously deadlocking scenario. All feature and receiver structs are completely migrated with no leftover manual Drop impls or raw handles.

channel.rs is the only file worth a second look — the missing #[must_use] on MessageListenerGuard is a minor gap worth addressing before the guard API sees wider use.

Important Files Changed

Filename	Overview
crates/openlogi-hidpp/src/channel.rs	Core change: dispatch loop now collects a Vec snapshot of Arc-cloned listeners before releasing the lock, preventing deadlock when a listener mutates the map during dispatch. Adds MessageListenerGuard (RAII remove-on-drop) backed by Weak to handle channel-outliving-guard and vice versa. Missing #[must_use] on MessageListenerGuard.
crates/openlogi-hid/src/pairing.rs	subscribe() now returns MessageListenerGuard instead of a raw u32; run_pairing uses explicit drop(listener) for clarity. Listener lifetime is correctly scoped to the drive() call.
crates/openlogi-hidpp/src/receiver/mod.rs	Removes the now-redundant ListenerDropGuard (superseded by MessageListenerGuard in channel.rs). Clean deletion with no functional change.
crates/openlogi-hidpp/src/feature/wireless_device_status/mod.rs	Removes the now-unnecessary chan: Arc field (only needed for the old manual Drop); migrated to MessageListenerGuard. Correct.
crates/openlogi-hid/src/gesture.rs	Migrates gesture capture listener from raw handle + explicit remove_msg_listener to add_msg_listener_guarded + drop(listener). Change is mechanical and correct.
crates/openlogi-hidpp/src/receiver/bolt.rs	Replaces Arc with Arc; listener registration migrated to add_msg_listener_guarded. Arc wrapping preserves correct semantics for a Clone receiver.
crates/openlogi-hidpp/src/receiver/unifying.rs	Same mechanical migration as bolt.rs – ListenerDropGuard replaced by Arc. Correct.
crates/openlogi-hidpp/src/feature/hires_wheel/mod.rs	msg_listener_hdl: u32 replaced by _msg_listener: MessageListenerGuard; manual Drop impl removed. Complete and correct.
crates/openlogi-hidpp/src/feature/thumbwheel/mod.rs	Same migration pattern as hires_wheel – manual Drop removed, guard field added. Correct.
crates/openlogi-hidpp/src/feature/unified_battery/mod.rs	Same migration pattern as hires_wheel – manual Drop removed, guard field added. Correct.
crates/openlogi-hidpp/src/feature/mod.rs	Removes pub(crate) chan() accessor from FeatureEndpoint, which was only needed by the now-deleted manual Drop impls. Clean.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant RT as Read Thread
    participant PM as pending_messages Mutex
    participant ML as message_listeners Mutex
    participant L1 as Listener 1 (Arc)
    participant L2 as Listener 2 (Arc)

    RT->>PM: lock → match response predicate
    PM-->>RT: release lock
    RT->>ML: lock → clone all Arc-Fn into Vec
    ML-->>RT: release lock
    Note over RT: dispatch outside both locks
    RT->>L1: listener(msg, matched)
    L1->>ML: remove_msg_listener(hdl) no deadlock
    RT->>L2: listener(msg, matched)
    Note over L2: guard drop also safe here

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant RT as Read Thread
    participant PM as pending_messages Mutex
    participant ML as message_listeners Mutex
    participant L1 as Listener 1 (Arc)
    participant L2 as Listener 2 (Arc)

    RT->>PM: lock → match response predicate
    PM-->>RT: release lock
    RT->>ML: lock → clone all Arc-Fn into Vec
    ML-->>RT: release lock
    Note over RT: dispatch outside both locks
    RT->>L1: listener(msg, matched)
    L1->>ML: remove_msg_listener(hdl) no deadlock
    RT->>L2: listener(msg, matched)
    Note over L2: guard drop also safe here

Loading

_{Reviews (2): Last reviewed commit: "fix(hidpp): keep listener guard opaque" | Re-trigger Greptile}

[greptile-apps]

handle() leaks the raw handle out of an otherwise opaque RAII guard

MessageListenerGuard::handle() is public but is never called anywhere in this PR (nor in any migrated site). Exposing it creates a footgun: a caller who retrieves the handle and passes it to remove_msg_listener will cause the guard's drop to silently skip the removal (entry already gone), while the guard still "looks active" to its owner. If the use case is only internal debugging or a future escape-hatch, pub(crate) would prevent accidental misuse from outside the crate, or the method could be removed entirely now that the migrated callers no longer need it.