Architecture remediation A/B/F: poison-tolerant locks, graceful backend init, doc fixes
Workstream A — poison-tolerant locking: - Add utils/lock.rs with MutexSafe/RwLockSafe extension traits that recover a poisoned std::sync lock instead of panicking, plus unit tests. - Replace all 153 .lock().unwrap() and 4 .read()/.write().unwrap() production sites with _safe variants across 14 files, eliminating the player crash-cascade class. Tokio async mutexes are unchanged. Workstream B — graceful backend init: - create_player_backend no longer panics when MPV/ExoPlayer fail to initialize; it falls back to NullBackend and emits a backend-init-failed event so the UI can show "playback unavailable" instead of the app crashing. Fatal DB-setup panics are kept. Workstream F — doc reconciliation: - Rewrite software-architecture.md's inaccurate "thin UI / ~800 lines" claims to reflect reality (~20.5k non-test frontend) and document the events+polling hybrid plus the new locking/backend-init behavior.
This commit is contained in:
@@ -0,0 +1,111 @@
|
||||
//! Poison-tolerant locking helpers.
|
||||
//!
|
||||
//! `std::sync::Mutex` and `RwLock` become *poisoned* if a thread panics while
|
||||
//! holding the guard. After that, every `.lock().unwrap()` / `.read().unwrap()`
|
||||
//! / `.write().unwrap()` panics as well — so a single failure can cascade into
|
||||
//! an unrecoverable crash. That is a real risk for stateful subsystems like the
|
||||
//! player, whose locks are touched from many background threads (the MPV event
|
||||
//! loop, sleep/autoplay timers, JNI callbacks, the session poller).
|
||||
//!
|
||||
//! These extension traits recover the guard from a poisoned lock instead of
|
||||
//! panicking. The data behind a poisoned lock may be in an unexpected state,
|
||||
//! but for this application's state (queues, settings, flags) recovering and
|
||||
//! continuing is far preferable to taking down playback entirely.
|
||||
//!
|
||||
//! Use `lock_safe()` / `read_safe()` / `write_safe()` in place of
|
||||
//! `.lock().unwrap()` / `.read().unwrap()` / `.write().unwrap()`.
|
||||
//!
|
||||
//! Note: these apply only to `std::sync` primitives. `tokio::sync::Mutex` does
|
||||
//! not poison, so async code keeps using `.lock().await` unchanged.
|
||||
|
||||
use std::sync::{Mutex, MutexGuard, PoisonError, RwLock, RwLockReadGuard, RwLockWriteGuard};
|
||||
|
||||
/// Poison-tolerant locking for [`std::sync::Mutex`].
|
||||
pub trait MutexSafe<T: ?Sized> {
|
||||
/// Lock the mutex, recovering the inner guard if the lock was poisoned.
|
||||
fn lock_safe(&self) -> MutexGuard<'_, T>;
|
||||
}
|
||||
|
||||
impl<T: ?Sized> MutexSafe<T> for Mutex<T> {
|
||||
fn lock_safe(&self) -> MutexGuard<'_, T> {
|
||||
self.lock().unwrap_or_else(PoisonError::into_inner)
|
||||
}
|
||||
}
|
||||
|
||||
/// Poison-tolerant locking for [`std::sync::RwLock`].
|
||||
pub trait RwLockSafe<T: ?Sized> {
|
||||
/// Acquire a read guard, recovering it if the lock was poisoned.
|
||||
fn read_safe(&self) -> RwLockReadGuard<'_, T>;
|
||||
/// Acquire a write guard, recovering it if the lock was poisoned.
|
||||
fn write_safe(&self) -> RwLockWriteGuard<'_, T>;
|
||||
}
|
||||
|
||||
impl<T: ?Sized> RwLockSafe<T> for RwLock<T> {
|
||||
fn read_safe(&self) -> RwLockReadGuard<'_, T> {
|
||||
self.read().unwrap_or_else(PoisonError::into_inner)
|
||||
}
|
||||
|
||||
fn write_safe(&self) -> RwLockWriteGuard<'_, T> {
|
||||
self.write().unwrap_or_else(PoisonError::into_inner)
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use std::sync::Arc;
|
||||
use std::thread;
|
||||
|
||||
/// Run `f` with the panic hook suppressed so deliberately-poisoning panics
|
||||
/// don't spam the test output.
|
||||
fn without_panic_noise<R>(f: impl FnOnce() -> R) -> R {
|
||||
let prev = std::panic::take_hook();
|
||||
std::panic::set_hook(Box::new(|_| {}));
|
||||
let result = f();
|
||||
std::panic::set_hook(prev);
|
||||
result
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn lock_safe_recovers_from_poison() {
|
||||
let mutex = Arc::new(Mutex::new(0u32));
|
||||
let poisoner = Arc::clone(&mutex);
|
||||
|
||||
without_panic_noise(|| {
|
||||
let _ = thread::spawn(move || {
|
||||
let mut guard = poisoner.lock().unwrap();
|
||||
*guard = 42;
|
||||
panic!("poison the mutex while holding the guard");
|
||||
})
|
||||
.join();
|
||||
});
|
||||
|
||||
// The mutex is now poisoned: the normal path would panic.
|
||||
assert!(mutex.lock().is_err());
|
||||
|
||||
// lock_safe recovers the guard with the last written value intact.
|
||||
let guard = mutex.lock_safe();
|
||||
assert_eq!(*guard, 42);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn rwlock_safe_recovers_from_poison() {
|
||||
let lock = Arc::new(RwLock::new(0u32));
|
||||
let poisoner = Arc::clone(&lock);
|
||||
|
||||
without_panic_noise(|| {
|
||||
let _ = thread::spawn(move || {
|
||||
let mut guard = poisoner.write().unwrap();
|
||||
*guard = 7;
|
||||
panic!("poison the rwlock while holding the write guard");
|
||||
})
|
||||
.join();
|
||||
});
|
||||
|
||||
assert!(lock.write().is_err());
|
||||
assert_eq!(*lock.read_safe(), 7);
|
||||
|
||||
*lock.write_safe() = 9;
|
||||
assert_eq!(*lock.read_safe(), 9);
|
||||
}
|
||||
}
|
||||
@@ -1 +1,2 @@
|
||||
pub mod conversions;
|
||||
pub mod lock;
|
||||
|
||||
Reference in New Issue
Block a user