From d91728bebb3883262de8397d4372954edc66d72f Mon Sep 17 00:00:00 2001 From: NimBold Date: Sun, 11 Oct 2026 21:56:29 +0330 Subject: [PATCH] fix: address regressions since v1.4.2 - Encode Linux autostart Exec paths and preserve Add row selection across discovery. - Queue media discovery updates during busy flows and bound session registries. - Journal terminal completions across shutdown and restore Aria2 dispatch state safely. - Add accessible Add row controls and lifecycle regression coverage. Refs #44 --- src-tauri/src/db.rs | 314 +++- src-tauri/src/download.rs | 186 +- src-tauri/src/extension_server.rs | 309 +++- src-tauri/src/lib.rs | 1509 +++++++++++++++-- src-tauri/src/login_start.rs | 79 +- src-tauri/src/queue.rs | 1480 +++++++++++++++- src/App.tsx | 57 +- src/components/AddDownloadPreviewRow.test.tsx | 38 + src/components/AddDownloadPreviewRow.tsx | 50 + src/components/AddDownloadsModal.tsx | 106 +- src/ipc.ts | 6 +- src/store/useDownloadStore.test.ts | 114 +- src/store/useDownloadStore.ts | 75 +- src/utils/addDownloadMetadata.test.ts | 25 + src/utils/addDownloadMetadata.ts | 30 + 15 files changed, 3974 insertions(+), 404 deletions(-) create mode 100644 src/components/AddDownloadPreviewRow.test.tsx create mode 100644 src/components/AddDownloadPreviewRow.tsx diff --git a/src-tauri/src/db.rs b/src-tauri/src/db.rs index 7bb1e22..c6dc688 100644 --- a/src-tauri/src/db.rs +++ b/src-tauri/src/db.rs @@ -7,7 +7,7 @@ use std::sync::Mutex; const DATABASE_NAME: &str = "firelink.sqlite"; const LEGACY_STORE_NAME: &str = "store.bin"; const LEGACY_BUNDLE_IDENTIFIER: &str = "com.nima.tauri-app"; -const CURRENT_SCHEMA_VERSION: i64 = 4; +const CURRENT_SCHEMA_VERSION: i64 = 5; pub(crate) const TOKEN_CHANGED_NOTICE: &str = "pairing-token-changed"; pub const PAIRING_TOKEN_KEYCHAIN_ID: &str = "extension-pairing-token"; // Development builds are a different executable identity from the packaged @@ -106,6 +106,7 @@ fn init_at_path_internal( import_legacy_data(&mut connection, app_data_dir, portable)?; recover_downloads_from_migration_backup(&mut connection, app_data_dir, portable)?; + recover_shutdown_terminal_journal(&mut connection)?; if portable { sanitize_persisted_downloads(&mut connection)?; } @@ -238,6 +239,19 @@ fn migrate_schema(connection: &mut Connection, from_version: i64) -> Result<(), ").map_err(|error| format!("failed to migrate removal jobs: {error}"))?; } + if from_version < 5 { + transaction + .execute_batch( + " + CREATE TABLE IF NOT EXISTS shutdown_terminal_journal ( + id TEXT PRIMARY KEY, + lifecycle_generation TEXT NOT NULL + ); + ", + ) + .map_err(|error| format!("failed to create shutdown terminal journal: {error}"))?; + } + transaction .pragma_update(None, "user_version", CURRENT_SCHEMA_VERSION) .map_err(|error| format!("failed to update database schema version: {error}"))?; @@ -1422,6 +1436,167 @@ pub fn load_downloads(connection: &Connection) -> Result, String> { query_string_column(connection, "SELECT data FROM downloads ORDER BY rowid") } +/// Confirm that both the indexed status and the canonical download document +/// durably describe a terminal completion. This is used only when the +/// shutdown journal could not commit and a final frontend flush is the +/// recovery path. +pub fn is_download_durably_completed(connection: &Connection, id: &str) -> Result { + let row = connection + .query_row( + "SELECT status, data FROM downloads WHERE id = ?1", + params![id], + |row| Ok((row.get::<_, String>(0)?, row.get::<_, String>(1)?)), + ) + .optional() + .map_err(|error| format!("failed to verify completed download '{id}': {error}"))?; + let Some((indexed_status, data)) = row else { + return Ok(false); + }; + let document: Value = serde_json::from_str(&data) + .map_err(|error| format!("persisted download '{id}' is malformed: {error}"))?; + let document_id = required_string(&document, "id")?; + let document_status = required_string(&document, "status")?; + Ok(document_id == id && indexed_status == "completed" && document_status == "completed") +} + +/// Persist a native completion observed while Firelink is shutting down. +/// +/// The frontend owns full download snapshots, so this journal is deliberately +/// separate: a late or failed frontend flush cannot overwrite the terminal +/// fact. Startup applies it to the matching durable row before interrupted +/// active rows are normalized for requeue. +pub fn record_shutdown_terminal( + connection: &mut Connection, + id: &str, + lifecycle_generation: u64, +) -> Result<(), String> { + if id.trim().is_empty() { + return Err("shutdown completion journal requires a download id".to_string()); + } + let transaction = connection + .transaction() + .map_err(|error| format!("failed to begin shutdown completion journal: {error}"))?; + let previous = transaction + .query_row( + "SELECT lifecycle_generation FROM shutdown_terminal_journal WHERE id = ?1", + params![id], + |row| row.get::<_, String>(0), + ) + .optional() + .map_err(|error| format!("failed to read shutdown completion journal: {error}"))?; + if let Some(previous) = previous { + let previous_generation = previous.parse::().map_err(|_| { + "shutdown completion journal contains an invalid lifecycle generation".to_string() + })?; + if previous_generation > lifecycle_generation { + transaction + .rollback() + .map_err(|error| format!("failed to keep newer shutdown completion: {error}"))?; + return Ok(()); + } + } + transaction + .execute( + "INSERT INTO shutdown_terminal_journal (id, lifecycle_generation) + VALUES (?1, ?2) + ON CONFLICT(id) DO UPDATE SET lifecycle_generation = excluded.lifecycle_generation", + params![id, lifecycle_generation.to_string()], + ) + .map_err(|error| format!("failed to record shutdown completion: {error}"))?; + transaction + .commit() + .map_err(|error| format!("failed to commit shutdown completion journal: {error}")) +} + +pub fn clear_shutdown_terminal( + connection: &mut Connection, + id: &str, +) -> Result<(), String> { + let transaction = connection + .transaction() + .map_err(|error| format!("failed to begin shutdown completion invalidation: {error}"))?; + transaction + .execute("DELETE FROM shutdown_terminal_journal WHERE id = ?1", params![id]) + .map_err(|error| format!("failed to invalidate shutdown completion for '{id}': {error}"))?; + transaction + .commit() + .map_err(|error| format!("failed to commit shutdown completion invalidation: {error}")) +} + +fn recover_shutdown_terminal_journal(connection: &mut Connection) -> Result { + let transaction = connection + .transaction() + .map_err(|error| format!("failed to begin shutdown completion recovery: {error}"))?; + let entries = { + let mut statement = transaction + .prepare("SELECT id, lifecycle_generation FROM shutdown_terminal_journal ORDER BY id") + .map_err(|error| format!("failed to read shutdown completion journal: {error}"))?; + let rows = statement + .query_map([], |row| { + Ok((row.get::<_, String>(0)?, row.get::<_, String>(1)?)) + }) + .map_err(|error| format!("failed to query shutdown completion journal: {error}"))?; + rows.collect::, _>>() + .map_err(|error| format!("failed to collect shutdown completion journal: {error}"))? + }; + + let mut recovered = 0; + for (id, generation) in entries { + generation.parse::().map_err(|_| { + format!("shutdown completion journal for '{id}' has an invalid lifecycle generation") + })?; + let persisted = transaction + .query_row( + "SELECT status, data FROM downloads WHERE id = ?1", + params![id], + |row| Ok((row.get::<_, String>(0)?, row.get::<_, String>(1)?)), + ) + .optional() + .map_err(|error| format!("failed to load shutdown-completed download '{id}': {error}"))?; + let Some((indexed_status, data)) = persisted else { + // A later user removal is authoritative. The journal is still + // consumed atomically so a deleted row cannot be resurrected. + continue; + }; + let mut value: Value = serde_json::from_str(&data) + .map_err(|error| format!("persisted download '{id}' is malformed: {error}"))?; + if required_string(&value, "id")? != id { + return Err(format!("persisted download '{id}' has mismatched document id")); + } + let document_status = required_string(&value, "status")?; + if document_status != indexed_status { + return Err(format!("persisted download '{id}' has mismatched indexed status")); + } + if document_status == "completed" { + continue; + } + let object = value + .as_object_mut() + .ok_or_else(|| format!("persisted download '{id}' is not an object"))?; + object.insert("status".to_string(), Value::String("completed".to_string())); + let data = serde_json::to_string(&value) + .map_err(|error| format!("failed to encode recovered completion '{id}': {error}"))?; + let changed = transaction + .execute( + "UPDATE downloads SET status = 'completed', data = ?1 WHERE id = ?2", + params![data, id], + ) + .map_err(|error| format!("failed to recover shutdown completion '{id}': {error}"))?; + if changed != 1 { + return Err(format!("persisted download '{id}' changed during recovery")); + } + recovered += 1; + } + + transaction + .execute("DELETE FROM shutdown_terminal_journal", []) + .map_err(|error| format!("failed to clear recovered shutdown completions: {error}"))?; + transaction + .commit() + .map_err(|error| format!("failed to commit shutdown completion recovery: {error}"))?; + Ok(recovered) +} + pub fn replace_downloads( connection: &mut Connection, data: &str, @@ -3529,6 +3704,143 @@ mod tests { assert!(load_queues(&connection).unwrap()[0].contains("new-queue")); } + #[test] + fn shutdown_terminal_journal_prevents_requeue_after_final_frontend_flush_failure() { + let root = TempDir::new().unwrap(); + let database = init_at_path(root.path()).unwrap(); + { + let mut connection = database.lock().unwrap(); + replace_downloads( + &mut connection, + &json!([{ + "id": "completed-during-shutdown", + "status": "downloading", + "queueId": "main", + "downloadedBytes": 900, + "totalBytes": 1000 + }]) + .to_string(), + false, + ) + .unwrap(); + // The native lifecycle observed completion, but the simulated + // final frontend flush times out and leaves the earlier active + // snapshot untouched. + record_shutdown_terminal(&mut connection, "completed-during-shutdown", 42).unwrap(); + let journal_generation: String = connection + .query_row( + "SELECT lifecycle_generation FROM shutdown_terminal_journal WHERE id = ?1", + params!["completed-during-shutdown"], + |row| row.get(0), + ) + .unwrap(); + assert_eq!(journal_generation, "42"); + } + drop(database); + + let restarted = init_at_path(root.path()).unwrap(); + let connection = restarted.lock().unwrap(); + let rows = load_downloads(&connection).unwrap(); + let row: Value = serde_json::from_str(&rows[0]).unwrap(); + assert_eq!(row["status"], "completed"); + assert_eq!(row["downloadedBytes"], 900); + let journal_entries: i64 = connection + .query_row("SELECT COUNT(*) FROM shutdown_terminal_journal", [], |row| row.get(0)) + .unwrap(); + assert_eq!(journal_entries, 0); + } + + #[test] + fn failed_shutdown_terminal_journal_commit_is_not_reported_as_success() { + let root = TempDir::new().unwrap(); + let database = init_at_path(root.path()).unwrap(); + let mut connection = database.lock().unwrap(); + connection + .execute_batch( + " + CREATE TRIGGER reject_shutdown_terminal_journal + BEFORE INSERT ON shutdown_terminal_journal + BEGIN + SELECT RAISE(ABORT, 'simulated journal failure'); + END; + ", + ) + .unwrap(); + + assert!(record_shutdown_terminal(&mut connection, "download-1", 7).is_err()); + let journal_entries: i64 = connection + .query_row("SELECT COUNT(*) FROM shutdown_terminal_journal", [], |row| row.get(0)) + .unwrap(); + assert_eq!(journal_entries, 0); + } + + #[test] + fn failed_journal_fallback_requires_the_index_and_document_to_be_completed() { + let root = TempDir::new().unwrap(); + let database = init_at_path(root.path()).unwrap(); + let mut connection = database.lock().unwrap(); + replace_downloads( + &mut connection, + &json!([ + { "id": "completed", "status": "completed", "queueId": "main" }, + { "id": "active", "status": "downloading", "queueId": "main" }, + { "id": "mismatched", "status": "completed", "queueId": "main" } + ]) + .to_string(), + false, + ) + .unwrap(); + connection + .execute( + "UPDATE downloads SET data = ?1 WHERE id = ?2", + params![json!({ "id": "mismatched", "status": "downloading" }).to_string(), "mismatched"], + ) + .unwrap(); + + assert!(is_download_durably_completed(&connection, "completed").unwrap()); + assert!(!is_download_durably_completed(&connection, "active").unwrap()); + assert!(!is_download_durably_completed(&connection, "mismatched").unwrap()); + assert!(!is_download_durably_completed(&connection, "missing").unwrap()); + } + + #[test] + fn new_download_lifecycle_clears_a_prior_shutdown_completion_record() { + let root = TempDir::new().unwrap(); + let database = init_at_path(root.path()).unwrap(); + let mut connection = database.lock().unwrap(); + record_shutdown_terminal(&mut connection, "redownload", 5).unwrap(); + clear_shutdown_terminal(&mut connection, "redownload").unwrap(); + + let journal_entries: i64 = connection + .query_row("SELECT COUNT(*) FROM shutdown_terminal_journal", [], |row| row.get(0)) + .unwrap(); + assert_eq!(journal_entries, 0); + } + + #[test] + fn shutdown_terminal_recovery_retains_journal_if_row_recovery_fails() { + let root = TempDir::new().unwrap(); + let database = init_at_path(root.path()).unwrap(); + let mut connection = database.lock().unwrap(); + connection + .execute( + "INSERT INTO downloads (id, status, queue_id, data) VALUES ('broken', 'downloading', 'main', 'not-json')", + [], + ) + .unwrap(); + record_shutdown_terminal(&mut connection, "broken", 1).unwrap(); + + assert!(recover_shutdown_terminal_journal(&mut connection).is_err()); + let journal_entries: i64 = connection + .query_row("SELECT COUNT(*) FROM shutdown_terminal_journal", [], |row| row.get(0)) + .unwrap(); + assert_eq!(journal_entries, 1); + let status: String = connection + .query_row("SELECT status FROM downloads WHERE id = 'broken'", [], |row| row.get(0)) + .unwrap(); + assert_eq!(status, "downloading"); + } + #[test] fn native_download_mutation_keeps_object_records_and_unrelated_rows_unchanged() { let temp = TempDir::new().unwrap(); diff --git a/src-tauri/src/download.rs b/src-tauri/src/download.rs index a3eb0ac..ea3821e 100644 --- a/src-tauri/src/download.rs +++ b/src-tauri/src/download.rs @@ -17,6 +17,13 @@ pub enum DownloadEvent { pub struct DownloadCoordinator { tx: mpsc::Sender, media_tx: mpsc::Sender, + media_registration: std::sync::Arc>, +} + +#[derive(Default)] +struct MediaRegistrationState { + closed: bool, + rejected_during_shutdown: HashSet<(String, u64)>, } impl DownloadCoordinator { @@ -36,7 +43,13 @@ impl DownloadCoordinator { let (tx, rx) = mpsc::channel(128); let (media_tx, media_rx) = mpsc::channel(32); tauri::async_runtime::spawn(run_coordinator(events, rx, media_rx)); - Self { tx, media_tx } + Self { + tx, + media_tx, + media_registration: std::sync::Arc::new(tokio::sync::Mutex::new( + MediaRegistrationState::default(), + )), + } } pub async fn send(&self, command: DownloadCmd) -> Result<(), String> { @@ -51,6 +64,14 @@ impl DownloadCoordinator { id: String, lifecycle_generation: u64, ) -> Result, String> { + let mut registration = self.media_registration.lock().await; + if registration.closed { + registration + .rejected_during_shutdown + .insert((id, lifecycle_generation)); + let (_cancel_tx, cancel_rx) = watch::channel(true); + return Ok(cancel_rx); + } let (cancel_tx, cancel_rx) = watch::channel(false); self.media_tx .send(MediaCmd::Register { @@ -60,9 +81,42 @@ impl DownloadCoordinator { }) .await .map_err(|_| "download coordinator is unavailable".to_string())?; + drop(registration); Ok(cancel_rx) } + /// Close media registration, cancel each active producer, and wait until + /// every accepted media runner has completed its terminal path. + pub async fn cancel_active_media_for_shutdown(&self) -> Result, String> { + let mut registration = self.media_registration.lock().await; + registration.closed = true; + let (ack_tx, ack_rx) = tokio::sync::oneshot::channel(); + if self + .media_tx + .send(MediaCmd::CancelAllForShutdown { ack: ack_tx }) + .await + .is_err() + { + registration.closed = false; + return Err("download coordinator is unavailable".to_string()); + } + drop(registration); + ack_rx + .await + .map_err(|_| "media runners stopped before shutdown cancellation completed".to_string()) + } + + pub async fn reopen_media_registration(&self) -> Vec<(String, u64)> { + let mut registration = self.media_registration.lock().await; + registration.closed = false; + let mut rejected = registration + .rejected_during_shutdown + .drain() + .collect::>(); + rejected.sort(); + rejected + } + pub async fn pause_media(&self, id: String, lifecycle_generation: u64) -> Result<(), String> { self.media_tx .send(MediaCmd::Pause { @@ -90,11 +144,22 @@ impl DownloadCoordinator { } pub async fn finish_media(&self, id: String, lifecycle_generation: u64) { + self.finish_media_with_shutdown_outcome(id, lifecycle_generation, false) + .await; + } + + pub async fn finish_media_with_shutdown_outcome( + &self, + id: String, + lifecycle_generation: u64, + shutdown_interrupted: bool, + ) { let _ = self .media_tx .send(MediaCmd::Finished { id, lifecycle_generation, + shutdown_interrupted, }) .await; } @@ -133,7 +198,17 @@ enum MediaCmd { Finished { id: String, lifecycle_generation: u64, + shutdown_interrupted: bool, }, + CancelAllForShutdown { + ack: tokio::sync::oneshot::Sender>, + }, +} + +struct PendingMediaShutdown { + remaining: HashSet<(String, u64)>, + interrupted: Vec<(String, u64)>, + ack: tokio::sync::oneshot::Sender>, } async fn run_coordinator( @@ -145,6 +220,7 @@ async fn run_coordinator( let mut cancelled_media_generations = HashMap::::new(); let mut pending_media_acks = HashMap::<(String, u64), tokio::sync::oneshot::Sender>::new(); + let mut pending_shutdown: Option = None; let mut pending_captured_urls = Vec::::new(); let mut frontend_ready = false; let mut command_open = true; @@ -234,7 +310,7 @@ async fn run_coordinator( let _ = ack.send(false); } } - MediaCmd::Finished { id, lifecycle_generation } => { + MediaCmd::Finished { id, lifecycle_generation, shutdown_interrupted } => { if active_media.get(&id).is_some_and(|(generation, _)| *generation == lifecycle_generation) { active_media.remove(&id); } @@ -251,6 +327,43 @@ async fn run_coordinator( { cancelled_media_generations.remove(&id); } + if let Some(waiting) = pending_shutdown.as_mut() { + waiting + .remaining + .remove(&(id.clone(), lifecycle_generation)); + if shutdown_interrupted { + waiting + .interrupted + .push((id.clone(), lifecycle_generation)); + } + if waiting.remaining.is_empty() { + if let Some(completed) = pending_shutdown.take() { + let mut interrupted = completed.interrupted; + interrupted.sort(); + let _ = completed.ack.send(interrupted); + } + } + } + } + MediaCmd::CancelAllForShutdown { ack } => { + let mut candidates = active_media + .iter() + .map(|(id, (generation, _))| (id.clone(), *generation)) + .collect::>(); + candidates.sort(); + let remaining = candidates.iter().cloned().collect::>(); + for (_, cancel_tx) in active_media.values() { + let _ = cancel_tx.send(true); + } + if remaining.is_empty() { + let _ = ack.send(Vec::new()); + } else { + pending_shutdown = Some(PendingMediaShutdown { + interrupted: Vec::new(), + remaining, + ack, + }); + } } }, None => media_open = false, @@ -498,4 +611,73 @@ mod tests { .unwrap() .unwrap(); } + + #[tokio::test] + async fn shutdown_cancels_and_drains_active_media_before_reopening_registration() { + let (coordinator, _events) = DownloadCoordinator::spawn_headless(); + let mut cancel_rx = coordinator + .register_media("shutdown-media".to_string(), 17) + .await + .unwrap(); + let mut completed_cancel_rx = coordinator + .register_media("completed-media".to_string(), 20) + .await + .unwrap(); + let shutdown_coordinator = coordinator.clone(); + let shutdown = tokio::spawn(async move { + shutdown_coordinator + .cancel_active_media_for_shutdown() + .await + }); + + tokio::time::timeout(Duration::from_secs(1), async { + while !*cancel_rx.borrow_and_update() { + cancel_rx.changed().await.unwrap(); + } + }) + .await + .expect("shutdown did not cancel the active media runner"); + tokio::time::timeout(Duration::from_secs(1), async { + while !*completed_cancel_rx.borrow_and_update() { + completed_cancel_rx.changed().await.unwrap(); + } + }) + .await + .expect("shutdown did not notify the finishing media runner"); + assert!(!shutdown.is_finished(), "shutdown must wait for runner cleanup"); + + coordinator + .finish_media_with_shutdown_outcome("completed-media".to_string(), 20, false) + .await; + coordinator + .finish_media_with_shutdown_outcome("shutdown-media".to_string(), 17, true) + .await; + assert_eq!( + tokio::time::timeout(Duration::from_secs(1), shutdown) + .await + .unwrap() + .unwrap() + .unwrap(), + vec![("shutdown-media".to_string(), 17)] + ); + + let late_registration = coordinator + .register_media("late-media".to_string(), 18) + .await + .unwrap(); + assert!(*late_registration.borrow()); + + assert_eq!( + coordinator.reopen_media_registration().await, + vec![("late-media".to_string(), 18)] + ); + let reopened_registration = coordinator + .register_media("reopened-media".to_string(), 19) + .await + .unwrap(); + assert!(!*reopened_registration.borrow()); + coordinator + .finish_media("reopened-media".to_string(), 19) + .await; + } } diff --git a/src-tauri/src/extension_server.rs b/src-tauri/src/extension_server.rs index 6540a6f..52712e2 100644 --- a/src-tauri/src/extension_server.rs +++ b/src-tauri/src/extension_server.rs @@ -52,6 +52,7 @@ const PROTOCOL_VERSION: &str = "7"; const MEDIA_HANDOFF_PROTOCOL_VERSION: u16 = 7; const MAX_MEDIA_HANDOFF_ID_BYTES: usize = 128; const MAX_ACTIVE_MEDIA_HANDOFFS: usize = 64; +const MAX_MEDIA_DISCOVERY_REPLAY_RECORDS: usize = 128; const MEDIA_HANDOFF_TTL: Duration = Duration::from_secs(30); const MEDIA_DISCOVERY_ATTEMPT_TTL: Duration = Duration::from_secs(10); const MAX_PENDING_EXTENSION_ACKS: usize = 64; @@ -62,7 +63,7 @@ pub type SharedExtensionToken = Arc>; pub type SharedFrontendReady = Arc; pub type SharedServerPort = Arc>>; pub type SharedExtensionAcks = Arc>>>; -pub type SharedMediaHandoffs = Arc>>; +pub type SharedMediaHandoffs = Arc>; type ReplayCache = Arc>>; #[derive(Clone, Copy, Debug, Deserialize, PartialEq, Eq, Serialize, TS)] @@ -94,9 +95,6 @@ enum MediaDiscoveryState { completion: watch::Sender>, attempt_expires_at: Instant, }, - Acknowledged { - content: MediaDiscoveryContent, - }, } #[derive(Clone, Debug)] @@ -105,6 +103,22 @@ pub struct MediaHandoffState { expires_at: Instant, } +#[derive(Clone, Debug)] +struct MediaDiscoveryReplay { + content: MediaDiscoveryContent, + expires_at: Instant, +} + +#[derive(Default)] +pub struct MediaHandoffRegistry { + active: HashMap, + replays: HashMap, +} + +pub fn new_media_handoffs() -> SharedMediaHandoffs { + Arc::new(Mutex::new(MediaHandoffRegistry::default())) +} + #[derive(Clone)] pub struct ServerState { pub app_handle: AppHandle, @@ -503,11 +517,21 @@ async fn download_handler( } if let Some(handoff_id) = media_handoff_id.as_deref() { - if !register_media_handoff(&state.media_handoffs, handoff_id) { - if cached_torrent { - crate::torrent::remove_managed_torrent(&state.app_handle, &request_id).await; + match register_media_handoff(&state.media_handoffs, handoff_id) { + MediaHandoffRegistration::Tracked + | MediaHandoffRegistration::TrackedAfterEviction => {} + MediaHandoffRegistration::UntrackedCapacity => { + log::warn!("media discovery capacity reached; preserving the initial page handoff"); + } + MediaHandoffRegistration::RegistryUnavailable => { + log::warn!("media discovery registry unavailable; preserving the initial page handoff"); + } + MediaHandoffRegistration::Duplicate => { + if cached_torrent { + crate::torrent::remove_managed_torrent(&state.app_handle, &request_id).await; + } + return Err(StatusCode::CONFLICT); } - return Err(StatusCode::CONFLICT); } } @@ -622,7 +646,12 @@ async fn deliver_media_discovery_update( let handoff_id = update.handoff_id.clone(); let request_id = uuid::Uuid::new_v4().simple().to_string(); match admit_media_discovery_update(&state.media_handoffs, &update, request_id.clone()) { - MediaDiscoveryAdmission::Missing => return Err(StatusCode::NOT_FOUND), + MediaDiscoveryAdmission::Missing => { + // The bounded active registry may evict an idle session under + // pressure. Its initial page handoff was already delivered, so a + // later optional manifest update is a successful no-op. + return signed_server_response(timestamp_str, nonce, state); + } MediaDiscoveryAdmission::Conflict => return Err(StatusCode::CONFLICT), MediaDiscoveryAdmission::RegistryUnavailable => { return Err(StatusCode::INTERNAL_SERVER_ERROR); @@ -890,9 +919,9 @@ fn media_discovery_content(update: &ExtensionMediaDiscoveryUpdate) -> MediaDisco } } -fn prune_media_handoffs(registry: &mut HashMap) { +fn prune_media_handoffs(registry: &mut MediaHandoffRegistry) { let now = Instant::now(); - for state in registry.values_mut() { + for state in registry.active.values_mut() { let attempt_expired = matches!( state.discovery.as_ref(), Some(MediaDiscoveryState::InFlight { @@ -906,30 +935,65 @@ fn prune_media_handoffs(registry: &mut HashMap) { } } } - registry.retain(|_, state| state.expires_at > now); + registry.active.retain(|_, state| state.expires_at > now); + registry.replays.retain(|_, replay| replay.expires_at > now); } -fn register_media_handoff(registry: &SharedMediaHandoffs, handoff_id: &str) -> bool { +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +enum MediaHandoffRegistration { + Tracked, + TrackedAfterEviction, + UntrackedCapacity, + Duplicate, + RegistryUnavailable, +} + +fn register_media_handoff( + registry: &SharedMediaHandoffs, + handoff_id: &str, +) -> MediaHandoffRegistration { let Ok(mut registry) = registry.lock() else { - return false; + return MediaHandoffRegistration::RegistryUnavailable; }; prune_media_handoffs(&mut registry); - if registry.contains_key(handoff_id) || registry.len() >= MAX_ACTIVE_MEDIA_HANDOFFS { - return false; + if registry.active.contains_key(handoff_id) || registry.replays.contains_key(handoff_id) { + return MediaHandoffRegistration::Duplicate; } - registry.insert( + + let registration = if registry.active.len() >= MAX_ACTIVE_MEDIA_HANDOFFS { + // An idle page session is less valuable than the new user's base + // handoff. Never evict a discovery request already being delivered. + let oldest_idle = registry + .active + .iter() + .filter(|(_, state)| state.discovery.is_none()) + .min_by_key(|(_, state)| state.expires_at) + .map(|(id, _)| id.clone()); + if let Some(id) = oldest_idle { + registry.active.remove(&id); + MediaHandoffRegistration::TrackedAfterEviction + } else { + // The initial page capture remains available. Its optional late + // manifest update will be acknowledged as a safe stale no-op. + return MediaHandoffRegistration::UntrackedCapacity; + } + } else { + MediaHandoffRegistration::Tracked + }; + registry.active.insert( handoff_id.to_string(), MediaHandoffState { discovery: None, expires_at: Instant::now() + MEDIA_HANDOFF_TTL, }, ); - true + registration } fn remove_media_handoff(registry: &SharedMediaHandoffs, handoff_id: &str) { if let Ok(mut registry) = registry.lock() { - registry.remove(handoff_id); + registry.active.remove(handoff_id); + registry.replays.remove(handoff_id); } } @@ -954,10 +1018,17 @@ fn admit_media_discovery_update( return MediaDiscoveryAdmission::RegistryUnavailable; }; prune_media_handoffs(&mut registry); - let Some(state) = registry.get_mut(&update.handoff_id) else { + let content = media_discovery_content(update); + if let Some(replay) = registry.replays.get(&update.handoff_id) { + return if replay.content == content { + MediaDiscoveryAdmission::Duplicate + } else { + MediaDiscoveryAdmission::Conflict + }; + } + let Some(state) = registry.active.get_mut(&update.handoff_id) else { return MediaDiscoveryAdmission::Missing; }; - let content = media_discovery_content(update); match state.discovery.take() { None => { let (completion, _waiter) = watch::channel(None); @@ -991,15 +1062,6 @@ fn admit_media_discovery_update( MediaDiscoveryAdmission::Conflict } } - Some(MediaDiscoveryState::Acknowledged { content: previous }) => { - let same_content = previous == content; - state.discovery = Some(MediaDiscoveryState::Acknowledged { content: previous }); - if same_content { - MediaDiscoveryAdmission::Duplicate - } else { - MediaDiscoveryAdmission::Conflict - } - } } } @@ -1014,7 +1076,7 @@ fn finish_media_discovery_attempt( let Ok(mut registry) = registry.lock() else { return false; }; - let Some(state) = registry.get_mut(handoff_id) else { + let Some(mut state) = registry.active.remove(handoff_id) else { return false; }; let Some(MediaDiscoveryState::InFlight { @@ -1024,6 +1086,7 @@ fn finish_media_discovery_attempt( attempt_expires_at, }) = state.discovery.take() else { + registry.active.insert(handoff_id.to_string(), state); return false; }; if current_attempt_id != attempt_id || content != media_discovery_content(update) { @@ -1033,11 +1096,33 @@ fn finish_media_discovery_attempt( completion, attempt_expires_at, }); + registry.active.insert(handoff_id.to_string(), state); return false; } if result == MediaDiscoveryDeliveryResult::Acknowledged { - state.discovery = Some(MediaDiscoveryState::Acknowledged { content }); + let now = Instant::now(); + registry.replays.insert( + handoff_id.to_string(), + MediaDiscoveryReplay { + content, + expires_at: now + MEDIA_HANDOFF_TTL, + }, + ); + if registry.replays.len() > MAX_MEDIA_DISCOVERY_REPLAY_RECORDS { + if let Some(oldest_id) = registry + .replays + .iter() + .min_by_key(|(_, replay)| replay.expires_at) + .map(|(id, _)| id.clone()) + { + registry.replays.remove(&oldest_id); + } + } + } else { + state.discovery = None; + state.expires_at = Instant::now() + MEDIA_HANDOFF_TTL; + registry.active.insert(handoff_id.to_string(), state); } completion }; @@ -1049,7 +1134,7 @@ fn finish_media_discovery_attempt( #[cfg(test)] fn expire_media_handoff(registry: &SharedMediaHandoffs, handoff_id: &str) { if let Ok(mut registry) = registry.lock() { - if let Some(state) = registry.get_mut(handoff_id) { + if let Some(state) = registry.active.get_mut(handoff_id) { state.expires_at = Instant::now() - Duration::from_secs(1); } } @@ -1591,7 +1676,8 @@ mod tests { is_valid_client_nonce, is_valid_media_handoff_id, normalize_download, normalize_headers, normalize_media_discovery_update, normalize_media_handoff_metadata, normalize_media_url, normalize_referer, normalize_url, - expire_media_handoff, finish_media_discovery_attempt, register_media_handoff, + expire_media_handoff, finish_media_discovery_attempt, new_media_handoffs, + register_media_handoff, reject_extension_download, wait_for_extension_acknowledgement, require_frontend_ready, required_client_nonce, same_origin_url, sanitize_filename, server_session_for_request, session_binding_requested, @@ -1599,7 +1685,9 @@ mod tests { wait_for_media_discovery_completion, ExtensionCookieScope, ExtensionMediaDiscoveryRequest, ExtensionRequest, MediaDiscoveryAdmission, MediaDiscoveryDeliveryResult, MediaHandoffPhase, - MAX_MEDIA_HEADER_VALUE_BYTES, MAX_MEDIA_HANDOFF_ID_BYTES, MAX_MEDIA_REFERER_BYTES, + MediaHandoffRegistration, MAX_ACTIVE_MEDIA_HANDOFFS, + MAX_MEDIA_DISCOVERY_REPLAY_RECORDS, MAX_MEDIA_HEADER_VALUE_BYTES, + MAX_MEDIA_HANDOFF_ID_BYTES, MAX_MEDIA_REFERER_BYTES, MAX_URL_COUNT, MEDIA_HANDOFF_PROTOCOL_VERSION, PROTOCOL_VERSION_HEADER, SERVER_HEADER, SERVER_SESSION_HEADER, @@ -1617,7 +1705,7 @@ mod tests { use std::collections::HashMap; use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::{Arc, Mutex, RwLock}; - use std::time::Duration; + use std::time::{Duration, Instant}; fn normalized_discovery_update( handoff_id: &str, @@ -1808,8 +1896,11 @@ mod tests { #[tokio::test] async fn discovery_updates_require_ack_before_deduplication_and_roll_back_on_failure() { - let registry = Arc::new(Mutex::new(HashMap::new())); - assert!(register_media_handoff(®istry, "media-opaque")); + let registry = new_media_handoffs(); + assert_eq!( + register_media_handoff(®istry, "media-opaque"), + MediaHandoffRegistration::Tracked + ); let update = normalized_discovery_update( "media-opaque", @@ -1866,6 +1957,8 @@ mod tests { success_waiter.await.unwrap(), Some(MediaDiscoveryDeliveryResult::Acknowledged) ); + assert_eq!(registry.lock().unwrap().active.len(), 0); + assert_eq!(registry.lock().unwrap().replays.len(), 1); assert!(matches!( admit_media_discovery_update(®istry, &update, "attempt-5".to_string()), MediaDiscoveryAdmission::Duplicate @@ -1887,7 +1980,7 @@ mod tests { #[test] fn missing_and_expired_discovery_handoffs_are_not_successful_duplicates() { - let missing_registry = Arc::new(Mutex::new(HashMap::new())); + let missing_registry = new_media_handoffs(); let update = normalized_discovery_update( "missing-handoff", "https://cdn.example/stream.m3u8", @@ -1899,8 +1992,11 @@ mod tests { MediaDiscoveryAdmission::Missing )); - let expired_registry = Arc::new(Mutex::new(HashMap::new())); - assert!(register_media_handoff(&expired_registry, "expired-handoff")); + let expired_registry = new_media_handoffs(); + assert_eq!( + register_media_handoff(&expired_registry, "expired-handoff"), + MediaHandoffRegistration::Tracked + ); expire_media_handoff(&expired_registry, "expired-handoff"); let expired = normalized_discovery_update( "expired-handoff", @@ -1914,6 +2010,137 @@ mod tests { )); } + #[test] + fn active_capacity_evicts_old_idle_sessions_and_keeps_initial_handoff_admissible() { + let registry = new_media_handoffs(); + for index in 0..MAX_ACTIVE_MEDIA_HANDOFFS { + assert_eq!( + register_media_handoff(®istry, &format!("idle-{index}")), + MediaHandoffRegistration::Tracked + ); + } + { + let mut registry = registry.lock().unwrap(); + registry.active.get_mut("idle-0").unwrap().expires_at = + Instant::now() + Duration::from_secs(10); + } + + assert_eq!( + register_media_handoff(®istry, "new-page-handoff"), + MediaHandoffRegistration::TrackedAfterEviction + ); + assert_eq!(registry.lock().unwrap().active.len(), MAX_ACTIVE_MEDIA_HANDOFFS); + let evicted_update = normalized_discovery_update( + "idle-0", + "https://cdn.example/late.m3u8", + None, + None, + ); + assert!(matches!( + admit_media_discovery_update(®istry, &evicted_update, "late-attempt".to_string()), + MediaDiscoveryAdmission::Missing + )); + } + + #[test] + fn full_inflight_registry_preserves_base_handoff_without_evicting_delivery() { + let registry = new_media_handoffs(); + for index in 0..MAX_ACTIVE_MEDIA_HANDOFFS { + let id = format!("inflight-{index}"); + assert_eq!( + register_media_handoff(®istry, &id), + MediaHandoffRegistration::Tracked + ); + let update = normalized_discovery_update( + &id, + &format!("https://cdn.example/{index}.m3u8"), + None, + None, + ); + assert!(matches!( + admit_media_discovery_update(®istry, &update, format!("attempt-{index}")), + MediaDiscoveryAdmission::Fresh + )); + } + + assert_eq!( + register_media_handoff(®istry, "untracked-page-handoff"), + MediaHandoffRegistration::UntrackedCapacity + ); + assert_eq!(registry.lock().unwrap().active.len(), MAX_ACTIVE_MEDIA_HANDOFFS); + let late_update = normalized_discovery_update( + "untracked-page-handoff", + "https://cdn.example/untracked.m3u8", + None, + None, + ); + assert!(matches!( + admit_media_discovery_update(®istry, &late_update, "late-attempt".to_string()), + MediaDiscoveryAdmission::Missing + )); + } + + #[tokio::test] + async fn replay_records_are_separate_from_active_capacity_and_remain_bounded() { + let registry = new_media_handoffs(); + for index in 0..(MAX_MEDIA_DISCOVERY_REPLAY_RECORDS + 1) { + let id = format!("replay-{index}"); + assert_eq!( + register_media_handoff(®istry, &id), + MediaHandoffRegistration::Tracked + ); + let update = normalized_discovery_update( + &id, + &format!("https://cdn.example/{index}.m3u8"), + None, + None, + ); + assert!(matches!( + admit_media_discovery_update(®istry, &update, format!("attempt-{index}")), + MediaDiscoveryAdmission::Fresh + )); + assert!(finish_media_discovery_attempt( + ®istry, + &id, + &format!("attempt-{index}"), + &update, + MediaDiscoveryDeliveryResult::Acknowledged, + )); + } + + for index in 0..MAX_ACTIVE_MEDIA_HANDOFFS { + assert_eq!( + register_media_handoff(®istry, &format!("active-{index}")), + MediaHandoffRegistration::Tracked + ); + } + { + let state = registry.lock().unwrap(); + assert_eq!(state.active.len(), MAX_ACTIVE_MEDIA_HANDOFFS); + assert_eq!(state.replays.len(), MAX_MEDIA_DISCOVERY_REPLAY_RECORDS); + } + let retained_replay = normalized_discovery_update( + "replay-128", + "https://cdn.example/128.m3u8", + None, + None, + ); + assert!(matches!( + admit_media_discovery_update(®istry, &retained_replay, "replay-check".to_string()), + MediaDiscoveryAdmission::Duplicate + )); + let evicted_replay = normalized_discovery_update( + "replay-0", + "https://cdn.example/0.m3u8", + None, + None, + ); + assert!(matches!( + admit_media_discovery_update(®istry, &evicted_replay, "evicted-check".to_string()), + MediaDiscoveryAdmission::Missing + )); + } + #[test] fn discovery_update_payload_has_a_stable_ipc_shape() { let update = normalized_discovery_update( diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index 73096a1..7a481a0 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -7,7 +7,7 @@ use regex::Regex; use serde::Serialize; use std::collections::{BTreeSet, HashMap, HashSet, VecDeque}; use std::hash::{Hash, Hasher}; -use std::ffi::OsStr; +use std::ffi::{OsStr, OsString}; use std::path::{Path, PathBuf}; use std::sync::OnceLock; use std::time::{Duration, Instant}; @@ -2764,7 +2764,123 @@ fn terminate_shell_process_tree(child: tauri_plugin_shell::process::CommandChild // direct kill is still needed when the root was not visible in the // snapshot, but its result is cleanup telemetry, not the operation's // primary error. - let _ = child.kill(); + if let Err(error) = child.kill() { + log::warn!("failed to signal cancelled media process: {error}"); + } +} + +async fn receive_media_termination_after_cancel( + rx: &mut tauri::async_runtime::Receiver, + stdout_buffer: &mut String, + stderr_buffer: &mut String, + final_output_path: &mut Option, + stderr_tail: &mut String, + stderr_tail_limit: usize, +) -> Result { + loop { + match rx.recv().await { + Some(tauri_plugin_shell::process::CommandEvent::Stdout(bytes)) => { + let chunk = String::from_utf8_lossy(&bytes); + for line in drain_media_output_lines(stdout_buffer, &chunk) { + if parse_media_progress_line(&line).is_none() { + capture_media_output_path(final_output_path, &line); + } + } + } + Some(tauri_plugin_shell::process::CommandEvent::Stderr(bytes)) => { + let chunk = String::from_utf8_lossy(&bytes); + append_bounded_stderr_tail(stderr_tail, &chunk, stderr_tail_limit); + let _ = drain_media_output_lines(stderr_buffer, &chunk); + } + Some(tauri_plugin_shell::process::CommandEvent::Terminated(payload)) => { + return Ok(payload); + } + Some(tauri_plugin_shell::process::CommandEvent::Error(_)) | None => { + return Err("could not confirm media process termination".to_string()); + } + Some(_) => {} + } + } +} + +async fn wait_for_media_termination_after_cancel( + rx: &mut tauri::async_runtime::Receiver, + stdout_buffer: &mut String, + stderr_buffer: &mut String, + final_output_path: &mut Option, + stderr_tail: &mut String, + stderr_tail_limit: usize, + grace_period: Duration, +) -> Result, String> { + match tokio::time::timeout( + grace_period, + receive_media_termination_after_cancel( + rx, + stdout_buffer, + stderr_buffer, + final_output_path, + stderr_tail, + stderr_tail_limit, + ), + ) + .await + { + Ok(result) => result.map(Some), + Err(_) => Ok(None), + } +} + +async fn terminate_and_wait_for_media_after_cancel( + rx: &mut tauri::async_runtime::Receiver, + stdout_buffer: &mut String, + stderr_buffer: &mut String, + final_output_path: &mut Option, + stderr_tail: &mut String, + stderr_tail_limit: usize, + grace_period: Duration, + terminate: F, +) -> Result +where + F: FnOnce(), +{ + match wait_for_media_termination_after_cancel( + rx, + stdout_buffer, + stderr_buffer, + final_output_path, + stderr_tail, + stderr_tail_limit, + grace_period, + ) + .await + { + Ok(Some(payload)) => Ok(payload), + Ok(None) => { + terminate(); + receive_media_termination_after_cancel( + rx, + stdout_buffer, + stderr_buffer, + final_output_path, + stderr_tail, + stderr_tail_limit, + ) + .await + } + Err(error) => { + terminate(); + receive_media_termination_after_cancel( + rx, + stdout_buffer, + stderr_buffer, + final_output_path, + stderr_tail, + stderr_tail_limit, + ) + .await + .map_err(|_| error) + } + } } async fn shell_command_output_with_timeout( @@ -4325,22 +4441,36 @@ mod pending_tray_action_tests { struct Aria2DaemonGuard { child: Mutex>, + restart_lock: tokio::sync::Mutex<()>, startup_error: Mutex>, last_stderr: Mutex, config_path: Mutex>, + launch_spec: Mutex>, route_contract_available: AtomicBool, shutdown_state: AtomicU8, + shutdown_journal_failures: Mutex>, +} + +#[derive(Clone)] +struct Aria2LaunchSpec { + binary_path: PathBuf, + args: Vec, + environment: Vec<(OsString, Option)>, + candidate_ports: Vec, } impl Aria2DaemonGuard { fn new() -> Self { Self { child: Mutex::new(None), + restart_lock: tokio::sync::Mutex::new(()), startup_error: Mutex::new(None), last_stderr: Mutex::new(String::new()), config_path: Mutex::new(None), + launch_spec: Mutex::new(None), route_contract_available: AtomicBool::new(false), shutdown_state: AtomicU8::new(0), + shutdown_journal_failures: Mutex::new(HashMap::new()), } } @@ -4360,10 +4490,79 @@ impl Aria2DaemonGuard { .clone() } - fn exit_allowed(&self) -> bool { + pub(crate) fn exit_allowed(&self) -> bool { self.shutdown_state.load(Ordering::SeqCst) == 2 } + pub(crate) fn is_shutting_down(&self) -> bool { + self.shutdown_state.load(Ordering::SeqCst) == 1 + } + + pub(crate) fn mark_shutdown_journal_failed(&self, id: &str, lifecycle_generation: u64) { + self.shutdown_journal_failures + .lock() + .unwrap_or_else(|error| error.into_inner()) + .insert(id.to_string(), lifecycle_generation); + } + + fn shutdown_journal_failed(&self) -> bool { + !self + .shutdown_journal_failures + .lock() + .unwrap_or_else(|error| error.into_inner()) + .is_empty() + } + + fn clear_shutdown_journal_failure(&self) { + self.shutdown_journal_failures + .lock() + .unwrap_or_else(|error| error.into_inner()) + .clear(); + } + + fn clear_shutdown_journal_failure_for(&self, id: &str) { + self.shutdown_journal_failures + .lock() + .unwrap_or_else(|error| error.into_inner()) + .remove(id); + } + + fn shutdown_journal_failures(&self) -> Vec<(String, u64)> { + self.shutdown_journal_failures + .lock() + .unwrap_or_else(|error| error.into_inner()) + .iter() + .map(|(id, generation)| (id.clone(), *generation)) + .collect() + } + + fn set_launch_spec(&self, spec: Aria2LaunchSpec) { + *self + .launch_spec + .lock() + .unwrap_or_else(|error| error.into_inner()) = Some(spec); + } + + fn launch_spec(&self) -> Option { + self.launch_spec + .lock() + .unwrap_or_else(|error| error.into_inner()) + .clone() + } + + /// Update a restart argument after a successful live option change. + /// Callers hold `restart_lock` across the RPC and this snapshot update so + /// a daemon restart cannot launch from an older setting. + fn update_launch_option(&self, option: &str, value: &str) { + let mut launch_spec = self + .launch_spec + .lock() + .unwrap_or_else(|error| error.into_inner()); + if let Some(spec) = launch_spec.as_mut() { + spec.args = set_aria2_option_argument(&spec.args, option, value); + } + } + fn begin_shutdown(&self) -> bool { self.shutdown_state .compare_exchange(0, 1, Ordering::SeqCst, Ordering::SeqCst) @@ -4407,36 +4606,56 @@ async fn shutdown_aria2_daemon(app_handle: tauri::AppHandle) { let child = guard.child.lock().ok().and_then(|mut child| child.take()); if let Some(mut child) = child { - let child_wait = tokio::task::spawn_blocking(move || { - // Aria2 1.37.0 intentionally schedules an RPC-requested shutdown - // three seconds after replying. Leave a bounded margin for that - // timer and the subsequent state flush before forcing termination. - let child_wait_started = Instant::now(); - let deadline = std::time::Instant::now() + std::time::Duration::from_secs(8); - let outcome = loop { - match child.try_wait() { - Ok(Some(_)) => break "exited", - Ok(None) if std::time::Instant::now() < deadline => { - std::thread::sleep(std::time::Duration::from_millis(50)); - } - _ => { + // Aria2 1.37.0 intentionally schedules an RPC-requested shutdown + // three seconds after replying. Leave a bounded margin for that + // timer and the subsequent state flush before forcing termination. + // Keep ownership here until the OS confirms exit. A timed-out + // spawn_blocking join would detach the waiter and permit a second + // daemon to start while the original process still owns its files. + let child_wait_started = Instant::now(); + let graceful_deadline = tokio::time::Instant::now() + Duration::from_secs(8); + let mut forced = false; + let mut last_kill_attempt = tokio::time::Instant::now(); + loop { + match child.try_wait() { + Ok(Some(_)) => break, + Ok(None) => { + if !forced && tokio::time::Instant::now() >= graceful_deadline { + match child.kill() { + Ok(()) => forced = true, + Err(error) => log::warn!( + "app_shutdown_phase=aria2_child kill request failed: {error}" + ), + } + last_kill_attempt = tokio::time::Instant::now(); + } else if forced + && tokio::time::Instant::now().duration_since(last_kill_attempt) + >= Duration::from_secs(1) + { + // A successful kill is still followed by try_wait; + // do not clear the published port until the process + // handle confirms that the old daemon has exited. let _ = child.kill(); - let _ = child.wait(); - break "forced"; + last_kill_attempt = tokio::time::Instant::now(); } } - }; - (outcome, child_wait_started.elapsed()) - }); - match tokio::time::timeout(std::time::Duration::from_secs(10), child_wait).await { - Ok(Ok((outcome, elapsed))) => log::info!( - "app_shutdown_phase=aria2_child outcome={} elapsed_ms={}", - outcome, - elapsed.as_millis() - ), - Ok(Err(error)) => log::warn!("aria2 child wait task failed: {error}"), - Err(_) => log::warn!("app_shutdown_phase=aria2_child outcome=timeout"), + Err(error) => { + log::warn!("app_shutdown_phase=aria2_child wait check failed: {error}"); + let _ = child.kill(); + forced = true; + last_kill_attempt = tokio::time::Instant::now(); + } + } + tokio::time::sleep(Duration::from_millis(50)).await; } + log::info!( + "app_shutdown_phase=aria2_child outcome={} elapsed_ms={}", + if forced { "forced" } else { "exited" }, + child_wait_started.elapsed().as_millis() + ); + } + if let Some(state) = app_handle.try_state::() { + state.aria2_port.store(0, Ordering::Release); } log::info!( "app_shutdown_phase=aria2_total elapsed_ms={}", @@ -4444,8 +4663,231 @@ async fn shutdown_aria2_daemon(app_handle: tauri::AppHandle) { ); } -fn reject_app_exit_request(app: &tauri::AppHandle, request: u64, reason: &str) { - app.state::().cancel_shutdown(); +fn monitor_aria2_stderr(app: tauri::AppHandle, stderr: std::process::ChildStderr) { + std::thread::spawn(move || { + use std::io::BufRead; + let reader = std::io::BufReader::new(stderr); + for line in reader.lines().map_while(Result::ok) { + let trimmed = line.trim().to_string(); + if let Ok(mut stderr_lock) = app.state::().last_stderr.lock() { + stderr_lock.push_str(&trimmed); + stderr_lock.push('\n'); + let excess = stderr_lock.len().saturating_sub(8192); + if excess > 0 { + let _ = stderr_lock.drain(..excess); + } + } + let lower = trimmed.to_lowercase(); + if lower.contains("error") || lower.contains("critical") { + log::error!("aria2c stderr: {}", trimmed); + } + } + }); +} + +fn set_aria2_option_argument(args: &[OsString], option: &str, value: &str) -> Vec { + let mut updated = args.to_vec(); + let prefix = format!("{option}="); + let replacement = OsString::from(format!("{option}={value}")); + for argument in &mut updated { + if argument + .to_str() + .is_some_and(|argument| argument.starts_with(&prefix)) + { + *argument = replacement; + return updated; + } + } + updated.push(replacement); + updated +} + +fn set_aria2_rpc_port_argument(args: &[OsString], port: u16) -> Vec { + set_aria2_option_argument(args, "--rpc-listen-port", &port.to_string()) +} + +async fn restart_aria2_daemon(app_handle: &tauri::AppHandle) -> Result<(), String> { + let guard = app_handle.state::(); + let _restart_guard = guard.restart_lock.lock().await; + if app_handle + .state::() + .aria2_port + .load(Ordering::Acquire) + != 0 + { + return Ok(()); + } + let spec = guard + .launch_spec() + .ok_or_else(|| "aria2 restart configuration is unavailable".to_string())?; + let config_path_available = guard + .config_path + .lock() + .unwrap_or_else(|error| error.into_inner()) + .as_ref() + .is_some_and(|path| path.exists()); + if !config_path_available { + return Err("aria2 restart configuration has expired".to_string()); + } + + let restart_deadline = tokio::time::Instant::now() + Duration::from_secs(15); + let mut last_error = "no RPC port could be restarted".to_string(); + for port in spec.candidate_ports.iter().copied() { + if tokio::time::Instant::now() >= restart_deadline { + break; + } + if aria2_rpc_port_is_occupied(port) { + continue; + } + let mut command = std::process::Command::new(&spec.binary_path); + crate::platform::hide_child_console(&mut command); + command.args(set_aria2_rpc_port_argument(&spec.args, port)); + for (key, value) in &spec.environment { + match value { + Some(value) => { + command.env(key, value); + } + None => { + command.env_remove(key); + } + } + } + command + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::piped()); + let mut child = match command.spawn() { + Ok(child) => child, + Err(error) => { + last_error = format!("failed to restart aria2 on RPC port {port}: {error}"); + continue; + } + }; + tokio::time::sleep( + Duration::from_millis(200).min( + restart_deadline.saturating_duration_since(tokio::time::Instant::now()), + ), + ) + .await; + if let Ok(Some(status)) = child.try_wait() { + use std::io::Read; + let mut stderr = String::new(); + if let Some(stderr_pipe) = child.stderr.take() { + let _ = stderr_pipe.take(4096).read_to_string(&mut stderr); + } + last_error = if stderr.trim().is_empty() { + format!("aria2 exited during restart on RPC port {port} with status {status}") + } else { + format!("aria2 exited during restart on RPC port {port}: {}", stderr.trim()) + }; + continue; + } + + if let Some(stderr) = child.stderr.take() { + monitor_aria2_stderr(app_handle.clone(), stderr); + } + *guard.child.lock().unwrap_or_else(|error| error.into_inner()) = Some(child); + let args = set_aria2_rpc_port_argument(&spec.args, port); + guard.set_launch_spec(Aria2LaunchSpec { + binary_path: spec.binary_path.clone(), + args, + environment: spec.environment.clone(), + candidate_ports: spec.candidate_ports.clone(), + }); + + let ready_started = tokio::time::Instant::now(); + let mut last_rpc_error = String::new(); + let readiness_deadline = (ready_started + Duration::from_secs(5)).min(restart_deadline); + while tokio::time::Instant::now() < readiness_deadline { + match rpc_call(port, &app_handle.state::().aria2_secret, "aria2.getVersion", serde_json::json!([])).await { + Ok(version) if crate::network::aria2_baseline_error(&version).is_none() => { + guard.set_route_contract_available( + crate::network::aria2_route_capability_error(&version).is_none(), + ); + guard + .startup_error + .lock() + .unwrap_or_else(|error| error.into_inner()) + .take(); + app_handle + .state::() + .aria2_port + .store(port, Ordering::Release); + log::info!("aria2 daemon restarted successfully on port {port}"); + return Ok(()); + } + Ok(version) => { + last_rpc_error = crate::network::aria2_baseline_error(&version) + .unwrap_or_else(|| "aria2 returned an invalid readiness response".to_string()); + } + Err(error) => last_rpc_error = error, + } + tokio::time::sleep(Duration::from_millis(100)).await; + } + last_error = format!("aria2 restart did not become ready on port {port}: {last_rpc_error}"); + let child = { + guard + .child + .lock() + .unwrap_or_else(|error| error.into_inner()) + .take() + }; + if let Some(child) = child { + let cleanup = tokio::task::spawn_blocking(move || { + let mut child = child; + let kill_error = child.kill().err(); + child.wait().map_err(|wait_error| { + format!( + "could not confirm aria2 candidate on port {port} exited after restart failure: {}; {}", + wait_error, + kill_error + .map(|error| format!("kill request also failed: {error}")) + .unwrap_or_else(|| "kill request succeeded".to_string()) + ) + }) + }) + .await + .map_err(|error| format!("aria2 restart cleanup task failed: {error}"))?; + if let Err(error) = cleanup { + return Err(error); + } + } + } + app_handle.state::().aria2_port.store(0, Ordering::Release); + guard.set_route_contract_available(false); + *guard + .startup_error + .lock() + .unwrap_or_else(|error| error.into_inner()) = Some(last_error.clone()); + Err(last_error) +} + +async fn reject_app_exit_request( + app: &tauri::AppHandle, + request: u64, + reason: &str, + interrupted_media_ids: &[String], +) { + let was_shutting_down = app.state::().cancel_shutdown(); + if was_shutting_down { + app.state::() + .queue_manager + .wake_after_exit_rejected(); + } + let rejected_media_registrations = app.state::() + .download_coordinator + .reopen_media_registration() + .await; + let mut interrupted_media_ids = interrupted_media_ids.to_vec(); + interrupted_media_ids.extend( + rejected_media_registrations + .into_iter() + .map(|(id, _generation)| id), + ); + app.state::() + .shutdown_terminal_barrier + .reopen_after_rejection(); + interrupted_media_ids.sort(); + interrupted_media_ids.dedup(); let tray_action_pending = app .state::() .reopen_after_exit_rejected(); @@ -4459,12 +4901,49 @@ fn reject_app_exit_request(app: &tauri::AppHandle, request: u64, reason: &str) { "app-exit-cancelled", serde_json::json!({ "requestId": request.to_string(), - "reason": reason + "reason": reason, + "interruptedMediaIds": interrupted_media_ids }), ); restore_main_window(app); } +fn shutdown_finalization_allowed( + shutdown_journal_failed: bool, + final_frontend_flush_succeeded: bool, + failed_rows_are_durably_completed: bool, +) -> bool { + !shutdown_journal_failed + || (final_frontend_flush_succeeded && failed_rows_are_durably_completed) +} + +fn shutdown_failed_rows_are_durably_completed( + guard: &Aria2DaemonGuard, + database: Option<&crate::db::DbState>, +) -> bool { + let failures = guard.shutdown_journal_failures(); + if failures.is_empty() { + return true; + } + let Some(database) = database else { + return false; + }; + failures.into_iter().all(|(id, generation)| { + let verified = database + .lock() + .and_then(|connection| crate::db::is_download_durably_completed(&connection, &id)) + .unwrap_or(false); + if !verified { + log::error!( + "shutdown completion fallback is not durable [id={} lifecycle_generation={}]", + id, + generation + ); + } + verified + }) +} + impl Drop for Aria2DaemonGuard { fn drop(&mut self) { if let Ok(mut lock) = self.child.lock() { @@ -4526,12 +5005,110 @@ pub struct AppState { frontend_exit_flush: Arc, pub aria2_port: std::sync::Arc, pub aria2_secret: String, + pub(crate) shutdown_terminal_barrier: Arc, pub media_semaphore: Arc, pub power_manager: Arc, pub scheduler_settings: Arc>>, pub queue_manager: Arc, } +#[derive(Default)] +struct ShutdownTerminalState { + sealed: bool, + exiting: bool, + active: usize, + late_writer: bool, +} + +#[derive(Default)] +pub(crate) struct ShutdownTerminalBarrier { + state: Mutex, + changed: tokio::sync::Notify, +} + +pub(crate) struct ShutdownTerminalLease { + barrier: Arc, +} + +impl ShutdownTerminalBarrier { + pub(crate) async fn enter(self: &Arc) -> Option { + loop { + let notified = self.changed.notified(); + tokio::pin!(notified); + notified.as_mut().enable(); + { + let mut state = self.state.lock().unwrap_or_else(|error| error.into_inner()); + if state.exiting { + return None; + } + if !state.sealed { + state.active += 1; + return Some(ShutdownTerminalLease { + barrier: Arc::clone(self), + }); + } + state.late_writer = true; + } + notified.await; + } + } + + async fn seal_and_wait(&self) { + { + let mut state = self.state.lock().unwrap_or_else(|error| error.into_inner()); + state.sealed = true; + } + loop { + let notified = self.changed.notified(); + tokio::pin!(notified); + notified.as_mut().enable(); + if self + .state + .lock() + .unwrap_or_else(|error| error.into_inner()) + .active + == 0 + { + return; + } + notified.await; + } + } + + fn try_finalize(&self) -> bool { + let mut state = self.state.lock().unwrap_or_else(|error| error.into_inner()); + if !state.sealed || state.active != 0 || state.late_writer { + return false; + } + state.exiting = true; + drop(state); + self.changed.notify_waiters(); + true + } + + fn reopen_after_rejection(&self) { + let mut state = self.state.lock().unwrap_or_else(|error| error.into_inner()); + state.sealed = false; + state.exiting = false; + state.late_writer = false; + drop(state); + self.changed.notify_waiters(); + } +} + +impl Drop for ShutdownTerminalLease { + fn drop(&mut self) { + let mut state = self + .barrier + .state + .lock() + .unwrap_or_else(|error| error.into_inner()); + state.active = state.active.saturating_sub(1); + drop(state); + self.barrier.changed.notify_waiters(); + } +} + #[derive(Default)] struct MainWindowRestoreState { requested: AtomicBool, @@ -6105,6 +6682,7 @@ pub(crate) async fn start_media_download_internal( let (mut rx, child) = cmd .spawn() .map_err(|e| format!("Failed to spawn yt-dlp: {}", e))?; + let mut child = Some(child); if strike > 0 { // The backoff path emits `Retrying`. Restore the live transfer // state when the replacement process actually starts so React @@ -6122,16 +6700,49 @@ pub(crate) async fn start_media_download_internal( let mut stdout_buffer = String::new(); let mut stderr_buffer = String::new(); let failure_reason = loop { - tokio::select! { + let event = tokio::select! { _ = cancel_rx.changed() => { - terminate_shell_process_tree(child); - if processing_started { - cleanup_media_processing_artifacts(&app_handle, id, &out_path).await; + // The shell plugin queues Terminated after draining both + // output pipes. Give an already-finishing child a bounded + // chance to report successful exit before killing it, so + // shutdown cancellation cannot turn a completed file into + // an interrupted row that startup requeues. + let termination = terminate_and_wait_for_media_after_cancel( + &mut rx, + &mut stdout_buffer, + &mut stderr_buffer, + &mut final_output_path, + &mut stderr_tail, + STDERR_TAIL, + Duration::from_secs(1), + || { + if let Some(child) = child.take() { + terminate_shell_process_tree(child); + } + }, + ) + .await; + let payload = match termination { + Ok(payload) => payload, + Err(error) => { + if processing_started { + cleanup_media_processing_artifacts(&app_handle, id, &out_path).await; + } + return Err(error); + } + }; + if payload.code == Some(0) { + Some(tauri_plugin_shell::process::CommandEvent::Terminated(payload)) + } else { + if processing_started { + cleanup_media_processing_artifacts(&app_handle, id, &out_path).await; + } + return Err(crate::queue::MEDIA_RUN_CANCELLED.to_string()); } - return Err(crate::queue::MEDIA_RUN_CANCELLED.to_string()); } - event = rx.recv() => { - match event { + event = rx.recv() => event, + }; + match event { Some(tauri_plugin_shell::process::CommandEvent::Stdout(line_bytes)) => { let chunk = String::from_utf8_lossy(&line_bytes); for line in drain_media_output_lines(&mut stdout_buffer, &chunk) { @@ -6285,8 +6896,6 @@ pub(crate) async fn start_media_download_internal( }; } } - } - } }; let transient = is_transient_network_error(&failure_reason); @@ -6348,6 +6957,42 @@ pub(crate) async fn start_media_download_internal( Err("yt-dlp retry loop exhausted".to_string()) } +/// Cancel a media runner without holding the per-download control lock while +/// waiting for its terminal acknowledgement. The runner needs that same lock +/// to arbitrate a successful completion against pause, remove, or reconfigure. +async fn cancel_media_and_wait_for_finish( + state: &AppState, + id: &str, + lifecycle_generation: u64, + control_guard: &mut queue::Aria2ControlGuard, +) -> Result { + control_guard.release(); + let result = async { + let (ack_tx, ack_rx) = tokio::sync::oneshot::channel(); + state + .download_coordinator + .pause_media_with_ack(id.to_string(), lifecycle_generation, ack_tx) + .await?; + ack_rx + .await + .map_err(|_| "download worker stopped without acknowledging pause".to_string()) + } + .await; + control_guard.reacquire().await; + + let media_was_registered = result?; + if !media_was_registered { + // No runner registered for this generation. Retire the cancellation + // tombstone only after registration and lifecycle control are fenced + // again by the reacquired lock. + state + .download_coordinator + .finish_media(id.to_string(), lifecycle_generation) + .await; + } + Ok(media_was_registered) +} + #[tauri::command] async fn pause_download( caller: tauri::WebviewWindow, @@ -6358,7 +7003,7 @@ async fn pause_download( properties_window::ensure_main_window(&caller)?; log::info!("pause_download called for id: {}", id); - let _control_guard = state.queue_manager.acquire_aria2_control(&id).await; + let mut control_guard = state.queue_manager.acquire_aria2_control(&id).await; removal_jobs::ensure_not_removing(&app_handle, &id)?; let active_kind = state.queue_manager.active_kind(&id).await; let registered_lifecycle_generation = state @@ -6594,26 +7239,54 @@ async fn pause_download( state.queue_manager.clear_aria2_retry_state(&id).await; } - let (tx, rx) = tokio::sync::oneshot::channel(); if matches!(active_kind, Some(crate::queue::TaskKind::Media)) { - state - .download_coordinator - .pause_media_with_ack(id.clone(), media_lifecycle_generation, tx) - .await?; - } else { - let _ = tx.send(false); - } - let media_was_registered = rx - .await - .map_err(|_| "download worker stopped without acknowledging pause".to_string())?; - if matches!(active_kind, Some(crate::queue::TaskKind::Media)) && !media_was_registered { - // No media runner reached the coordinator. The queue control lock is - // still held, and run_media now checks the same lifecycle generation - // before registration, so this tombstone can be retired safely. - state - .download_coordinator - .finish_media(id.clone(), media_lifecycle_generation) + let _ = cancel_media_and_wait_for_finish( + state.inner(), + &id, + media_lifecycle_generation, + &mut control_guard, + ) + .await?; + + // A successful media process records this marker before acknowledging + // Finished. If it won the race, leave terminal reconciliation to the + // worker instead of downgrading the completed row to Paused. + let current_generation = state + .queue_manager + .registered_lifecycle_generation(&id) .await; + let current_kind = state.queue_manager.active_kind(&id).await; + let terminal_outcome = state + .queue_manager + .media_terminal_outcome(&id, media_lifecycle_generation) + .await; + let lifecycle_is_current = current_generation == registered_lifecycle_generation + && matches!(current_kind, Some(crate::queue::TaskKind::Media)); + if !lifecycle_is_current { + if terminal_outcome == Some(crate::queue::MediaTerminalOutcome::Cancelled) + && current_generation.is_none() + && current_kind.is_none() + { + // The worker completed cancellation and released ownership + // before this pause reacquired the lock. Preserve the user's + // pause request without reopening a stale lifecycle. + state + .queue_manager + .emit_state(id.clone(), crate::ipc::DownloadStatus::Paused); + return Ok(true); + } + return Ok(false); + } + if matches!( + terminal_outcome, + Some(crate::queue::MediaTerminalOutcome::Completed | crate::queue::MediaTerminalOutcome::Failed) + ) || state + .queue_manager + .media_payload_completed(&id, media_lifecycle_generation) + .await + { + return Ok(false); + } } let seed_remaining = if state.queue_manager.is_seed_owner(&id) { @@ -7420,52 +8093,78 @@ async fn remove_download_inner( // removal so its late result cannot resurrect the download. let removal_epoch = state.queue_manager.next_aria2_control_epoch(&id).await; state.queue_manager.cancel_aria2_retries(&id).await; - let (tx, rx) = tokio::sync::oneshot::channel(); if matches!(active_kind, Some(crate::queue::TaskKind::Media)) { - state - .download_coordinator - .pause_media_with_ack(id.clone(), media_lifecycle_generation, tx) - .await?; - } else { - let _ = tx.send(false); - } - let media_was_registered = rx.await.unwrap_or(false); - if matches!(active_kind, Some(crate::queue::TaskKind::Media)) && !media_was_registered { - state - .download_coordinator - .finish_media(id.clone(), media_lifecycle_generation) - .await; - } + let guard = control_guard + .as_mut() + .expect("remove lifecycle guard is held before media cancellation"); + let _ = cancel_media_and_wait_for_finish( + state.inner(), + &id, + media_lifecycle_generation, + guard, + ) + .await?; - // The initial addUri call intentionally runs outside the control lock. - // Do not delete the guessed magnet path and ownership record until a - // late GID has been removed; otherwise the daemon can finish creating - // an output after cleanup and leave an untracked file behind. - drop(control_guard.take()); - state.queue_manager.wait_for_aria2_dispatch(&id).await; - let reacquired_control_guard = state.queue_manager.acquire_aria2_control(&id).await; - let current_generation = state - .queue_manager - .registered_lifecycle_generation(&id) - .await; - let current_active_kind = state.queue_manager.active_kind(&id).await; - let lifecycle_changed = registered_lifecycle_generation.is_some() - && current_generation != registered_lifecycle_generation - || registered_lifecycle_generation.is_none() && current_generation.is_some(); - let late_native_lifecycle = !native_lifecycle_was_admitted - && current_active_kind.is_some(); - if lifecycle_changed || late_native_lifecycle { + let current_generation = state + .queue_manager + .registered_lifecycle_generation(&id) + .await; + let current_active_kind = state.queue_manager.active_kind(&id).await; + let terminal_outcome = state + .queue_manager + .media_terminal_outcome(&id, media_lifecycle_generation) + .await; + let newer_lifecycle = current_generation + .is_some_and(|generation| Some(generation) != registered_lifecycle_generation) + || current_generation.is_none() && current_active_kind.is_some(); + if newer_lifecycle { + return Err("download lifecycle changed while waiting for media shutdown".to_string()); + } + if terminal_outcome == Some(crate::queue::MediaTerminalOutcome::Completed) { + permanent_asset_removal = false; + } + + // Finished has drained the media writer. Keep the reacquired lock + // through ownership lookup and cleanup so a newer lifecycle + // cannot claim the output path while removal is in progress. state .queue_manager .release_permit_for_generation(&id, media_lifecycle_generation) .await; state .queue_manager - .clear_aria2_allocation_for_epoch(&id, removal_epoch) + .release_registered_id_for_generation(&id, media_lifecycle_generation) .await; - return Err("download lifecycle changed while waiting for aria2 dispatch".to_string()); - } - if let Some(late_gid) = state.queue_manager.aria2_gid_for_download(&id) { + cleanup_control_guard = control_guard.take(); + } else { + // The initial addUri call intentionally runs outside the control + // lock. Do not delete a guessed path until any late GID has been + // removed, or Aria2 could recreate an untracked output. + drop(control_guard.take()); + state.queue_manager.wait_for_aria2_dispatch(&id).await; + let reacquired_control_guard = state.queue_manager.acquire_aria2_control(&id).await; + let current_generation = state + .queue_manager + .registered_lifecycle_generation(&id) + .await; + let current_active_kind = state.queue_manager.active_kind(&id).await; + let lifecycle_changed = registered_lifecycle_generation.is_some() + && current_generation != registered_lifecycle_generation + || registered_lifecycle_generation.is_none() && current_generation.is_some(); + let late_native_lifecycle = !native_lifecycle_was_admitted + && current_active_kind.is_some(); + if lifecycle_changed || late_native_lifecycle { + state + .queue_manager + .release_permit_for_generation(&id, media_lifecycle_generation) + .await; + state + .queue_manager + .clear_aria2_allocation_for_epoch(&id, removal_epoch) + .await; + return Err("download lifecycle changed while waiting for aria2 dispatch".to_string()); + } + if let Some(late_gid) = state.queue_manager.aria2_gid_for_download(&id) { if let Err(error) = state .queue_manager .reconcile_aria2_torrent_ownership(&id, &late_gid) @@ -7512,21 +8211,22 @@ async fn remove_download_inner( state.queue_manager.clear_aria2_retry_state(&id).await; state.queue_manager.forget_torrent_telemetry(&id).await; state.queue_manager.forget_aria2_gid(&id).await; - } else if !state - .queue_manager - .is_aria2_control_epoch_current(&id, removal_epoch) - .await - { - // A same-generation pause/resume control action may advance the - // epoch while the late add is being retired. Removal still owns - // the registered row, so fence that action before cleaning it. - state.queue_manager.next_aria2_control_epoch(&id).await; + } else if !state + .queue_manager + .is_aria2_control_epoch_current(&id, removal_epoch) + .await + { + // A same-generation pause/resume control action may advance the + // epoch while the late add is being retired. Removal still owns + // the registered row, so fence that action before cleaning it. + state.queue_manager.next_aria2_control_epoch(&id).await; + } + state.queue_manager.release_permit(&id).await; + state.queue_manager.clear_aria2_retry_state(&id).await; + state.queue_manager.forget_torrent_telemetry(&id).await; + state.queue_manager.forget_aria2_gid(&id).await; + cleanup_control_guard = Some(reacquired_control_guard); } - state.queue_manager.release_permit(&id).await; - state.queue_manager.clear_aria2_retry_state(&id).await; - state.queue_manager.forget_torrent_telemetry(&id).await; - state.queue_manager.forget_aria2_gid(&id).await; - cleanup_control_guard = Some(reacquired_control_guard); } // Keep lifecycle ownership fenced while the exact owned paths and the @@ -7734,6 +8434,12 @@ async fn remove_download_inner( log::info!("download removal [id={} stage=cleanup success={} elapsed_ms={}]", id, cleanup_result.is_ok(), removal_cleanup_started.elapsed().as_millis()); state.queue_manager.release_registered_id(&id).await; + if cleanup_result.is_ok() { + state + .queue_manager + .clear_media_terminal_outcome(&id, media_lifecycle_generation) + .await; + } cleanup_result } @@ -9151,13 +9857,13 @@ async fn detach_download_for_reconfigure( ) -> Result<(), String> { properties_window::ensure_main_window(&caller)?; log::info!("detach_download_for_reconfigure called for id: {}", id); - let control_guard = state.queue_manager.acquire_aria2_control(&id).await; + let mut control_guard = state.queue_manager.acquire_aria2_control(&id).await; removal_jobs::ensure_not_removing(&app_handle, &id)?; detach_download_for_reconfigure_locked( &app_handle, state.inner(), &id, - &control_guard, + &mut control_guard, ) .await } @@ -9166,7 +9872,7 @@ async fn detach_download_for_reconfigure_locked( app_handle: &tauri::AppHandle, state: &AppState, id: &str, - _control_guard: &queue::Aria2ControlGuard, + control_guard: &mut queue::Aria2ControlGuard, ) -> Result<(), String> { let active_kind = state.queue_manager.active_kind(id).await; let media_lifecycle_generation = state @@ -9219,27 +9925,46 @@ async fn detach_download_for_reconfigure_locked( // Invalidate a queued/addUri lifecycle that has not published a GID. state.queue_manager.next_aria2_control_epoch(id).await; state.queue_manager.cancel_aria2_retries(id).await; - let (tx, rx) = tokio::sync::oneshot::channel(); if matches!(active_kind, Some(crate::queue::TaskKind::Media)) { - state - .download_coordinator - .pause_media_with_ack(id.to_string(), media_lifecycle_generation, tx) - .await?; - } else { - let _ = tx.send(false); // Fallback if no task exists - } - let media_was_registered = rx.await.unwrap_or(false); // Wait for the writer to stop - if matches!(active_kind, Some(crate::queue::TaskKind::Media)) && !media_was_registered { - state - .download_coordinator - .finish_media(id.to_string(), media_lifecycle_generation) + let _ = cancel_media_and_wait_for_finish( + state, + id, + media_lifecycle_generation, + control_guard, + ) + .await?; + let terminal_outcome = state + .queue_manager + .media_terminal_outcome(id, media_lifecycle_generation) .await; + if matches!( + terminal_outcome, + Some(crate::queue::MediaTerminalOutcome::Completed | crate::queue::MediaTerminalOutcome::Failed) + ) { + return Err("media lifecycle reached a terminal result while detaching".to_string()); + } + let current_generation = state + .queue_manager + .registered_lifecycle_generation(id) + .await; + let current_active_kind = state.queue_manager.active_kind(id).await; + if current_generation.is_some_and(|generation| generation != media_lifecycle_generation) + || current_generation.is_none() && current_active_kind.is_some() + { + return Err("download lifecycle changed while waiting for media shutdown".to_string()); + } } - state.queue_manager.release_permit(id).await; + state + .queue_manager + .release_permit_for_generation(id, media_lifecycle_generation) + .await; state.queue_manager.clear_aria2_retry_state(id).await; state.queue_manager.forget_aria2_gid(id).await; - state.queue_manager.release_registered_id(id).await; + state + .queue_manager + .release_registered_id_for_generation(id, media_lifecycle_generation) + .await; } use tauri::Emitter; @@ -9390,11 +10115,12 @@ async fn verify_aria2_resume_status_snapshot( last_observation } -async fn reconcile_aria2_downloads(app_handle: &tauri::AppHandle) { +async fn reconcile_aria2_downloads(app_handle: &tauri::AppHandle) -> bool { let state = app_handle.state::(); let port = state.aria2_port.load(std::sync::atomic::Ordering::Relaxed); let secret = state.aria2_secret.clone(); let mappings = state.queue_manager.aria2_gid_mappings(); + let mut reconciled = true; for (gid, id) in mappings { let status = match rpc_call( @@ -9407,6 +10133,7 @@ async fn reconcile_aria2_downloads(app_handle: &tauri::AppHandle) { { Ok(status) => status, Err(error) => { + reconciled = false; log::debug!( "aria2 reconnect reconciliation [{}]: could not query gid {}: {}", id, @@ -9431,6 +10158,7 @@ async fn reconcile_aria2_downloads(app_handle: &tauri::AppHandle) { .refresh_aria2_connections(&id, &gid, mapping.epoch) .await { + reconciled = false; log::warn!( "aria2 reconnect reconciliation [{}]: payload recovery for missing gid {} failed: {}", id, @@ -9445,6 +10173,10 @@ async fn reconcile_aria2_downloads(app_handle: &tauri::AppHandle) { }; let status_name = status.get("status").and_then(|value| value.as_str()); + if status_name.is_none() { + reconciled = false; + continue; + } let outcome = match status_name { Some("complete") => Some(crate::queue::PendingOutcome::Complete), Some("active") @@ -9473,17 +10205,22 @@ async fn reconcile_aria2_downloads(app_handle: &tauri::AppHandle) { None => error_message.to_string(), })) } - _ => None, + Some("active") | Some("waiting") | Some("paused") => None, + _ => { + reconciled = false; + None + } }; if let Some(outcome) = outcome { let progress = aria2_download_state_progress(Some(&status)); state .queue_manager - .handle_aria2_event_with_progress(&gid, outcome, progress) + .handle_aria2_event_with_progress_under_terminal_lease(&gid, outcome, progress) .await; } } + reconciled } async fn aria2_daemon_is_reachable(port: u16, secret: &str) -> bool { @@ -12141,7 +12878,7 @@ async fn set_torrent_file_selection( selected_indices: Option>, ) -> Result { properties_window::ensure_properties_or_main(&caller, &properties, &id)?; - let control_guard = state.queue_manager.acquire_aria2_control(&id).await; + let mut control_guard = state.queue_manager.acquire_aria2_control(&id).await; removal_jobs::ensure_not_removing(&app_handle, &id)?; let item = load_persisted_torrent_item(database.inner(), &id)?; if item.is_torrent != Some(true) { @@ -12212,7 +12949,7 @@ async fn set_torrent_file_selection( &app_handle, state.inner(), &id, - &control_guard, + &mut control_guard, ) .await { @@ -13501,7 +14238,7 @@ async fn verify_torrent_data( // Verification replaces the current Aria2 lifecycle. Serialize that // replacement with pause/resume/remove so a paused GID cannot reject the // maintenance enqueue as a duplicate task or race it with a late event. - let control_guard = state.queue_manager.acquire_aria2_control(&id).await; + let mut control_guard = state.queue_manager.acquire_aria2_control(&id).await; removal_jobs::ensure_not_removing(&app_handle, &id)?; let item = load_persisted_torrent_item(database.inner(), &id)?; if item.is_torrent != Some(true) { @@ -13534,7 +14271,7 @@ async fn verify_torrent_data( &app_handle, state.inner(), &id, - &control_guard, + &mut control_guard, ) .await?; } else if state.queue_manager.is_registered(&id).await @@ -14125,6 +14862,8 @@ async fn set_torrent_max_open_files( ) -> Result<(), String> { properties_window::ensure_main_window(&caller)?; let max_open_files = queue::normalize_torrent_max_open_files(max_open_files)?; + let guard = caller.app_handle().state::(); + let _restart_guard = guard.restart_lock.lock().await; rpc_call( state.aria2_port.load(std::sync::atomic::Ordering::Relaxed), &state.aria2_secret, @@ -14133,7 +14872,9 @@ async fn set_torrent_max_open_files( ) .await .and_then(|result| ensure_aria2_ok_result("changeGlobalOption", &result)) - .map_err(|error| format!("Failed to set Torrent maximum open files: {error}")) + .map_err(|error| format!("Failed to set Torrent maximum open files: {error}"))?; + guard.update_launch_option("--bt-max-open-files", &max_open_files.to_string()); + Ok(()) } #[tauri::command] @@ -14145,6 +14886,8 @@ async fn set_torrent_overall_upload_limit( properties_window::ensure_main_window(&caller)?; let normalized_limit = normalize_torrent_overall_upload_limit(limit.as_deref())?; let limit_str = normalized_limit.as_deref().unwrap_or("0"); + let guard = caller.app_handle().state::(); + let _restart_guard = guard.restart_lock.lock().await; rpc_call( state.aria2_port.load(std::sync::atomic::Ordering::Relaxed), &state.aria2_secret, @@ -14153,7 +14896,9 @@ async fn set_torrent_overall_upload_limit( ) .await .and_then(|result| ensure_aria2_ok_result("changeGlobalOption", &result)) - .map_err(|error| format!("Failed to set Torrent overall upload limit: {error}")) + .map_err(|error| format!("Failed to set Torrent overall upload limit: {error}"))?; + guard.update_launch_option("--max-overall-upload-limit", limit_str); + Ok(()) } #[tauri::command] @@ -14171,6 +14916,8 @@ async fn set_global_speed_limit( ), }; let limit_str = normalized_limit.clone().unwrap_or_else(|| "0".to_string()); + let guard = caller.app_handle().state::(); + let _restart_guard = guard.restart_lock.lock().await; rpc_call( state.aria2_port.load(std::sync::atomic::Ordering::Relaxed), &state.aria2_secret, @@ -14179,15 +14926,15 @@ async fn set_global_speed_limit( ) .await .and_then(|result| ensure_aria2_ok_result("changeGlobalOption", &result)) - .map(|_| { - state - .queue_manager - .set_aria2_global_speed_limit(normalized_limit); - }) .map_err(|e| { eprintln!("Failed to set global speed limit: {}", e); e - }) + })?; + guard.update_launch_option("--max-overall-download-limit", &limit_str); + state + .queue_manager + .set_aria2_global_speed_limit(normalized_limit); + Ok(()) } #[cfg(target_os = "macos")] @@ -16056,6 +16803,9 @@ fn fail_extension_media_discovery( #[cfg(test)] mod tests { + use std::ffi::OsString; + use std::sync::Arc; + #[test] fn shell_fast_exit_output_fixture() { let Some(output_path) = std::env::var_os("FIRELINK_SHELL_TEST_OUTPUT_PATH") else { @@ -17322,6 +18072,189 @@ mod tests { assert!(!guard.begin_shutdown()); } + #[test] + fn shutdown_completion_journal_failure_needs_a_successful_final_frontend_flush() { + assert!(super::shutdown_finalization_allowed(false, false, false)); + assert!(!super::shutdown_finalization_allowed(true, false, true)); + assert!(!super::shutdown_finalization_allowed(true, true, false)); + assert!(super::shutdown_finalization_allowed(true, true, true)); + + let guard = Aria2DaemonGuard::new(); + assert!(guard.begin_shutdown()); + guard.mark_shutdown_journal_failed("download-1", 7); + assert!(guard.shutdown_journal_failed()); + assert_eq!(guard.shutdown_journal_failures(), vec![("download-1".into(), 7)]); + guard.clear_shutdown_journal_failure_for("download-1"); + assert!(!guard.shutdown_journal_failed()); + guard.mark_shutdown_journal_failed("download-1", 7); + guard.clear_shutdown_journal_failure(); + assert!(!guard.shutdown_journal_failed()); + } + + #[tokio::test] + async fn shutdown_terminal_barrier_drains_and_reopens_late_writers() { + let barrier = Arc::new(super::ShutdownTerminalBarrier::default()); + let active = barrier.enter().await.expect("barrier starts open"); + let drain_barrier = Arc::clone(&barrier); + let drain = tokio::spawn(async move { drain_barrier.seal_and_wait().await }); + tokio::task::yield_now().await; + assert!(!drain.is_finished(), "active terminal work must be drained"); + drop(active); + drain.await.unwrap(); + + let late_barrier = Arc::clone(&barrier); + let late_writer = tokio::spawn(async move { late_barrier.enter().await }); + tokio::task::yield_now().await; + assert!( + !barrier.try_finalize(), + "a writer arriving after sealing must reject finalization" + ); + + barrier.reopen_after_rejection(); + let resumed = late_writer + .await + .unwrap() + .expect("rejection must resume late terminal work"); + drop(resumed); + barrier.seal_and_wait().await; + assert!(barrier.try_finalize()); + assert!(barrier.enter().await.is_none()); + } + + #[tokio::test] + async fn shutdown_waits_for_the_websocket_reader_to_drain_buffered_events() { + let barrier = Arc::new(super::ShutdownTerminalBarrier::default()); + // The connection reader acquires this lease before connecting and + // retains it until its buffered messages have been processed. + let reader = barrier.enter().await.expect("reader starts tracked"); + let shutdown_barrier = Arc::clone(&barrier); + let drain = tokio::spawn(async move { shutdown_barrier.seal_and_wait().await }); + tokio::task::yield_now().await; + assert!(!drain.is_finished(), "shutdown must wait for buffered events"); + drop(reader); + drain.await.unwrap(); + assert!(barrier.try_finalize()); + } + + #[tokio::test] + async fn queued_successful_media_termination_wins_over_shutdown_cancellation() { + let (sender, mut receiver) = tauri::async_runtime::channel(1); + sender + .send(tauri_plugin_shell::process::CommandEvent::Terminated( + tauri_plugin_shell::process::TerminatedPayload { + code: Some(0), + signal: None, + }, + )) + .await + .unwrap(); + + let mut stdout_buffer = String::new(); + let mut stderr_buffer = String::new(); + let mut output_path = None; + let mut stderr_tail = String::new(); + let termination = super::wait_for_media_termination_after_cancel( + &mut receiver, + &mut stdout_buffer, + &mut stderr_buffer, + &mut output_path, + &mut stderr_tail, + 2048, + Duration::from_secs(1), + ) + .await + .expect("queued shell termination should be preserved") + .expect("queued shell termination should be returned"); + assert_eq!(termination.code, Some(0)); + } + + #[tokio::test] + async fn delayed_media_termination_is_observed_after_kill_request() { + let (sender, mut receiver) = tauri::async_runtime::channel(1); + let (terminate_tx, terminate_rx) = tokio::sync::oneshot::channel(); + let termination_sender = sender.clone(); + let termination = tokio::spawn(async move { + terminate_rx.await.expect("kill closure should signal"); + termination_sender + .send(tauri_plugin_shell::process::CommandEvent::Terminated( + tauri_plugin_shell::process::TerminatedPayload { + code: Some(143), + signal: Some(15), + }, + )) + .await + .expect("termination event should be delivered"); + }); + + let mut stdout_buffer = String::new(); + let mut stderr_buffer = String::new(); + let mut output_path = None; + let mut stderr_tail = String::new(); + let payload = super::terminate_and_wait_for_media_after_cancel( + &mut receiver, + &mut stdout_buffer, + &mut stderr_buffer, + &mut output_path, + &mut stderr_tail, + 2048, + Duration::from_millis(1), + move || terminate_tx.send(()).expect("termination task is waiting"), + ) + .await + .expect("kill must be followed by observed process termination"); + termination.await.unwrap(); + assert_eq!(payload.signal, Some(15)); + } + + #[test] + fn aria2_restart_changes_only_the_rpc_port_argument() { + let args = vec![ + OsString::from("--enable-rpc=true"), + OsString::from("--rpc-listen-port=6800"), + OsString::from("--continue=true"), + ]; + let updated = super::set_aria2_rpc_port_argument(&args, 6812); + assert_eq!( + updated, + vec![ + OsString::from("--enable-rpc=true"), + OsString::from("--rpc-listen-port=6812"), + OsString::from("--continue=true"), + ] + ); + assert_eq!(args[1], OsString::from("--rpc-listen-port=6800")); + } + + #[test] + fn aria2_restart_snapshot_updates_existing_and_missing_options() { + let guard = super::Aria2DaemonGuard::new(); + guard.set_launch_spec(super::Aria2LaunchSpec { + binary_path: std::path::PathBuf::from("aria2c"), + args: vec![ + OsString::from("--enable-rpc=true"), + OsString::from("--bt-max-open-files=16"), + OsString::from("--max-overall-upload-limit=1M"), + ], + environment: Vec::new(), + candidate_ports: vec![6800], + }); + + guard.update_launch_option("--bt-max-open-files", "64"); + guard.update_launch_option("--max-overall-upload-limit", "0"); + guard.update_launch_option("--max-overall-download-limit", "512K"); + + let args = guard.launch_spec().expect("launch spec").args; + assert_eq!( + args, + vec![ + OsString::from("--enable-rpc=true"), + OsString::from("--bt-max-open-files=64"), + OsString::from("--max-overall-upload-limit=0"), + OsString::from("--max-overall-download-limit=512K"), + ] + ); + } + #[test] fn frontend_exit_flush_acknowledges_the_current_request_generation() { let flush = FrontendExitFlush::new(); @@ -21653,7 +22586,7 @@ pub fn run() { let server_frontend_ready = extension_frontend_ready.clone(); let extension_acks = Arc::new(Mutex::new(HashMap::new())); let server_extension_acks = extension_acks.clone(); - let extension_media_handoffs = Arc::new(Mutex::new(HashMap::new())); + let extension_media_handoffs = extension_server::new_media_handoffs(); let server_extension_media_handoffs = extension_media_handoffs.clone(); let extension_server_port = Arc::new(RwLock::new(None)); let server_extension_port = extension_server_port.clone(); @@ -21666,6 +22599,7 @@ pub fn run() { // WebSocket listener attach to an unrelated local service. let aria2_port = Arc::new(std::sync::atomic::AtomicU16::new(0)); let aria2_port_clone = Arc::clone(&aria2_port); + let shutdown_terminal_barrier = Arc::new(ShutdownTerminalBarrier::default()); let aria2_secret = uuid::Uuid::new_v4().to_string(); let builder = tauri::Builder::default() .manage(MainWindowRestoreState::default()) @@ -21967,6 +22901,7 @@ pub fn run() { frontend_exit_flush, aria2_port: aria2_port.clone(), aria2_secret: aria2_secret.clone(), + shutdown_terminal_barrier: shutdown_terminal_barrier.clone(), media_semaphore: Arc::new(tokio::sync::Semaphore::new(3)), power_manager: Arc::clone(&power_manager), scheduler_settings: Arc::clone(&scheduler_settings), @@ -22105,17 +23040,19 @@ pub fn run() { let mut success = false; let mut attempted_rpc_port = false; let mut startup_failure = None; - for attempt_port in 6800..6900 { - if queue::torrent_port_spec_contains( + let candidate_ports = (6800..6900) + .filter(|attempt_port| { + !queue::torrent_port_spec_contains( if torrent_startup_settings.listen_port.is_empty() { queue::DEFAULT_TORRENT_LISTEN_PORT_SPEC } else { &torrent_startup_settings.listen_port }, - attempt_port, - ) { - continue; - } + *attempt_port, + ) + }) + .collect::>(); + for attempt_port in candidate_ports.iter().copied() { attempted_rpc_port = true; app_handle_bg .state::() @@ -22199,6 +23136,14 @@ pub fn run() { cmd.arg(format!("--max-overall-download-limit={}", limit)); } + let launch_args = cmd.get_args().map(OsString::from).collect(); + let launch_environment = cmd + .get_envs() + .map(|(key, value)| { + (OsString::from(key), value.map(OsString::from)) + }) + .collect(); + cmd.stdout(std::process::Stdio::null()); cmd.stderr(std::process::Stdio::piped()); @@ -22243,32 +23188,19 @@ pub fn run() { log::info!("aria2c spawned successfully on port {}", attempt_port); - let daemon_app = app_handle_bg.clone(); if let Some(stderr) = child.stderr.take() { - std::thread::spawn(move || { - use std::io::BufRead; - let reader = std::io::BufReader::new(stderr); - for line in reader.lines().map_while(Result::ok) { - let trimmed = line.trim().to_string(); - if let Ok(mut stderr_lock) = daemon_app.state::().last_stderr.lock() { - stderr_lock.push_str(&trimmed); - stderr_lock.push('\n'); - let excess = stderr_lock.len().saturating_sub(8192); - if excess > 0 { - let _ = stderr_lock.drain(..excess); - } - } - let lower = trimmed.to_lowercase(); - if lower.contains("error") || lower.contains("critical") { - log::error!("aria2c stderr: {}", trimmed); - } - } - }); + monitor_aria2_stderr(app_handle_bg.clone(), stderr); } let guard = app_handle_bg.state::(); *guard.child.lock().unwrap() = Some(child); *guard.config_path.lock().unwrap() = Some(config_path); + guard.set_launch_spec(Aria2LaunchSpec { + binary_path: binary_path.clone(), + args: launch_args, + environment: launch_environment, + candidate_ports: candidate_ports.clone(), + }); let mut last_err = String::new(); let start = std::time::Instant::now(); @@ -22364,7 +23296,7 @@ pub fn run() { } } - let Some(ws_port) = ws_port else { + let Some(_initial_ws_port) = ws_port else { log::error!("aria2 startup completed without a verified WebSocket port"); aria2_port_clone.store( 0, @@ -22375,6 +23307,31 @@ pub fn run() { let mut ws_retries = 0; loop { + let state = app_handle_bg.state::(); + let current_port = state.aria2_port.load(std::sync::atomic::Ordering::Acquire); + if current_port == 0 { + let guard = app_handle_bg.state::(); + if !guard.is_shutting_down() + && guard.startup_error_message().is_some() + && guard.launch_spec().is_some() + { + match restart_aria2_daemon(&app_handle_bg).await { + Ok(()) => log::info!( + "app_shutdown_phase=aria2_restart outcome=recovered" + ), + Err(error) => { + log::error!( + "app_shutdown_phase=aria2_restart outcome=retrying error={}", + error + ); + tokio::time::sleep(Duration::from_secs(2)).await; + } + } + } else { + tokio::time::sleep(Duration::from_millis(200)).await; + } + continue; + } if ws_retries == 10 { log::error!("Aria2 WebSocket is still unavailable after repeated reconnection attempts; continuing to retry."); let guard = app_handle_bg.state::(); @@ -22385,8 +23342,27 @@ pub fn run() { ws_retries = 0; } tokio::time::sleep(std::time::Duration::from_secs(2)).await; - let ws_url = format!("ws://127.0.0.1:{}/jsonrpc", ws_port); - if let Ok((ws_stream, _)) = tokio_tungstenite::connect_async(&ws_url).await { + let current_port = state.aria2_port.load(std::sync::atomic::Ordering::Acquire); + if current_port == 0 { + continue; + } + let ws_url = format!("ws://127.0.0.1:{}/jsonrpc", current_port); + // Own a producer lease from before the socket can receive + // events until its buffered messages have been drained. + // Per-message leases leave a gap between the network + // reader and `enter()` where shutdown could finalize past + // a terminal event already queued by aria2. + let Some(_terminal_work) = + Arc::clone(&state.shutdown_terminal_barrier).enter().await + else { + break; + }; + let connection = tokio::time::timeout( + Duration::from_secs(2), + tokio_tungstenite::connect_async(&ws_url), + ) + .await; + if let Ok(Ok((ws_stream, _))) = connection { ws_retries = 0; // reset on success if let Ok(mut startup_error) = app_handle_bg .state::() @@ -22399,7 +23375,22 @@ pub fn run() { *startup_error = None; } } - reconcile_aria2_downloads(&app_handle_bg).await; + let recovery_pending = state.queue_manager.aria2_dispatch_held(); + let reconciled = reconcile_aria2_downloads(&app_handle_bg).await; + if recovery_pending && !reconciled { + log::warn!( + "aria2 restart reconciliation remains incomplete; keeping queued Aria2 work parked" + ); + drop(_terminal_work); + tokio::time::sleep(Duration::from_secs(2)).await; + continue; + } + if recovery_pending { + state.queue_manager.set_aria2_dispatch_hold(false); + log::info!( + "aria2 restart reconciliation complete; queued Aria2 work resumed" + ); + } use futures_util::StreamExt; let (_, mut read) = ws_stream.split(); while let Some(msg) = read.next().await { @@ -22497,8 +23488,11 @@ pub fn run() { _ => None, }; if let Some(outcome) = outcome { - Arc::clone(&state.queue_manager) - .handle_aria2_event_with_progress(gid, outcome, progress) + let manager = Arc::clone(&state.queue_manager); + manager + .handle_aria2_event_with_progress_under_terminal_lease( + gid, outcome, progress, + ) .await; } } @@ -22509,10 +23503,13 @@ pub fn run() { } } } + drop(_terminal_work); ws_retries += 1; let state = app_handle_bg.state::(); let port = state.aria2_port.load(std::sync::atomic::Ordering::Relaxed); - if !aria2_daemon_is_reachable(port, &state.aria2_secret).await + if !app_handle_bg.state::().is_shutting_down() + && port != 0 + && !aria2_daemon_is_reachable(port, &state.aria2_secret).await && aria2_daemon_process_exited(&app_handle_bg) { state.queue_manager.clear_aria2_permits().await; @@ -22543,6 +23540,16 @@ pub fn run() { // RPC failures while startup is pending or unavailable. continue; } + let Some(_terminal_work) = Arc::clone( + &app_handle_poll + .state::() + .shutdown_terminal_barrier, + ) + .enter() + .await + else { + return; + }; // Terminal cleanup removes a download's GID mapping. Do // not retain one observation per historical download for // the lifetime of the poller. @@ -22587,6 +23594,13 @@ pub fn run() { { Ok(active_list) => active_list, Err(error) => { + if poll_port.load(std::sync::atomic::Ordering::Acquire) == 0 + && app_handle_poll + .state::() + .is_shutting_down() + { + break; + } log::warn!( "aria2 poller [stage=poll operation=tell_active result=failed error_class={} error_code={} elapsed_ms={}]", crate::retry::network_error_class(&error), @@ -23013,7 +24027,7 @@ pub fn run() { && poll_mgr.aria2_torrent_seeding_requested(&id).await { poll_mgr - .handle_aria2_event( + .handle_aria2_event_under_terminal_lease( &gid, crate::queue::PendingOutcome::Seeding, ) @@ -23267,7 +24281,9 @@ pub fn run() { .unwrap_or_else(|| "none".to_string()) ); poll_mgr - .handle_aria2_event_with_progress(&gid, outcome, progress) + .handle_aria2_event_with_progress_under_terminal_lease( + &gid, outcome, progress, + ) .await; } } @@ -23755,7 +24771,7 @@ pub fn run() { })) { log::warn!("app_shutdown_phase=frontend_flush signal failed: {error}"); frontend_exit_flush.finish(request, false); - reject_app_exit_request(&app, request, "signal-failed"); + reject_app_exit_request(&app, request, "signal-failed", &[]).await; log::info!( "app_shutdown_phase=total outcome=cancelled elapsed_ms={}", exit_started.elapsed().as_millis() @@ -23818,15 +24834,79 @@ pub fn run() { } else { "rejected" }; - reject_app_exit_request(&app, request, cancel_reason); + reject_app_exit_request(&app, request, cancel_reason, &[]).await; log::info!( "app_shutdown_phase=total outcome=cancelled elapsed_ms={}", exit_started.elapsed().as_millis() ); return; } - let _ = extension_server_shutdown.send(true); + // Stop any addUri handoff admitted just before the + // shutdown fence. If one cannot settle promptly, + // reject this exit while the existing daemon is + // still available to finish the transfer. + let dispatch_drain = tokio::time::timeout( + Duration::from_secs(15), + app.state::() + .queue_manager + .wait_for_dispatch_handoffs(), + ) + .await; + if !matches!(dispatch_drain, Ok(true)) { + log::warn!( + "app_shutdown_phase=aria2_dispatch_drain outcome=timeout-or-incomplete" + ); + reject_app_exit_request( + &app, + request, + "aria2-dispatch-drain-failed", + &[], + ) + .await; + return; + } + // Close media registration and drain accepted + // workers before stopping aria2. A successful + // yt-dlp result then stays inside the terminal + // barrier through its durable journal and row + // update, and cancellation leaves the active row + // restartable if this exit is later rejected. + let interrupted_media_ids = match app + .state::() + .download_coordinator + .cancel_active_media_for_shutdown() + .await + { + Ok(interrupted) => interrupted + .into_iter() + .map(|(id, _generation)| id) + .collect::>(), + Err(error) => { + log::error!( + "app_shutdown_phase=media_cancel outcome=failed error={error}" + ); + reject_app_exit_request( + &app, + request, + "media-shutdown-failed", + &[], + ) + .await; + return; + } + }; + // The accepted snapshot durably reconciles any + // journal failure left by a rejected prior exit. shutdown_aria2_daemon(app.clone()).await; + let terminal_barrier = Arc::clone( + &app.state::().shutdown_terminal_barrier, + ); + let terminal_drained = tokio::time::timeout( + Duration::from_secs(15), + terminal_barrier.seal_and_wait(), + ) + .await + .is_ok(); let final_flush_started = Instant::now(); let mut final_completed = frontend_exit_flush.completed.subscribe(); @@ -23837,6 +24917,7 @@ pub fn run() { "requestId": request.to_string() }), ); + let mut final_flush_succeeded = false; if let Err(error) = final_flush_emitted { log::warn!("app_shutdown_phase=final_flush signal failed: {error}"); } else { @@ -23861,10 +24942,13 @@ pub fn run() { ) .await { - Ok(true) => log::info!( - "app_shutdown_phase=final_flush outcome=complete elapsed_ms={}", - final_flush_started.elapsed().as_millis() - ), + Ok(true) => { + final_flush_succeeded = true; + log::info!( + "app_shutdown_phase=final_flush outcome=complete elapsed_ms={}", + final_flush_started.elapsed().as_millis() + ); + } Ok(false) => log::warn!( "app_shutdown_phase=final_flush outcome=failed elapsed_ms={}", final_flush_started.elapsed().as_millis() @@ -23875,7 +24959,58 @@ pub fn run() { ), } } + let shutdown_guard = app.state::(); + let failed_rows_are_durably_completed = + shutdown_failed_rows_are_durably_completed( + &shutdown_guard, + app.try_state::().as_deref(), + ); + let finalization_checks_passed = terminal_drained + && shutdown_finalization_allowed( + shutdown_guard.shutdown_journal_failed(), + final_flush_succeeded, + failed_rows_are_durably_completed, + ); + let finalization_allowed = finalization_checks_passed + && terminal_barrier.try_finalize(); + if !finalization_allowed { + log::error!( + "app_shutdown_phase=finalization outcome=blocked reason=terminal-state-not-durable-or-event-drain-failed" + ); + let restart_required = app + .state::() + .launch_spec() + .is_some(); + app.state::() + .queue_manager + .set_aria2_dispatch_hold(true); + if restart_required { + if let Err(error) = restart_aria2_daemon(&app).await { + log::error!( + "app_shutdown_phase=aria2_restart outcome=unavailable error={}", + error + ); + *app.state::() + .startup_error + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()) = + Some(error); + } + } + reject_app_exit_request( + &app, + request, + "terminal-journal-failed", + &interrupted_media_ids, + ) + .await; + return; + } + if final_flush_succeeded { + app.state::().clear_shutdown_journal_failure(); + } app.state::().allow_exit(); + let _ = extension_server_shutdown.send(true); log::info!( "app_shutdown_phase=total elapsed_ms={}", exit_started.elapsed().as_millis() diff --git a/src-tauri/src/login_start.rs b/src-tauri/src/login_start.rs index 1149818..7e6b282 100644 --- a/src-tauri/src/login_start.rs +++ b/src-tauri/src/login_start.rs @@ -216,11 +216,55 @@ fn launch_path(path: String) -> String { format!("\"{}\"", path.replace('"', "\\\"")) } +#[cfg(any(target_os = "linux", test))] +fn encode_linux_exec_path(path: &str) -> Result { + if path.chars().any(char::is_control) { + return Err( + "Linux executable path contains an unsupported control character".to_string(), + ); + } + if path.contains('=') { + return Err( + "Linux executable path contains '=' which Desktop Entry Exec does not support" + .to_string(), + ); + } + + // First encode the argument according to the Exec quoting rules. Percent + // signs are doubled so the launcher does not treat them as field codes. + let mut quoted_argument = String::with_capacity(path.len() + 2); + quoted_argument.push('"'); + for character in path.chars() { + match character { + '\\' | '"' | '`' | '$' => { + quoted_argument.push('\\'); + quoted_argument.push(character); + } + '%' => quoted_argument.push_str("%%"), + _ => quoted_argument.push(character), + } + } + quoted_argument.push('"'); + + // Exec is also a desktop-entry string value. Escape its backslashes and + // supported control characters so the parser can apply the Exec quoting + // rules to the intended argument above. + let mut encoded = String::with_capacity(quoted_argument.len() + 2); + for character in quoted_argument.chars() { + match character { + '\\' => encoded.push_str("\\\\"), + '\t' => encoded.push_str("\\t"), + '\n' => encoded.push_str("\\n"), + '\r' => encoded.push_str("\\r"), + _ => encoded.push(character), + } + } + Ok(encoded) +} + #[cfg(target_os = "linux")] -fn launch_path(path: String) -> String { - // XDG Exec values use double-quoted arguments rather than shell parsing. - let escaped = path.replace('\\', "\\\\").replace('"', "\\\""); - format!("\"{escaped}\"") +fn launch_path(path: String) -> Result { + encode_linux_exec_path(&path) } #[cfg(target_os = "macos")] @@ -248,7 +292,7 @@ fn build_auto_launch() -> Result { fn build_auto_launch() -> Result { use auto_launch::{AutoLaunchBuilder, LinuxLaunchMode}; - let executable = launch_path(linux_executable_path()?); + let executable = launch_path(linux_executable_path()?)?; let mut builder = AutoLaunchBuilder::new(); builder .set_app_name("Firelink") @@ -481,7 +525,7 @@ pub fn open_login_items_settings() -> Result<(), String> { #[cfg(test)] mod tests { - use super::{has_autostart_argument, AUTOSTART_ARGUMENT}; + use super::{encode_linux_exec_path, has_autostart_argument, AUTOSTART_ARGUMENT}; #[test] fn accepts_only_the_exact_autostart_argument() { @@ -495,4 +539,27 @@ mod tests { assert!(!has_autostart_argument(["--firelink-autostart-extra"])); assert!(!has_autostart_argument(["firelink-autostart"])); } + + #[test] + fn encodes_linux_exec_path_special_characters() { + let path = r#"/opt/Firelink app/quoted"/back\slash/$cost/`tick`/100%"#; + + assert_eq!( + encode_linux_exec_path(path).unwrap(), + r#""/opt/Firelink app/quoted\\"/back\\\\slash/\\$cost/\\`tick\\`/100%%""# + ); + } + + #[test] + fn rejects_control_characters_in_linux_executable_paths() { + for character in ['\0', '\t', '\n', '\r', '\u{1}', '\u{7f}', '\u{85}'] { + let path = format!("/opt/Firelink{character}path"); + assert!(encode_linux_exec_path(&path).is_err()); + } + } + + #[test] + fn rejects_equals_in_linux_executable_path() { + assert!(encode_linux_exec_path("/opt/Fire=link").is_err()); + } } diff --git a/src-tauri/src/queue.rs b/src-tauri/src/queue.rs index 0cda487..1a34750 100644 --- a/src-tauri/src/queue.rs +++ b/src-tauri/src/queue.rs @@ -908,6 +908,23 @@ impl Drop for Aria2ControlGuard { } } +impl Aria2ControlGuard { + /// Release this download's control lock while waiting for a worker that + /// needs the same lock to publish its terminal result. + pub(crate) fn release(&mut self) { + self.guard.take(); + } + + /// Reacquire the same per-download lock after the worker acknowledgement. + /// Keeping the original lock Arc prevents a second lock from being + /// installed for this download while the guard is temporarily released. + pub(crate) async fn reacquire(&mut self) { + if self.guard.is_none() { + self.guard = Some(self.lock.clone().lock_owned().await); + } + } +} + /// Outcome of an aria2 completion that arrived before its gid was stored. /// Carries the outcome so the correct state emit survives the race. #[derive(Debug, Clone)] @@ -1065,6 +1082,13 @@ pub enum TaskKind { Media, } +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) enum MediaTerminalOutcome { + Completed, + Cancelled, + Failed, +} + /// Everything needed to start a sidecar, captured at enqueue time so the /// dispatcher can spawn it later without round-tripping back to the frontend. #[derive(Debug, Clone)] @@ -1085,6 +1109,31 @@ struct QueuePermitOwnership { active: bool, } +#[derive(Clone, Debug)] +struct PendingOrderEntry { + queue_id: String, + ordinal: u64, +} + +struct DispatchHandoffGuard { + count: Arc, + notify: Arc, +} + +impl DispatchHandoffGuard { + fn new(count: Arc, notify: Arc) -> Self { + count.fetch_add(1, Ordering::AcqRel); + Self { count, notify } + } +} + +impl Drop for DispatchHandoffGuard { + fn drop(&mut self) { + self.count.fetch_sub(1, Ordering::AcqRel); + self.notify.notify_waiters(); + } +} + #[derive(Debug, Clone)] struct SeedWaiter { id: String, @@ -1289,9 +1338,20 @@ pub struct QueueManager { /// transfer. Keeping both phases in one map makes queue-slot ownership /// exactly-once across the async addUri handoff. queue_permit_ownership: Mutex>, + /// Stable logical positions for pending tasks and selected tasks whose + /// dispatch has not yet been accepted by the sidecar. + pending_order_ordinals: Mutex>, + next_pending_order_ordinal: Mutex>, + /// Selection-to-dispatch handoffs that shutdown must wait for before + /// finalizing Aria2 exit. + dispatch_handoff_count: Arc, + dispatch_handoff_notify: Arc, /// Serializes queue-slot selection with global permit acquisition and /// ownership transitions. admission_gate: Mutex<()>, + /// Keeps pending Aria2 work queued while its daemon is offline or being + /// recovered. Media work uses a separate runner and remains admissible. + aria2_dispatch_hold: AtomicBool, /// Prevents new enqueue/admission work after a system action has passed /// its final safety check. The flag is set while holding admission_gate so /// the check and the fence are one state transition. @@ -1327,6 +1387,10 @@ pub struct QueueManager { /// The generation fences late start/clear events from an older GID. aria2_allocation_pending: Mutex>, media_completed_generations: Mutex>, + /// Last media terminal outcome for each download. Keep it after queue + /// ownership is released so a control command that was waiting for the + /// worker can distinguish completion, failure, and cancellation. + media_terminal_outcomes: Mutex>, /// download id -> spawn payload for aria2 transient-error re-addUri retries. aria2_payloads: Mutex>, @@ -1418,7 +1482,12 @@ impl QueueManager { queue_limits: Mutex::new(HashMap::new()), queue_dispatch_holds: Mutex::new(HashMap::new()), queue_permit_ownership: Mutex::new(HashMap::new()), + pending_order_ordinals: Mutex::new(HashMap::new()), + next_pending_order_ordinal: Mutex::new(HashMap::new()), + dispatch_handoff_count: Arc::new(AtomicUsize::new(0)), + dispatch_handoff_notify: Arc::new(Notify::new()), admission_gate: Mutex::new(()), + aria2_dispatch_hold: AtomicBool::new(false), system_action_pending: AtomicBool::new(false), dispatch_cursor: Mutex::new(None), target_capacity: AtomicUsize::new(capacity), @@ -1438,6 +1507,7 @@ impl QueueManager { pending_download_starts: Arc::new(Mutex::new(HashSet::new())), aria2_allocation_pending: Mutex::new(HashMap::new()), media_completed_generations: Mutex::new(HashMap::new()), + media_terminal_outcomes: Mutex::new(HashMap::new()), aria2_payloads: Mutex::new(HashMap::new()), aria2_connection_options: Mutex::new(HashMap::new()), aria2_dispatch_inflight: Mutex::new(HashMap::new()), @@ -1832,6 +1902,19 @@ impl QueueManager { } } + async fn enter_shutdown_terminal_work( + &self, + ) -> Result, ()> { + let Some(app_state) = self.app_handle.try_state::() else { + return Ok(None); + }; + Arc::clone(&app_state.shutdown_terminal_barrier) + .enter() + .await + .map(Some) + .ok_or(()) + } + fn emit_waiting_to_seed(&self, id: &str, remaining: Option) { use tauri::Emitter; let _ = self.app_handle.emit( @@ -1841,6 +1924,11 @@ impl QueueManager { } async fn try_start_waiting_seed(self: &Arc) -> bool { + if self.app_handle.try_state::() + .is_some_and(|guard| guard.is_shutting_down()) + { + return false; + } let Some(waiter) = self.reserve_seed_waiter() else { return false; }; @@ -1881,6 +1969,15 @@ impl QueueManager { return; }; let _control_guard = self.acquire_aria2_control(&id).await; + if self.app_handle.try_state::() + .is_some_and(|guard| guard.is_shutting_down()) + { + self.release_aria2_permit_candidate(&id, waiter.lifecycle_generation) + .await; + self.abandon_seed_start(&id); + self.add_seed_waiter(waiter); + return; + } if !self .is_registered_generation_or_legacy(&id, waiter.lifecycle_generation) .await @@ -1917,6 +2014,10 @@ impl QueueManager { self.abandon_seed_start(&id); return; } + let _terminal_work = match self.enter_shutdown_terminal_work().await { + Ok(terminal_work) => terminal_work, + Err(()) => return, + }; match self.spawner.resume_for_seed(&gid).await { Ok(Aria2SeedControlOutcome::Resumed) => { if !self.is_aria2_control_epoch_current(&id, epoch).await @@ -1937,7 +2038,12 @@ impl QueueManager { } Ok(Aria2SeedControlOutcome::Complete) => { self.finish_seed_start(&id); - self.apply_completion_locked(&id, PendingOutcome::Complete).await; + self.apply_completion_locked_with_progress_unfenced( + &id, + PendingOutcome::Complete, + None, + ) + .await; } Ok(Aria2SeedControlOutcome::Paused) => { let remaining = self.capture_seed_remaining(&id).await; @@ -1992,7 +2098,36 @@ impl QueueManager { _ = self.notify.notified() => {} } + if self + .app_handle + .try_state::() + .is_some_and(|guard| guard.exit_allowed()) + { + return; + } + if self + .app_handle + .try_state::() + .is_some_and(|guard| guard.is_shutting_down()) + { + continue; + } + + let terminal_work = match self.enter_shutdown_terminal_work().await { + Ok(terminal_work) => terminal_work, + Err(()) => return, + }; + let _control_guard = self.acquire_aria2_control(&waiter.id).await; + if self + .app_handle + .try_state::() + .is_some_and(|guard| guard.is_shutting_down()) + { + drop(_control_guard); + drop(terminal_work); + continue; + } let lifecycle_current = self .is_registered_generation_or_legacy(&waiter.id, waiter.lifecycle_generation) .await @@ -2023,7 +2158,6 @@ impl QueueManager { }, ) { - let _ = self.spawner.pause_for_seed(&gid).await; let _ = self.spawner.pause_for_seed(&gid).await; self.release_download_permit_for_seed(&waiter.id).await; self.abandon_seed_start(&waiter.id); @@ -2037,7 +2171,11 @@ impl QueueManager { } Ok(Aria2SeedControlOutcome::Complete) => { self.finish_seed_start(&waiter.id); - self.apply_completion_locked(&waiter.id, PendingOutcome::Complete) + self.apply_completion_locked_with_progress_unfenced( + &waiter.id, + PendingOutcome::Complete, + None, + ) .await; return; } @@ -2083,6 +2221,107 @@ impl QueueManager { .collect() } + async fn ensure_pending_order_ordinal(&self, task: &QueuedTask) -> u64 { + let key = (task.id.clone(), task.lifecycle_generation); + let mut ordinals = self.pending_order_ordinals.lock().await; + if let Some(entry) = ordinals.get(&key) { + return entry.ordinal; + } + let mut next_ordinals = self.next_pending_order_ordinal.lock().await; + let next = next_ordinals.entry(task.queue_id.clone()).or_default(); + *next = next.saturating_add(1); + let ordinal = *next; + ordinals.insert( + key, + PendingOrderEntry { + queue_id: task.queue_id.clone(), + ordinal, + }, + ); + ordinal + } + + async fn forget_pending_order_ordinal(&self, id: &str, lifecycle_generation: u64) { + self.pending_order_ordinals + .lock() + .await + .remove(&(id.to_string(), lifecycle_generation)); + } + + /// Rebuild current pending order into the existing logical slots for this + /// queue. Selected dispatches stay in their slots, so a manual reorder + /// remains effective when one of those dispatches later returns to pending. + async fn refresh_pending_order_ordinals( + &self, + queue_id: &str, + reordered_pending_tasks: &[QueuedTask], + ) { + let mut ordinals = self.pending_order_ordinals.lock().await; + let pending_keys = reordered_pending_tasks + .iter() + .map(|task| (task.id.clone(), task.lifecycle_generation)) + .collect::>(); + let mut logical_keys = ordinals + .iter() + .filter_map(|(key, entry)| { + (entry.queue_id == queue_id).then_some((entry.ordinal, key.clone())) + }) + .collect::>(); + logical_keys.sort_by_key(|(ordinal, _)| *ordinal); + + let mut reordered = reordered_pending_tasks.iter().map(|task| { + (task.id.clone(), task.lifecycle_generation) + }); + let mut next_logical_keys = Vec::with_capacity(logical_keys.len().max(pending_keys.len())); + for (_, key) in logical_keys { + if pending_keys.contains(&key) { + if let Some(reordered_key) = reordered.next() { + next_logical_keys.push(reordered_key); + } + } else { + next_logical_keys.push(key); + } + } + next_logical_keys.extend(reordered); + + let mut next_ordinals = self.next_pending_order_ordinal.lock().await; + let next = next_ordinals.entry(queue_id.to_string()).or_default(); + for key in next_logical_keys { + *next = next.saturating_add(1); + if let Some(entry) = ordinals.get_mut(&key) { + entry.ordinal = *next; + } + } + } + + fn stable_pending_insert_index( + pending: &VecDeque, + task: &QueuedTask, + ordinal: u64, + fallback_index: usize, + order_ordinals: &HashMap<(String, u64), PendingOrderEntry>, + ) -> usize { + let mut lower_bound = 0; + let mut upper_bound = pending.len(); + for (index, candidate) in pending.iter().enumerate() { + if candidate.queue_id != task.queue_id { + continue; + } + let key = (candidate.id.clone(), candidate.lifecycle_generation); + let Some(candidate_entry) = order_ordinals.get(&key) else { + continue; + }; + if candidate_entry.ordinal <= ordinal { + lower_bound = index + 1; + } else { + upper_bound = index; + break; + } + } + let lower_bound = lower_bound.min(upper_bound); + fallback_index.clamp(lower_bound, upper_bound) + } + /// Temporarily remove one not-yet-admitted task while a caller performs a /// lifecycle-safe reconfiguration. The admission gate prevents the /// dispatcher from popping the same task between the caller's validation @@ -2098,17 +2337,40 @@ impl QueueManager { /// position even when another queue's work was admitted meanwhile. pub async fn restore_pending_task(&self, index: usize, task: QueuedTask) { let _admission_gate = self.admission_gate.lock().await; + let mut ordinals = self.pending_order_ordinals.lock().await; + let key = (task.id.clone(), task.lifecycle_generation); + let ordinal = if let Some(entry) = ordinals.get(&key) { + entry.ordinal + } else { + let mut next_ordinals = self.next_pending_order_ordinal.lock().await; + let next = next_ordinals.entry(task.queue_id.clone()).or_default(); + *next = (*next).saturating_add(1); + let ordinal = *next; + ordinals.insert( + key, + PendingOrderEntry { + queue_id: task.queue_id.clone(), + ordinal, + }, + ); + ordinal + }; let mut pending = self.pending.lock().await; - let insert_at = index.min(pending.len()); + let insert_at = Self::stable_pending_insert_index(&pending, &task, ordinal, index, &ordinals); pending.insert(insert_at, task); + drop(pending); + drop(ordinals); self.notify.notify_one(); } /// Explicitly release a backend registry id (e.g. on un-resumable false paths, removals, or detach). pub async fn release_registered_id(&self, id: &str) { - self.media_completed_generations.lock().await.remove(id); self.registered_ids.lock().await.remove(id); self.registered_lifecycle_generations.lock().await.remove(id); + self.pending_order_ordinals + .lock() + .await + .retain(|(download_id, _), _| download_id != id); // A released lifecycle cannot be resumed by a delayed retry worker. // Epoch checks remain the authoritative guard; removing this marker // prevents terminal downloads from accumulating cancellation entries. @@ -2158,7 +2420,7 @@ impl QueueManager { } }; if released { - self.media_completed_generations.lock().await.remove(id); + self.forget_pending_order_ordinal(id, generation).await; self.aria2_retry_cancelled.lock().await.remove(id); self.release_seed_tracking(id); self.notify.notify_waiters(); @@ -2197,6 +2459,11 @@ impl QueueManager { id: &str, generation: u64, ) -> Result, String> { + if self.app_handle.try_state::() + .is_some_and(|guard| guard.is_shutting_down()) + { + return Err("Firelink is preparing to exit".to_string()); + } let cancellations = self.enqueue_cancellations.lock().await; if cancellations .get(id) @@ -2220,6 +2487,14 @@ impl QueueManager { .await .insert(id.to_string(), generation); generations.insert(id.to_string(), generation); + drop(registered); + drop(generations); + drop(cancellations); + if let Err(error) = self.clear_shutdown_completion_journal(id) { + self.rollback_enqueue_reservation(id, generation, previous_generation) + .await; + return Err(error); + } Ok(previous_generation) } @@ -2276,6 +2551,13 @@ impl QueueManager { { let id = task.id.clone(); let _admission_gate = self.admission_gate.lock().await; + if self.app_handle.try_state::() + .is_some_and(|guard| guard.is_shutting_down()) + { + self.rollback_enqueue_reservation(&id, generation, previous_generation) + .await; + return Err("Firelink is preparing to exit".to_string()); + } if self.system_action_pending.load(Ordering::Acquire) { self.rollback_enqueue_reservation(&id, generation, previous_generation) .await; @@ -2302,6 +2584,7 @@ impl QueueManager { } } task.lifecycle_generation = generation; + self.ensure_pending_order_ordinal(&task).await; self.pending.lock().await.push_back(task); if let Err(error) = finalizer().await { @@ -2309,6 +2592,8 @@ impl QueueManager { pending.retain(|candidate| { !(candidate.id == id && candidate.lifecycle_generation == generation) }); + drop(pending); + self.forget_pending_order_ordinal(&id, generation).await; self.rollback_enqueue_reservation(&id, generation, previous_generation) .await; return Err(error); @@ -2328,12 +2613,18 @@ impl QueueManager { pending.retain(|candidate| { !(candidate.id == id && candidate.lifecycle_generation == generation) }); + drop(pending); + self.forget_pending_order_ordinal(&id, generation).await; self.rollback_enqueue_reservation(&id, generation, previous_generation) .await; return Err("Download enqueue was superseded by a newer user action".to_string()); } } + // A committed replacement owns a new lifecycle. Retire terminal + // evidence from the prior generation only after admission can no + // longer roll back. + self.clear_media_terminal_state(&id).await; self.emit_state(id, DownloadStatus::Queued); self.notify.notify_one(); Ok(()) @@ -3734,7 +4025,12 @@ impl QueueManager { /// Pop the next task, or None if empty. pub async fn pop_front(&self) -> Option { let _admission_gate = self.admission_gate.lock().await; - self.pending.lock().await.pop_front() + let task = self.pending.lock().await.pop_front(); + if let Some(task) = &task { + self.forget_pending_order_ordinal(&task.id, task.lifecycle_generation) + .await; + } + task } /// Acquire a permit from the semaphore (blocks until one is available). @@ -3895,6 +4191,121 @@ impl QueueManager { } } + /// Return a dispatched task to its original pending position while + /// releasing the queue reservation and global permit. A stale lifecycle + /// still relinquishes its ownership, but is never reinserted into pending. + async fn restore_dispatch_task_to_pending( + &self, + index: usize, + selected_ordinal: u64, + task: QueuedTask, + previous_cursor: Option, + unparked_permit: Option, + ) -> bool { + let id = task.id.clone(); + let generation = task.lifecycle_generation; + let removal_requested = self.removal_requested(&id); + let _admission_gate = self.admission_gate.lock().await; + + let mut ownership = self.queue_permit_ownership.lock().await; + let Some(owned) = ownership + .get(&id) + .filter(|owned| owned.lifecycle_generation == generation) + .cloned() + else { + self.pending_order_ordinals + .lock() + .await + .remove(&(id.clone(), generation)); + drop(ownership); + drop(_admission_gate); + drop(unparked_permit); + return false; + }; + + let registered_generation = self + .registered_lifecycle_generations + .lock() + .await + .get(&id) + .copied(); + let registered = registered_generation == Some(generation) + || (generation == 0 + && registered_generation.is_none() + && self.registered_ids.lock().await.contains(&id)); + let cancelled = self + .enqueue_cancellations + .lock() + .await + .get(&id) + .is_some_and(|cancelled| *cancelled >= generation); + let should_restore = registered && !cancelled && !removal_requested; + + ownership.remove(&id); + drop(ownership); + + let active_generation = self + .active_permit_generations + .lock() + .await + .get(&id) + .copied(); + let remove_active_permit = active_generation == Some(generation) + || (owned.active && active_generation.is_none()); + let active_permit = if remove_active_permit { + if active_generation == Some(generation) { + self.active_permit_generations.lock().await.remove(&id); + } + self.active_permits.lock().await.remove(&id) + } else { + None + }; + if remove_active_permit { + self.active_kinds.lock().await.remove(&id); + } + + let order_key = (id.clone(), generation); + let mut pending_ordinals = self.pending_order_ordinals.lock().await; + if should_restore { + let ordinal = pending_ordinals + .get(&order_key) + .filter(|entry| entry.queue_id == owned.queue_id) + .map_or(selected_ordinal, |entry| entry.ordinal); + pending_ordinals.insert( + order_key, + PendingOrderEntry { + queue_id: owned.queue_id.clone(), + ordinal, + }, + ); + let mut pending = self.pending.lock().await; + let insert_at = Self::stable_pending_insert_index( + &pending, + &task, + ordinal, + index, + &pending_ordinals, + ); + pending.insert(insert_at, task); + let mut cursor = self.dispatch_cursor.lock().await; + if cursor.as_deref() == Some(owned.queue_id.as_str()) { + *cursor = previous_cursor; + } + } else { + pending_ordinals.remove(&order_key); + } + drop(pending_ordinals); + + drop(active_permit); + drop(unparked_permit); + drop(_admission_gate); + self.notify.notify_waiters(); + if remove_active_permit { + self.sync_power_activity().await; + } + should_restore + } + async fn activate_admitted_permit( &self, id: &str, @@ -3942,8 +4353,22 @@ impl QueueManager { true } - async fn try_admit_next_task(&self) -> Option<(OwnedSemaphorePermit, QueuedTask)> { + async fn try_admit_next_task( + &self, + ) -> Option<( + OwnedSemaphorePermit, + QueuedTask, + usize, + Option, + u64, + DispatchHandoffGuard, + )> { let _admission_gate = self.admission_gate.lock().await; + if self.app_handle.try_state::() + .is_some_and(|guard| guard.is_shutting_down()) + { + return None; + } if self.system_action_pending.load(Ordering::Acquire) { return None; } @@ -3956,8 +4381,10 @@ impl QueueManager { let ownership = self.queue_permit_ownership.lock().await; let mut queue_ids = Vec::new(); let mut seen = HashSet::new(); + let aria2_dispatch_held = self.aria2_dispatch_held(); for task in pending.iter() { - if !ownership.contains_key(&task.id) + if (!aria2_dispatch_held || !matches!(&task.kind, TaskKind::Aria2)) + && !ownership.contains_key(&task.id) && !held_queues.contains(&task.queue_id) && seen.insert(task.queue_id.clone()) { @@ -3990,11 +4417,16 @@ impl QueueManager { })?; let task_index = pending .iter() - .position(|task| task.queue_id == selected_queue && !ownership.contains_key(&task.id))?; + .position(|task| { + task.queue_id == selected_queue + && (!aria2_dispatch_held || !matches!(&task.kind, TaskKind::Aria2)) + && !ownership.contains_key(&task.id) + })?; drop(queue_limits); drop(ownership); let permit = self.try_acquire_permit_after_retirement()?; + let previous_cursor = cursor; let task = pending.remove(task_index)?; self.queue_permit_ownership.lock().await.insert( task.id.clone(), @@ -4005,7 +4437,19 @@ impl QueueManager { }, ); *self.dispatch_cursor.lock().await = Some(selected_queue); - Some((permit, task)) + let pending_ordinal = self.ensure_pending_order_ordinal(&task).await; + let handoff_guard = DispatchHandoffGuard::new( + Arc::clone(&self.dispatch_handoff_count), + Arc::clone(&self.dispatch_handoff_notify), + ); + Some(( + permit, + task, + task_index, + previous_cursor, + pending_ordinal, + handoff_guard, + )) } /// Park an already-acquired permit under `id`. @@ -4296,6 +4740,19 @@ impl QueueManager { self.semaphore.available_permits() } + /// Hold only queued Aria2 work while its daemon is offline or recovering. + /// The dispatcher is woken for both transitions so Media work can keep + /// flowing during a hold and Aria2 work resumes promptly after it clears. + pub fn set_aria2_dispatch_hold(&self, held: bool) { + self.aria2_dispatch_hold.store(held, Ordering::Release); + self.notify.notify_waiters(); + } + + /// Whether the Aria2 queue is currently held for daemon recovery. + pub fn aria2_dispatch_held(&self) -> bool { + self.aria2_dispatch_hold.load(Ordering::Acquire) + } + pub(crate) fn emit_state(&self, id: impl Into, status: DownloadStatus) { use tauri::Emitter; let _ = self @@ -4534,7 +4991,15 @@ impl QueueManager { if self.try_start_waiting_seed().await { continue; } - if let Some((permit, task)) = self.try_admit_next_task().await { + if let Some(( + permit, + task, + pending_index, + previous_cursor, + pending_ordinal, + handoff_guard, + )) = self.try_admit_next_task().await + { // Admission owns the global and per-queue reservation before // this task is spawned. Keep the dispatcher free to admit // other work while an Aria2 addUri call is retrying or a @@ -4542,7 +5007,16 @@ impl QueueManager { // until that lifecycle reaches its own terminal path. let manager = Arc::clone(&self); tauri::async_runtime::spawn(async move { - manager.dispatch_one(permit, task).await; + manager + .dispatch_one( + permit, + task, + pending_index, + previous_cursor, + pending_ordinal, + handoff_guard, + ) + .await; }); } else { // This covers both an empty pending list and the case where @@ -4561,18 +5035,84 @@ impl QueueManager { } } - async fn dispatch_one(self: Arc, permit: OwnedSemaphorePermit, task: QueuedTask) { + pub(crate) fn wake_after_exit_rejected(&self) { + self.notify.notify_waiters(); + } + + pub(crate) async fn wait_for_dispatch_handoffs(&self) -> bool { + loop { + let notified = self.dispatch_handoff_notify.notified(); + tokio::pin!(notified); + notified.as_mut().enable(); + + let admission_gate = self.admission_gate.lock().await; + if self.dispatch_handoff_count.load(Ordering::Acquire) == 0 { + return true; + } + drop(admission_gate); + notified.await; + } + } + + async fn dispatch_one( + self: Arc, + permit: OwnedSemaphorePermit, + task: QueuedTask, + pending_index: usize, + previous_cursor: Option, + pending_ordinal: u64, + _handoff_guard: DispatchHandoffGuard, + ) { let id = task.id.clone(); let lifecycle_generation = task.lifecycle_generation; + if matches!(&task.kind, TaskKind::Aria2) && self.aria2_dispatch_held() { + self.restore_dispatch_task_to_pending( + pending_index, + pending_ordinal, + task, + previous_cursor, + Some(permit), + ) + .await; + return; + } // Serialize activation with pause/remove/detach. The dispatcher can // pop a task just as a control command arrives; registering the // permit and active kind under the same guard prevents that command // from observing a half-started lifecycle. let control_guard = self.acquire_aria2_control(&id).await; + if matches!(&task.kind, TaskKind::Aria2) && self.aria2_dispatch_held() { + drop(control_guard); + self.restore_dispatch_task_to_pending( + pending_index, + pending_ordinal, + task, + previous_cursor, + Some(permit), + ) + .await; + return; + } + if self.app_handle.try_state::() + .is_some_and(|guard| guard.is_shutting_down()) + { + drop(control_guard); + self.restore_dispatch_task_to_pending( + pending_index, + pending_ordinal, + task, + previous_cursor, + Some(permit), + ) + .await; + return; + } if self.removal_requested(&id) || !self .is_registered_generation(&id, lifecycle_generation) .await { + self.forget_pending_order_ordinal(&id, lifecycle_generation) + .await; self.release_queue_reservation_for_generation(&id, lifecycle_generation) .await; drop(control_guard); @@ -4587,6 +5127,8 @@ impl QueueManager { .activate_admitted_permit(&id, lifecycle_generation, permit) .await { + self.forget_pending_order_ordinal(&id, lifecycle_generation) + .await; drop(control_guard); return; } @@ -4634,10 +5176,25 @@ impl QueueManager { self.emit_state(&id, DownloadStatus::Downloading); } - match task.kind { + match &task.kind { TaskKind::Aria2 => { let lifecycle_epoch = aria2_lifecycle_epoch .expect("aria2 dispatch must initialize a control epoch"); + if self.aria2_dispatch_held() { + let _control_guard = self.acquire_aria2_control(&id).await; + self.clear_aria2_allocation_for_epoch(&id, lifecycle_epoch) + .await; + self.finish_aria2_dispatch(&id, lifecycle_epoch).await; + self.restore_dispatch_task_to_pending( + pending_index, + pending_ordinal, + task, + previous_cursor, + None, + ) + .await; + return; + } let add_result = self.spawner.add_uri(&id, &task.payload).await; match add_result { Ok(gid) => { @@ -4648,6 +5205,8 @@ impl QueueManager { .await && self.is_registered(&id).await; if cancelled || !current_lifecycle { + self.forget_pending_order_ordinal(&id, lifecycle_generation) + .await; drop(control_guard); log::info!( "aria2 dispatch cancellation [{}]: removing late gid {}", @@ -4695,6 +5254,8 @@ impl QueueManager { // lets the UI (and a concurrent Properties pause) // act on a lifecycle that does not yet exist. let buffered_outcome = self.remember_gid(id.clone(), gid.clone()).await; + self.forget_pending_order_ordinal(&id, lifecycle_generation) + .await; if buffered_outcome.is_none() { self.emit_state(&id, DownloadStatus::Downloading); } @@ -4757,6 +5318,13 @@ impl QueueManager { } } Err(error) => { + let daemon_unavailable = is_aria2_daemon_recovery_required(&error); + if daemon_unavailable { + // An unavailable daemon is a recoverable queue + // condition. Hold later Aria2 work before restoring + // this task so the dispatcher cannot spin on it. + self.set_aria2_dispatch_hold(true); + } self.clear_aria2_allocation_for_epoch(&id, lifecycle_epoch) .await; let _control_guard = self.acquire_aria2_control(&id).await; @@ -4765,6 +5333,20 @@ impl QueueManager { .await && self.is_registered(&id).await; if current_lifecycle { + if daemon_unavailable { + self.finish_aria2_dispatch(&id, lifecycle_epoch).await; + self.restore_dispatch_task_to_pending( + pending_index, + pending_ordinal, + task, + previous_cursor, + None, + ) + .await; + return; + } + self.forget_pending_order_ordinal(&id, lifecycle_generation) + .await; self.next_aria2_control_epoch(&id).await; self.cancel_aria2_retries(&id).await; self.clear_aria2_retry_state(&id).await; @@ -4772,6 +5354,8 @@ impl QueueManager { self.release_registered_id(&id).await; self.emit_failed(&id, error); } else { + self.forget_pending_order_ordinal(&id, lifecycle_generation) + .await; log::info!( "aria2 dispatch [{}]: ignoring stale addUri failure after a newer lifecycle took ownership", id @@ -4782,10 +5366,30 @@ impl QueueManager { } } TaskKind::Media => { + self.forget_pending_order_ordinal(&id, lifecycle_generation) + .await; + let producer_lease = self + .app_handle + .try_state::() + .map(|state| Arc::clone(&state.shutdown_terminal_barrier)); + let producer_lease = match producer_lease { + Some(barrier) => match barrier.enter().await { + Some(lease) => Some(lease), + None => { + self.release_permit_for_generation(&id, lifecycle_generation) + .await; + self.release_registered_id_for_generation(&id, lifecycle_generation) + .await; + return; + } + }, + None => None, + }; let this = Arc::clone(&self); let payload = task.payload.clone(); let id_for_task = id.clone(); tauri::async_runtime::spawn(async move { + let _producer_lease = producer_lease; let outcome = this .spawner .run_media(&id_for_task, &payload, lifecycle_generation) @@ -4801,6 +5405,87 @@ impl QueueManager { self.media_completed_generations.lock().await.get(id).copied() == Some(generation) } + pub(crate) async fn record_media_terminal_outcome( + &self, + id: &str, + generation: u64, + outcome: MediaTerminalOutcome, + ) { + self.media_terminal_outcomes + .lock() + .await + .insert(id.to_string(), (generation, outcome)); + } + + pub(crate) async fn media_terminal_outcome( + &self, + id: &str, + generation: u64, + ) -> Option { + self.media_terminal_outcomes + .lock() + .await + .get(id) + .filter(|(current_generation, _)| *current_generation == generation) + .map(|(_, outcome)| *outcome) + } + + pub(crate) async fn clear_media_terminal_outcome(&self, id: &str, generation: u64) { + let mut outcomes = self.media_terminal_outcomes.lock().await; + if outcomes + .get(id) + .is_some_and(|(current_generation, _)| *current_generation == generation) + { + outcomes.remove(id); + } + drop(outcomes); + let mut completed = self.media_completed_generations.lock().await; + if completed.get(id).copied() == Some(generation) { + completed.remove(id); + } + } + + async fn clear_media_terminal_state(&self, id: &str) { + self.media_terminal_outcomes.lock().await.remove(id); + self.media_completed_generations.lock().await.remove(id); + } + + fn clear_shutdown_completion_journal(&self, id: &str) -> Result<(), String> { + if let Some(database) = self.app_handle.try_state::() { + let mut connection = database.lock()?; + crate::db::clear_shutdown_terminal(&mut connection, id)?; + } + if let Some(shutdown_guard) = self.app_handle.try_state::() { + shutdown_guard.clear_shutdown_journal_failure_for(id); + } + Ok(()) + } + + fn persist_shutdown_completion_journal(&self, id: &str, lifecycle_generation: u64) { + let Some(shutdown_guard) = self.app_handle.try_state::() else { + return; + }; + if !shutdown_guard.is_shutting_down() { + return; + } + let result = self + .app_handle + .try_state::() + .ok_or_else(|| "persistence state is unavailable".to_string()) + .and_then(|database| { + let mut connection = database.lock()?; + crate::db::record_shutdown_terminal(&mut connection, id, lifecycle_generation) + }); + if result.is_err() { + shutdown_guard.mark_shutdown_journal_failed(id, lifecycle_generation); + log::error!( + "could not persist shutdown completion journal [id={} lifecycle_generation={}]", + id, + lifecycle_generation + ); + } + } + fn persist_completed_removal_payload(&self, id: &str) -> Result<(), String> { if !self.removal_requested(id) { return Ok(()); } if let Some(db) = self.app_handle.try_state::() { @@ -4823,8 +5508,9 @@ impl QueueManager { outcome: Result<(), String>, ) { // Completion and pause/remove/detach must observe one ordered - // lifecycle. Without this guard a terminal media outcome could emit - // Completed, then a racing pause could emit Paused afterward. + // lifecycle. The media dispatch holds a producer lease across both + // run_media and this reconciliation, so shutdown cannot finalize while + // a producer is between its result and terminal update. let _control_guard = self.acquire_aria2_control(id).await; if !self.is_registered_generation(id, lifecycle_generation).await { log::info!( @@ -4838,11 +5524,32 @@ impl QueueManager { match outcome { Ok(()) => { - if self.persist_completed_removal_payload(id).is_err() { - log::error!("could not persist media completion during removal [id={}]", id); - return; + self.persist_shutdown_completion_journal(id, lifecycle_generation); + if let Err(error) = self.persist_completed_removal_payload(id) { + // The output is already complete. Do not leak the permit + // or leave the lifecycle registered because an auxiliary + // removal-state write failed. + log::error!("could not persist media completion during removal [id={}]: {}", id, error); + } + let completed_with_file = crate::download_ownership::primary_path_for_id( + &self.app_handle, + id, + ) + .ok() + .flatten() + .and_then(|path| path.file_name().and_then(|name| name.to_str()).map(str::to_owned)) + .is_some_and(|file_name| { + use tauri::Emitter; + self.app_handle + .emit( + "download-state", + DownloadStateEvent::completed_with_file(id, file_name), + ) + .is_ok() + }); + if !completed_with_file { + self.emit_state(id, DownloadStatus::Completed); } - self.emit_state(id, DownloadStatus::Completed); self.release_registered_id_for_generation(id, lifecycle_generation) .await; } @@ -5012,6 +5719,25 @@ impl QueueManager { id: &str, outcome: PendingOutcome, progress: Option, + ) { + let terminal_barrier = self + .app_handle + .try_state::() + .map(|state| Arc::clone(&state.shutdown_terminal_barrier)); + if let Some(terminal_barrier) = terminal_barrier { + let Some(_terminal_work) = terminal_barrier.enter().await else { + return; + }; + } + self.apply_completion_locked_with_progress_unfenced(id, outcome, progress) + .await; + } + + async fn apply_completion_locked_with_progress_unfenced( + &self, + id: &str, + outcome: PendingOutcome, + progress: Option, ) { self.clear_aria2_magnet_handoff(id).await; self.clear_aria2_allocation(id).await; @@ -5082,6 +5808,29 @@ impl QueueManager { PendingOutcome::Seeding => PendingOutcome::Complete, other => other, }; + let restored_status = if verification_only { + if verification_restore_status.as_deref() == Some("completed") { + DownloadStatus::Completed + } else { + DownloadStatus::Paused + } + } else { + match verification_restore_status.as_deref() { + Some("paused") => DownloadStatus::Paused, + Some("failed") => DownloadStatus::Failed, + Some("ready") => DownloadStatus::Ready, + Some("staged") => DownloadStatus::Staged, + Some("completed") => DownloadStatus::Completed, + _ => DownloadStatus::Completed, + } + }; + if matches!(&outcome, PendingOutcome::Complete) + && matches!(restored_status, DownloadStatus::Completed) + { + if let Some(lifecycle_generation) = self.registered_lifecycle_generation(id).await { + self.persist_shutdown_completion_journal(id, lifecycle_generation); + } + } if let Some(gid) = self.aria2_gid_for_download(id) { if let Err(error) = self.reconcile_aria2_torrent_ownership(id, &gid).await { log::debug!( @@ -5130,22 +5879,6 @@ impl QueueManager { } self.release_registered_id(id).await; self.release_permit(id).await; - let restored_status = if verification_only { - if verification_restore_status.as_deref() == Some("completed") { - DownloadStatus::Completed - } else { - DownloadStatus::Paused - } - } else { - match verification_restore_status.as_deref() { - Some("paused") => DownloadStatus::Paused, - Some("failed") => DownloadStatus::Failed, - Some("ready") => DownloadStatus::Ready, - Some("staged") => DownloadStatus::Staged, - Some("completed") => DownloadStatus::Completed, - _ => DownloadStatus::Completed, - } - }; let mut event = DownloadStateEvent::new(id, restored_status); if let Some(progress) = progress { event = event.with_progress(progress); @@ -6175,7 +6908,7 @@ impl QueueManager { let payload = self.aria2_payloads.lock().await.get(&id).cloned(); if payload.is_none() { - self.apply_completion_locked_with_progress( + self.apply_completion_locked_with_progress_unfenced( &id, PendingOutcome::Error(error), progress, @@ -6220,7 +6953,7 @@ impl QueueManager { effective_connections ); if retry_action == Aria2RetryAction::Terminal { - self.apply_completion_locked_with_progress( + self.apply_completion_locked_with_progress_unfenced( &id, PendingOutcome::Error(error), progress, @@ -6437,10 +7170,51 @@ impl QueueManager { .await; } + pub(crate) async fn handle_aria2_event_with_progress_under_terminal_lease( + self: &Arc, + gid: &str, + outcome: PendingOutcome, + progress: Option, + ) { + self.handle_aria2_pending_event_unfenced( + gid, + PendingAria2Outcome { outcome, progress }, + ) + .await; + } + + pub(crate) async fn handle_aria2_event_under_terminal_lease( + self: &Arc, + gid: &str, + outcome: PendingOutcome, + ) { + self.handle_aria2_pending_event_unfenced(gid, PendingAria2Outcome::new(outcome)) + .await; + } + async fn handle_aria2_pending_event( self: &Arc, gid: &str, pending: PendingAria2Outcome, + ) { + let terminal_barrier = self + .app_handle + .try_state::() + .map(|state| Arc::clone(&state.shutdown_terminal_barrier)); + if let Some(terminal_barrier) = terminal_barrier { + let Some(_terminal_work) = terminal_barrier.enter().await else { + return; + }; + self.handle_aria2_pending_event_unfenced(gid, pending).await; + return; + } + self.handle_aria2_pending_event_unfenced(gid, pending).await; + } + + async fn handle_aria2_pending_event_unfenced( + self: &Arc, + gid: &str, + pending: PendingAria2Outcome, ) { let PendingAria2Outcome { outcome, progress } = pending; if let PendingOutcome::Error(error) = outcome { @@ -6491,7 +7265,7 @@ impl QueueManager { return; } Aria2MagnetCompletionDisposition::Failed(error) => { - self.apply_completion_locked_with_progress( + self.apply_completion_locked_with_progress_unfenced( &mapping.id, PendingOutcome::Error(error), progress, @@ -6509,19 +7283,24 @@ impl QueueManager { // payload lifecycle. self.remove_aria2_magnet_parent_result(gid).await; if let Some(pending) = child.pending { - Box::pin(self.handle_aria2_pending_event(&child.gid, pending)).await; + Box::pin(self.handle_aria2_pending_event_unfenced(&child.gid, pending)).await; } else { match child.status.as_str() { "complete" => { - Box::pin(self.handle_aria2_event(&child.gid, PendingOutcome::Complete)) + Box::pin(self.handle_aria2_pending_event_unfenced( + &child.gid, + PendingAria2Outcome::new(PendingOutcome::Complete), + )) .await; } "error" | "removed" => { - Box::pin(self.handle_aria2_event( + Box::pin(self.handle_aria2_pending_event_unfenced( &child.gid, - PendingOutcome::Error(child.error.unwrap_or_else(|| { - "aria2 magnet payload child ended unexpectedly".to_string() - })), + PendingAria2Outcome::new(PendingOutcome::Error( + child.error.unwrap_or_else(|| { + "aria2 magnet payload child ended unexpectedly".to_string() + }), + )), )) .await; } @@ -6547,7 +7326,7 @@ impl QueueManager { } } } - self.apply_completion_locked_with_progress(&mapping.id, outcome, progress) + self.apply_completion_locked_with_progress_unfenced(&mapping.id, outcome, progress) .await; } @@ -6586,6 +7365,7 @@ impl QueueManager { .enumerate() .filter_map(|(position, index)| ids.iter().any(|id| id == &pending[*index].id).then_some(position)) .collect::>(); + let mut refreshed_queue_order = None; if !selected_positions.is_empty() { let queue_tasks = queue_positions @@ -6616,12 +7396,18 @@ impl QueueManager { for (queue_index, pending_index) in queue_positions.iter().enumerate() { pending[*pending_index] = reordered[queue_index].clone(); } + refreshed_queue_order = Some(reordered); } - pending + let result = pending .iter() .filter(|task| task.queue_id == queue_id) .map(|task| task.id.clone()) - .collect() + .collect(); + drop(pending); + if let Some(reordered) = refreshed_queue_order { + self.refresh_pending_order_ordinals(queue_id, &reordered).await; + } + result } /// Atomically place a selected block at an insertion index among the @@ -6646,17 +7432,23 @@ impl QueueManager { .map(|index| pending[*index].clone()) .collect::>(); - if let Some(reordered) = reorder_selected_queue_tasks_in_order(&queue_tasks, ids, target_index) { + let reordered = reorder_selected_queue_tasks_in_order(&queue_tasks, ids, target_index); + if let Some(reordered) = reordered.as_ref() { for (queue_index, pending_index) in queue_positions.iter().enumerate() { pending[*pending_index] = reordered[queue_index].clone(); } } - pending + let result = pending .iter() .filter(|task| task.queue_id == queue_id) .map(|task| task.id.clone()) - .collect() + .collect(); + drop(pending); + if let Some(reordered) = reordered { + self.refresh_pending_order_ordinals(queue_id, &reordered).await; + } + result } /// Remove a task from pending if present (used by remove_download). @@ -6666,8 +7458,17 @@ impl QueueManager { let _admission_gate = self.admission_gate.lock().await; let mut pending = self.pending.lock().await; let before = pending.len(); + let removed_keys = pending + .iter() + .filter(|task| task.id == id) + .map(|task| (task.id.clone(), task.lifecycle_generation)) + .collect::>(); pending.retain(|t| t.id != id); let removed = pending.len() < before; + drop(pending); + for (id, generation) in removed_keys { + self.forget_pending_order_ordinal(&id, generation).await; + } if removed { self.notify.notify_one(); } @@ -6678,9 +7479,14 @@ impl QueueManager { let _admission_gate = self.admission_gate.lock().await; let mut pending = self.pending.lock().await; let before = pending.len(); + let removed = pending.iter().any(|task| { + task.id == id && task.lifecycle_generation == generation + }); pending.retain(|task| !(task.id == id && task.lifecycle_generation == generation)); - let removed = pending.len() < before; + let removed = removed && pending.len() < before; + drop(pending); if removed { + self.forget_pending_order_ordinal(id, generation).await; self.notify.notify_one(); } removed @@ -6762,6 +7568,14 @@ fn is_aria2_rpc_unavailable(error: &str) -> bool { || lower.contains("connection reset") } +fn is_aria2_daemon_recovery_required(error: &str) -> bool { + let lower = error.to_ascii_lowercase(); + lower.contains("aria2 daemon unavailable") + || lower.contains("aria2 daemon is not ready") + || lower.contains("aria2 daemon did not become ready") + || lower.contains("aria2 did not become ready") +} + fn aria2_startup_failure_for_port(port: u16, startup_error: Option<&str>) -> Option { if port != 0 { return None; @@ -9332,26 +10146,51 @@ impl SidecarSpawner for ProductionSpawner { ) .await }; - if let Ok(path) = outcome.as_ref() { - // Publish completion evidence before Finished acknowledges a racing - // removal. The control lock may be held by that waiting remover. - state.queue_manager.media_completed_generations.lock().await - .insert(id.to_string(), lifecycle_generation); - if state.queue_manager.persist_completed_removal_payload(id).is_err() { - log::error!("could not persist media completion before removal acknowledgement [id={}]", id); + let terminal_outcome = match &outcome { + Ok(_) => MediaTerminalOutcome::Completed, + Err(error) if error == crate::queue::MEDIA_RUN_CANCELLED => { + MediaTerminalOutcome::Cancelled } - let _ = crate::download_ownership::set_primary_path(&self.app_handle, id, path); - if let Some(file_name) = path.file_name().and_then(|name| name.to_str()) { - use tauri::Emitter; - let _ = self.app_handle.emit( - "download-state", - crate::ipc::DownloadStateEvent::completed_with_file(id, file_name), - ); + Err(_) => MediaTerminalOutcome::Failed, + }; + { + // Publish the exact terminal result before Finished acknowledges a + // racing control operation. That command can then distinguish a + // successful completion from a failed or cancelled media run. + let _control_guard = state.queue_manager.acquire_aria2_control(id).await; + if state + .queue_manager + .is_registered_generation(id, lifecycle_generation) + .await + { + state + .queue_manager + .record_media_terminal_outcome(id, lifecycle_generation, terminal_outcome) + .await; + if let Ok(path) = outcome.as_ref() { + state.queue_manager.media_completed_generations.lock().await + .insert(id.to_string(), lifecycle_generation); + state.queue_manager.persist_shutdown_completion_journal(id, lifecycle_generation); + if state.queue_manager.persist_completed_removal_payload(id).is_err() { + log::error!("could not persist media completion before removal acknowledgement [id={}]", id); + } + if let Err(error) = crate::download_ownership::set_primary_path(&self.app_handle, id, path) { + log::error!("could not persist completed media path [id={}]: {}", id, error); + } + } } } + let shutdown_interrupted = matches!( + &outcome, + Err(error) if error == crate::queue::MEDIA_RUN_CANCELLED + ); let _ = state .download_coordinator - .finish_media(id.to_string(), lifecycle_generation) + .finish_media_with_shutdown_outcome( + id.to_string(), + lifecycle_generation, + shutdown_interrupted, + ) .await; outcome.map(|_| ()) } @@ -9596,6 +10435,49 @@ mod tests { release: Arc, } + struct UnavailableAria2Spawner { + add_uri_calls: AtomicUsize, + gate_add_uri: bool, + add_uri_started: Arc, + release_add_uri: Arc, + } + + impl UnavailableAria2Spawner { + fn new(gate_add_uri: bool) -> Self { + Self { + add_uri_calls: AtomicUsize::new(0), + gate_add_uri, + add_uri_started: Arc::new(tokio::sync::Notify::new()), + release_add_uri: Arc::new(tokio::sync::Notify::new()), + } + } + } + + #[async_trait::async_trait] + impl SidecarSpawner for UnavailableAria2Spawner { + async fn add_uri(&self, _id: &str, _payload: &SpawnPayload) -> Result { + self.add_uri_calls.fetch_add(1, Ordering::SeqCst); + self.add_uri_started.notify_one(); + if self.gate_add_uri { + self.release_add_uri.notified().await; + } + Err("aria2 daemon is not ready".to_string()) + } + + async fn remove_uri(&self, _gid: &str) -> Result<(), String> { + Ok(()) + } + + async fn run_media( + &self, + _id: &str, + _payload: &SpawnPayload, + _lifecycle_generation: u64, + ) -> Result<(), String> { + Ok(()) + } + } + #[async_trait::async_trait] impl SidecarSpawner for BlockingSpawner { async fn add_uri(&self, _id: &str, _payload: &SpawnPayload) -> Result { @@ -9629,6 +10511,33 @@ mod tests { .and_then(|pending| pending.get(id).copied()) } + async fn enqueue_test_task( + manager: &QueueManager, + id: &str, + queue_id: &str, + kind: TaskKind, + generation: u64, + ) { + let previous_generation = manager + .reserve_enqueue_generation(id, generation) + .await + .expect("test lifecycle reservation"); + manager + .commit_reserved_enqueue( + QueuedTask { + id: id.to_string(), + queue_id: queue_id.to_string(), + kind, + payload: SpawnPayload::default(), + lifecycle_generation: generation, + }, + generation, + previous_generation, + ) + .await + .expect("test task admission"); + } + #[test] fn aria2_allocation_eligibility_matches_download_type_and_torrent_policy() { assert!(QueueManager::::aria2_allocation_phase_eligible( @@ -9663,6 +10572,32 @@ mod tests { )); } + #[tokio::test] + async fn control_guard_can_be_released_for_a_media_terminal_ack() { + let app = tauri::test::mock_builder() + .build(tauri::test::mock_context(tauri::test::noop_assets())) + .expect("mock app"); + let manager = Arc::new(QueueManager::test_new( + app.handle().clone(), + 1, + Arc::new(TestSpawner), + )); + let mut control = manager.acquire_aria2_control("media-control").await; + control.release(); + + let worker_manager = Arc::clone(&manager); + tokio::time::timeout(Duration::from_secs(1), async move { + let _worker_control = worker_manager + .acquire_aria2_control("media-control") + .await; + }) + .await + .expect("media terminal writer must acquire the released control lock"); + + control.reacquire().await; + drop(control); + } + #[tokio::test] async fn media_removal_completion_evidence_is_lifecycle_fenced() { let app = tauri::test::mock_builder() @@ -9670,12 +10605,51 @@ mod tests { let manager = QueueManager::test_new(app.handle().clone(), 1, Arc::new(TestSpawner)); manager.reserve_enqueue_generation("media-removal", 7).await.unwrap(); manager.media_completed_generations.lock().await.insert("media-removal".into(), 7); + manager + .record_media_terminal_outcome( + "media-removal", + 7, + MediaTerminalOutcome::Completed, + ) + .await; assert!(manager.media_payload_completed("media-removal", 7).await); assert!(!manager.media_payload_completed("media-removal", 8).await); manager.release_registered_id_for_generation("media-removal", 6).await; assert!(manager.media_payload_completed("media-removal", 7).await); manager.release_registered_id_for_generation("media-removal", 7).await; + assert!(manager.media_payload_completed("media-removal", 7).await); + assert_eq!( + manager + .media_terminal_outcome("media-removal", 7) + .await, + Some(MediaTerminalOutcome::Completed) + ); + + let previous = manager + .reserve_enqueue_generation("media-removal", 8) + .await + .unwrap(); + manager + .commit_reserved_enqueue( + QueuedTask { + id: "media-removal".to_string(), + queue_id: "main".to_string(), + kind: TaskKind::Media, + payload: SpawnPayload::default(), + lifecycle_generation: 8, + }, + 8, + previous, + ) + .await + .unwrap(); assert!(!manager.media_payload_completed("media-removal", 7).await); + assert_eq!( + manager + .media_terminal_outcome("media-removal", 7) + .await, + None + ); } #[tokio::test] @@ -9725,6 +10699,357 @@ mod tests { assert_eq!(allocation_pending_epoch(&manager, "telemetry"), None); } + #[tokio::test] + async fn aria2_recovery_hold_skips_aria2_but_admits_media() { + let app = tauri::test::mock_builder() + .build(tauri::test::mock_context(tauri::test::noop_assets())) + .expect("mock app"); + let manager = QueueManager::test_new(app.handle().clone(), 2, Arc::new(TestSpawner)); + enqueue_test_task(&manager, "aria2-held", "shared", TaskKind::Aria2, 1).await; + enqueue_test_task(&manager, "media-ready", "shared", TaskKind::Media, 1).await; + manager.set_aria2_dispatch_hold(true); + + let (permit, task, _, _, _, _handoff_guard) = manager + .try_admit_next_task() + .await + .expect("Media remains eligible during an Aria2 recovery hold"); + assert_eq!(task.id, "media-ready"); + assert!(matches!(task.kind, TaskKind::Media)); + manager + .release_queue_reservation_for_generation(&task.id, task.lifecycle_generation) + .await; + drop(permit); + + assert_eq!(manager.pending_order(None).await, ["aria2-held"]); + assert!(manager.try_admit_next_task().await.is_none()); + } + + #[tokio::test] + async fn aria2_hold_after_selection_restores_task_before_add_uri() { + let app = tauri::test::mock_builder() + .build(tauri::test::mock_context(tauri::test::noop_assets())) + .expect("mock app"); + let spawner = Arc::new(UnavailableAria2Spawner::new(false)); + let manager = Arc::new(QueueManager::test_new( + app.handle().clone(), + 1, + spawner.clone(), + )); + enqueue_test_task(&manager, "selected-aria2", "main", TaskKind::Aria2, 4).await; + enqueue_test_task(&manager, "queued-media", "main", TaskKind::Media, 4).await; + let ( + permit, + task, + pending_index, + previous_cursor, + pending_ordinal, + handoff_guard, + ) = manager + .try_admit_next_task() + .await + .expect("Aria2 task should be selected before recovery starts"); + + manager.set_aria2_dispatch_hold(true); + Arc::clone(&manager) + .dispatch_one( + permit, + task, + pending_index, + previous_cursor, + pending_ordinal, + handoff_guard, + ) + .await; + + assert_eq!(spawner.add_uri_calls.load(Ordering::SeqCst), 0); + assert_eq!( + manager.pending_order(None).await, + ["selected-aria2", "queued-media"] + ); + assert_eq!(manager.registered_lifecycle_generation("selected-aria2").await, Some(4)); + assert!(!manager.has_active_permit("selected-aria2").await); + assert!(manager.queue_permit_ownership.lock().await.is_empty()); + assert_eq!(manager.available_permits(), 1); + assert!(manager.aria2_dispatch_held()); + } + + #[tokio::test] + async fn concurrent_same_queue_restores_keep_fifo_when_ordinals_share_snapshot_index() { + let app = tauri::test::mock_builder() + .build(tauri::test::mock_context(tauri::test::noop_assets())) + .expect("mock app"); + let manager = QueueManager::test_new(app.handle().clone(), 2, Arc::new(TestSpawner)); + for id in ["first", "second"] { + enqueue_test_task(&manager, id, "main", TaskKind::Aria2, 7).await; + } + + let ( + first_permit, + first, + first_index, + first_cursor, + first_ordinal, + first_handoff, + ) = manager + .try_admit_next_task() + .await + .expect("first same-queue task should be selected"); + let ( + second_permit, + second, + second_index, + second_cursor, + second_ordinal, + second_handoff, + ) = manager + .try_admit_next_task() + .await + .expect("second same-queue task should be selected concurrently"); + assert_eq!(first.id, "first"); + assert_eq!(second.id, "second"); + assert_eq!(first_index, second_index); + assert!(first_ordinal < second_ordinal); + + // Restore in admission order, which makes two index-only insertions + // put the later task ahead of the earlier task at the shared index. + manager + .restore_dispatch_task_to_pending( + first_index, + first_ordinal, + first, + first_cursor, + Some(first_permit), + ) + .await; + drop(first_handoff); + manager + .restore_dispatch_task_to_pending( + second_index, + second_ordinal, + second, + second_cursor, + Some(second_permit), + ) + .await; + drop(second_handoff); + + assert_eq!(manager.pending_order(Some("main")).await, ["first", "second"]); + assert_eq!(manager.registered_lifecycle_generation("first").await, Some(7)); + assert_eq!(manager.registered_lifecycle_generation("second").await, Some(7)); + assert_eq!(manager.available_permits(), 2); + } + + #[tokio::test] + async fn pending_reorder_during_dispatch_is_preserved_when_task_is_restored() { + let app = tauri::test::mock_builder() + .build(tauri::test::mock_context(tauri::test::noop_assets())) + .expect("mock app"); + let manager = QueueManager::test_new(app.handle().clone(), 1, Arc::new(TestSpawner)); + for id in ["selected", "second", "third"] { + enqueue_test_task(&manager, id, "main", TaskKind::Aria2, 8).await; + } + + let (permit, task, index, cursor, ordinal, handoff_guard) = manager + .try_admit_next_task() + .await + .expect("first task should be selected for dispatch"); + let reordered = manager + .move_many_in_queue_to(&["third".to_string()], "main", 0) + .await; + assert_eq!(reordered, ["third", "second"]); + + manager + .restore_dispatch_task_to_pending(index, ordinal, task, cursor, Some(permit)) + .await; + drop(handoff_guard); + + assert_eq!(manager.pending_order(Some("main")).await, ["selected", "third", "second"]); + } + + #[tokio::test] + async fn rejected_shutdown_restores_selected_task_and_dispatches_it() { + let app = tauri::test::mock_builder() + .manage(crate::Aria2DaemonGuard::new()) + .build(tauri::test::mock_context(tauri::test::noop_assets())) + .expect("mock app"); + let guard = app.handle().state::(); + let manager = Arc::new(QueueManager::test_new( + app.handle().clone(), + 1, + Arc::new(TestSpawner), + )); + enqueue_test_task(&manager, "shutdown-held", "main", TaskKind::Aria2, 11).await; + + let (permit, task, pending_index, previous_cursor, pending_ordinal, handoff_guard) = manager + .try_admit_next_task() + .await + .expect("task is admitted before shutdown starts"); + assert!(guard.begin_shutdown()); + Arc::clone(&manager) + .dispatch_one( + permit, + task, + pending_index, + previous_cursor, + pending_ordinal, + handoff_guard, + ) + .await; + + assert_eq!(manager.pending_order(None).await, ["shutdown-held"]); + assert_eq!(manager.registered_lifecycle_generation("shutdown-held").await, Some(11)); + assert!(!manager.has_active_permit("shutdown-held").await); + assert!(manager.queue_permit_ownership.lock().await.is_empty()); + assert_eq!(manager.available_permits(), 1); + assert_eq!(manager.aria2_gid_for_download("shutdown-held"), None); + + assert!(guard.cancel_shutdown(), "exit rejection reopens dispatch"); + manager.wake_after_exit_rejected(); + let dispatcher = tokio::spawn(Arc::clone(&manager).run_dispatcher()); + tokio::time::timeout(Duration::from_secs(1), async { + loop { + if manager.aria2_gid_for_download("shutdown-held").as_deref() + == Some("test-gid") + { + break; + } + tokio::task::yield_now().await; + } + }) + .await + .expect("rejected shutdown must redispatch the preserved task"); + + assert!(manager.pending_order(None).await.is_empty()); + assert_eq!(manager.registered_lifecycle_generation("shutdown-held").await, Some(11)); + assert!(manager.has_active_permit("shutdown-held").await); + assert_eq!(manager.available_permits(), 0); + dispatcher.abort(); + } + + #[tokio::test] + async fn shutdown_waits_for_gated_add_uri_dispatch_handoff_to_restore() { + let app = tauri::test::mock_builder() + .manage(crate::Aria2DaemonGuard::new()) + .build(tauri::test::mock_context(tauri::test::noop_assets())) + .expect("mock app"); + let guard = app.handle().state::(); + let spawner = Arc::new(UnavailableAria2Spawner::new(true)); + let manager = Arc::new(QueueManager::test_new( + app.handle().clone(), + 1, + spawner.clone(), + )); + enqueue_test_task(&manager, "gated-shutdown", "main", TaskKind::Aria2, 13).await; + let (permit, task, index, cursor, ordinal, handoff_guard) = manager + .try_admit_next_task() + .await + .expect("task should be selected before shutdown begins"); + let dispatch = tokio::spawn(Arc::clone(&manager).dispatch_one( + permit, + task, + index, + cursor, + ordinal, + handoff_guard, + )); + + tokio::time::timeout(Duration::from_secs(1), spawner.add_uri_started.notified()) + .await + .expect("dispatch should reach the gated addUri call"); + assert!(guard.begin_shutdown()); + + let waiting_manager = Arc::clone(&manager); + let shutdown_waiter = tokio::spawn(async move { + waiting_manager.wait_for_dispatch_handoffs().await; + }); + tokio::task::yield_now().await; + assert_eq!(manager.dispatch_handoff_count.load(Ordering::Acquire), 1); + assert!(!shutdown_waiter.is_finished(), "shutdown must wait for addUri"); + + spawner.release_add_uri.notify_one(); + tokio::time::timeout(Duration::from_secs(1), dispatch) + .await + .expect("dispatch should finish after addUri is released") + .expect("dispatch task should not panic"); + tokio::time::timeout(Duration::from_secs(1), shutdown_waiter) + .await + .expect("shutdown handoff wait should drain") + .expect("shutdown waiter should not panic"); + + assert_eq!(manager.dispatch_handoff_count.load(Ordering::Acquire), 0); + assert_eq!(manager.pending_order(None).await, ["gated-shutdown"]); + assert_eq!(manager.registered_lifecycle_generation("gated-shutdown").await, Some(13)); + assert_eq!(manager.available_permits(), 1); + } + + #[tokio::test] + async fn unavailable_add_uri_requeues_selected_task_without_terminal_failure() { + use tauri::Listener; + + let app = tauri::test::mock_builder() + .build(tauri::test::mock_context(tauri::test::noop_assets())) + .expect("mock app"); + let spawner = Arc::new(UnavailableAria2Spawner::new(true)); + let manager = Arc::new(QueueManager::test_new( + app.handle().clone(), + 1, + spawner.clone(), + )); + let failures = Arc::new(AtomicUsize::new(0)); + let observed_failures = Arc::clone(&failures); + let listener = app.handle().listen("download-state", move |event| { + if let Ok(payload) = serde_json::from_str::(event.payload()) { + if payload.get("id").and_then(serde_json::Value::as_str) + == Some("unavailable-aria2") + && payload.get("status").and_then(serde_json::Value::as_str) + == Some("failed") + { + observed_failures.fetch_add(1, Ordering::SeqCst); + } + } + }); + enqueue_test_task( + &manager, + "unavailable-aria2", + "main", + TaskKind::Aria2, + 9, + ) + .await; + + let dispatcher = tokio::spawn(Arc::clone(&manager).run_dispatcher()); + tokio::time::timeout(Duration::from_secs(1), spawner.add_uri_started.notified()) + .await + .expect("selected task should reach addUri"); + manager.set_aria2_dispatch_hold(true); + spawner.release_add_uri.notify_one(); + + tokio::time::timeout(Duration::from_secs(1), async { + loop { + if spawner.add_uri_calls.load(Ordering::SeqCst) == 1 + && manager.aria2_dispatch_held() + && manager.pending_order(None).await == ["unavailable-aria2"] + && !manager.has_active_permit("unavailable-aria2").await + { + break; + } + tokio::task::yield_now().await; + } + }) + .await + .expect("unavailable Aria2 task should return to pending under the recovery hold"); + + assert_eq!(manager.registered_lifecycle_generation("unavailable-aria2").await, Some(9)); + assert_eq!(manager.available_permits(), 1); + assert!(manager.queue_permit_ownership.lock().await.is_empty()); + assert!(manager.aria2_dispatch_inflight.lock().await.is_empty()); + assert_eq!(allocation_pending_epoch(&manager, "unavailable-aria2"), None); + assert_eq!(failures.load(Ordering::SeqCst), 0); + + dispatcher.abort(); + app.handle().unlisten(listener); + } + #[tokio::test] async fn allocation_stays_pending_while_async_add_uri_is_in_flight() { let app = tauri::test::mock_builder() @@ -12628,6 +13953,25 @@ mod tests { )); } + #[test] + fn aria2_dispatch_recovery_hold_requires_explicit_daemon_unavailability() { + assert!(is_aria2_daemon_recovery_required( + "aria2 daemon unavailable: startup failed" + )); + assert!(is_aria2_daemon_recovery_required( + "aria2 did not become ready within 15 seconds" + )); + assert!(is_aria2_daemon_recovery_required( + "aria2 daemon is not ready" + )); + assert!(!is_aria2_daemon_recovery_required( + "error trying to connect: tcp connect error: Connection refused" + )); + assert!(!is_aria2_daemon_recovery_required( + "aria2 RPC connection reset after a temporary network fault" + )); + } + #[test] fn aria2_transfer_gid_is_a_stable_16_digit_option() { let gid = new_aria2_transfer_gid(); diff --git a/src/App.tsx b/src/App.tsx index ad4f422..d895333 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -299,6 +299,7 @@ function App() { const pendingScheduledStopIds = useRef(new Set()); const startupInputReady = useRef(false); const extensionProcessing = useRef(createSerialTaskQueue()); + const mediaDiscoveryProcessing = useRef(createSerialTaskQueue()); const frontendReadyUpdate = useRef>(Promise.resolve()); const pendingStartupInputs = useRef { - let handled = false; + let disposition = 'retryable'; try { - await useDownloadStore.getState().handleExtensionMediaDiscoveryUpdate(payload); - handled = true; - } finally { - // Only a completed projection is a success ACK. On failure, close the - // native waiter as retryable so the extension can safely replay the - // bounded discovery update against the native admission state. - if (handled) { - await acknowledgeExtensionMediaDiscovery(payload.request_id); - } else { - await failExtensionMediaDiscovery(payload.request_id); - } + disposition = await useDownloadStore.getState().handleExtensionMediaDiscoveryUpdate(payload); + } catch (error) { + console.error('Could not apply browser media discovery:', error); + } + // A queued update is safe to acknowledge because the store now owns it; + // stale updates are intentional no-ops. Only a rejected update stays + // retryable at the native handoff boundary. + if (disposition === 'retryable') { + await failExtensionMediaDiscovery(payload.request_id); + } else { + await acknowledgeExtensionMediaDiscovery(payload.request_id); } }, [acknowledgeExtensionMediaDiscovery, failExtensionMediaDiscovery]); @@ -414,6 +415,10 @@ function App() { return extensionProcessing.current(task); }, []); + const enqueueMediaDiscoveryInput = useCallback((task: () => void | Promise) => { + return mediaDiscoveryProcessing.current(task); + }, []); + const schedulePostQueueAction = useCallback((action: Exclude) => { clearPendingPostActionTimer(); @@ -686,7 +691,26 @@ function App() { } }); const exitCancelledListener = listen('app-exit-cancelled', event => { + if (activeExitRequestId !== event.payload.requestId) return; restoreExitAttempt(event.payload.requestId, true); + const interruptedMediaIds = Array.isArray(event.payload.interruptedMediaIds) + ? event.payload.interruptedMediaIds.filter((id): id is string => typeof id === 'string') + : []; + if (interruptedMediaIds.length > 0) { + void Promise.allSettled(interruptedMediaIds.map(async id => { + const item = useDownloadStore.getState().downloads.find(download => download.id === id); + if ( + !item + || !['downloading', 'processing', 'verifying', 'retrying'].includes(item.status) + ) { + return; + } + const resumed = await useDownloadStore.getState().resumeDownload(id); + if (!resumed) { + console.error(`Could not resume media download ${id} after exit was cancelled`); + } + })); + } }); const exitFinalFlushListener = listen('app-exit-final-flush-requested', async event => { if (activeExitRequestId !== event.payload.requestId || !exiting) { @@ -717,7 +741,6 @@ function App() { requestId: event.payload.requestId, succeeded }); - activeExitRequestId = null; } catch (error) { console.error('Failed to acknowledge the final exit flush:', error); } @@ -951,7 +974,7 @@ function App() { pendingStartupInputs.current.push({ type: 'extension-media-discovery', payload: event.payload }); return; } - enqueueAddInput(() => processExtensionMediaDiscovery(event.payload)).catch(error => { + enqueueMediaDiscoveryInput(() => processExtensionMediaDiscovery(event.payload)).catch(error => { console.error('Failed to handle browser media discovery:', error); }); }); @@ -1044,7 +1067,7 @@ function App() { disposePersistence?.(); disposePersistence = null; }; - }, [addToast, enqueueAddInput, processExtensionDownload, processExtensionMediaDiscovery, queueFrontendReadyUpdate]); + }, [addToast, enqueueAddInput, enqueueMediaDiscoveryInput, processExtensionDownload, processExtensionMediaDiscovery, queueFrontendReadyUpdate]); useEffect(() => { if (!coreReady || exitPreparing) return; @@ -1070,14 +1093,14 @@ function App() { console.error('Failed to handle queued browser extension download:', error); }); } else if (input.type === 'extension-media-discovery') { - enqueueAddInput(() => processExtensionMediaDiscovery(input.payload)).catch(error => { + enqueueMediaDiscoveryInput(() => processExtensionMediaDiscovery(input.payload)).catch(error => { console.error('Failed to handle queued browser media discovery:', error); }); } else { enqueueAddInput(() => useDownloadStore.getState().openAddModalWithUrls(input.payload)); } } - }, [coreReady, enqueueAddInput, exitPreparing, processExtensionDownload, processExtensionMediaDiscovery, showKeychainModal]); + }, [coreReady, enqueueAddInput, enqueueMediaDiscoveryInput, exitPreparing, processExtensionDownload, processExtensionMediaDiscovery, showKeychainModal]); useEffect(() => { if (!coreReady || showKeychainModal || startupResumeStarted.current || exitPreparing || frontendExitPreparing) return; diff --git a/src/components/AddDownloadPreviewRow.test.tsx b/src/components/AddDownloadPreviewRow.test.tsx new file mode 100644 index 0000000..f3b7338 --- /dev/null +++ b/src/components/AddDownloadPreviewRow.test.tsx @@ -0,0 +1,38 @@ +import { renderToStaticMarkup } from 'react-dom/server'; +import { describe, expect, it } from 'vitest'; +import { AddDownloadPreviewRow } from './AddDownloadPreviewRow'; + +describe('AddDownloadPreviewRow accessibility', () => { + it('keeps details, download selection, and media mode as separate native controls', () => { + const markup = renderToStaticMarkup( +
+ undefined} + onSelectDetails={() => undefined} + details={episode.mkv 1080p ready} + mediaControl={( + + )} + /> +
+ ); + + expect(markup).toContain('role="listitem"'); + expect(markup).not.toMatch(/role="(?:listbox|option)"/); + expect(markup).toContain('')).toBeLessThan(listItem.lastIndexOf(']*>[^]*? void; + onSelectDetails: () => void; + details: ReactNode; + mediaControl?: ReactNode; +}; + +export const AddDownloadPreviewRow = ({ + file, + selected, + detailsSelected, + selectionLabel, + onToggleSelection, + onSelectDetails, + details, + mediaControl +}: AddDownloadPreviewRowProps) => ( +
+
+ + + {mediaControl} +
+
+); diff --git a/src/components/AddDownloadsModal.tsx b/src/components/AddDownloadsModal.tsx index e897794..452dd8d 100644 --- a/src/components/AddDownloadsModal.tsx +++ b/src/components/AddDownloadsModal.tsx @@ -15,6 +15,7 @@ import { open } from '@tauri-apps/plugin-dialog'; import { writeText as writeClipboardText } from '@tauri-apps/plugin-clipboard-manager'; import { invokeCommand as invoke } from '../ipc'; import { DuplicateResolutionModal, DuplicateConflict } from './DuplicateResolutionModal'; +import { AddDownloadPreviewRow } from './AddDownloadPreviewRow'; import { canonicalizeDownloadFileName, categoryForFileName, downloadFileNameWithSuffix, downloadFileNamesMatch, downloadMediaKindsMatch, headerNameHasCredentialMaterial, isDirectMediaManifestUrl, isHttpMediaRouteUrl, isMediaUrl, isValidTorrentExcludeTrackerList, isValidTorrentTrackerList, MAX_TORRENT_STOP_TIMEOUT, MAX_TORRENT_TRACKER_INTERVAL, MAX_TORRENT_TRACKER_TIMEOUT, normalizeSpeedLimitForBackend, normalizeTorrentWebSeedDrafts, normalizeTorrentTrackerInterval, normalizeTorrentTrackerTimeout, serializeTorrentPreviewPriority, TORRENT_ENCRYPTION_POLICY_DISABLED, TORRENT_ENCRYPTION_POLICY_FORCE_ENCRYPTION, TORRENT_ENCRYPTION_POLICY_REQUIRE_CRYPTO, type MediaMode, type TorrentEncryptionPolicy, type TorrentFileAllocation } from '../utils/downloads'; import { fetchMediaMetadataDeduped, fetchMediaPlaylistMetadataDeduped } from '../utils/mediaMetadata'; import { @@ -37,6 +38,7 @@ import { canSubmitMetadataRows, applyMediaModeToRow, appendRequestUrlsAfterVersion, + buildSelectedBySourceUrl, commonMediaFormatsForRows, commonMediaQualitiesForRows, durableDownloadUrl, @@ -424,9 +426,15 @@ export const AddDownloadsModal = () => { // Run these after the busy flag is cleared. The first queued handoff opens // a fresh modal after a completed submission, while later handoffs append // to that same review session. - for (const request of queued) { + for (const request of queued.handoffs) { void useDownloadStore.getState().handleExtensionDownload(request); } + // Initial handoffs establish their row contexts synchronously. Apply + // accepted discovery updates afterward so a discovery that arrived while + // its initial capture was queued can still upgrade the correct row. + for (const update of queued.mediaDiscoveryUpdates) { + void useDownloadStore.getState().handleExtensionMediaDiscoveryUpdate(update); + } }, [releasePendingAddHandoffs, toggleAddModal]); const closeModalFromDismissAction = useCallback(() => { @@ -676,14 +684,7 @@ export const AddDownloadsModal = () => { mediaModesBySourceUrl[normalizeComparableUrl(row.sourceUrl)] = row.mediaMode; } } - const selectedBySourceUrl = Object.fromEntries( - current.map(row => [row.sourceUrl, row.selected !== false]) - ); - for (const row of current) { - if (row.playlistSourceUrl && !(row.playlistSourceUrl in selectedBySourceUrl)) { - selectedBySourceUrl[row.playlistSourceUrl] = row.selected !== false; - } - } + const selectedBySourceUrl = buildSelectedBySourceUrl(current, pendingAddRequestContexts); return reconcileDownloadRows( urls, current, @@ -2535,54 +2536,37 @@ export const AddDownloadsModal = () => {
{t($ => $.addDownloads.size)}
{t($ => $.addDownloads.status)}
-
$.addDownloads.downloadPreview)}> +
$.addDownloads.downloadPreview)}> {parsedItems.length === 0 ? (
{t($ => $.addDownloads.noLinks)}
) : ( parsedItems.map((item, i) => ( -
setSelectedItemIndex(i)} - onKeyDown={(event) => { - if (event.key === 'Enter' || event.key === ' ') { - event.preventDefault(); - setSelectedItemIndex(i); - } - }} - role="option" - aria-selected={selectedItemIndex === i} - tabIndex={0} - className={`add-download-preview-row flex flex-col text-xs px-2 py-2 cursor-pointer rounded-md group ${ - selectedItemIndex === i - ? 'is-selected border' - : 'border border-transparent' - }`} - > -
- toggleRowSelection(i)} - onClick={event => event.stopPropagation()} - aria-label={t($ => $.addDownloads.selectItem, { file: item.file })} - className="me-2 shrink-0 accent-purple-500" - /> -
+ file={item.file} + selected={item.selected !== false} + detailsSelected={selectedItemIndex === i} + selectionLabel={t($ => $.addDownloads.selectItem, { file: item.file })} + onToggleSelection={() => toggleRowSelection(i)} + onSelectDetails={() => setSelectedItemIndex(i)} + details={( + + {item.file} {item.isMedia && item.status === 'ready' && mediaQualityForRow(item) ? ( $.addDownloads.quality)}> {mediaQualityForRow(item)} ) : null} -
-
{item.size || t($ => $.addDownloads.unknown)}
-
+ + {item.size || t($ => $.addDownloads.unknown)} + {isAddDownloadMetadataLoading(item) ? ( -
+ {item.isPlaylist ? t($ => $.addDownloads.fetchingPlaylist) : t($ => $.addDownloads.fetching)} -
+ ) : ( isYtDlpDefaultMediaFormatSelected(item) ? t($ => $.addDownloads.defaultMediaFormat) @@ -2602,27 +2586,23 @@ export const AddDownloadsModal = () => { ? t($ => $.addDownloads.invalid) : t($ => $.addDownloads.ready) )} -
- {!item.isTorrent && ( - - )} -
-
+ + + )} + mediaControl={!item.isTorrent ? ( + + ) : undefined} + /> )) )}
diff --git a/src/ipc.ts b/src/ipc.ts index c8a5515..a57f7c2 100644 --- a/src/ipc.ts +++ b/src/ipc.ts @@ -268,7 +268,11 @@ type EventMap = { 'tray-action-available': null; 'tray-action-result': TrayActionResult; 'app-exit-requested': { requestId: string }; - 'app-exit-cancelled': { requestId: string; reason: 'rejected' | 'timeout' }; + 'app-exit-cancelled': { + requestId: string; + reason: 'rejected' | 'timeout' | 'signal-failed' | 'media-shutdown-failed' | 'terminal-journal-failed'; + interruptedMediaIds: string[]; + }; 'app-exit-final-flush-requested': { requestId: string }; }; diff --git a/src/store/useDownloadStore.test.ts b/src/store/useDownloadStore.test.ts index 2a5f457..05bec0d 100644 --- a/src/store/useDownloadStore.test.ts +++ b/src/store/useDownloadStore.test.ts @@ -111,6 +111,7 @@ describe('useDownloadStore', () => { pendingAddRequestContexts: {}, pendingAddRequestVersion: 0, pendingAddHandoffs: [], + pendingAddMediaDiscoveryUpdates: [], pendingAddModalBusy: false, queues: [{ id: MAIN_QUEUE_ID, name: 'Main Queue', isMain: true }], }); @@ -3652,6 +3653,38 @@ describe('useDownloadStore', () => { expect(useDownloadStore.getState().backendRegisteredIds.has('startup-destination-access')).toBe(false); }); + it('does not requeue a completion recovered from the shutdown journal', async () => { + const id = 'shutdown-recovered-completion'; + vi.mocked(ipc.invokeCommand).mockImplementation(async (command: string) => { + if (command === 'list_download_removals') return []; + if (command === 'db_get_all_queues') return []; + if (command === 'db_get_all_downloads') { + return [JSON.stringify({ + id, + url: 'https://example.com/completed.mp4', + fileName: 'completed.mp4', + status: 'completed', + category: 'Movies', + dateAdded: '', + queueId: MAIN_QUEUE_ID, + isMedia: true, + hasBeenDispatched: true + })]; + } + return undefined; + }); + + await useDownloadStore.getState().initDB(); + expect(useDownloadStore.getState().downloads.find(download => download.id === id)?.status) + .toBe('completed'); + + await useDownloadStore.getState().resumePendingDownloads(); + + expect(ipc.invokeCommand).not.toHaveBeenCalledWith('enqueue_many', expect.anything()); + expect(useDownloadStore.getState().downloads.find(download => download.id === id)?.status) + .toBe('completed'); + }); + it('does not show allocation for a startup Torrent batch while it is merely queued', async () => { let releaseEnqueue!: (value: Array<{ id: string; success: boolean; filename: string }>) => void; const enqueue = new Promise>(resolve => { @@ -5114,7 +5147,7 @@ describe('useDownloadStore', () => { referer: pageUrl, headers: 'Origin: https://video.example' }; - await useDownloadStore.getState().handleExtensionMediaDiscoveryUpdate(update); + expect(await useDownloadStore.getState().handleExtensionMediaDiscoveryUpdate(update)).toBe('applied'); const state = useDownloadStore.getState(); expect(state.pendingAddUrls).toBe(manifestUrl); @@ -5177,19 +5210,87 @@ describe('useDownloadStore', () => { }); it('ignores a discovered manifest when its initial handoff is no longer reviewable', async () => { - await useDownloadStore.getState().handleExtensionMediaDiscoveryUpdate({ + expect(await useDownloadStore.getState().handleExtensionMediaDiscoveryUpdate({ request_id: 'late-discovery', handoff_id: 'm-missing', phase: 'discovered', urls: ['https://cdn.example/late.m3u8'], referer: null, headers: null - }); + })).toBe('stale'); expect(useDownloadStore.getState().pendingAddUrls).toBe(''); expect(useDownloadStore.getState().pendingAddRequestContexts).toEqual({}); }); + it('queues a discovery update during a busy Add flow and applies it after release', async () => { + const handoffId = 'm-busy-discovery'; + const pageUrl = 'https://video.example/watch?id=busy'; + const manifestUrl = 'https://cdn.example/busy.m3u8'; + useDownloadStore.getState().openAddModalWithUrls( + pageUrl, + pageUrl, + null, + null, + null, + true, + undefined, + false, + undefined, + false, + undefined, + undefined, + handoffId + ); + useDownloadStore.getState().setPendingAddModalBusy(true); + + const update: ExtensionMediaDiscoveryUpdate = { + request_id: 'busy-discovery-request', + handoff_id: handoffId, + phase: 'discovered', + urls: [manifestUrl], + referer: pageUrl, + headers: null + }; + expect(await useDownloadStore.getState().handleExtensionMediaDiscoveryUpdate(update)).toBe('queued'); + expect(useDownloadStore.getState().pendingAddUrls).toBe(pageUrl); + expect(useDownloadStore.getState().pendingAddMediaDiscoveryUpdates).toEqual([update]); + + const queued = useDownloadStore.getState().releasePendingAddHandoffs(); + expect(queued).toEqual({ handoffs: [], mediaDiscoveryUpdates: [update] }); + expect(await useDownloadStore.getState().handleExtensionMediaDiscoveryUpdate(queued.mediaDiscoveryUpdates[0])).toBe('applied'); + expect(useDownloadStore.getState().pendingAddUrls).toBe(manifestUrl); + expect(useDownloadStore.getState().pendingAddRequestContexts[manifestUrl]).toMatchObject({ + handoffId, + replacesUrl: pageUrl + }); + }); + + it('returns retryable instead of acknowledging when a busy discovery queue is full', async () => { + useDownloadStore.getState().setPendingAddModalBusy(true); + const queue = Array.from({ length: 64 }, (_, index): ExtensionMediaDiscoveryUpdate => ({ + request_id: `queued-${index}`, + handoff_id: `handoff-${index}`, + phase: 'discovered', + urls: [`https://cdn.example/${index}.m3u8`], + referer: null, + headers: null + })); + useDownloadStore.setState({ pendingAddMediaDiscoveryUpdates: queue }); + + const disposition = await useDownloadStore.getState().handleExtensionMediaDiscoveryUpdate({ + request_id: 'queue-overflow', + handoff_id: 'handoff-overflow', + phase: 'discovered', + urls: ['https://cdn.example/overflow.m3u8'], + referer: null, + headers: null + }); + + expect(disposition).toBe('retryable'); + expect(useDownloadStore.getState().pendingAddMediaDiscoveryUpdates).toHaveLength(64); + }); + it('routes direct manifests through media header and cookie suppression without forcing the mode', () => { const url = 'https://cdn.example/live/stream.m3u8?token=abc'; useDownloadStore.getState().openAddModalWithUrls( @@ -5285,12 +5386,13 @@ describe('useDownloadStore', () => { expect(duplicateResolved).toBe(false); const queued = useDownloadStore.getState().releasePendingAddHandoffs(); - expect(queued).toHaveLength(1); - expect(queued[0]).toMatchObject({ request_id: 'busy-handoff-1' }); + expect(queued.handoffs).toHaveLength(1); + expect(queued.handoffs[0]).toMatchObject({ request_id: 'busy-handoff-1' }); + expect(queued.mediaDiscoveryUpdates).toEqual([]); expect(useDownloadStore.getState().pendingAddHandoffs).toEqual([]); expect(useDownloadStore.getState().pendingAddModalBusy).toBe(false); - await useDownloadStore.getState().handleExtensionDownload(queued[0]); + await useDownloadStore.getState().handleExtensionDownload(queued.handoffs[0]); await Promise.all([firstDelivery, duplicateDelivery]); expect(firstResolved).toBe(true); expect(duplicateResolved).toBe(true); diff --git a/src/store/useDownloadStore.ts b/src/store/useDownloadStore.ts index 7a416f6..2d7ec24 100644 --- a/src/store/useDownloadStore.ts +++ b/src/store/useDownloadStore.ts @@ -1201,6 +1201,10 @@ export type PendingAddRequestContext = { torrentCacheId?: string; }; +export type MediaDiscoveryDisposition = 'applied' | 'queued' | 'stale' | 'retryable'; + +const MAX_PENDING_ADD_MEDIA_DISCOVERY_UPDATES = 64; + export type DeleteModalState = { isOpen: boolean; downloadIds?: string[]; @@ -1246,11 +1250,16 @@ interface DownloadState { pendingAddRequestVersion: number; /** Extension handoffs received while the Add window is mutating its rows. */ pendingAddHandoffs: ExtensionDownloadRequest[]; + /** Accepted media-discovery updates waiting for the current Add mutation. */ + pendingAddMediaDiscoveryUpdates: ExtensionMediaDiscoveryUpdate[]; pendingAddModalBusy: boolean; selectedPropertiesDownloadId: string | null; toggleAddModal: (isOpen: boolean) => void; setPendingAddModalBusy: (busy: boolean) => void; - releasePendingAddHandoffs: () => ExtensionDownloadRequest[]; + releasePendingAddHandoffs: () => { + handoffs: ExtensionDownloadRequest[]; + mediaDiscoveryUpdates: ExtensionMediaDiscoveryUpdate[]; + }; openAddModalWithUrls: ( urls: string, referer?: string | null, @@ -1267,7 +1276,9 @@ interface DownloadState { handoffId?: string ) => void; handleExtensionDownload: (request: ExtensionDownloadRequest) => Promise; - handleExtensionMediaDiscoveryUpdate: (update: ExtensionMediaDiscoveryUpdate) => Promise; + handleExtensionMediaDiscoveryUpdate: ( + update: ExtensionMediaDiscoveryUpdate + ) => Promise; deleteModalState: DeleteModalState; openDeleteModal: (downloadIds?: string | string[]) => void; closeDeleteModal: () => void; @@ -2010,6 +2021,7 @@ export const useDownloadStore = create((set, get) => { pendingAddRequestContexts: {}, pendingAddRequestVersion: 0, pendingAddHandoffs: [], + pendingAddMediaDiscoveryUpdates: [], pendingAddModalBusy: false, selectedPropertiesDownloadId: null, deleteModalState: { isOpen: false }, @@ -2032,6 +2044,7 @@ export const useDownloadStore = create((set, get) => { pendingAddBatch: false, pendingAddBatchName: '', pendingAddRequestContexts: {}, + pendingAddMediaDiscoveryUpdates: [], // Invalidate any in-flight Add-modal handoff even when the modal is // opened or closed without URLs. pendingAddRequestVersion: state.pendingAddRequestVersion + 1 @@ -2039,14 +2052,17 @@ export const useDownloadStore = create((set, get) => { setPendingAddModalBusy: (busy) => set({ pendingAddModalBusy: busy }), releasePendingAddHandoffs: () => { let handoffs: ExtensionDownloadRequest[] = []; + let mediaDiscoveryUpdates: ExtensionMediaDiscoveryUpdate[] = []; set(state => { handoffs = state.pendingAddHandoffs; + mediaDiscoveryUpdates = state.pendingAddMediaDiscoveryUpdates; return { pendingAddHandoffs: [], + pendingAddMediaDiscoveryUpdates: [], pendingAddModalBusy: false }; }); - return handoffs; + return { handoffs, mediaDiscoveryUpdates }; }, openAddModalWithUrls: ( urls, @@ -2212,26 +2228,59 @@ export const useDownloadStore = create((set, get) => { handleExtensionMediaDiscoveryUpdate: async (update) => { const handoffId = update.handoff_id?.trim(); const discoveredUrl = update.urls?.length === 1 ? update.urls[0]?.trim() : ''; - if (!handoffId || update.phase !== 'discovered' || !discoveredUrl) return; + if (!handoffId || update.phase !== 'discovered' || !discoveredUrl) return 'retryable'; + + const comparable = (url: string) => { + try { return new URL(url).href; } catch { return url; } + }; + const sameQueuedUpdate = (queued: ExtensionMediaDiscoveryUpdate) => + queued.handoff_id?.trim() === handoffId + && queued.phase === update.phase + && queued.urls?.length === 1 + && comparable(queued.urls[0]?.trim() || '') === comparable(discoveredUrl) + && (queued.referer?.trim() || '') === (update.referer?.trim() || '') + && (stripSensitiveMediaHeaders(queued.headers) || '') + === (stripSensitiveMediaHeaders(update.headers) || ''); + + let disposition: MediaDiscoveryDisposition = 'stale'; set(state => { - if (!state.isAddModalOpen || state.pendingAddModalBusy) return state; + if (state.pendingAddModalBusy) { + const queuedForHandoff = state.pendingAddMediaDiscoveryUpdates.find( + candidate => candidate.handoff_id?.trim() === handoffId + ); + if (queuedForHandoff) { + disposition = sameQueuedUpdate(queuedForHandoff) ? 'queued' : 'retryable'; + return state; + } + if (state.pendingAddMediaDiscoveryUpdates.length >= MAX_PENDING_ADD_MEDIA_DISCOVERY_UPDATES) { + disposition = 'retryable'; + return state; + } + disposition = 'queued'; + return { + pendingAddMediaDiscoveryUpdates: [ + ...state.pendingAddMediaDiscoveryUpdates, + update + ] + }; + } + if (!state.isAddModalOpen) return state; const matchingEntry = Object.entries(state.pendingAddRequestContexts) .find(([, context]) => context.handoffId === handoffId && context.media === true); if (!matchingEntry) return state; const [currentUrl, currentContext] = matchingEntry; const nextUrl = discoveredUrl; - const currentComparable = (() => { - try { return new URL(currentUrl).href; } catch { return currentUrl; } - })(); - const nextComparable = (() => { - try { return new URL(nextUrl).href; } catch { return nextUrl; } - })(); + const currentComparable = comparable(currentUrl); + const nextComparable = comparable(nextUrl); // Backend retries are idempotent, so a duplicate delivery of the same // discovered target must not create a new request version or restart // metadata work for an already-upgraded row. - if (currentComparable === nextComparable) return state; + if (currentComparable === nextComparable) { + disposition = 'applied'; + return state; + } const existingDifferentContext = Object.entries(state.pendingAddRequestContexts) .some(([url, context]) => url !== currentUrl && context.handoffId !== handoffId @@ -2272,6 +2321,7 @@ export const useDownloadStore = create((set, get) => { const pendingAddMediaUrls = Object.entries(nextContexts) .filter(([, context]) => context.mediaMode === 'media') .map(([url]) => url); + disposition = 'applied'; return { pendingAddUrls, pendingAddReferer: nextContexts[nextComparable]?.referer || state.pendingAddReferer, @@ -2282,6 +2332,7 @@ export const useDownloadStore = create((set, get) => { pendingAddRequestVersion: nextVersion }; }); + return disposition; }, setSelectedPropertiesDownloadId: (id) => set({ selectedPropertiesDownloadId: id }), addDownload: async (item, action) => { diff --git a/src/utils/addDownloadMetadata.test.ts b/src/utils/addDownloadMetadata.test.ts index 3c5e414..b140764 100644 --- a/src/utils/addDownloadMetadata.test.ts +++ b/src/utils/addDownloadMetadata.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from 'vitest'; import { appendRequestUrlsAfterVersion, applyMediaModeToRow, + buildSelectedBySourceUrl, classifyMediaMetadataError, commonMediaFormatsForRows, canSubmitMetadataRows, @@ -79,6 +80,30 @@ describe('add download metadata workflow', () => { }); }); + it('keeps an unchecked row unchecked when discovery replaces its page URL', () => { + const pageUrl = 'https://video.example/watch?id=1'; + const manifestUrl = 'https://cdn.example/master.m3u8'; + const uncheckedPage = row({ sourceUrl: pageUrl, downloadUrl: pageUrl, selected: false }); + const selectedBySourceUrl = buildSelectedBySourceUrl([uncheckedPage], { + [manifestUrl]: { replacesUrl: pageUrl } + }); + + const [replacement] = reconcileDownloadRows( + manifestUrl, + [uncheckedPage], + undefined, + new Set(), + undefined, + {}, + {}, + {}, + selectedBySourceUrl + ); + + expect(replacement.sourceUrl).toBe(manifestUrl); + expect(replacement.selected).toBe(false); + }); + it('deduplicates normalized URLs and marks malformed or unsupported URLs invalid', () => { let nextId = 0; const rows = reconcileDownloadRows( diff --git a/src/utils/addDownloadMetadata.ts b/src/utils/addDownloadMetadata.ts index 061f1b2..26eb076 100644 --- a/src/utils/addDownloadMetadata.ts +++ b/src/utils/addDownloadMetadata.ts @@ -558,6 +558,36 @@ const comparableUrl = (rawUrl: string): string => { } }; +export const buildSelectedBySourceUrl = ( + rows: readonly Pick[], + requestContexts: Readonly> +): Record => { + const selectedBySourceUrl: Record = {}; + for (const row of rows) { + const selected = row.selected !== false; + selectedBySourceUrl[comparableUrl(row.sourceUrl)] = selected; + if (row.playlistSourceUrl) { + const playlistUrl = comparableUrl(row.playlistSourceUrl); + if (!(playlistUrl in selectedBySourceUrl)) { + selectedBySourceUrl[playlistUrl] = selected; + } + } + } + + // Discovery changes the input URL while retaining the original row as its + // logical identity. Carry an explicit unchecked state across that alias so + // reconciliation cannot default the replacement manifest back to selected. + for (const [url, context] of Object.entries(requestContexts)) { + if (!context.replacesUrl) continue; + const originalUrl = comparableUrl(context.replacesUrl); + const replacementUrl = comparableUrl(url); + if (originalUrl in selectedBySourceUrl && !(replacementUrl in selectedBySourceUrl)) { + selectedBySourceUrl[replacementUrl] = selectedBySourceUrl[originalUrl]; + } + } + return selectedBySourceUrl; +}; + export const appendRequestUrlsAfterVersion = ( rawText: string, requestContexts: Readonly>,