From 99722aedaf2edcf0408d9ed159447013f55f10b8 Mon Sep 17 00:00:00 2001 From: vicrodh <103399283+vicrodh@users.noreply.github.com> Date: Tue, 18 Aug 2026 12:40:52 -0600 Subject: [PATCH] feat: an unreachable track can no longer stop playback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two owner requirements, one shape: something that cannot play must be skipped, and cleaned when there is nowhere left to skip to. The causes are outside our control — Qobuz withdraws a track overnight, or a local/Plex file sits on a share this network cannot see today. THE FUNNEL, which is where the real gap was. `play_current_if_local` returned a bool whose two `false`s meant opposite things: "not my row, run the Qobuz path" and "my row, and it cannot play". The caller could only act on the first, so an unreachable local file fell through to the Qobuz tier-walk, failed there with an unrelated error, and `auto_skip_unavailable` then declined it as non-terminal. One dead file stopped playback silently. It is now a three-state `LocalPlay`, and a local failure takes the SAME bounded walk a pulled Qobuz track takes — to the user they are the same event. CLEANING. Both dead ends of that walk (budget spent, queue edge) now call `purge_unavailable_upcoming`, which drops only rows `queue_track_unavailable` already refuses to play, walking backwards so a removal cannot shift an index still to be visited. A track on an unreachable share can never land there: it is Unreachable, not dead. PROBES THAT CANNOT HANG (`qbz-library/src/reachability.rs`, 4 tests). The machinery for network shares already existed — mount_info, `library_folders.is_network`, a populated `local_tracks.is_network_mount`, even an unreachable-folder guard in scan.rs. Every one of them probed with a bare blocking syscall, and a mounted-but-unreachable share does not make `exists()`/`read_dir()` fail, it makes them BLOCK for the mount's timeout. The guards could not fire; scan.rs hung on the very condition it existed to detect. Three states, not two, and that distinction is the whole safety property: `Missing` is the filesystem saying the file is gone and may be cleaned; `Unreachable` is it saying nothing, and may only ever be skipped. Collapsing them — which a bare `exists()` does — would delete a user's NAS library because they changed networks. The deadline goes on the PROBE only, never the transfer: a hi-res FLAC over a working-but-slow share legitimately takes longer than any probe budget, and timing the read out would turn "your network is slow" into "this track is gone". A blocked syscall cannot be cancelled, so a timed-out probe leaks its thread; the mount is therefore remembered with a cool-down, one probe per dead mount rather than one per track, which on a 2000-row queue is the difference between a defence and a thread bomb. PERSISTENCE. `busy_timeout=1000` on all 18 store pragmas. Deliberately NOT the 2-3s discussed: these writes run on the Qt thread, so a 3s wait would install exactly the stall this whole sweep has been removing, while today's 0 means contention loses a setting silently and instantly. One second resolves transient contention without a visible stall. `home_bridge::ui` announces both of its silent failure modes. Three builds went into proving it was NOT dropping the Home document during the launch freeze; one warn line would have answered it in one. Verified against the owner's real session state (1935-row queue): the app launches to a painted Discover with no QML errors and no dropped publishes. cc --- .../src/settings/album_play_history.rs | 2 +- .../qbz-app/src/settings/artist_blacklist.rs | 2 +- crates/qbz-app/src/settings/bundle.rs | 2 +- crates/qbz-app/src/settings/developer.rs | 2 +- crates/qbz-app/src/settings/discover_prefs.rs | 2 +- crates/qbz-app/src/settings/favorites.rs | 2 +- .../qbz-app/src/settings/favorites_cache.rs | 2 +- crates/qbz-app/src/settings/graphics.rs | 2 +- .../qbz-app/src/settings/local_favorites.rs | 2 +- crates/qbz-app/src/settings/pinned_items.rs | 2 +- crates/qbz-app/src/settings/playback.rs | 2 +- crates/qbz-app/src/settings/plex.rs | 2 +- crates/qbz-app/src/settings/reco_store.rs | 2 +- crates/qbz-app/src/settings/remote_control.rs | 4 +- crates/qbz-app/src/settings/scrobblers.rs | 2 +- crates/qbz-app/src/settings/subscription.rs | 2 +- crates/qbz-app/src/settings/tray.rs | 2 +- crates/qbz-library/src/lib.rs | 2 + crates/qbz-library/src/reachability.rs | 205 ++++++++++++++++++ crates/qbz-library/src/scan.rs | 24 +- crates/qbz-qt/src/home_bridge.rs | 22 +- crates/qbz-qt/src/local_library_qt.rs | 2 +- crates/qbz-qt/src/local_playback.rs | 90 ++++++-- crates/qbz-qt/src/playback_qt.rs | 33 ++- crates/qbz-qt/src/queue_qt.rs | 39 ++++ 25 files changed, 408 insertions(+), 45 deletions(-) create mode 100644 crates/qbz-library/src/reachability.rs diff --git a/crates/qbz-app/src/settings/album_play_history.rs b/crates/qbz-app/src/settings/album_play_history.rs index 28ce4cf2a..c9b444ad7 100644 --- a/crates/qbz-app/src/settings/album_play_history.rs +++ b/crates/qbz-app/src/settings/album_play_history.rs @@ -71,7 +71,7 @@ fn open_db() -> Option { } }; // ADR-002: WAL for any SQLite store touched off the UI thread. - if let Err(e) = conn.execute_batch("PRAGMA journal_mode=WAL; PRAGMA synchronous=NORMAL;") { + if let Err(e) = conn.execute_batch("PRAGMA journal_mode=WAL; PRAGMA synchronous=NORMAL; PRAGMA busy_timeout=1000;") { log::warn!("[qbz-slint] album_play_history pragma failed: {e}"); } if let Err(e) = init_schema(&conn) { diff --git a/crates/qbz-app/src/settings/artist_blacklist.rs b/crates/qbz-app/src/settings/artist_blacklist.rs index cd54d824c..178a93f04 100644 --- a/crates/qbz-app/src/settings/artist_blacklist.rs +++ b/crates/qbz-app/src/settings/artist_blacklist.rs @@ -84,7 +84,7 @@ impl BlacklistService { .map_err(|e| format!("Failed to open blacklist database: {}", e))?; // Enable WAL mode for better concurrent access (ADR-002). - conn.execute_batch("PRAGMA journal_mode=WAL; PRAGMA synchronous=NORMAL;") + conn.execute_batch("PRAGMA journal_mode=WAL; PRAGMA synchronous=NORMAL; PRAGMA busy_timeout=1000;") .map_err(|e| format!("Failed to set WAL mode: {}", e))?; let service = Self { diff --git a/crates/qbz-app/src/settings/bundle.rs b/crates/qbz-app/src/settings/bundle.rs index d461e44d8..7603299b8 100644 --- a/crates/qbz-app/src/settings/bundle.rs +++ b/crates/qbz-app/src/settings/bundle.rs @@ -1425,7 +1425,7 @@ fn qconnect_kv_read(db: &Path, key: &str) -> Option { fn qconnect_kv_write(db: &Path, key: &str, value: Option<&str>) -> Result<(), String> { let conn = rusqlite::Connection::open(db).map_err(|e| e.to_string())?; - conn.execute_batch("PRAGMA journal_mode=WAL; PRAGMA synchronous=NORMAL;") + conn.execute_batch("PRAGMA journal_mode=WAL; PRAGMA synchronous=NORMAL; PRAGMA busy_timeout=1000;") .map_err(|e| e.to_string())?; conn.execute_batch( "CREATE TABLE IF NOT EXISTS settings (key TEXT PRIMARY KEY, value TEXT NOT NULL)", diff --git a/crates/qbz-app/src/settings/developer.rs b/crates/qbz-app/src/settings/developer.rs index f7f4bd9fb..3f0c2149d 100644 --- a/crates/qbz-app/src/settings/developer.rs +++ b/crates/qbz-app/src/settings/developer.rs @@ -63,7 +63,7 @@ impl DeveloperSettingsStore { let conn = Connection::open(&db_path) .map_err(|e| format!("Failed to open developer settings database: {}", e))?; - conn.execute_batch("PRAGMA journal_mode=WAL; PRAGMA synchronous=NORMAL;") + conn.execute_batch("PRAGMA journal_mode=WAL; PRAGMA synchronous=NORMAL; PRAGMA busy_timeout=1000;") .map_err(|e| { format!( "Failed to enable WAL for developer settings database: {}", diff --git a/crates/qbz-app/src/settings/discover_prefs.rs b/crates/qbz-app/src/settings/discover_prefs.rs index a094834cb..8d06d4b9b 100644 --- a/crates/qbz-app/src/settings/discover_prefs.rs +++ b/crates/qbz-app/src/settings/discover_prefs.rs @@ -431,7 +431,7 @@ impl DiscoverPrefsStore { let conn = Connection::open(&db_path) .map_err(|e| format!("Failed to open discover prefs database: {}", e))?; - conn.execute_batch("PRAGMA journal_mode=WAL; PRAGMA synchronous=NORMAL;") + conn.execute_batch("PRAGMA journal_mode=WAL; PRAGMA synchronous=NORMAL; PRAGMA busy_timeout=1000;") .map_err(|e| format!("Failed to enable WAL for discover prefs database: {}", e))?; conn.execute_batch( diff --git a/crates/qbz-app/src/settings/favorites.rs b/crates/qbz-app/src/settings/favorites.rs index 48b09757e..66bdb3a4e 100644 --- a/crates/qbz-app/src/settings/favorites.rs +++ b/crates/qbz-app/src/settings/favorites.rs @@ -42,7 +42,7 @@ impl FavoritesPreferencesStore { let db_path = dir.join(db_name); let conn = Connection::open(&db_path) .map_err(|e| format!("Failed to open favorites preferences database: {}", e))?; - conn.execute_batch("PRAGMA journal_mode=WAL; PRAGMA synchronous=NORMAL;") + conn.execute_batch("PRAGMA journal_mode=WAL; PRAGMA synchronous=NORMAL; PRAGMA busy_timeout=1000;") .map_err(|e| format!("Failed to set WAL mode on favorites preferences: {}", e))?; conn.execute( diff --git a/crates/qbz-app/src/settings/favorites_cache.rs b/crates/qbz-app/src/settings/favorites_cache.rs index c4c127bb6..143457343 100644 --- a/crates/qbz-app/src/settings/favorites_cache.rs +++ b/crates/qbz-app/src/settings/favorites_cache.rs @@ -25,7 +25,7 @@ impl FavoritesCacheStore { let conn = Connection::open(&db_path) .map_err(|e| format!("Failed to open favorites cache database: {}", e))?; - conn.execute_batch("PRAGMA journal_mode=WAL; PRAGMA synchronous=NORMAL;") + conn.execute_batch("PRAGMA journal_mode=WAL; PRAGMA synchronous=NORMAL; PRAGMA busy_timeout=1000;") .map_err(|e| format!("Failed to enable WAL for favorites cache database: {}", e))?; // Create tables diff --git a/crates/qbz-app/src/settings/graphics.rs b/crates/qbz-app/src/settings/graphics.rs index 5df9261cc..95aae3ef6 100644 --- a/crates/qbz-app/src/settings/graphics.rs +++ b/crates/qbz-app/src/settings/graphics.rs @@ -86,7 +86,7 @@ impl GraphicsSettingsStore { let conn = Connection::open(&db_path) .map_err(|e| format!("Failed to open graphics settings database: {}", e))?; - conn.execute_batch("PRAGMA journal_mode=WAL; PRAGMA synchronous=NORMAL;") + conn.execute_batch("PRAGMA journal_mode=WAL; PRAGMA synchronous=NORMAL; PRAGMA busy_timeout=1000;") .map_err(|e| format!("Failed to enable WAL for graphics settings database: {}", e))?; conn.execute_batch( diff --git a/crates/qbz-app/src/settings/local_favorites.rs b/crates/qbz-app/src/settings/local_favorites.rs index e26e40bdb..4aac4eb4c 100644 --- a/crates/qbz-app/src/settings/local_favorites.rs +++ b/crates/qbz-app/src/settings/local_favorites.rs @@ -56,7 +56,7 @@ impl LocalFavoritesService { let conn = Connection::open(db_path) .map_err(|e| format!("Failed to open local favorites database: {}", e))?; - conn.execute_batch("PRAGMA journal_mode=WAL; PRAGMA synchronous=NORMAL;") + conn.execute_batch("PRAGMA journal_mode=WAL; PRAGMA synchronous=NORMAL; PRAGMA busy_timeout=1000;") .map_err(|e| format!("Failed to set WAL mode: {}", e))?; let service = Self { diff --git a/crates/qbz-app/src/settings/pinned_items.rs b/crates/qbz-app/src/settings/pinned_items.rs index 77ecf7d99..ee84a9d83 100644 --- a/crates/qbz-app/src/settings/pinned_items.rs +++ b/crates/qbz-app/src/settings/pinned_items.rs @@ -53,7 +53,7 @@ impl PinnedItemsService { .map_err(|e| format!("Failed to open pinned items database: {}", e))?; // Enable WAL mode for better concurrent access (ADR-002). - conn.execute_batch("PRAGMA journal_mode=WAL; PRAGMA synchronous=NORMAL;") + conn.execute_batch("PRAGMA journal_mode=WAL; PRAGMA synchronous=NORMAL; PRAGMA busy_timeout=1000;") .map_err(|e| format!("Failed to set WAL mode: {}", e))?; let service = Self { diff --git a/crates/qbz-app/src/settings/playback.rs b/crates/qbz-app/src/settings/playback.rs index 4c2de8915..413a206e3 100644 --- a/crates/qbz-app/src/settings/playback.rs +++ b/crates/qbz-app/src/settings/playback.rs @@ -85,7 +85,7 @@ impl PlaybackPreferencesStore { let conn = Connection::open(&db_path) .map_err(|e| format!("Failed to open playback preferences database: {}", e))?; - conn.execute_batch("PRAGMA journal_mode=WAL; PRAGMA synchronous=NORMAL;") + conn.execute_batch("PRAGMA journal_mode=WAL; PRAGMA synchronous=NORMAL; PRAGMA busy_timeout=1000;") .map_err(|e| { format!( "Failed to enable WAL for playback preferences database: {}", diff --git a/crates/qbz-app/src/settings/plex.rs b/crates/qbz-app/src/settings/plex.rs index 1070c1af2..58db60411 100644 --- a/crates/qbz-app/src/settings/plex.rs +++ b/crates/qbz-app/src/settings/plex.rs @@ -79,7 +79,7 @@ impl PlexSettingsStore { let conn = Connection::open(&db_path) .map_err(|e| format!("Failed to open Plex settings database: {}", e))?; - conn.execute_batch("PRAGMA journal_mode=WAL; PRAGMA synchronous=NORMAL;") + conn.execute_batch("PRAGMA journal_mode=WAL; PRAGMA synchronous=NORMAL; PRAGMA busy_timeout=1000;") .map_err(|e| format!("Failed to enable WAL for Plex settings database: {}", e))?; // Base (slice 2a) schema. New columns are added by idempotent ALTERs diff --git a/crates/qbz-app/src/settings/reco_store.rs b/crates/qbz-app/src/settings/reco_store.rs index 8cbd4880a..a95f5ba5b 100644 --- a/crates/qbz-app/src/settings/reco_store.rs +++ b/crates/qbz-app/src/settings/reco_store.rs @@ -196,7 +196,7 @@ impl RecoStore { let conn = Connection::open(&db_path) .map_err(|e| format!("Failed to open reco database: {}", e))?; - conn.execute_batch("PRAGMA journal_mode=WAL; PRAGMA synchronous=NORMAL;") + conn.execute_batch("PRAGMA journal_mode=WAL; PRAGMA synchronous=NORMAL; PRAGMA busy_timeout=1000;") .map_err(|e| format!("Failed to enable WAL for reco database: {}", e))?; let store = Self { conn }; diff --git a/crates/qbz-app/src/settings/remote_control.rs b/crates/qbz-app/src/settings/remote_control.rs index 86eaee898..c4a4c1ba4 100644 --- a/crates/qbz-app/src/settings/remote_control.rs +++ b/crates/qbz-app/src/settings/remote_control.rs @@ -49,7 +49,7 @@ impl RemoteControlSettingsStore { let conn = Connection::open(&db_path) .map_err(|e| format!("Failed to open remote control settings database: {}", e))?; - conn.execute_batch("PRAGMA journal_mode=WAL; PRAGMA synchronous=NORMAL;") + conn.execute_batch("PRAGMA journal_mode=WAL; PRAGMA synchronous=NORMAL; PRAGMA busy_timeout=1000;") .map_err(|e| { format!( "Failed to enable WAL for remote control settings database: {}", @@ -305,7 +305,7 @@ impl AllowedOriginsStore { let conn = Connection::open(&db_path) .map_err(|e| format!("Failed to open allowed origins database: {}", e))?; - conn.execute_batch("PRAGMA journal_mode=WAL; PRAGMA synchronous=NORMAL;") + conn.execute_batch("PRAGMA journal_mode=WAL; PRAGMA synchronous=NORMAL; PRAGMA busy_timeout=1000;") .map_err(|e| format!("Failed to enable WAL for allowed origins database: {}", e))?; conn.execute_batch( diff --git a/crates/qbz-app/src/settings/scrobblers.rs b/crates/qbz-app/src/settings/scrobblers.rs index 0e0afec14..712f83d09 100644 --- a/crates/qbz-app/src/settings/scrobblers.rs +++ b/crates/qbz-app/src/settings/scrobblers.rs @@ -93,7 +93,7 @@ impl ScrobblerSettingsStore { let conn = Connection::open(&db_path) .map_err(|e| format!("Failed to open scrobbler settings database: {}", e))?; - conn.execute_batch("PRAGMA journal_mode=WAL; PRAGMA synchronous=NORMAL;") + conn.execute_batch("PRAGMA journal_mode=WAL; PRAGMA synchronous=NORMAL; PRAGMA busy_timeout=1000;") .map_err(|e| format!("Failed to enable WAL for scrobbler settings database: {}", e))?; conn.execute_batch( diff --git a/crates/qbz-app/src/settings/subscription.rs b/crates/qbz-app/src/settings/subscription.rs index 722a61eab..dd1fd4804 100644 --- a/crates/qbz-app/src/settings/subscription.rs +++ b/crates/qbz-app/src/settings/subscription.rs @@ -55,7 +55,7 @@ impl SubscriptionStateStore { let conn = Connection::open(&db_path) .map_err(|e| format!("Failed to open subscription state database: {}", e))?; - conn.execute_batch("PRAGMA journal_mode=WAL; PRAGMA synchronous=NORMAL;") + conn.execute_batch("PRAGMA journal_mode=WAL; PRAGMA synchronous=NORMAL; PRAGMA busy_timeout=1000;") .map_err(|e| { format!( "Failed to enable WAL for subscription state database: {}", diff --git a/crates/qbz-app/src/settings/tray.rs b/crates/qbz-app/src/settings/tray.rs index 64e491b16..e4efa79d2 100644 --- a/crates/qbz-app/src/settings/tray.rs +++ b/crates/qbz-app/src/settings/tray.rs @@ -78,7 +78,7 @@ impl TraySettingsStore { let conn = Connection::open(&db_path) .map_err(|e| format!("Failed to open tray settings database: {}", e))?; - conn.execute_batch("PRAGMA journal_mode=WAL; PRAGMA synchronous=NORMAL;") + conn.execute_batch("PRAGMA journal_mode=WAL; PRAGMA synchronous=NORMAL; PRAGMA busy_timeout=1000;") .map_err(|e| format!("Failed to enable WAL for tray settings database: {}", e))?; conn.execute_batch( diff --git a/crates/qbz-library/src/lib.rs b/crates/qbz-library/src/lib.rs index 58f2355d0..85d05f90a 100644 --- a/crates/qbz-library/src/lib.rs +++ b/crates/qbz-library/src/lib.rs @@ -38,6 +38,7 @@ mod errors; mod metadata; mod models; mod mount_info; +pub mod reachability; mod scan; mod scanner; mod tag_writer; @@ -54,6 +55,7 @@ pub use errors::LibraryError; pub use metadata::MetadataExtractor; pub use models::*; pub use mount_info::{is_network_path, network_fs_label}; +pub use reachability::{probe, probe_default, Reach, DEFAULT_PROBE}; pub use scan::{scan_with_progress, ScanEvent}; pub use tag_writer::{ compute_track_artist_match, write_album_tags_to_files, write_purchase_tags, AlbumTagWrite, diff --git a/crates/qbz-library/src/reachability.rs b/crates/qbz-library/src/reachability.rs new file mode 100644 index 000000000..571a5e6d4 --- /dev/null +++ b/crates/qbz-library/src/reachability.rs @@ -0,0 +1,205 @@ +//! Reachability probes that cannot hang the caller. +//! +//! # Why this exists +//! +//! A network share that is MOUNTED BUT UNREACHABLE — the user is on a +//! different network today, the NAS is off, the VPN dropped — does not make +//! filesystem calls fail. It makes them **block**, for however long the mount +//! was configured to wait (an NFS hard mount waits forever). `Path::exists()`, +//! `read_dir()` and even `canonicalize()` are all syscalls, and all of them sit +//! there. +//! +//! That matters because the codebase already tries to defend against exactly +//! this case and cannot, since every guard probes with a bare blocking call: +//! +//! * `scan.rs` decides a folder is unreachable with `read_dir(..).is_err()` — +//! on a hung mount that never returns an error, it never returns at all; +//! * `mount_info::is_network_path()` canonicalizes the path it is classifying, +//! so the network detector hangs on the very thing it exists to detect; +//! * the local playback path awaits an `exists()` with no deadline, so +//! playing one dead file never resolves. +//! +//! # The two rules this module encodes +//! +//! **1. Deadline the PROBE, never the transfer.** Reading a hi-res FLAC over a +//! working-but-slow share legitimately takes many seconds; a blanket timeout +//! around the whole operation turns "your network is slow" into "this track +//! does not exist", which is worse than the bug. Only the question *does this +//! answer at all?* gets a deadline. +//! +//! **2. Probe a MOUNT once, not a file each time.** A blocking syscall cannot +//! be cancelled: when the deadline expires the helper thread is still stuck in +//! the kernel, and it stays stuck until the mount gives up. One leaked thread +//! per dead mount per cool-down is the price of asking at all. One per TRACK +//! would be a thread bomb on a 2000-row queue, which is how a defence turns +//! into the outage it was meant to prevent. + +use std::collections::HashMap; +use std::path::{Path, PathBuf}; +use std::sync::{Mutex, OnceLock}; +use std::time::{Duration, Instant}; + +/// How long a mount stays marked unreachable before anyone probes it again. +/// Long enough that a dead NAS is asked about rarely; short enough that +/// reconnecting to the right network recovers without a restart. +const COOLDOWN: Duration = Duration::from_secs(30); + +/// The default probe deadline. The owner's spec: a couple of seconds, then +/// stop waiting and treat it as gone. +pub const DEFAULT_PROBE: Duration = Duration::from_secs(3); + +/// What a probe learned about a path. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Reach { + /// The filesystem answered and the file is there. + Present, + /// The filesystem answered and the file is not there. This is a REAL + /// answer — the caller may delete/clean on it. + Missing, + /// The filesystem did not answer inside the deadline. NOT the same as + /// missing: the file may be perfectly fine on a share we cannot see from + /// this network, so the caller should skip it, never clean it. + Unreachable, +} + +impl Reach { + /// Can playback proceed? + pub fn is_playable(self) -> bool { + self == Reach::Present + } +} + +fn unreachable_until() -> &'static Mutex> { + static MAP: OnceLock>> = OnceLock::new(); + MAP.get_or_init(|| Mutex::new(HashMap::new())) +} + +/// The key a path is remembered under: its first two components, which is a +/// good enough stand-in for "the mount" without calling into the filesystem to +/// find the real mount point (that lookup is itself a syscall on the path we +/// are trying not to touch). +fn mount_key(path: &Path) -> PathBuf { + path.components().take(3).collect() +} + +/// True when this path's mount answered too slowly recently and its cool-down +/// has not elapsed. Costs no syscall — that is the whole point. +pub fn is_cooling_down(path: &Path) -> bool { + let key = mount_key(path); + let mut map = match unreachable_until().lock() { + Ok(m) => m, + Err(_) => return false, + }; + match map.get(&key) { + Some(until) if Instant::now() < *until => true, + Some(_) => { + map.remove(&key); + false + } + None => false, + } +} + +fn mark_unreachable(path: &Path) { + if let Ok(mut map) = unreachable_until().lock() { + map.insert(mount_key(path), Instant::now() + COOLDOWN); + } +} + +/// Forget every cool-down. Call when connectivity changes — the user joined +/// the right network and should not wait out a timer for it. +pub fn reset_cooldowns() { + if let Ok(mut map) = unreachable_until().lock() { + map.clear(); + } +} + +/// Does `path` exist, answered within `deadline`? +/// +/// Blocking, but bounded: safe to call from `spawn_blocking` or a plain +/// thread. The probe itself runs on its own thread precisely because it may +/// never return; this function stops WAITING for it, which is all anyone can +/// do about a syscall stuck in the kernel. +pub fn probe(path: &Path, deadline: Duration) -> Reach { + if is_cooling_down(path) { + return Reach::Unreachable; + } + let (tx, rx) = std::sync::mpsc::channel(); + let probe_path = path.to_path_buf(); + // Detached on purpose: if it is wedged, joining it would re-introduce the + // hang this function exists to remove. + std::thread::Builder::new() + .name("qbz-reach-probe".to_string()) + .spawn(move || { + // `try_exists` rather than `exists`: it distinguishes "answered, + // not there" from "could not answer", which is exactly the + // distinction the caller needs to decide clean-vs-skip. + let _ = tx.send(probe_path.try_exists().unwrap_or(false)); + }) + .ok(); + + match rx.recv_timeout(deadline) { + Ok(true) => Reach::Present, + Ok(false) => Reach::Missing, + Err(_) => { + log::warn!( + "[reach] {} did not answer in {:?} — treating the mount as unreachable for {:?}", + path.display(), + deadline, + COOLDOWN + ); + mark_unreachable(path); + Reach::Unreachable + } + } +} + +/// [`probe`] with the default deadline. +pub fn probe_default(path: &Path) -> Reach { + probe(path, DEFAULT_PROBE) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn a_real_file_is_present() { + let f = std::env::temp_dir().join("qbz-reach-present"); + std::fs::write(&f, b"x").unwrap(); + assert_eq!(probe_default(&f), Reach::Present); + let _ = std::fs::remove_file(&f); + } + + #[test] + fn an_absent_file_is_missing_not_unreachable() { + // The distinction the caller acts on: Missing may be cleaned, + // Unreachable may only be skipped. + let f = std::env::temp_dir().join("qbz-reach-no-such-file-9e1f"); + assert_eq!(probe_default(&f), Reach::Missing); + } + + #[test] + fn a_cooling_mount_answers_without_touching_the_filesystem() { + let p = PathBuf::from("/mnt/qbz-test-dead/album/track.flac"); + mark_unreachable(&p); + assert!(is_cooling_down(&p)); + // A sibling under the SAME mount is covered by the one probe. + assert!(is_cooling_down(Path::new("/mnt/qbz-test-dead/other/x.flac"))); + assert_eq!(probe_default(&p), Reach::Unreachable); + reset_cooldowns(); + assert!(!is_cooling_down(&p)); + } + + #[test] + fn mount_key_groups_siblings_and_separates_mounts() { + assert_eq!( + mount_key(Path::new("/mnt/nas/a/b.flac")), + mount_key(Path::new("/mnt/nas/c/d.flac")) + ); + assert_ne!( + mount_key(Path::new("/mnt/nas/a.flac")), + mount_key(Path::new("/mnt/other/a.flac")) + ); + } +} diff --git a/crates/qbz-library/src/scan.rs b/crates/qbz-library/src/scan.rs index d7d14dbf5..1aef7f177 100644 --- a/crates/qbz-library/src/scan.rs +++ b/crates/qbz-library/src/scan.rs @@ -364,7 +364,29 @@ pub fn scan_with_progress( .map(|folders| { folders .iter() - .filter(|f| f.is_network && std::fs::read_dir(&f.path).is_err()) + // BOUNDED. This filter is the guard that keeps a scan from + // deleting tracks whose share is merely out of reach — and it + // used to probe with a bare `read_dir`, which on a + // mounted-but-unreachable mount does not return an error, it + // BLOCKS for the mount's timeout. The guard hung on exactly + // the condition it exists to detect. + // + // Anything that is not a clear `Present` counts as + // unavailable, which preserves the original semantics (an + // unreadable folder was already "unavailable") while adding a + // deadline. `read_dir` still runs, but only once the mount has + // proven it answers, so it can no longer wedge the scan. + .filter(|f| { + if !f.is_network { + return false; + } + match crate::reachability::probe_default(Path::new(&f.path)) { + crate::reachability::Reach::Present => { + std::fs::read_dir(&f.path).is_err() + } + _ => true, + } + }) .map(|f| folder_prefix(&f.path)) .collect() }) diff --git a/crates/qbz-qt/src/home_bridge.rs b/crates/qbz-qt/src/home_bridge.rs index 3869df6d7..00a491761 100644 --- a/crates/qbz-qt/src/home_bridge.rs +++ b/crates/qbz-qt/src/home_bridge.rs @@ -394,9 +394,27 @@ static QT_THREAD: OnceLock> = OnceLock::new(); /// Queue a home-bridge mutation onto the Qt event loop (no-op before /// boot registers the thread). +/// Queue a home-bridge mutation onto the Qt event loop. +/// +/// BOTH failure modes are announced, and that is not cosmetic. This function +/// could previously lose a publish two different silent ways — an unregistered +/// thread dropped the closure, and a failed `queue()` was swallowed by +/// `let _ =` — so a document that never reached the UI looked exactly like a +/// document that was never built. Three builds went into proving that was NOT +/// what was happening during the 2026-08-17 launch freeze; a single warn line +/// would have answered it in one. pub(crate) fn ui(f: impl FnOnce(Pin<&mut QbzHome>) + Send + 'static) { - if let Some(thread) = QT_THREAD.get() { - let _ = thread.queue(f); + match QT_THREAD.get() { + Some(thread) => { + if thread.queue(f).is_err() { + log::warn!( + "[qbz-qt] home bridge: a publish was REJECTED by the Qt queue and is lost" + ); + } + } + None => log::warn!( + "[qbz-qt] home bridge: a publish was DROPPED — boot() has not registered the Qt thread yet" + ), } } diff --git a/crates/qbz-qt/src/local_library_qt.rs b/crates/qbz-qt/src/local_library_qt.rs index eb9b0f0f8..05c9e5000 100644 --- a/crates/qbz-qt/src/local_library_qt.rs +++ b/crates/qbz-qt/src/local_library_qt.rs @@ -80,7 +80,7 @@ pub use crate::local_artwork::{fetch_plex_misses, resolve_window_blocking, Artwo // --- playback -------------------------------------------------------------- pub use crate::local_playback::{ - enqueue, local_queue_track, play_album, play_current_if_local, play_folder, play_folder_track, + enqueue, local_queue_track, play_album, play_current_if_local, LocalPlay, play_folder, play_folder_track, play_local_file, play_plex_track, play_tracks_visible, }; diff --git a/crates/qbz-qt/src/local_playback.rs b/crates/qbz-qt/src/local_playback.rs index dee21638a..9a50ec5e8 100644 --- a/crates/qbz-qt/src/local_playback.rs +++ b/crates/qbz-qt/src/local_playback.rs @@ -16,7 +16,7 @@ use std::sync::Arc; use qbz_app::shell::AppRuntime; use qbz_core::LoggingAdapter; -use qbz_library::LocalTrack; +use qbz_library::{LocalTrack, Reach}; use qbz_models::QueueTrack; use crate::local_albums::fetch_album_tracks_blocking; @@ -271,18 +271,42 @@ pub async fn play_local_file(runtime: &Runtime, row_id: u64) -> bool { return true; } } + // BOUNDED PROBE, then an unbounded read — and the split is the whole + // point. A mounted-but-unreachable share (the user is on a different + // network today) does not make `exists()` fail, it makes it BLOCK for the + // mount's timeout, so this await could never return: playing one dead file + // wedged playback with no way out. The probe now stops WAITING. + // + // The READ deliberately keeps no deadline. A hi-res FLAC over a + // working-but-slow share legitimately takes longer than any probe budget, + // and timing the transfer out would turn "your network is slow today" into + // "this track is gone" — worse than the bug being fixed. let read_path = path.clone(); - let bytes = tokio::task::spawn_blocking(move || { - if !Path::new(&read_path).exists() { - return None; - } - std::fs::read(&read_path).ok() + let (reach, bytes) = tokio::task::spawn_blocking(move || { + let reach = qbz_library::probe_default(Path::new(&read_path)); + let bytes = if reach == Reach::Present { + std::fs::read(&read_path).ok() + } else { + None + }; + (reach, bytes) }) .await - .ok() - .flatten(); + .unwrap_or((Reach::Unreachable, None)); let Some(bytes) = bytes else { - log::error!("[qbz-qt] local play: file not available at {path} (drive unmounted?)"); + // Missing and Unreachable are NOT the same answer and must not be + // logged as one. `Missing` is the filesystem saying the file is gone — + // a caller may clean on it. `Unreachable` is the filesystem saying + // NOTHING; the file may be perfectly fine on a share this network + // cannot see, so it may only ever be SKIPPED. + match reach { + Reach::Unreachable => log::warn!( + "[qbz-qt] local play: {path} did not answer — share unreachable from this network; skipping, NOT removing" + ), + _ => log::error!( + "[qbz-qt] local play: file not available at {path} (drive unmounted?)" + ), + } return false; }; if let Err(e) = runtime.core().player().play_data(bytes, row_id) { @@ -437,22 +461,54 @@ async fn play_audible(runtime: &Runtime, track: &QueueTrack) -> bool { } } -/// Source-aware AUDIBLE step for the shared poll-loop advance: when the -/// CURRENT queue track is a local file or a Plex row, play it here and report -/// true; otherwise report false so the Qobuz tier-walk runs unchanged. +/// What the local/Plex audible step did with a queue row. +/// +/// This used to be a `bool`, and the two `false`s meant OPPOSITE things: "this +/// row is not mine, go run the Qobuz walk" and "this row IS mine and it cannot +/// play". The caller could only treat both as the first, so a local file on a +/// share that had gone away fell through to the Qobuz tier-walk, failed there +/// with an unrelated error ("No Qobuz client available"), and +/// `auto_skip_unavailable` then declined to skip because that error is not +/// terminal-unavailable. Net effect: one unreachable local file stopped +/// playback dead, silently — the exact shape of "a track that cannot play must +/// never be blocking". +pub enum LocalPlay { + /// Not a local/Plex row. The Qobuz path must run, unchanged. + NotLocal, + /// Playing. + Played, + /// Ours, and it cannot play. Carries a reason phrased so the shared skip + /// walk recognises it as terminal (see `is_terminal_unavailable`). + Unavailable(String), +} + +/// Source-aware AUDIBLE step for the shared poll-loop advance. /// /// Without this seam, auto-advance to the next local/Plex track goes down /// `play_track_resolved` and fails with "No Qobuz client available". -pub async fn play_current_if_local(runtime: &Runtime, track_id: u64) -> bool { +pub async fn play_current_if_local(runtime: &Runtime, track_id: u64) -> LocalPlay { let Some(qt) = runtime.core().current_track().await else { - return false; + return LocalPlay::NotLocal; }; if qt.id != track_id { - return false; + return LocalPlay::NotLocal; } match qt.source.as_deref() { - Some("local") | Some("plex") => play_audible(runtime, &qt).await, - _ => false, + Some("local") | Some("plex") => { + if play_audible(runtime, &qt).await { + LocalPlay::Played + } else { + // The wording is load-bearing: `is_terminal_unavailable` + // matches on "no longer available", which is what routes this + // into the SAME bounded skip walk a pulled Qobuz track takes. + // One seam for both, which is the point — to the user they are + // the same failure. + LocalPlay::Unavailable(format!( + "local file no longer available (track {track_id})" + )) + } + } + _ => LocalPlay::NotLocal, } } diff --git a/crates/qbz-qt/src/playback_qt.rs b/crates/qbz-qt/src/playback_qt.rs index d1d5c19d1..5bf6633af 100644 --- a/crates/qbz-qt/src/playback_qt.rs +++ b/crates/qbz-qt/src/playback_qt.rs @@ -338,7 +338,7 @@ fn queue_track_blacklisted(track: &QueueTrack) -> bool { /// LOCAL / Plex rows can never be dropped here, and that is not luck: their /// builder (`local_playback::local_queue_track`) sets `streamable: true` /// because a file on disk is outside Qobuz's streaming rights entirely. -fn queue_track_unavailable(track: &QueueTrack) -> bool { +pub(crate) fn queue_track_unavailable(track: &QueueTrack) -> bool { // Clause 2 (D3): a track the player already found terminally unavailable // THIS RUN counts as dead even while the API keeps advertising it as // streamable — which is the owner's test case exactly, since `/album/get` @@ -1979,10 +1979,26 @@ async fn play_queue_track( // Source-aware audible step: a LOCAL file plays from disk through the // player's play_data seam. The Qobuz tier-walk below needs a client and // would fail with "No Qobuz client available" on every local advance. - if crate::local_library_qt::play_current_if_local(runtime, track_id).await { - refresh_now_playing(runtime).await; - publish_queue(runtime).await; - return; + // ONE seam for both kinds of "cannot play". A local/Plex row that fails + // must NOT fall through to the Qobuz walk below: it would fail there with + // an unrelated error, `auto_skip_unavailable` would decline it as + // non-terminal, and playback would stop dead on one unreachable file. To + // the user a withdrawn Qobuz track and a file on a share that went away + // are the same event, so they take the same bounded skip walk. + match crate::local_library_qt::play_current_if_local(runtime, track_id).await { + crate::local_playback::LocalPlay::Played => { + refresh_now_playing(runtime).await; + publish_queue(runtime).await; + return; + } + crate::local_playback::LocalPlay::Unavailable(reason) => { + log::warn!("[qbz-qt] playback: {reason} — skipping"); + refresh_now_playing(runtime).await; + publish_queue(runtime).await; + auto_skip_unavailable(runtime, reason, Some(track_id)).await; + return; + } + crate::local_playback::LocalPlay::NotLocal => {} } if let Err(e) = play_resolved_offline_aware(runtime, track_id, start_position_secs) @@ -2076,13 +2092,18 @@ fn auto_skip_unavailable<'a>( crate::toast_qt::warning(qbz_i18n::t("No available tracks to play")); crate::now_playing::set_playing(false); UNAVAILABLE_SKIPS.store(0, Ordering::SeqCst); + // Nowhere left to skip TO, so clean rather than leave the user a + // queue of rows that each stop playback when reached. + crate::queue_qt::purge_unavailable_upcoming(runtime).await; return; } let Some(track) = runtime.core().next_track().await else { // Queue edge reached while skipping — stop quietly, the warning - // above already told the user why. + // above already told the user why. Same clean-up as the budget + // arm: the rows behind us cannot play either. crate::now_playing::set_playing(false); UNAVAILABLE_SKIPS.store(0, Ordering::SeqCst); + crate::queue_qt::purge_unavailable_upcoming(runtime).await; return; }; log::info!( diff --git a/crates/qbz-qt/src/queue_qt.rs b/crates/qbz-qt/src/queue_qt.rs index 47002ade3..9be5355ac 100644 --- a/crates/qbz-qt/src/queue_qt.rs +++ b/crates/qbz-qt/src/queue_qt.rs @@ -650,6 +650,45 @@ pub async fn remove_upcoming(runtime: &Arc>, page_ind publish(runtime).await; } +/// Drop every UPCOMING row that cannot play, and report how many went. +/// +/// The last half of "an unavailable track is never blocking": it is skipped, +/// and when there is nowhere left to skip TO, the dead rows are cleaned out +/// rather than left for the user to walk into one at a time. Called only from +/// the dead ends of the skip walk — the budget running out, or the queue edge +/// being reached — never speculatively. +/// +/// Walks BACKWARDS so each removal cannot shift an index still to be visited. +/// +/// It removes what `queue_track_unavailable` already refuses to play, so this +/// can only delete rows that were unplayable anyway — never a track that is +/// merely on a share this network cannot see, because an unreachable local row +/// is `Unreachable`, not dead, and never enters that predicate. +pub async fn purge_unavailable_upcoming(runtime: &Arc>) -> usize { + let doomed: Vec = { + let state = runtime.core().get_queue_state_full().await; + state + .upcoming + .iter() + .enumerate() + .filter(|(_, t)| crate::playback_qt::queue_track_unavailable(t)) + .map(|(i, _)| i) + .collect() + }; + if doomed.is_empty() { + return 0; + } + for index in doomed.iter().rev() { + runtime.core().remove_upcoming_track(*index).await; + } + log::info!( + "[qbz-qt] queue: purged {} unplayable upcoming row(s)", + doomed.len() + ); + publish(runtime).await; + doomed.len() +} + pub async fn remove_all_after(runtime: &Arc>, page_index: usize) { let Some(&upcoming_index) = current_page_indices(runtime).await.get(page_index) else { log::warn!("[qbz-qt] queue: remove_all_after {page_index} out of range");