cat / goq
clone

turns a linux box into a private steam streaming appliance: plug in, tap your key, play from another computer

fix(input): neutralize cancelled and panicked sessions (#109)

FelineStateMachine committed f84cb13
* fix(input): neutralize cancelled sessions

* fix(input): reset through poisoned device lock

Recover a poisoned uinput mutex only during teardown so held transitions are still released after a panic. Keep ordinary input injection fail-closed and cover the poisoned-lock path with a focused regression test.
commit
f84cb137987ebc38118c882343bece07ebe868ed
parent
0065c99d0157ab6fc44528f7ab944995a0fa6062
author
FelineStateMachine <dami.etoile@gmail.com>
2 files changed, +222 -14
Mcrates/sigil-host/src/input.rs54++++++++-
Mcrates/sigil-host/src/server/handlers.rs182++++++++++++++++++++++++++++--
M crates/sigil-host/src/input.rs +50 -4
@@ -3,6 +3,8 @@ use std::fmt;
33 use std::mem::size_of;44 #[cfg(target_os = "linux")]55 use std::sync::Arc;6+#[cfg(any(target_os = "linux", test))]7+use std::sync::Mutex;68 79 #[cfg(target_os = "linux")]810 use anyhow::Context;
@@ -282,15 +284,30 @@ impl InputBackend {
282284 pub fn reset_session(&self) -> Result<()> {283285 #[cfg(target_os = "linux")]284286 if let Some(device) = &self.device {285- device286- .lock()287- .map_err(|_| anyhow::anyhow!("uinput backend lock poisoned"))?288- .release_all()?;287+ reset_through_poisoned_lock(device, linux::UinputDevice::release_all)?;289288 }290289 Ok(())291290 }292291 }293292 293+#[cfg(any(target_os = "linux", test))]294+fn reset_through_poisoned_lock<T>(295+ device: &Mutex<T>,296+ reset: impl FnOnce(&mut T) -> Result<()>,297+) -> Result<()> {298+ let mut device = match device.lock() {299+ Ok(device) => device,300+ Err(poisoned) => {301+ // A panic while applying input poisons this mutex. Teardown must302+ // still recover the device solely to release held transitions;303+ // ordinary input continues to reject the poisoned lock.304+ tracing::warn!("recovering poisoned uinput lock for session reset");305+ poisoned.into_inner()306+ }307+ };308+ reset(&mut device)309+}310+294311 fn ensure_event_was_negotiated(295312 mode: &InputMode,296313 event: &InputEvent,
@@ -1395,6 +1412,9 @@ mod linux {
13951412 13961413 #[cfg(test)]13971414 mod tests {1415+ use std::panic::{AssertUnwindSafe, catch_unwind};1416+ use std::sync::Arc;1417+13981418 use super::*;13991419 14001420 fn acl_entry(tag: u16, permissions: u16, id: u32) -> [u8; POSIX_ACL_ENTRY_BYTES] {
@@ -1501,6 +1521,32 @@ mod tests {
15011521 assert_ne!(KEYBOARD_PRODUCT_ID, GAMEPAD_PRODUCT_ID);15021522 }15031523 1524+ #[test]1525+ fn session_reset_recovers_a_lock_poisoned_while_applying_input() {1526+ #[derive(Debug, Default)]1527+ struct DeviceState {1528+ released: bool,1529+ }1530+1531+ let device = Arc::new(Mutex::new(DeviceState::default()));1532+ let panic_device = Arc::clone(&device);1533+ let panic_result = catch_unwind(AssertUnwindSafe(move || {1534+ let _device = panic_device.lock().unwrap();1535+ panic!("simulate panic while applying uinput");1536+ }));1537+ assert!(panic_result.is_err());1538+ assert!(device.lock().is_err());1539+1540+ reset_through_poisoned_lock(device.as_ref(), |device| {1541+ device.released = true;1542+ Ok(())1543+ })1544+ .unwrap();1545+1546+ let device = device.lock().unwrap_err().into_inner();1547+ assert!(device.released);1548+ }1549+15041550 #[test]15051551 fn protocol_events_route_to_disjoint_virtual_devices() {15061552 for event in [
M crates/sigil-host/src/server/handlers.rs +172 -10
@@ -1,3 +1,4 @@
1+use super::session::InputLease;12 use super::*;23 34 pub(super) const MEDIA_CAPABILITIES: &[Capability] = &[Capability::VideoH264];
@@ -6,6 +7,49 @@ pub(super) const HANDSHAKE_TIMEOUT: Duration = Duration::from_secs(10);
67 const INPUT_ACK_TIMEOUT: Duration = Duration::from_secs(1);78 const REJECTION_DRAIN_TIMEOUT: Duration = Duration::from_secs(1);89 10+#[must_use = "the guard must remain armed for the lifetime of the input session"]11+struct InputSessionGuard<F>12+where13+ F: FnOnce() -> Result<()>,14+{15+ _lease: InputLease,16+ reset: Option<F>,17+}18+19+impl<F> InputSessionGuard<F>20+where21+ F: FnOnce() -> Result<()>,22+{23+ fn new(lease: InputLease, reset: F) -> Self {24+ Self {25+ _lease: lease,26+ reset: Some(reset),27+ }28+ }29+30+ fn finish(mut self) -> Result<()> {31+ self.run()32+ }33+34+ fn run(&mut self) -> Result<()> {35+ let Some(reset) = self.reset.take() else {36+ return Ok(());37+ };38+ reset()39+ }40+}41+42+impl<F> Drop for InputSessionGuard<F>43+where44+ F: FnOnce() -> Result<()>,45+{46+ fn drop(&mut self) {47+ if let Err(error) = self.run() {48+ error!(%error, "failed to release held input transitions while dropping session");49+ }50+ }51+}52+953 #[derive(Clone, Debug)]1054 pub struct MediaHandler {1155 pub config: HostConfig,
@@ -225,9 +269,14 @@ async fn serve_input(
225269 return Err(error);226270 }227271 };272+ let session_id = lease.session_id;273+ let grants = lease.grants;274+ // Owning the lease guarantees cancellation or unwinding neutralizes the275+ // virtual devices before another input stream can be admitted.276+ let session_guard = InputSessionGuard::new(lease, || backend.reset_session());228277 let supported =229278 supported_input_capabilities(backend.capabilities(), pointer_positions.is_some());230- let negotiated = negotiated_input_capabilities(&hello, &supported, lease.grants);279+ let negotiated = negotiated_input_capabilities(&hello, &supported, grants);231280 232281 let ack_enabled = negotiated.contains(&Capability::InputAck);233282 let feedback_enabled = negotiated.contains(&Capability::PointerPositionFeedback);
@@ -237,10 +286,10 @@ async fn serve_input(
237286 .map(PointerPositionTracker::subscribe);238287 write_host_hello(239288 &mut send,240- &HostHello::accepted(lease.session_id, negotiated.clone()),289+ &HostHello::accepted(session_id, negotiated.clone()),241290 )242291 .await?;243- info!(%remote, session_id = lease.session_id, "input client accepted");292+ info!(%remote, session_id, "input client accepted");244293 245294 let session_result: Result<()> = async {246295 let mut received_events = 0_u64;
@@ -263,8 +312,8 @@ async fn serve_input(
263312 .context("timed out writing initial pointer position")??;264313 }265314 loop {266- if !sessions.is_active(remote, lease.session_id) {267- debug!(%remote, session_id = lease.session_id, "media session ended; closing input");315+ if !sessions.is_active(remote, session_id) {316+ debug!(%remote, session_id, "media session ended; closing input");268317 break;269318 }270319 tokio::select! {
@@ -303,8 +352,8 @@ async fn serve_input(
303352 let Some(event) = event? else {304353 break;305354 };306- if !sessions.is_active(remote, lease.session_id) {307- debug!(%remote, session_id = lease.session_id, "discarding input after media ended");355+ if !sessions.is_active(remote, session_id) {356+ debug!(%remote, session_id, "discarding input after media ended");308357 break;309358 }310359 match backend.apply(&event, &negotiated)? {
@@ -352,11 +401,15 @@ async fn serve_input(
352401 Ok(())353402 }354403 .await;355- let reset_result = backend356- .reset_session()404+ let reset_result = session_guard405+ .finish()357406 .context("releasing held input transitions at session end");407+ if session_result.is_err()408+ && let Err(error) = &reset_result409+ {410+ error!(%error, "input session and held-transition release both failed");411+ }358412 let result = session_result.and(reset_result);359- drop(lease);360413 info!(%remote, "input client released");361414 result362415 }
@@ -601,8 +654,117 @@ where
601654 602655 #[cfg(test)]603656 mod tests {657+ use std::cell::Cell;658+ use std::panic::{AssertUnwindSafe, catch_unwind};659+ use std::sync::atomic::{AtomicBool, Ordering};660+604661 use super::*;605662 663+ #[test]664+ fn input_session_guard_resets_before_releasing_lease_on_drop() {665+ let sessions = Arc::new(SessionRegistry::default());666+ let remote = endpoint(1);667+ let nonce = [1; 16];668+ let _media = sessions669+ .claim(remote, nonce, InvitationGrants::ALL)670+ .unwrap();671+ let input = sessions.claim_input(remote, nonce).unwrap();672+ let resets = Cell::new(0);673+ let reset_saw_lease_held = Cell::new(false);674+675+ {676+ let _guard = InputSessionGuard::new(input, || {677+ resets.set(resets.get() + 1);678+ reset_saw_lease_held.set(sessions.claim_input(remote, nonce).is_err());679+ Ok(())680+ });681+ }682+683+ assert_eq!(resets.get(), 1);684+ assert!(reset_saw_lease_held.get());685+ assert!(sessions.claim_input(remote, nonce).is_ok());686+ }687+688+ #[test]689+ fn input_session_guard_finish_propagates_reset_error_once() {690+ let sessions = Arc::new(SessionRegistry::default());691+ let remote = endpoint(1);692+ let nonce = [2; 16];693+ let _media = sessions694+ .claim(remote, nonce, InvitationGrants::ALL)695+ .unwrap();696+ let input = sessions.claim_input(remote, nonce).unwrap();697+ let resets = Cell::new(0);698+699+ let error = InputSessionGuard::new(input, || {700+ resets.set(resets.get() + 1);701+ Err(anyhow::anyhow!("reset failed"))702+ })703+ .finish()704+ .unwrap_err();705+706+ assert_eq!(error.to_string(), "reset failed");707+ assert_eq!(resets.get(), 1);708+ assert!(sessions.claim_input(remote, nonce).is_ok());709+ }710+711+ #[test]712+ fn input_session_guard_resets_before_releasing_lease_during_unwind() {713+ let sessions = Arc::new(SessionRegistry::default());714+ let remote = endpoint(1);715+ let nonce = [3; 16];716+ let _media = sessions717+ .claim(remote, nonce, InvitationGrants::ALL)718+ .unwrap();719+ let input = sessions.claim_input(remote, nonce).unwrap();720+ let reset_saw_lease_held = Cell::new(false);721+722+ let panic_result = catch_unwind(AssertUnwindSafe(|| {723+ let _guard = InputSessionGuard::new(input, || {724+ reset_saw_lease_held.set(sessions.claim_input(remote, nonce).is_err());725+ Ok(())726+ });727+ panic!("simulate input task panic");728+ }));729+730+ assert!(panic_result.is_err());731+ assert!(reset_saw_lease_held.get());732+ assert!(sessions.claim_input(remote, nonce).is_ok());733+ }734+735+ #[tokio::test]736+ async fn input_session_guard_resets_before_releasing_lease_when_task_is_cancelled() {737+ let sessions = Arc::new(SessionRegistry::default());738+ let remote = endpoint(1);739+ let nonce = [4; 16];740+ let _media = sessions741+ .claim(remote, nonce, InvitationGrants::ALL)742+ .unwrap();743+ let input = sessions.claim_input(remote, nonce).unwrap();744+ let reset_saw_lease_held = Arc::new(AtomicBool::new(false));745+ let task_sessions = Arc::clone(&sessions);746+ let task_reset_saw_lease_held = Arc::clone(&reset_saw_lease_held);747+ let (armed_tx, armed_rx) = tokio::sync::oneshot::channel();748+749+ let task = tokio::spawn(async move {750+ let _guard = InputSessionGuard::new(input, move || {751+ task_reset_saw_lease_held.store(752+ task_sessions.claim_input(remote, nonce).is_err(),753+ Ordering::SeqCst,754+ );755+ Ok(())756+ });757+ armed_tx.send(()).unwrap();758+ std::future::pending::<()>().await;759+ });760+761+ armed_rx.await.unwrap();762+ task.abort();763+ assert!(task.await.unwrap_err().is_cancelled());764+ assert!(reset_saw_lease_held.load(Ordering::SeqCst));765+ assert!(sessions.claim_input(remote, nonce).is_ok());766+ }767+606768 #[test]607769 fn capability_negotiation_is_an_exact_intersection() {608770 let hello = ClientHello::new(