diff --git a/src/main.rs b/src/main.rs index cffd32a..d9887f8 100644 --- a/src/main.rs +++ b/src/main.rs @@ -29,6 +29,7 @@ mod pot_provider; mod stats; mod theme; mod tray; +mod util; mod web; mod ytdlp_bin; diff --git a/src/util.rs b/src/util.rs new file mode 100644 index 0000000..9e31fb1 --- /dev/null +++ b/src/util.rs @@ -0,0 +1,48 @@ +//! Small shared helpers. + +use std::sync::{Mutex, MutexGuard}; + +/// Lock a [`Mutex`], recovering the inner data if the lock was poisoned by +/// a panic in another thread while it was held. +/// +/// The web server holds several long-lived `Mutex`es in `WebState`. With +/// the default `.lock().unwrap()`, a single panic while one of them is held +/// would poison it and turn *every* subsequent access into a panic — +/// cascading one handler's bug into a permanently dead server. Recovering +/// via [`std::sync::PoisonError::into_inner`] keeps it serving: our +/// critical sections are short and don't leave half-updated invariants a +/// later reader would choke on, so the data behind the lock is still +/// usable, and one stuck request is far better than a stuck process. +pub trait LockExt { + /// Acquire the guard, recovering from poisoning instead of panicking. + fn lock_recover(&self) -> MutexGuard<'_, T>; +} + +impl LockExt for Mutex { + fn lock_recover(&self) -> MutexGuard<'_, T> { + self.lock().unwrap_or_else(|poisoned| poisoned.into_inner()) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use std::sync::Arc; + + #[test] + fn recovers_after_a_panic_poisoned_the_lock() { + let m = Arc::new(Mutex::new(41)); + // Poison the mutex: panic inside a thread while holding the guard. + let m2 = m.clone(); + let _ = std::thread::spawn(move || { + let _g = m2.lock().unwrap(); + panic!("boom while holding the lock"); + }) + .join(); + // Plain lock() would now return Err(PoisonError). lock_recover + // hands back the data so the server keeps going. + assert!(m.lock().is_err(), "precondition: mutex is poisoned"); + *m.lock_recover() += 1; + assert_eq!(*m.lock_recover(), 42); + } +} diff --git a/src/web.rs b/src/web.rs index 046e149..61a8f7a 100644 --- a/src/web.rs +++ b/src/web.rs @@ -43,6 +43,7 @@ use tower_http::services::ServeDir; use argon2::{Argon2, PasswordHash, PasswordHasher, PasswordVerifier}; use argon2::password_hash::SaltString; +use crate::util::LockExt; use crate::config::Config; use crate::database::Database; use crate::downloader::{DownloadQuality, Downloader, JobState}; @@ -868,7 +869,7 @@ fn session_token_from_headers(headers: &HeaderMap) -> Option { /// `sessions` map doesn't grow without bound for users who never log out. fn is_authed(state: &WebState, headers: &HeaderMap) -> bool { let Some(token) = session_token_from_headers(headers) else { return false }; - let mut sessions = state.sessions.lock().unwrap(); + let mut sessions = state.sessions.lock_recover(); let now = std::time::Instant::now(); // Lazy prune: drop anything older than the TTL. sessions.retain(|_, issued| now.duration_since(*issued) < SESSION_TTL); @@ -933,7 +934,7 @@ async fn post_login( let now = std::time::Instant::now(); // Check lockout first. { - let mut attempts = state.login_attempts.lock().unwrap(); + let mut attempts = state.login_attempts.lock_recover(); // GC entries whose lockout has elapsed. attempts.retain(|_, a| a.until.map_or(true, |u| u > now)); if let Some(a) = attempts.get(&ip) { @@ -951,7 +952,7 @@ async fn post_login( return (StatusCode::OK, "no password set").into_response(); }; if !verify_password(&body.password, &hash) { - let mut attempts = state.login_attempts.lock().unwrap(); + let mut attempts = state.login_attempts.lock_recover(); let entry = attempts.entry(ip).or_insert(LoginAttempt { failures: 0, until: None }); entry.failures += 1; if entry.failures >= LOGIN_LOCKOUT_AFTER { @@ -961,10 +962,10 @@ async fn post_login( return (StatusCode::UNAUTHORIZED, "invalid password").into_response(); } // Success: reset the failure counter for this IP. - state.login_attempts.lock().unwrap().remove(&ip); + state.login_attempts.lock_recover().remove(&ip); let token = generate_session_token(); - state.sessions.lock().unwrap().insert(token.clone(), now); + state.sessions.lock_recover().insert(token.clone(), now); let cookie = session_cookie(&token, &headers, SESSION_TTL.as_secs()); ([(header::SET_COOKIE, cookie)], StatusCode::OK).into_response() } @@ -975,7 +976,7 @@ async fn post_logout( headers: HeaderMap, ) -> Response { if let Some(token) = session_token_from_headers(&headers) { - state.sessions.lock().unwrap().remove(&token); + state.sessions.lock_recover().remove(&token); } let cookie = session_cookie("", &headers, 0); ([(header::SET_COOKIE, cookie)], StatusCode::OK).into_response() @@ -1079,7 +1080,7 @@ async fn get_library( // reliably). Serializing a multi-MB library JSON for every reload // burns measurable CPU on large installations. { - let cache = state.library_body_cache.lock().unwrap(); + let cache = state.library_body_cache.lock_recover(); if let Some((cached_ver, body)) = cache.as_ref() { if *cached_ver == version { let body = body.clone(); @@ -1105,7 +1106,7 @@ async fn get_library( let body = serde_json::to_string(&payload).unwrap_or_else(|_| "{}".to_string()); if post_version == version { let body_arc = std::sync::Arc::new(body.clone()); - *state.library_body_cache.lock().unwrap() = Some((version, body_arc)); + *state.library_body_cache.lock_recover() = Some((version, body_arc)); } ( [ @@ -1118,10 +1119,10 @@ async fn get_library( } async fn build_library_payload(state: &Arc) -> LibraryResponse { - let lib = state.library.lock().unwrap(); - let watched = state.watched.lock().unwrap(); - let positions = state.positions.lock().unwrap(); - let flags = state.flags.lock().unwrap(); + let lib = state.library.lock_recover(); + let watched = state.watched.lock_recover(); + let positions = state.positions.lock_recover(); + let flags = state.flags.lock_recover(); // file_url() now resolves relative to library_root (= parent of // channels_root) so non-YouTube platforms are reachable at // `/files///