diff --git a/software-architecture.md b/software-architecture.md index 75c4a5d..0d88490 100644 --- a/software-architecture.md +++ b/software-architecture.md @@ -2,19 +2,21 @@ This document describes the current architecture of JellyTau, a cross-platform Jellyfin client built with Tauri, SvelteKit, and Rust. -**Last Updated:** 2026-03-01 +**Last Updated:** 2026-06-20 ## Architecture Overview -JellyTau uses a modern client-server architecture with a thin Svelte UI layer and comprehensive Rust backend: +JellyTau uses a client-server architecture: business logic lives in a comprehensive Rust backend, while a UI-rich Svelte frontend handles presentation and interaction. ### Architecture Principles -- **Thin UI Layer**: TypeScript reduced from ~3,300 to ~800 lines -- **Business Logic in Rust**: Performance, reliability, type safety -- **Event-Driven**: Rust emits events, TypeScript listens and updates UI -- **Handle-Based Resources**: UUID handles for stateful Rust objects -- **Cache-First**: Parallel queries with intelligent fallback +- **Business Logic in Rust**: Core logic — playback, repository, sync, downloads, connectivity — lives in Rust for performance, reliability, and type safety. +- **Presentation in Svelte**: The frontend (~20.5k non-test lines) owns UI, layout, navigation, and interaction state and invokes Rust commands. It is intentionally UI-heavy, **not** a thin wrapper. Largest pieces: components + routes (~14.6k lines), stores (~3.4k), api/services/utils (~2.4k); `VideoPlayer.svelte` alone is ~1.6k lines. +- **Events + Polling hybrid**: Rust emits events the frontend listens to, and the UI also polls status on short intervals in a few hot spots (e.g. queue status in `library/+layout.svelte`, playback progress in `VideoPlayer.svelte`). +- **Handle-Based Resources**: UUID handles for stateful Rust objects. +- **Cache-First**: Parallel queries with intelligent fallback. +- **Poison-tolerant locking**: Shared `std::sync` state is accessed via the `MutexSafe`/`RwLockSafe` helpers in `utils/lock.rs`, which recover a poisoned lock instead of cascading a panic across the player. +- **Graceful backend init**: If a native player backend (MPV/ExoPlayer) fails to initialize, the app falls back to a no-op backend and emits a `backend-init-failed` event rather than crashing. ```mermaid flowchart TB @@ -190,11 +192,11 @@ src/lib/ 4. **Database Service** - Async wrapper preventing UI freezing 5. **Playback Mode** (303 lines) - Local/remote transfer coordination -**TypeScript Layer (now ~800 lines, down from ~3,300):** -- Svelte stores (reactive wrappers) -- Type definitions -- UI event handling -- Tauri command invocation -- Event listeners for Rust events +**Svelte/TypeScript frontend (~20.5k non-test lines, plus ~9.6k test lines):** +- Components + routes (~14.6k lines) — UI and presentation +- Stores (~3.4k lines) — reactive state that invokes Rust commands and listens for events +- api / services / utils (~2.4k lines) — typed clients, event listeners, conversion helpers + +The frontend is genuinely UI-heavy; business decisions live in Rust, but the UI owns layout, navigation, and interaction state. **Total Commands:** 90+ Tauri commands across 14 command modules diff --git a/src-tauri/src/commands/download.rs b/src-tauri/src/commands/download.rs index 39abf12..81f1ad9 100644 --- a/src-tauri/src/commands/download.rs +++ b/src-tauri/src/commands/download.rs @@ -1,5 +1,7 @@ //! Tauri commands for download operations +#[cfg(test)] +use crate::utils::lock::MutexSafe; use std::sync::{Arc, Mutex}; use tauri::State; use log::{debug, error, info, warn}; @@ -1577,7 +1579,7 @@ mod tests { fn setup_test_db() -> Database { let db = Database::open_in_memory().unwrap(); let conn = db.connection(); - let conn = conn.lock().unwrap(); + let conn = conn.lock_safe(); // Create server conn.execute( @@ -1620,7 +1622,7 @@ mod tests { fn test_download_item_returns_correct_id_on_insert() { let db = setup_test_db(); let conn = db.connection(); - let conn = conn.lock().unwrap(); + let conn = conn.lock_safe(); // Insert a new download conn.execute( @@ -1658,7 +1660,7 @@ mod tests { fn test_download_item_upsert_returns_correct_id() { let db = setup_test_db(); let conn = db.connection(); - let conn = conn.lock().unwrap(); + let conn = conn.lock_safe(); // First insert conn.execute( @@ -1733,7 +1735,7 @@ mod tests { // to get the correct download ID, regardless of whether INSERT or UPDATE occurred. let db = setup_test_db(); let conn = db.connection(); - let conn = conn.lock().unwrap(); + let conn = conn.lock_safe(); // First insert conn.execute( @@ -1781,7 +1783,7 @@ mod tests { fn test_download_album_returns_correct_ids() { let db = setup_test_db(); let conn = db.connection(); - let conn = conn.lock().unwrap(); + let conn = conn.lock_safe(); // Insert downloads for multiple items (simulating album download) let track_ids = vec!["item1", "item2", "item3"]; @@ -1827,7 +1829,7 @@ mod tests { fn test_download_album_upsert_returns_correct_ids() { let db = setup_test_db(); let conn = db.connection(); - let conn = conn.lock().unwrap(); + let conn = conn.lock_safe(); // First download attempt - insert all let track_ids = vec!["item1", "item2", "item3"]; @@ -1887,7 +1889,7 @@ mod tests { fn test_get_downloads_returns_correct_metadata() { let db = setup_test_db(); let conn = db.connection(); - let conn = conn.lock().unwrap(); + let conn = conn.lock_safe(); // Insert a download conn.execute( @@ -1932,7 +1934,7 @@ mod tests { fn test_download_status_transitions() { let db = setup_test_db(); let conn = db.connection(); - let conn = conn.lock().unwrap(); + let conn = conn.lock_safe(); // Insert pending download conn.execute( @@ -1978,7 +1980,7 @@ mod tests { fn test_download_progress_updates() { let db = setup_test_db(); let conn = db.connection(); - let conn = conn.lock().unwrap(); + let conn = conn.lock_safe(); conn.execute( "INSERT INTO downloads (item_id, user_id, file_path, status, progress, bytes_downloaded, file_size) diff --git a/src-tauri/src/commands/player.rs b/src-tauri/src/commands/player.rs index 0267146..66c747e 100644 --- a/src-tauri/src/commands/player.rs +++ b/src-tauri/src/commands/player.rs @@ -1,5 +1,6 @@ //! TRACES: UR-003, UR-004, UR-005, UR-010, UR-020, UR-021 | JA-022, JA-023, JA-024, JA-025, JA-026 | DR-001 +use crate::utils::lock::MutexSafe; use log::{debug, error, info, warn}; use serde::{Deserialize, Serialize}; use std::path::PathBuf; @@ -1291,7 +1292,7 @@ fn get_player_status(controller: &PlayerController) -> PlayerStatus { fn get_queue_status(controller: &PlayerController) -> QueueStatus { let queue = controller.queue(); - let queue_lock = queue.lock().unwrap(); + let queue_lock = queue.lock_safe(); QueueStatus { items: queue_lock.items().to_vec(), diff --git a/src-tauri/src/commands/repository.rs b/src-tauri/src/commands/repository.rs index 7915e1e..f8070b8 100644 --- a/src-tauri/src/commands/repository.rs +++ b/src-tauri/src/commands/repository.rs @@ -3,6 +3,7 @@ //! //! TRACES: UR-007, UR-035, UR-036 | JA-004, JA-005, JA-029, JA-030, JA-031 +use crate::utils::lock::MutexSafe; use std::collections::HashMap; use std::sync::{Arc, Mutex}; @@ -26,17 +27,17 @@ impl RepositoryManager { } pub fn create(&self, handle: String, repository: HybridRepository) { - let mut repos = self.repositories.lock().unwrap(); + let mut repos = self.repositories.lock_safe(); repos.insert(handle, Arc::new(repository)); } pub fn get(&self, handle: &str) -> Option> { - let repos = self.repositories.lock().unwrap(); + let repos = self.repositories.lock_safe(); repos.get(handle).cloned() } pub fn destroy(&self, handle: &str) { - let mut repos = self.repositories.lock().unwrap(); + let mut repos = self.repositories.lock_safe(); repos.remove(handle); } } diff --git a/src-tauri/src/download/cache.rs b/src-tauri/src/download/cache.rs index 78ee2c2..02053ab 100644 --- a/src-tauri/src/download/cache.rs +++ b/src-tauri/src/download/cache.rs @@ -1,5 +1,7 @@ //! Smart caching engine for predictive downloads +#[cfg(test)] +use crate::utils::lock::MutexSafe; use log::{debug, info}; use std::collections::HashMap; use std::sync::{Arc, Mutex}; @@ -309,7 +311,7 @@ mod tests { // Add some downloads { - let conn_guard = conn_arc.lock().unwrap(); + let conn_guard = conn_arc.lock_safe(); conn_guard.execute( "INSERT INTO downloads (user_id, status, file_size) VALUES ('user1', 'completed', 600)", [], diff --git a/src-tauri/src/download/mod.rs b/src-tauri/src/download/mod.rs index 29ce565..8357f46 100644 --- a/src-tauri/src/download/mod.rs +++ b/src-tauri/src/download/mod.rs @@ -10,6 +10,7 @@ pub mod cache; pub mod events; pub mod worker; +use crate::utils::lock::MutexSafe; use std::path::PathBuf; use std::collections::HashSet; use std::sync::{Arc, Mutex}; @@ -45,18 +46,18 @@ impl DownloadManager { /// Check if a new download can be started based on concurrent limit pub fn can_start_download(&self) -> bool { - let active = self.active_downloads.lock().unwrap(); + let active = self.active_downloads.lock_safe(); active.len() < self.max_concurrent } /// Get the number of currently active downloads pub fn active_count(&self) -> usize { - self.active_downloads.lock().unwrap().len() + self.active_downloads.lock_safe().len() } /// Register a download as active pub fn register_download(&self, download_id: i64) -> bool { - let mut active = self.active_downloads.lock().unwrap(); + let mut active = self.active_downloads.lock_safe(); if active.len() >= self.max_concurrent { return false; } @@ -65,7 +66,7 @@ impl DownloadManager { /// Unregister a download when it completes or fails pub fn unregister_download(&self, download_id: i64) { - let mut active = self.active_downloads.lock().unwrap(); + let mut active = self.active_downloads.lock_safe(); active.remove(&download_id); } diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index 0bf2c95..6e44d6a 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -16,7 +16,7 @@ mod utils; use std::sync::{Arc, Mutex}; use tokio::sync::Mutex as TokioMutex; -use tauri::Manager; +use tauri::{Emitter, Manager}; use log::{error, info}; #[cfg(target_os = "android")] use log::warn; @@ -120,8 +120,9 @@ use download::cache::{CacheConfig as SmartCacheConfig, SmartCache}; use download::DownloadManager; use jellyfin::{HttpClient, HttpConfig}; use player::{MediaSessionManager, PlayerBackend, PlayerController, TauriEventEmitter}; -// NullBackend fallback for platforms without native backends (not Linux or Android) -#[cfg(not(any(target_os = "linux", target_os = "android")))] +// NullBackend is used both for platforms without a native backend AND as a graceful +// fallback when a native backend (MPV/ExoPlayer) fails to initialize, so the app can +// still launch (browse library, manage downloads, see an error) instead of crashing. use player::NullBackend; #[cfg(target_os = "linux")] @@ -225,13 +226,39 @@ impl RemoteVolumeHandler for RemoteVolumeSessionHandler { } } +/// Payload emitted to the frontend when a native player backend fails to +/// initialize and the app falls back to a no-op backend. +#[derive(Clone, serde::Serialize)] +struct BackendInitError { + platform: &'static str, + backend: &'static str, + message: String, +} + +/// Log a backend-initialization failure and notify the frontend, so the UI can +/// surface "playback unavailable" instead of the app hard-crashing. +fn emit_backend_init_failed(app_handle: &tauri::AppHandle, backend: &'static str, message: String) { + error!( + "[INIT] Player backend '{}' failed to initialize: {}. Falling back to NullBackend (playback disabled).", + backend, message + ); + let _ = app_handle.emit( + "backend-init-failed", + BackendInitError { + platform: std::env::consts::OS, + backend, + message, + }, + ); +} + /// Create the appropriate player backend for the current platform. fn create_player_backend( app_handle: tauri::AppHandle, playback_reporter: Arc>>, position_throttler: Arc, ) -> Box { - let _event_emitter = Arc::new(TauriEventEmitter::new(app_handle)); + let _event_emitter = Arc::new(TauriEventEmitter::new(app_handle.clone())); #[cfg(target_os = "android")] { @@ -255,17 +282,21 @@ fn create_player_backend( return Box::new(backend); } Err(e) => { - panic!("FATAL: Failed to initialize ExoPlayer backend on Android: {}. This is a critical error - playback will not work.", e); + // Degrade gracefully instead of crashing the app. + emit_backend_init_failed(&app_handle, "exoplayer", e.to_string()); + return Box::new(NullBackend::new()); } } } Err(e) => { - panic!("FATAL: Failed to attach JNI thread on Android: {}. This is a critical error - playback will not work.", e); + emit_backend_init_failed(&app_handle, "exoplayer", format!("attach JNI thread failed: {}", e)); + return Box::new(NullBackend::new()); } } } Err(e) => { - panic!("FATAL: Failed to create JavaVM on Android: {}. This is a critical error - playback will not work.", e); + emit_backend_init_failed(&app_handle, "exoplayer", format!("create JavaVM failed: {}", e)); + return Box::new(NullBackend::new()); } } } @@ -298,7 +329,11 @@ fn create_player_backend( error!("\nAudio playback will NOT work until this is fixed."); error!("========================================\n"); - panic!("Cannot start application: MPV backend initialization failed. See error message above."); + // Degrade gracefully: launch with a no-op backend so the user can + // still browse the library and manage downloads, and the frontend + // can show a "playback unavailable" notice via this event. + emit_backend_init_failed(&app_handle, "mpv", e.to_string()); + return Box::new(NullBackend::new()); } } } diff --git a/src-tauri/src/playback_mode/mod.rs b/src-tauri/src/playback_mode/mod.rs index bee8b86..1a2e93a 100644 --- a/src-tauri/src/playback_mode/mod.rs +++ b/src-tauri/src/playback_mode/mod.rs @@ -1,3 +1,4 @@ +use crate::utils::lock::{MutexSafe, RwLockSafe}; use log::{debug, error, info}; use serde::{Deserialize, Serialize}; use std::sync::{ @@ -43,13 +44,13 @@ impl PlaybackModeManager { /// Get current playback mode pub fn get_mode(&self) -> PlaybackMode { - self.current_mode.read().unwrap().clone() + self.current_mode.read_safe().clone() } /// Set playback mode (internal use) pub fn set_mode(&self, mode: PlaybackMode) { log::info!("[PlaybackMode] Setting mode to: {:?}", mode); - let mut current = self.current_mode.write().unwrap(); + let mut current = self.current_mode.write_safe(); *current = mode; } @@ -193,7 +194,7 @@ impl PlaybackModeManager { debug!("[PlaybackMode] Player controller lock acquired"); let queue_arc = player.queue(); - let queue = queue_arc.lock().unwrap(); + let queue = queue_arc.lock_safe(); let state = player.state(); let original_index = queue.current_index().unwrap_or(0); @@ -399,7 +400,7 @@ impl PlaybackModeManager { // Log queue state BEFORE stop { let queue_arc = player.queue(); - let queue = queue_arc.lock().unwrap(); + let queue = queue_arc.lock_safe(); info!( "[PlaybackMode] BEFORE STOP: Queue has {} items, current_index={:?}", queue.items().len(), @@ -412,7 +413,7 @@ impl PlaybackModeManager { // Log queue state AFTER stop (should be unchanged) { let queue_arc = player.queue(); - let queue = queue_arc.lock().unwrap(); + let queue = queue_arc.lock_safe(); info!( "[PlaybackMode] AFTER STOP: Queue has {} items, current_index={:?}", queue.items().len(), diff --git a/src-tauri/src/playback_reporting/throttle.rs b/src-tauri/src/playback_reporting/throttle.rs index fd066f4..e30d688 100644 --- a/src-tauri/src/playback_reporting/throttle.rs +++ b/src-tauri/src/playback_reporting/throttle.rs @@ -5,6 +5,7 @@ #![allow(dead_code)] +use crate::utils::lock::MutexSafe; use std::collections::HashMap; use std::sync::{Arc, Mutex}; use std::time::{Duration, Instant}; @@ -34,7 +35,7 @@ impl EventThrottler { /// Checks if enough time has elapsed since the last report for this item pub fn should_report(&self, item_id: &str) -> bool { - let last_times = self.last_report_time.lock().unwrap(); + let last_times = self.last_report_time.lock_safe(); if let Some(last_time) = last_times.get(item_id) { let elapsed = last_time.elapsed(); @@ -54,7 +55,7 @@ impl EventThrottler { /// Marks the item as reported at the current time pub fn mark_reported(&self, item_id: &str) { - let mut last_times = self.last_report_time.lock().unwrap(); + let mut last_times = self.last_report_time.lock_safe(); last_times.insert(item_id.to_string(), Instant::now()); log::debug!( @@ -66,14 +67,14 @@ impl EventThrottler { /// Clears all tracked report times pub fn clear(&self) { - let mut last_times = self.last_report_time.lock().unwrap(); + let mut last_times = self.last_report_time.lock_safe(); last_times.clear(); log::debug!("[EventThrottler] Cleared all tracked report times"); } /// Removes a specific item from tracking pub fn clear_item(&self, item_id: &str) { - let mut last_times = self.last_report_time.lock().unwrap(); + let mut last_times = self.last_report_time.lock_safe(); last_times.remove(item_id); log::debug!("[EventThrottler] Cleared tracking for {}", item_id); } diff --git a/src-tauri/src/player/android/mod.rs b/src-tauri/src/player/android/mod.rs index 2018959..9e9b1d0 100644 --- a/src-tauri/src/player/android/mod.rs +++ b/src-tauri/src/player/android/mod.rs @@ -3,6 +3,7 @@ //! This module provides a `PlayerBackend` implementation using Android's ExoPlayer //! through JNI calls to Kotlin code. +use crate::utils::lock::MutexSafe; use std::sync::{Arc, Mutex, OnceLock}; use tokio::sync::Mutex as TokioMutex; use log::debug; @@ -303,7 +304,7 @@ impl PlayerBackend for ExoPlayerBackend { // Update local state { - let mut state = self.shared_state.lock().unwrap(); + let mut state = self.shared_state.lock_safe(); state.current_media = Some(media.clone()); state.state = PlayerState::Loading { media: media.clone(), @@ -430,7 +431,7 @@ impl PlayerBackend for ExoPlayerBackend { fn stop(&mut self) -> Result<(), PlayerError> { { - let mut state = self.shared_state.lock().unwrap(); + let mut state = self.shared_state.lock_safe(); state.state = PlayerState::Idle; state.is_loaded = false; state.current_media = None; @@ -479,24 +480,24 @@ impl PlayerBackend for ExoPlayerBackend { ) .map_err(|e| PlayerError::playback_failed(format!("Failed to call setVolume: {}", e)))?; - self.shared_state.lock().unwrap().volume = clamped; + self.shared_state.lock_safe().volume = clamped; Ok(()) } fn position(&self) -> f64 { - self.shared_state.lock().unwrap().position + self.shared_state.lock_safe().position } fn duration(&self) -> Option { - self.shared_state.lock().unwrap().duration + self.shared_state.lock_safe().duration } fn state(&self) -> PlayerState { - self.shared_state.lock().unwrap().state.clone() + self.shared_state.lock_safe().state.clone() } fn volume(&self) -> f32 { - self.shared_state.lock().unwrap().volume + self.shared_state.lock_safe().volume } fn set_audio_track(&mut self, stream_index: i32) -> Result<(), PlayerError> { @@ -564,7 +565,7 @@ pub extern "system" fn Java_com_dtourolle_jellytau_player_JellyTauPlayer_nativeO // Update state and get the preserved duration to emit let duration_to_emit = if let Some(state) = SHARED_STATE.get() { - let mut state = state.lock().unwrap(); + let mut state = state.lock_safe(); state.position = position; if duration > 0.0 { state.duration = Some(duration); @@ -606,7 +607,7 @@ pub extern "system" fn Java_com_dtourolle_jellytau_player_JellyTauPlayer_nativeO // Update shared state if let Some(shared) = SHARED_STATE.get() { - let mut shared = shared.lock().unwrap(); + let mut shared = shared.lock_safe(); if let Some(media) = shared.current_media.clone() { let duration = shared.duration.unwrap_or(0.0); let position = shared.position; @@ -651,7 +652,7 @@ pub extern "system" fn Java_com_dtourolle_jellytau_player_JellyTauPlayer_nativeO duration: jdouble, ) { if let Some(state) = SHARED_STATE.get() { - let mut state = state.lock().unwrap(); + let mut state = state.lock_safe(); state.duration = Some(duration); state.is_loaded = true; } @@ -692,7 +693,7 @@ pub extern "system" fn Java_com_dtourolle_jellytau_player_JellyTauPlayer_nativeO // Log queue state before advancing let queue_info = { - let queue = ctrl.queue.lock().unwrap(); + let queue = ctrl.queue.lock_safe(); format!("current_index={:?}, len={}", queue.current_index(), queue.items().len()) }; log::debug!("[Autoplay] Queue state before next(): {}", queue_info); @@ -702,7 +703,7 @@ pub extern "system" fn Java_com_dtourolle_jellytau_player_JellyTauPlayer_nativeO log::info!("[Autoplay] Successfully advanced to next track"); // Log queue state after advancing let queue_info = { - let queue = ctrl.queue.lock().unwrap(); + let queue = ctrl.queue.lock_safe(); format!("current_index={:?}, len={}", queue.current_index(), queue.items().len()) }; log::debug!("[Autoplay] Queue state after next(): {}", queue_info); @@ -811,7 +812,7 @@ pub extern "system" fn Java_com_dtourolle_jellytau_player_JellyTauPlayer_nativeO muted: jboolean, ) { if let Some(state) = SHARED_STATE.get() { - state.lock().unwrap().volume = volume; + state.lock_safe().volume = volume; } if let Some(emitter) = EVENT_EMITTER.get() { diff --git a/src-tauri/src/player/events.rs b/src-tauri/src/player/events.rs index 3b206ab..60d9373 100644 --- a/src-tauri/src/player/events.rs +++ b/src-tauri/src/player/events.rs @@ -5,6 +5,8 @@ //! //! TRACES: UR-005, UR-019, UR-023, UR-026 | DR-001, DR-028, DR-047 +#[cfg(test)] +use crate::utils::lock::MutexSafe; use log::error; use serde::{Deserialize, Serialize}; use std::sync::Arc; @@ -168,17 +170,17 @@ mod tests { } pub fn events(&self) -> Vec { - self.events.lock().unwrap().clone() + self.events.lock_safe().clone() } pub fn clear(&self) { - self.events.lock().unwrap().clear(); + self.events.lock_safe().clear(); } } impl PlayerEventEmitter for TestEventEmitter { fn emit(&self, event: PlayerStatusEvent) { - self.events.lock().unwrap().push(event); + self.events.lock_safe().push(event); } } diff --git a/src-tauri/src/player/mod.rs b/src-tauri/src/player/mod.rs index 6ea5a34..00bc393 100644 --- a/src-tauri/src/player/mod.rs +++ b/src-tauri/src/player/mod.rs @@ -44,6 +44,7 @@ pub use android::{ set_media_command_handler, set_remote_volume_handler, get_detected_codecs, }; +use crate::utils::lock::MutexSafe; use log::{debug, error, warn}; use std::sync::{Arc, Mutex}; use std::time::Duration; @@ -121,7 +122,7 @@ impl PlayerController { /// Configure the Jellyfin API client for automatic playback reporting pub fn set_jellyfin_client(&self, client: Option) { - let mut jellyfin = self.jellyfin_client.lock().unwrap(); + let mut jellyfin = self.jellyfin_client.lock_safe(); *jellyfin = client; log::info!("[PlayerController] Jellyfin client configured: {}", jellyfin.is_some()); } @@ -157,24 +158,24 @@ impl PlayerController { /// Set the end reason for the next playback end event fn set_end_reason(&self, reason: EndReason) { log::debug!("[PlayerController] Setting end reason: {:?}", reason); - *self.end_reason.lock().unwrap() = Some(reason); + *self.end_reason.lock_safe() = Some(reason); } /// Get and clear the current end reason fn take_end_reason(&self) -> Option { - self.end_reason.lock().unwrap().take() + self.end_reason.lock_safe().take() } /// Increment autoplay episode counter. Returns true if limit is reached. fn increment_autoplay_count(&self) -> bool { - let max = self.autoplay_settings.lock().unwrap().max_episodes; + let max = self.autoplay_settings.lock_safe().max_episodes; if max == 0 { // Unlimited return false; } - let mut count = self.autoplay_episode_count.lock().unwrap(); + let mut count = self.autoplay_episode_count.lock_safe(); *count += 1; debug!("[PlayerController] Autoplay episode count: {}/{}", *count, max); @@ -183,7 +184,7 @@ impl PlayerController { /// Reset autoplay episode counter (called on manual play actions) fn reset_autoplay_count(&self) { - let mut count = self.autoplay_episode_count.lock().unwrap(); + let mut count = self.autoplay_episode_count.lock_safe(); if *count > 0 { debug!("[PlayerController] Resetting autoplay episode counter (was {})", *count); } @@ -199,7 +200,7 @@ impl PlayerController { // Update queue with this single item { - let mut queue = self.queue.lock().unwrap(); + let mut queue = self.queue.lock_safe(); queue.set_queue(vec![item.clone()], 0); } @@ -217,7 +218,7 @@ impl PlayerController { // Set end reason to NewTrackLoaded to prevent autoplay when MPV ends current track self.set_end_reason(EndReason::NewTrackLoaded); - let mut backend = self.backend.lock().unwrap(); + let mut backend = self.backend.lock_safe(); backend.load(item)?; backend.play()?; drop(backend); @@ -297,12 +298,12 @@ impl PlayerController { self.reset_autoplay_count(); { - let mut queue = self.queue.lock().unwrap(); + let mut queue = self.queue.lock_safe(); queue.set_queue(items, start_index); } // Play the current item (without modifying the queue we just set) - if let Some(item) = self.queue.lock().unwrap().current().cloned() { + if let Some(item) = self.queue.lock_safe().current().cloned() { self.load_and_play(&item)?; } @@ -312,19 +313,19 @@ impl PlayerController { /// Play/resume playback pub fn play(&self) -> Result<(), PlayerError> { debug!("[PlayerController] play"); - let mut backend = self.backend.lock().unwrap(); + let mut backend = self.backend.lock_safe(); backend.play() } /// Pause playback pub fn pause(&self) -> Result<(), PlayerError> { - let mut backend = self.backend.lock().unwrap(); + let mut backend = self.backend.lock_safe(); backend.pause() } /// Toggle play/pause pub fn toggle_playback(&self) -> Result<(), PlayerError> { - let mut backend = self.backend.lock().unwrap(); + let mut backend = self.backend.lock_safe(); if backend.state().is_playing() { backend.pause() } else { @@ -339,16 +340,16 @@ impl PlayerController { // Get current playback info before stopping let jellyfin_id = { - let queue = self.queue.lock().unwrap(); + let queue = self.queue.lock_safe(); queue.current().and_then(|item| item.jellyfin_id().map(|s| s.to_string())) }; let position_ticks = { - let backend = self.backend.lock().unwrap(); + let backend = self.backend.lock_safe(); (backend.position() * 10_000_000.0) as i64 }; - let mut backend = self.backend.lock().unwrap(); + let mut backend = self.backend.lock_safe(); backend.stop()?; drop(backend); @@ -412,7 +413,7 @@ impl PlayerController { self.reset_autoplay_count(); let next_item = { - let mut queue = self.queue.lock().unwrap(); + let mut queue = self.queue.lock_safe(); queue.next().cloned() }; @@ -435,7 +436,7 @@ impl PlayerController { self.reset_autoplay_count(); // If we're more than 3 seconds in, restart current track { - let backend = self.backend.lock().unwrap(); + let backend = self.backend.lock_safe(); if backend.position() > 3.0 { debug!("[PlayerController] previous: restarting current track (position > 3s)"); drop(backend); @@ -444,7 +445,7 @@ impl PlayerController { } let prev_item = { - let mut queue = self.queue.lock().unwrap(); + let mut queue = self.queue.lock_safe(); queue.previous().cloned() }; @@ -459,40 +460,40 @@ impl PlayerController { /// Seek to a position in seconds pub fn seek(&self, position: f64) -> Result<(), PlayerError> { - let mut backend = self.backend.lock().unwrap(); + let mut backend = self.backend.lock_safe(); backend.seek(position) } /// Set volume (0.0 - 1.0) pub fn set_volume(&self, volume: f32) -> Result<(), PlayerError> { - self.backend.lock().unwrap().set_volume(volume) + self.backend.lock_safe().set_volume(volume) } /// Set the active audio track by stream index pub fn set_audio_track(&self, stream_index: i32) -> Result<(), PlayerError> { - let mut backend = self.backend.lock().unwrap(); + let mut backend = self.backend.lock_safe(); backend.set_audio_track(stream_index) } /// Set the active subtitle track by stream index (None to disable subtitles) pub fn set_subtitle_track(&self, stream_index: Option) -> Result<(), PlayerError> { - let mut backend = self.backend.lock().unwrap(); + let mut backend = self.backend.lock_safe(); backend.set_subtitle_track(stream_index) } /// Get current state pub fn state(&self) -> PlayerState { - self.backend.lock().unwrap().state() + self.backend.lock_safe().state() } /// Get current position pub fn position(&self) -> f64 { - self.backend.lock().unwrap().position() + self.backend.lock_safe().position() } /// Get duration pub fn duration(&self) -> Option { - self.backend.lock().unwrap().duration() + self.backend.lock_safe().duration() } /// Get queue reference @@ -502,27 +503,27 @@ impl PlayerController { /// Toggle shuffle pub fn toggle_shuffle(&self) { - self.queue.lock().unwrap().toggle_shuffle(); + self.queue.lock_safe().toggle_shuffle(); } /// Cycle repeat mode pub fn cycle_repeat(&self) { - self.queue.lock().unwrap().cycle_repeat(); + self.queue.lock_safe().cycle_repeat(); } /// Check if shuffle is enabled pub fn is_shuffle(&self) -> bool { - self.queue.lock().unwrap().is_shuffle() + self.queue.lock_safe().is_shuffle() } /// Get repeat mode pub fn repeat_mode(&self) -> RepeatMode { - self.queue.lock().unwrap().repeat_mode() + self.queue.lock_safe().repeat_mode() } /// Get current volume (0.0 - 1.0) pub fn volume(&self) -> f32 { - self.backend.lock().unwrap().volume() + self.backend.lock_safe().volume() } /// Check if muted @@ -532,35 +533,35 @@ impl PlayerController { /// Set audio settings (crossfade, gapless, normalization) pub fn set_audio_settings(&mut self, settings: &AudioSettings) -> Result<(), PlayerError> { - self.backend.lock().unwrap().set_audio_settings(settings) + self.backend.lock_safe().set_audio_settings(settings) } /// Get current audio settings pub fn audio_settings(&self) -> AudioSettings { - self.backend.lock().unwrap().audio_settings() + self.backend.lock_safe().audio_settings() } // ===== Sleep Timer Methods ===== /// Set the event emitter for notifications pub fn set_event_emitter(&self, emitter: Arc) { - let mut event_emitter = self.event_emitter.lock().unwrap(); + let mut event_emitter = self.event_emitter.lock_safe(); *event_emitter = Some(emitter); } /// Get the event emitter pub fn event_emitter(&self) -> Option> { - self.event_emitter.lock().unwrap().clone() + self.event_emitter.lock_safe().clone() } /// Get sleep timer state pub fn sleep_timer_state(&self) -> SleepTimerState { - self.sleep_timer.lock().unwrap().clone() + self.sleep_timer.lock_safe().clone() } /// Set sleep timer mode (in-memory only, not persisted) pub fn set_sleep_timer(&self, mode: SleepTimerMode) { - let mut timer = self.sleep_timer.lock().unwrap(); + let mut timer = self.sleep_timer.lock_safe(); timer.mode = mode.clone(); if let SleepTimerMode::Time { end_time } = mode { let now = chrono::Utc::now().timestamp_millis(); @@ -589,7 +590,7 @@ impl PlayerController { loop { std::thread::sleep(Duration::from_secs(1)); - let mut timer = sleep_timer.lock().unwrap(); + let mut timer = sleep_timer.lock_safe(); if timer.is_active() { timer.update_remaining_seconds(); @@ -599,7 +600,7 @@ impl PlayerController { timer.cancel(); // Emit cancelled state - if let Some(emitter) = event_emitter.lock().unwrap().as_ref() { + if let Some(emitter) = event_emitter.lock_safe().as_ref() { emitter.emit(PlayerStatusEvent::SleepTimerChanged { mode: SleepTimerMode::Off, remaining_seconds: 0, @@ -608,14 +609,14 @@ impl PlayerController { drop(timer); // Stop the backend - if let Err(e) = backend.lock().unwrap().stop() { + if let Err(e) = backend.lock_safe().stop() { error!("[SleepTimer] Failed to stop playback: {}", e); } continue; } // Emit update event - if let Some(emitter) = event_emitter.lock().unwrap().as_ref() { + if let Some(emitter) = event_emitter.lock_safe().as_ref() { emitter.emit(PlayerStatusEvent::SleepTimerChanged { mode: timer.mode.clone(), remaining_seconds: timer.remaining_seconds, @@ -629,9 +630,9 @@ impl PlayerController { /// Emit sleep timer changed event to frontend fn emit_sleep_timer_changed(&self) { - let timer = self.sleep_timer.lock().unwrap().clone(); + let timer = self.sleep_timer.lock_safe().clone(); - if let Some(emitter) = self.event_emitter.lock().unwrap().as_ref() { + if let Some(emitter) = self.event_emitter.lock_safe().as_ref() { emitter.emit(PlayerStatusEvent::SleepTimerChanged { mode: timer.mode, remaining_seconds: timer.remaining_seconds, @@ -641,12 +642,12 @@ impl PlayerController { /// Emit queue changed event to frontend pub fn emit_queue_changed(&self) { - let queue = self.queue.lock().unwrap(); + let queue = self.queue.lock_safe(); debug!("PlayerController::emit_queue_changed() - Emitting queue with {} items, current_index: {:?}", queue.items().len(), queue.current_index()); - if let Some(emitter) = self.event_emitter.lock().unwrap().as_ref() { + if let Some(emitter) = self.event_emitter.lock_safe().as_ref() { emitter.emit(PlayerStatusEvent::QueueChanged { items: queue.items().to_vec(), current_index: queue.current_index(), @@ -664,19 +665,19 @@ impl PlayerController { /// Get autoplay settings pub fn autoplay_settings(&self) -> AutoplaySettings { - self.autoplay_settings.lock().unwrap().clone() + self.autoplay_settings.lock_safe().clone() } /// Set autoplay settings (in-memory only, persistence handled by command layer) pub fn set_autoplay_settings(&self, settings: AutoplaySettings) { let validated = settings.with_validated_countdown(); - *self.autoplay_settings.lock().unwrap() = validated; + *self.autoplay_settings.lock_safe() = validated; } /// Cancel active autoplay countdown pub fn cancel_autoplay_countdown(&self) { - if let Some(cancel_flag) = self.countdown_cancel.lock().unwrap().as_ref() { - *cancel_flag.lock().unwrap() = true; + if let Some(cancel_flag) = self.countdown_cancel.lock_safe().as_ref() { + *cancel_flag.lock_safe() = true; } } @@ -719,7 +720,7 @@ impl PlayerController { } let current_item = { - let queue = self.queue.lock().unwrap(); + let queue = self.queue.lock_safe(); queue.current().cloned() }; @@ -729,7 +730,7 @@ impl PlayerController { // Check sleep timer state let timer_mode = { - let timer = self.sleep_timer.lock().unwrap(); + let timer = self.sleep_timer.lock_safe(); timer.mode.clone() }; @@ -739,14 +740,14 @@ impl PlayerController { let now = chrono::Utc::now().timestamp_millis(); if now >= *end_time { debug!("[PlayerController] Time-based sleep timer expired at track boundary"); - self.sleep_timer.lock().unwrap().cancel(); + self.sleep_timer.lock_safe().cancel(); self.emit_sleep_timer_changed(); return Ok(AutoplayDecision::Stop); } } SleepTimerMode::EndOfTrack => { // Stop at end of track - self.sleep_timer.lock().unwrap().cancel(); + self.sleep_timer.lock_safe().cancel(); self.emit_sleep_timer_changed(); return Ok(AutoplayDecision::Stop); } @@ -756,7 +757,7 @@ impl PlayerController { && self.is_episode_item(¤t).await; if is_episode { - let should_stop = self.sleep_timer.lock().unwrap().decrement_episode(); + let should_stop = self.sleep_timer.lock_safe().decrement_episode(); self.emit_sleep_timer_changed(); if should_stop { @@ -773,7 +774,7 @@ impl PlayerController { // Note: This path is typically not hit for HTML5 video (which uses on_video_playback_ended). // It's here for the Android ExoPlayer path where video items may be in the backend queue. if current.media_type == MediaType::Video && self.is_episode_item(¤t).await { - let repo = self.repository.lock().unwrap().clone(); + let repo = self.repository.lock_safe().clone(); let jellyfin_id = current.jellyfin_id().unwrap_or(¤t.id); let next_ep_result = if let Some(repo) = &repo { self.fetch_next_episode_for_item(jellyfin_id, repo).await? @@ -782,7 +783,7 @@ impl PlayerController { None }; if let Some(next_ep) = next_ep_result { - let settings = self.autoplay_settings.lock().unwrap().clone(); + let settings = self.autoplay_settings.lock_safe().clone(); // Check if auto-play episode limit is reached let limit_reached = self.increment_autoplay_count(); @@ -803,7 +804,7 @@ impl PlayerController { // For audio/movies, check if there's a next track in the queue let has_next = { - let queue = self.queue.lock().unwrap(); + let queue = self.queue.lock_safe(); queue.has_next() }; @@ -837,7 +838,7 @@ impl PlayerController { // Check sleep timer state let timer_mode = { - let timer = self.sleep_timer.lock().unwrap(); + let timer = self.sleep_timer.lock_safe(); timer.mode.clone() }; @@ -846,18 +847,18 @@ impl PlayerController { let now = chrono::Utc::now().timestamp_millis(); if now >= *end_time { debug!("[PlayerController] Time-based sleep timer expired at video end"); - self.sleep_timer.lock().unwrap().cancel(); + self.sleep_timer.lock_safe().cancel(); self.emit_sleep_timer_changed(); return Ok(AutoplayDecision::Stop); } } SleepTimerMode::EndOfTrack => { - self.sleep_timer.lock().unwrap().cancel(); + self.sleep_timer.lock_safe().cancel(); self.emit_sleep_timer_changed(); return Ok(AutoplayDecision::Stop); } SleepTimerMode::Episodes { .. } => { - let should_stop = self.sleep_timer.lock().unwrap().decrement_episode(); + let should_stop = self.sleep_timer.lock_safe().decrement_episode(); self.emit_sleep_timer_changed(); if should_stop { return Ok(AutoplayDecision::Stop); @@ -868,7 +869,7 @@ impl PlayerController { // Fetch next episode for the video that just ended if let Some(next_ep) = self.fetch_next_episode_for_item(item_id, &repo).await? { - let settings = self.autoplay_settings.lock().unwrap().clone(); + let settings = self.autoplay_settings.lock_safe().clone(); let limit_reached = self.increment_autoplay_count(); if limit_reached { @@ -961,7 +962,7 @@ impl PlayerController { pub fn start_autoplay_countdown(&self, _next_item: crate::repository::types::MediaItem, countdown_seconds: u32) { // Create cancellation flag let cancel_flag = Arc::new(Mutex::new(false)); - *self.countdown_cancel.lock().unwrap() = Some(cancel_flag.clone()); + *self.countdown_cancel.lock_safe() = Some(cancel_flag.clone()); let event_emitter = self.event_emitter.clone(); @@ -972,7 +973,7 @@ impl PlayerController { std::thread::sleep(Duration::from_secs(1)); // Check cancellation - if *cancel_flag.lock().unwrap() { + if *cancel_flag.lock_safe() { log::info!("[PlayerController] Autoplay countdown cancelled"); return; } @@ -980,7 +981,7 @@ impl PlayerController { remaining -= 1; // Emit countdown tick event - if let Some(emitter) = event_emitter.lock().unwrap().as_ref() { + if let Some(emitter) = event_emitter.lock_safe().as_ref() { emitter.emit(PlayerStatusEvent::CountdownTick { remaining_seconds: remaining, }); @@ -1081,7 +1082,7 @@ mod tests { // Verify initial state { let queue = controller.queue(); - let queue_lock = queue.lock().unwrap(); + let queue_lock = queue.lock_safe(); assert_eq!(queue_lock.items().len(), 5, "Queue should have 5 items"); assert_eq!(queue_lock.current_index(), Some(0), "Should start at index 0"); assert_eq!(queue_lock.current().unwrap().id, "item_0", "Current item should be item_0"); @@ -1093,7 +1094,7 @@ mod tests { // Verify queue is intact and index advanced { let queue = controller.queue(); - let queue_lock = queue.lock().unwrap(); + let queue_lock = queue.lock_safe(); assert_eq!(queue_lock.items().len(), 5, "Queue should still have 5 items after skip"); assert_eq!(queue_lock.current_index(), Some(1), "Index should advance to 1"); assert_eq!(queue_lock.current().unwrap().id, "item_1", "Current item should be item_1"); @@ -1112,7 +1113,7 @@ mod tests { // Verify queue still intact and index advanced again { let queue = controller.queue(); - let queue_lock = queue.lock().unwrap(); + let queue_lock = queue.lock_safe(); assert_eq!(queue_lock.items().len(), 5, "Queue should still have 5 items after second skip"); assert_eq!(queue_lock.current_index(), Some(2), "Index should advance to 2"); assert_eq!(queue_lock.current().unwrap().id, "item_2", "Current item should be item_2"); @@ -1125,7 +1126,7 @@ mod tests { // Verify we're at the last item { let queue = controller.queue(); - let queue_lock = queue.lock().unwrap(); + let queue_lock = queue.lock_safe(); assert_eq!(queue_lock.items().len(), 5, "Queue should still have 5 items at end"); assert_eq!(queue_lock.current_index(), Some(4), "Index should be at last item (4)"); assert_eq!(queue_lock.current().unwrap().id, "item_4", "Current item should be item_4"); @@ -1147,7 +1148,7 @@ mod tests { // Verify we're at the last item { let queue = controller.queue(); - let queue_lock = queue.lock().unwrap(); + let queue_lock = queue.lock_safe(); assert_eq!(queue_lock.current_index(), Some(2), "Should be at last item"); } @@ -1158,7 +1159,7 @@ mod tests { // Verify queue is still intact { let queue = controller.queue(); - let queue_lock = queue.lock().unwrap(); + let queue_lock = queue.lock_safe(); assert_eq!(queue_lock.items().len(), 3, "Queue should still have 3 items after skip at end"); // When we skip past the end, the queue index should stay at the last item // or become None (depending on implementation) @@ -1187,7 +1188,7 @@ mod tests { // Verify we wrapped to the first item { let queue = controller.queue(); - let queue_lock = queue.lock().unwrap(); + let queue_lock = queue.lock_safe(); assert_eq!(queue_lock.items().len(), 3, "Queue should still have 3 items"); assert_eq!(queue_lock.current_index(), Some(0), "Should wrap to index 0"); assert_eq!(queue_lock.current().unwrap().id, "item_0", "Should be back at item_0"); @@ -1206,7 +1207,7 @@ mod tests { // Verify starting position { let queue = controller.queue(); - let queue_lock = queue.lock().unwrap(); + let queue_lock = queue.lock_safe(); assert_eq!(queue_lock.current_index(), Some(3), "Should start at index 3"); } @@ -1216,7 +1217,7 @@ mod tests { // Verify queue is intact and index moved back { let queue = controller.queue(); - let queue_lock = queue.lock().unwrap(); + let queue_lock = queue.lock_safe(); assert_eq!(queue_lock.items().len(), 5, "Queue should still have 5 items after previous"); assert_eq!(queue_lock.current_index(), Some(2), "Index should move to 2"); assert_eq!(queue_lock.current().unwrap().id, "item_2", "Current item should be item_2"); @@ -1385,7 +1386,7 @@ mod tests { // Set sleep timer to end of track { - let mut timer = controller.sleep_timer.lock().unwrap(); + let mut timer = controller.sleep_timer.lock_safe(); timer.mode = SleepTimerMode::EndOfTrack; } @@ -1400,7 +1401,7 @@ mod tests { // Verify timer was cancelled { - let timer = controller.sleep_timer.lock().unwrap(); + let timer = controller.sleep_timer.lock_safe(); assert!( matches!(timer.mode, SleepTimerMode::Off), "Sleep timer should be cancelled after EndOfTrack" diff --git a/src-tauri/src/player/mpv_backend.rs b/src-tauri/src/player/mpv_backend.rs index f31ae42..059c434 100644 --- a/src-tauri/src/player/mpv_backend.rs +++ b/src-tauri/src/player/mpv_backend.rs @@ -1,3 +1,4 @@ +use crate::utils::lock::MutexSafe; use log::{debug, error, info, warn}; use super::backend::{PlayerBackend, PlayerError}; use super::events::{PlayerEventEmitter, PlayerStatusEvent}; @@ -190,7 +191,7 @@ impl MpvBackend { libmpv::events::Event::PlaybackRestart => { debug!("[MpvBackend] Playback started/resumed"); - let media_id = state.lock().unwrap().current_media.as_ref().map(|m| m.id.clone()); + let media_id = state.lock_safe().current_media.as_ref().map(|m| m.id.clone()); if let Some(emitter) = &event_emitter { emitter.emit(PlayerStatusEvent::StateChanged { @@ -202,7 +203,7 @@ impl MpvBackend { libmpv::events::Event::PropertyChange { name, .. } if name == "pause" => { // Handle pause state changes if let Ok(is_paused) = mpv.get_property::("pause") { - let media_id = state.lock().unwrap().current_media.as_ref().map(|m| m.id.clone()); + let media_id = state.lock_safe().current_media.as_ref().map(|m| m.id.clone()); if let Some(emitter) = &event_emitter { emitter.emit(PlayerStatusEvent::StateChanged { @@ -308,7 +309,7 @@ impl MpvBackend { if !is_paused { // Throttled progress reporting (every 30s) let jellyfin_id = { - let state = state_for_position.lock().unwrap(); + let state = state_for_position.lock_safe(); state.current_media.as_ref() .and_then(|m| m.jellyfin_id().map(|s| s.to_string())) }; @@ -376,7 +377,7 @@ impl PlayerBackend for MpvBackend { // Update state { - let mut state = self.state.lock().unwrap(); + let mut state = self.state.lock_safe(); state.current_media = Some(media.clone()); } @@ -422,7 +423,7 @@ impl PlayerBackend for MpvBackend { message: format!("Failed to stop: {:?}", e), })?; - let mut state = self.state.lock().unwrap(); + let mut state = self.state.lock_safe(); state.current_media = None; Ok(()) @@ -460,7 +461,7 @@ impl PlayerBackend for MpvBackend { message: format!("Failed to set volume: {:?}", e), })?; - let mut state = self.state.lock().unwrap(); + let mut state = self.state.lock_safe(); state.volume = clamped; Ok(()) @@ -480,7 +481,7 @@ impl PlayerBackend for MpvBackend { } fn state(&self) -> PlayerState { - let state = self.state.lock().unwrap(); + let state = self.state.lock_safe(); if let Some(ref media) = state.current_media { let is_paused = self.mpv.get_property::("pause").unwrap_or(true); @@ -506,7 +507,7 @@ impl PlayerBackend for MpvBackend { } fn volume(&self) -> f32 { - let state = self.state.lock().unwrap(); + let state = self.state.lock_safe(); state.volume } diff --git a/src-tauri/src/repository/hybrid.rs b/src-tauri/src/repository/hybrid.rs index 159c2f3..508ee1d 100644 --- a/src-tauri/src/repository/hybrid.rs +++ b/src-tauri/src/repository/hybrid.rs @@ -5,6 +5,8 @@ // @req: DR-012 - Local database for media metadata cache // @req: DR-013 - Repository pattern for online/offline data access +#[cfg(test)] +use crate::utils::lock::MutexSafe; use std::sync::Arc; use async_trait::async_trait; @@ -546,16 +548,16 @@ mod tests { } fn get_query_count(&self) -> usize { - *self.query_count.lock().unwrap() + *self.query_count.lock_safe() } fn get_save_count(&self) -> usize { - *self.save_count.lock().unwrap() + *self.save_count.lock_safe() } async fn save_to_cache(&self, _parent_id: &str, items: &[MediaItem]) -> Result { - *self.save_count.lock().unwrap() += 1; - *self.items.lock().unwrap() = items.to_vec(); + *self.save_count.lock_safe() += 1; + *self.items.lock_safe() = items.to_vec(); Ok(items.len()) } } @@ -567,8 +569,8 @@ mod tests { } async fn get_items(&self, _parent_id: &str, _options: Option) -> Result { - *self.query_count.lock().unwrap() += 1; - let items = self.items.lock().unwrap().clone(); + *self.query_count.lock_safe() += 1; + let items = self.items.lock_safe().clone(); let count = items.len(); Ok(SearchResult { items, @@ -715,7 +717,7 @@ mod tests { } fn get_query_count(&self) -> usize { - *self.query_count.lock().unwrap() + *self.query_count.lock_safe() } } @@ -726,7 +728,7 @@ mod tests { } async fn get_items(&self, _parent_id: &str, _options: Option) -> Result { - *self.query_count.lock().unwrap() += 1; + *self.query_count.lock_safe() += 1; Ok(SearchResult { items: self.items.clone(), total_record_count: self.items.len(), diff --git a/src-tauri/src/session_poller/mod.rs b/src-tauri/src/session_poller/mod.rs index cd1597b..4ebdf2b 100644 --- a/src-tauri/src/session_poller/mod.rs +++ b/src-tauri/src/session_poller/mod.rs @@ -5,6 +5,7 @@ //! //! TRACES: UR-010 | JA-021 +use crate::utils::lock::{MutexSafe, RwLockSafe}; use log::{debug, info, warn}; use std::sync::{Arc, Mutex, RwLock}; use std::sync::atomic::{AtomicBool, AtomicU64, Ordering}; @@ -60,7 +61,7 @@ impl SessionPollerManager { /// Set event emitter for broadcasting session updates pub fn set_event_emitter(&self, emitter: Arc) { - *self.event_emitter.lock().unwrap() = Some(emitter); + *self.event_emitter.lock_safe() = Some(emitter); } /// Start the background polling thread @@ -88,7 +89,7 @@ impl SessionPollerManager { // Calculate poll interval based on mode and hint let new_interval = Self::calculate_interval( &mode_manager.get_mode(), - *hint.read().unwrap(), + *hint.read_safe(), ); interval_ms.store(new_interval, Ordering::Relaxed); @@ -97,7 +98,7 @@ impl SessionPollerManager { // Fetch sessions let sessions_result = rt.block_on(async { - let client_opt = client.lock().unwrap().clone(); + let client_opt = client.lock_safe().clone(); match client_opt { Some(c) => c.get_sessions().await, None => { @@ -111,7 +112,7 @@ impl SessionPollerManager { match sessions_result { Ok(sessions) => { debug!("[SessionPoller] Fetched {} sessions", sessions.len()); - if let Some(em) = emitter.lock().unwrap().as_ref() { + if let Some(em) = emitter.lock_safe().as_ref() { em.emit(crate::player::PlayerStatusEvent::SessionsUpdated { sessions, }); @@ -129,7 +130,7 @@ impl SessionPollerManager { info!("[SessionPoller] Polling thread stopped"); }); - *self.thread_handle.lock().unwrap() = Some(handle); + *self.thread_handle.lock_safe() = Some(handle); } /// Stop the polling thread @@ -138,7 +139,7 @@ impl SessionPollerManager { self.is_running.store(false, Ordering::Relaxed); // Join the thread if possible (don't block indefinitely) - if let Some(handle) = self.thread_handle.lock().unwrap().take() { + if let Some(handle) = self.thread_handle.lock_safe().take() { let _ = handle.join(); } } @@ -146,7 +147,7 @@ impl SessionPollerManager { /// Set UI hint for polling frequency adjustment pub fn set_polling_hint(&self, hint: PollingHint) { debug!("[SessionPoller] Setting polling hint: {:?}", hint); - *self.current_hint.write().unwrap() = hint; + *self.current_hint.write_safe() = hint; } /// Calculate polling interval based on mode and hint @@ -165,7 +166,7 @@ impl SessionPollerManager { /// Manually trigger a poll (for frontend refresh button) pub async fn poll_now(&self) -> Result, String> { - let client = self.jellyfin_client.lock().unwrap().clone() + let client = self.jellyfin_client.lock_safe().clone() .ok_or("Jellyfin client not configured")?; client.get_sessions().await diff --git a/src-tauri/src/storage/mod.rs b/src-tauri/src/storage/mod.rs index a6ca4d2..8cb5589 100644 --- a/src-tauri/src/storage/mod.rs +++ b/src-tauri/src/storage/mod.rs @@ -7,6 +7,7 @@ pub mod db_service; pub mod models; pub mod schema; +use crate::utils::lock::MutexSafe; use std::path::PathBuf; use std::sync::{Arc, Mutex}; @@ -77,7 +78,7 @@ impl Database { /// Run all pending migrations pub fn migrate(&self) -> SqliteResult<()> { info!("Starting database migrations..."); - let conn = self.conn.lock().unwrap(); + let conn = self.conn.lock_safe(); // Create migrations table if it doesn't exist debug!("Creating _migrations table if it doesn't exist..."); @@ -172,7 +173,7 @@ mod tests { fn test_migrations_run() { let db = Database::open_in_memory().unwrap(); let conn = db.connection(); - let conn = conn.lock().unwrap(); + let conn = conn.lock_safe(); // Check that tables exist let mut stmt = conn @@ -186,7 +187,7 @@ mod tests { fn test_all_tables_created() { let db = Database::open_in_memory().unwrap(); let conn = db.connection(); - let conn = conn.lock().unwrap(); + let conn = conn.lock_safe(); let expected_tables = [ "servers", @@ -218,7 +219,7 @@ mod tests { fn test_fts_table_created() { let db = Database::open_in_memory().unwrap(); let conn = db.connection(); - let conn = conn.lock().unwrap(); + let conn = conn.lock_safe(); let exists: Option = conn .query_row( @@ -234,7 +235,7 @@ mod tests { fn test_server_crud() { let db = Database::open_in_memory().unwrap(); let conn = db.connection(); - let conn = conn.lock().unwrap(); + let conn = conn.lock_safe(); // Insert a server conn.execute( @@ -282,7 +283,7 @@ mod tests { fn test_user_crud() { let db = Database::open_in_memory().unwrap(); let conn = db.connection(); - let conn = conn.lock().unwrap(); + let conn = conn.lock_safe(); // Create a server first (foreign key) conn.execute( @@ -329,7 +330,7 @@ mod tests { fn test_cascade_delete_server_removes_users() { let db = Database::open_in_memory().unwrap(); let conn = db.connection(); - let conn = conn.lock().unwrap(); + let conn = conn.lock_safe(); // Create server and user conn.execute( @@ -367,7 +368,7 @@ mod tests { fn test_item_insert_and_fts_search() { let db = Database::open_in_memory().unwrap(); let conn = db.connection(); - let conn = conn.lock().unwrap(); + let conn = conn.lock_safe(); // Create server first conn.execute( @@ -424,7 +425,7 @@ mod tests { fn test_user_data_playback_position() { let db = Database::open_in_memory().unwrap(); let conn = db.connection(); - let conn = conn.lock().unwrap(); + let conn = conn.lock_safe(); // Setup: server, user, item conn.execute( @@ -487,7 +488,7 @@ mod tests { fn test_sync_queue_operations() { let db = Database::open_in_memory().unwrap(); let conn = db.connection(); - let conn = conn.lock().unwrap(); + let conn = conn.lock_safe(); // Setup conn.execute( @@ -552,7 +553,7 @@ mod tests { fn test_downloads_table() { let db = Database::open_in_memory().unwrap(); let conn = db.connection(); - let conn = conn.lock().unwrap(); + let conn = conn.lock_safe(); // Setup conn.execute( @@ -627,7 +628,7 @@ mod tests { // Tables should still exist let conn = db.connection(); - let conn = conn.lock().unwrap(); + let conn = conn.lock_safe(); let count: i32 = conn .query_row( @@ -643,7 +644,7 @@ mod tests { fn test_global_active_user_deactivation() { let db = Database::open_in_memory().unwrap(); let conn = db.connection(); - let conn = conn.lock().unwrap(); + let conn = conn.lock_safe(); // Create two servers conn.execute( @@ -713,7 +714,7 @@ mod tests { fn test_active_session_query_ordering() { let db = Database::open_in_memory().unwrap(); let conn = db.connection(); - let conn = conn.lock().unwrap(); + let conn = conn.lock_safe(); // Create server conn.execute( diff --git a/src-tauri/src/utils/lock.rs b/src-tauri/src/utils/lock.rs new file mode 100644 index 0000000..3816a2c --- /dev/null +++ b/src-tauri/src/utils/lock.rs @@ -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 { + /// Lock the mutex, recovering the inner guard if the lock was poisoned. + fn lock_safe(&self) -> MutexGuard<'_, T>; +} + +impl MutexSafe for Mutex { + fn lock_safe(&self) -> MutexGuard<'_, T> { + self.lock().unwrap_or_else(PoisonError::into_inner) + } +} + +/// Poison-tolerant locking for [`std::sync::RwLock`]. +pub trait RwLockSafe { + /// 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 RwLockSafe for RwLock { + 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(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); + } +} diff --git a/src-tauri/src/utils/mod.rs b/src-tauri/src/utils/mod.rs index 4974d5c..04c1b38 100644 --- a/src-tauri/src/utils/mod.rs +++ b/src-tauri/src/utils/mod.rs @@ -1 +1,2 @@ pub mod conversions; +pub mod lock;