jellytau_lib/player/lock_order.rs
1//! A declared lock hierarchy for [`PlayerController`], and a tripwire that
2//! enforces it.
3//!
4//! The controller carries seventeen separate mutexes, reached from the MPV event
5//! loop, JNI callbacks, sleep/autoplay timers, the session poller and every IPC
6//! command. Nothing about that arrangement prevents two threads taking the same
7//! two locks in opposite orders, which deadlocks the player outright — and this
8//! subsystem has already produced one deadlock (a tokio `MutexGuard` held in a
9//! `match` scrutinee, which stalled the `AdvanceToNext` arm).
10//!
11//! Today the code is disciplined: acquisitions are scoped, and `previous()` for
12//! instance explicitly drops the backend guard before touching the queue. But
13//! that holds by convention, and convention is not checked. [`LOCK_ORDER`]
14//! writes the convention down and `every_overlapping_acquisition_respects_the_order`
15//! fails the build when a change breaks it.
16//!
17//! Ordering only matters where one guard is **still held** while another lock is
18//! taken. Acquiring two locks one after another, each released before the next,
19//! cannot deadlock — so the analysis looks for overlap, not for mere sequence.
20//!
21//! TRACES: UR-005 | DR-052
22
23// This module is a static analysis of `player/mod.rs` plus the hierarchy it
24// checks against. Its only caller is its own test module, but `LOCK_ORDER` is
25// the documentation of record for how these locks nest, so it stays compiled
26// (and rustdoc'd) rather than hidden behind `cfg(test)`.
27#![allow(dead_code)]
28
29/// The order in which `PlayerController`'s locks may be nested.
30///
31/// A thread already holding one of these may only acquire a lock that appears
32/// **later** in this list. The order is not arbitrary — it follows the nesting
33/// the code already relies on:
34///
35/// - `repository` and `sleep_timer` are taken by long-running decisions that go
36/// on to consult playback state, so they sit outermost.
37/// - `backend` outranks `queue`: "what is playing" is read before "what is
38/// next", never the reverse.
39/// - `event_emitter` is last. Emitting is a leaf — notifying the frontend must
40/// never reach back for more player state.
41///
42/// TRACES: UR-005 | DR-052
43pub const LOCK_ORDER: &[&str] = &[
44 "repository",
45 "sleep_timer",
46 "countdown_cancel",
47 "jellyfin_client",
48 "backend",
49 "queue",
50 "stream_resume",
51 "end_reason",
52 "reported_time",
53 "background_audio_active",
54 "background_audio_base",
55 "html5_playing",
56 "autoplay_settings",
57 "autoplay_episode_count",
58 "reports",
59 "event_emitter",
60];
61
62/// Rank of `field` in [`LOCK_ORDER`], or `None` if it is not a declared lock.
63pub fn rank(field: &str) -> Option<usize> {
64 LOCK_ORDER.iter().position(|f| *f == field)
65}
66
67/// One lock acquired while another is still held.
68#[derive(Debug, PartialEq, Eq)]
69pub struct Overlap {
70 /// The lock already held.
71 pub outer: String,
72 /// The lock acquired underneath it.
73 pub inner: String,
74 /// 1-indexed line of the inner acquisition, for a useful failure message.
75 pub line: usize,
76}
77
78/// Find every place `src` takes a lock while holding another.
79///
80/// Deliberately simple and line-based: it tracks `let … = self.FIELD.lock_safe()`
81/// bindings and looks for a different `self.OTHER.lock_safe()` before the
82/// binding goes out of scope or is explicitly dropped. A guard that is not bound
83/// to a name (`*self.flag.lock_safe() = false;`) is released at the end of its
84/// statement and cannot overlap anything, so it is only ever an *inner*
85/// acquisition here.
86///
87/// TRACES: UR-005 | DR-052 | UT-052
88pub fn overlapping_acquisitions(src: &str) -> Vec<Overlap> {
89 let lines: Vec<&str> = src.lines().collect();
90 let mut found = Vec::new();
91
92 for (i, line) in lines.iter().enumerate() {
93 let Some((var, field)) = parse_binding(line) else {
94 continue;
95 };
96 let indent = line.len() - line.trim_start().len();
97
98 for (j, later) in lines.iter().enumerate().skip(i + 1) {
99 if later.contains(&format!("drop({var})")) {
100 break;
101 }
102 let trimmed = later.trim();
103 if trimmed.is_empty() {
104 continue;
105 }
106 // Left the block the guard lives in.
107 let later_indent = later.len() - later.trim_start().len();
108 if later_indent < indent && !trimmed.starts_with(['.', ')', '}']) {
109 break;
110 }
111 if trimmed == "}" && later_indent < indent {
112 break;
113 }
114 if let Some(inner) = parse_acquisition(later, field) {
115 found.push(Overlap {
116 outer: field.to_string(),
117 inner,
118 line: j + 1,
119 });
120 break;
121 }
122 }
123 }
124 found
125}
126
127/// `let [mut] name = self.field.lock_safe()` → `(name, field)`.
128fn parse_binding(line: &str) -> Option<(&str, &str)> {
129 let rest = line.trim_start().strip_prefix("let ")?;
130 let rest = rest.strip_prefix("mut ").unwrap_or(rest);
131 let (name, rest) = rest.split_once(" = self.")?;
132 let (field, _) = rest.split_once(".lock_safe()")?;
133 if name.contains(' ') || field.contains('.') {
134 return None;
135 }
136 Some((name, field))
137}
138
139/// The first `self.other.lock_safe()` on `line` that is not `held`.
140fn parse_acquisition(line: &str, held: &str) -> Option<String> {
141 let mut search = line;
142 while let Some(at) = search.find("self.") {
143 let after = &search[at + 5..];
144 if let Some((field, _)) = after.split_once(".lock_safe()") {
145 if !field.contains(['.', '(', ' ']) && field != held {
146 return Some(field.to_string());
147 }
148 }
149 search = after;
150 }
151 None
152}
153
154// TRACES: UR-005 | DR-052 | UT-052
155#[cfg(test)]
156mod tests {
157 use super::*;
158
159 /// **The tripwire.** Every nested acquisition in the controller must follow
160 /// [`LOCK_ORDER`].
161 ///
162 /// A violation is a lock-order inversion: two threads taking the same pair
163 /// in opposite orders deadlock the player, and the symptom is a frozen app
164 /// with no error anywhere.
165 ///
166 /// TRACES: UR-005 | DR-052 | UT-052
167 #[test]
168 fn every_overlapping_acquisition_respects_the_order() {
169 let src = include_str!("mod.rs");
170 let mut violations = Vec::new();
171
172 for overlap in overlapping_acquisitions(src) {
173 let (Some(outer), Some(inner)) = (rank(&overlap.outer), rank(&overlap.inner)) else {
174 violations.push(format!(
175 "player/mod.rs:{} takes '{}' while holding '{}', and one of them \
176 is not declared in LOCK_ORDER",
177 overlap.line, overlap.inner, overlap.outer
178 ));
179 continue;
180 };
181 if outer >= inner {
182 violations.push(format!(
183 "player/mod.rs:{} takes '{}' (rank {inner}) while holding '{}' \
184 (rank {outer}) — an inversion against LOCK_ORDER",
185 overlap.line, overlap.inner, overlap.outer
186 ));
187 }
188 }
189
190 assert!(
191 violations.is_empty(),
192 "lock-order inversions in PlayerController:\n {}\n\nEither reorder the \
193 acquisitions or, if the new order is the correct one, change LOCK_ORDER \
194 and re-check every other site.",
195 violations.join("\n ")
196 );
197 }
198
199 /// The analysis must actually see the nesting the controller does today,
200 /// or the tripwire above passes by finding nothing.
201 ///
202 /// TRACES: UR-005 | DR-052 | UT-052
203 #[test]
204 fn the_analysis_finds_the_nesting_that_exists() {
205 let found = overlapping_acquisitions(include_str!("mod.rs"));
206 assert!(
207 found.len() >= 5,
208 "expected the controller's known nested acquisitions, found {found:?}"
209 );
210 assert!(
211 found
212 .iter()
213 .any(|o| o.outer == "backend" && o.inner == "queue"),
214 "the backend->queue nesting in state() should be detected: {found:?}"
215 );
216 }
217
218 /// A guard held across a lock taken in the wrong order must be caught.
219 ///
220 /// TRACES: UR-005 | DR-052 | UT-052
221 #[test]
222 fn an_inversion_is_detected() {
223 let src = " fn bad(&self) {\n\
224 \x20 let queue = self.queue.lock_safe();\n\
225 \x20 let b = self.backend.lock_safe();\n\
226 \x20 }\n";
227 let found = overlapping_acquisitions(src);
228 assert_eq!(found.len(), 1, "{found:?}");
229 assert_eq!(found[0].outer, "queue");
230 assert_eq!(found[0].inner, "backend");
231 assert!(rank("queue").unwrap() > rank("backend").unwrap());
232 }
233
234 /// Sequential, non-overlapping acquisitions cannot deadlock and must not be
235 /// reported — `previous()` drops the backend guard before taking the queue.
236 ///
237 /// TRACES: UR-005 | DR-052 | UT-052
238 #[test]
239 fn a_dropped_guard_is_not_an_overlap() {
240 let src = " fn fine(&self) {\n\
241 \x20 let backend = self.backend.lock_safe();\n\
242 \x20 drop(backend);\n\
243 \x20 let queue = self.queue.lock_safe();\n\
244 \x20 }\n";
245 assert!(overlapping_acquisitions(src).is_empty());
246 }
247
248 /// Every declared lock name must be a real field, or the order documents
249 /// something that no longer exists.
250 ///
251 /// TRACES: UR-005 | DR-052 | UT-052
252 #[test]
253 fn every_declared_lock_is_a_real_field() {
254 let src = include_str!("mod.rs");
255 let decl = src
256 .split_once("pub struct PlayerController {")
257 .expect("PlayerController struct")
258 .1;
259 let decl = decl.split_once("\n}").expect("end of struct").0;
260
261 for name in LOCK_ORDER {
262 assert!(
263 decl.contains(&format!("{name}:")),
264 "LOCK_ORDER names '{name}', which is not a PlayerController field"
265 );
266 }
267 }
268}