//! A declared lock hierarchy for [`PlayerController`], and a tripwire that //! enforces it. //! //! The controller carries seventeen separate mutexes, reached from the MPV event //! loop, JNI callbacks, sleep/autoplay timers, the session poller and every IPC //! command. Nothing about that arrangement prevents two threads taking the same //! two locks in opposite orders, which deadlocks the player outright — and this //! subsystem has already produced one deadlock (a tokio `MutexGuard` held in a //! `match` scrutinee, which stalled the `AdvanceToNext` arm). //! //! Today the code is disciplined: acquisitions are scoped, and `previous()` for //! instance explicitly drops the backend guard before touching the queue. But //! that holds by convention, and convention is not checked. [`LOCK_ORDER`] //! writes the convention down and `every_overlapping_acquisition_respects_the_order` //! fails the build when a change breaks it. //! //! Ordering only matters where one guard is **still held** while another lock is //! taken. Acquiring two locks one after another, each released before the next, //! cannot deadlock — so the analysis looks for overlap, not for mere sequence. //! //! TRACES: UR-005 | DR-052 // This module is a static analysis of `player/mod.rs` plus the hierarchy it // checks against. Its only caller is its own test module, but `LOCK_ORDER` is // the documentation of record for how these locks nest, so it stays compiled // (and rustdoc'd) rather than hidden behind `cfg(test)`. #![allow(dead_code)] /// The order in which `PlayerController`'s locks may be nested. /// /// A thread already holding one of these may only acquire a lock that appears /// **later** in this list. The order is not arbitrary — it follows the nesting /// the code already relies on: /// /// - `repository` and `sleep_timer` are taken by long-running decisions that go /// on to consult playback state, so they sit outermost. /// - `backend` outranks `queue`: "what is playing" is read before "what is /// next", never the reverse. /// - `event_emitter` is last. Emitting is a leaf — notifying the frontend must /// never reach back for more player state. /// /// TRACES: UR-005 | DR-052 pub const LOCK_ORDER: &[&str] = &[ "repository", "sleep_timer", "countdown_cancel", "jellyfin_client", "backend", "queue", "stream_resume", "end_reason", "reported_time", "background_audio_active", "background_audio_base", "html5_playing", "autoplay_settings", "autoplay_episode_count", "reports", "event_emitter", ]; /// Rank of `field` in [`LOCK_ORDER`], or `None` if it is not a declared lock. pub fn rank(field: &str) -> Option { LOCK_ORDER.iter().position(|f| *f == field) } /// One lock acquired while another is still held. #[derive(Debug, PartialEq, Eq)] pub struct Overlap { /// The lock already held. pub outer: String, /// The lock acquired underneath it. pub inner: String, /// 1-indexed line of the inner acquisition, for a useful failure message. pub line: usize, } /// Find every place `src` takes a lock while holding another. /// /// Deliberately simple and line-based: it tracks `let … = self.FIELD.lock_safe()` /// bindings and looks for a different `self.OTHER.lock_safe()` before the /// binding goes out of scope or is explicitly dropped. A guard that is not bound /// to a name (`*self.flag.lock_safe() = false;`) is released at the end of its /// statement and cannot overlap anything, so it is only ever an *inner* /// acquisition here. /// /// TRACES: UR-005 | DR-052 | UT-052 pub fn overlapping_acquisitions(src: &str) -> Vec { let lines: Vec<&str> = src.lines().collect(); let mut found = Vec::new(); for (i, line) in lines.iter().enumerate() { let Some((var, field)) = parse_binding(line) else { continue; }; let indent = line.len() - line.trim_start().len(); for (j, later) in lines.iter().enumerate().skip(i + 1) { if later.contains(&format!("drop({var})")) { break; } let trimmed = later.trim(); if trimmed.is_empty() { continue; } // Left the block the guard lives in. let later_indent = later.len() - later.trim_start().len(); if later_indent < indent && !trimmed.starts_with(['.', ')', '}']) { break; } if trimmed == "}" && later_indent < indent { break; } if let Some(inner) = parse_acquisition(later, field) { found.push(Overlap { outer: field.to_string(), inner, line: j + 1, }); break; } } } found } /// `let [mut] name = self.field.lock_safe()` → `(name, field)`. fn parse_binding(line: &str) -> Option<(&str, &str)> { let rest = line.trim_start().strip_prefix("let ")?; let rest = rest.strip_prefix("mut ").unwrap_or(rest); let (name, rest) = rest.split_once(" = self.")?; let (field, _) = rest.split_once(".lock_safe()")?; if name.contains(' ') || field.contains('.') { return None; } Some((name, field)) } /// The first `self.other.lock_safe()` on `line` that is not `held`. fn parse_acquisition(line: &str, held: &str) -> Option { let mut search = line; while let Some(at) = search.find("self.") { let after = &search[at + 5..]; if let Some((field, _)) = after.split_once(".lock_safe()") { if !field.contains(['.', '(', ' ']) && field != held { return Some(field.to_string()); } } search = after; } None } // TRACES: UR-005 | DR-052 | UT-052 #[cfg(test)] mod tests { use super::*; /// **The tripwire.** Every nested acquisition in the controller must follow /// [`LOCK_ORDER`]. /// /// A violation is a lock-order inversion: two threads taking the same pair /// in opposite orders deadlock the player, and the symptom is a frozen app /// with no error anywhere. /// /// TRACES: UR-005 | DR-052 | UT-052 #[test] fn every_overlapping_acquisition_respects_the_order() { let src = include_str!("mod.rs"); let mut violations = Vec::new(); for overlap in overlapping_acquisitions(src) { let (Some(outer), Some(inner)) = (rank(&overlap.outer), rank(&overlap.inner)) else { violations.push(format!( "player/mod.rs:{} takes '{}' while holding '{}', and one of them \ is not declared in LOCK_ORDER", overlap.line, overlap.inner, overlap.outer )); continue; }; if outer >= inner { violations.push(format!( "player/mod.rs:{} takes '{}' (rank {inner}) while holding '{}' \ (rank {outer}) — an inversion against LOCK_ORDER", overlap.line, overlap.inner, overlap.outer )); } } assert!( violations.is_empty(), "lock-order inversions in PlayerController:\n {}\n\nEither reorder the \ acquisitions or, if the new order is the correct one, change LOCK_ORDER \ and re-check every other site.", violations.join("\n ") ); } /// The analysis must actually see the nesting the controller does today, /// or the tripwire above passes by finding nothing. /// /// TRACES: UR-005 | DR-052 | UT-052 #[test] fn the_analysis_finds_the_nesting_that_exists() { let found = overlapping_acquisitions(include_str!("mod.rs")); assert!( found.len() >= 5, "expected the controller's known nested acquisitions, found {found:?}" ); assert!( found .iter() .any(|o| o.outer == "backend" && o.inner == "queue"), "the backend->queue nesting in state() should be detected: {found:?}" ); } /// A guard held across a lock taken in the wrong order must be caught. /// /// TRACES: UR-005 | DR-052 | UT-052 #[test] fn an_inversion_is_detected() { let src = " fn bad(&self) {\n\ \x20 let queue = self.queue.lock_safe();\n\ \x20 let b = self.backend.lock_safe();\n\ \x20 }\n"; let found = overlapping_acquisitions(src); assert_eq!(found.len(), 1, "{found:?}"); assert_eq!(found[0].outer, "queue"); assert_eq!(found[0].inner, "backend"); assert!(rank("queue").unwrap() > rank("backend").unwrap()); } /// Sequential, non-overlapping acquisitions cannot deadlock and must not be /// reported — `previous()` drops the backend guard before taking the queue. /// /// TRACES: UR-005 | DR-052 | UT-052 #[test] fn a_dropped_guard_is_not_an_overlap() { let src = " fn fine(&self) {\n\ \x20 let backend = self.backend.lock_safe();\n\ \x20 drop(backend);\n\ \x20 let queue = self.queue.lock_safe();\n\ \x20 }\n"; assert!(overlapping_acquisitions(src).is_empty()); } /// Every declared lock name must be a real field, or the order documents /// something that no longer exists. /// /// TRACES: UR-005 | DR-052 | UT-052 #[test] fn every_declared_lock_is_a_real_field() { let src = include_str!("mod.rs"); let decl = src .split_once("pub struct PlayerController {") .expect("PlayerController struct") .1; let decl = decl.split_once("\n}").expect("end of struct").0; for name in LOCK_ORDER { assert!( decl.contains(&format!("{name}:")), "LOCK_ORDER names '{name}', which is not a PlayerController field" ); } } }