gsadmin/Firelink
1
0
Fork 0
mirror of https://github.com/nimbold/Firelink.git synced 2026-07-26 12:08:27 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(downloads): harden lifecycle race handling

Browse Source 
Audit Assignment 06 lifecycle paths and preserve truthful state across ambiguous Aria2 resume failures. Clean up partial startup listener registration and prevent stale resume workers from leaking or releasing current permits.
This commit is contained in:
NimBold
2026-07-15 05:34:32 +03:30
parent 80a29356e0
commit 2479ead4ed
6 changed files with 313 additions and 132 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src-tauri/src/download.rs
+47 -25
View File
@@ -147,40 +147,39 @@ async fn run_coordinator(
HashMap::<(String, u64), tokio::sync::oneshot::Sender<()>>::new();
let mut pending_captured_urls = Vec::<String>::new();
let mut frontend_ready = false;
let mut command_open = true;
let mut media_open = true;
loop {
while command_open || media_open {
tokio::select! {
command = command_rx.recv() => {
let Some(command) = command else {
break;
};
command = command_rx.recv(), if command_open => {
match command {
DownloadCmd::CaptureUrls(urls) => {
append_unique_urls(&mut pending_captured_urls, urls);
if frontend_ready && !pending_captured_urls.is_empty() {
let payload = pending_captured_urls.join("\n");
if events.emit_captured_urls(payload) {
pending_captured_urls.clear();
Some(command) => match command {
DownloadCmd::CaptureUrls(urls) => {
append_unique_urls(&mut pending_captured_urls, urls);
if frontend_ready && !pending_captured_urls.is_empty() {
let payload = pending_captured_urls.join("\n");
if events.emit_captured_urls(payload) {
pending_captured_urls.clear();
}
}
}
}
DownloadCmd::FrontendReady(ready) => {
frontend_ready = ready;
if ready && !pending_captured_urls.is_empty() {
let payload = pending_captured_urls.join("\n");
if events.emit_captured_urls(payload) {
pending_captured_urls.clear();
DownloadCmd::FrontendReady(ready) => {
frontend_ready = ready;
if ready && !pending_captured_urls.is_empty() {
let payload = pending_captured_urls.join("\n");
if events.emit_captured_urls(payload) {
pending_captured_urls.clear();
}
}
}
}
},
None => command_open = false,
}
}
command = media_rx.recv() => {
let Some(command) = command else {
continue;
};
command = media_rx.recv(), if media_open => {
match command {
Some(command) => match command {
MediaCmd::Register { id, lifecycle_generation, cancel_tx } => {
if active_media
.get(&id)
@@ -248,6 +247,8 @@ async fn run_coordinator(
cancelled_media_generations.remove(&id);
}
}
},
None => media_open = false,
}
}
}
@@ -297,9 +298,30 @@ pub(crate) fn format_duration(seconds: f64) -> String {
#[cfg(test)]
mod tests {
use super::{DownloadCmd, DownloadCoordinator, DownloadEvent};
use super::{CoordinatorEventSink, DownloadCmd, DownloadCoordinator, DownloadEvent};
use tokio::sync::mpsc;
use std::time::Duration;
#[tokio::test]
async fn coordinator_exits_when_both_command_channels_close() {
let (event_tx, _event_rx) = mpsc::unbounded_channel();
let (command_tx, command_rx) = mpsc::channel(1);
let (media_tx, media_rx) = mpsc::channel(1);
let coordinator = tokio::spawn(super::run_coordinator(
CoordinatorEventSink::Headless(event_tx),
command_rx,
media_rx,
));
drop(command_tx);
drop(media_tx);
tokio::time::timeout(Duration::from_secs(1), coordinator)
.await
.expect("coordinator did not exit after both channels closed")
.expect("coordinator task panicked");
}
#[tokio::test]
async fn buffers_captured_urls_until_frontend_is_ready() {
let (coordinator, mut events) = DownloadCoordinator::spawn_headless();
src-tauri/src/lib.rs
+103 -30
View File
@@ -3389,6 +3389,12 @@ async fn pause_download(
.await
{
Ok(status) if status == "paused" => {
// forcePause may have returned an RPC error after
// the daemon actually paused the GID. Invalidate
// terminal events already in flight before
// preserving the resumable mapping.
state.queue_manager.next_aria2_control_epoch(&id).await;
state.queue_manager.cancel_aria2_retries(&id).await;
log::info!(
"aria2 pause [{}]: forcePause failed but gid {} is paused",
id,
@@ -3574,20 +3580,25 @@ async fn resume_download(
let app_handle_clone = app_handle.clone();
drop(control_guard);
tauri::async_runtime::spawn(async move {
let acquired = queue_manager.ensure_aria2_permit(&id_clone).await;
if !acquired && !queue_manager.has_active_permit(&id_clone).await {
let had_permit = queue_manager.has_active_permit(&id_clone).await;
let permit_candidate = if had_permit {
None
} else {
queue_manager.acquire_aria2_permit_candidate().await
};
if permit_candidate.is_none() && !had_permit {
return;
}
let _control_guard = queue_manager.acquire_aria2_control(&id_clone).await;
let current_epoch = queue_manager
.is_aria2_control_epoch_current(&id_clone, control_epoch)
.await;
if queue_manager.is_aria2_retry_cancelled(&id_clone).await
|| !queue_manager
.is_aria2_control_epoch_current(&id_clone, control_epoch)
.await
|| !current_epoch
|| queue_manager.aria2_gid_for_download(&id_clone).as_deref()
!= Some(gid_clone.as_str())
|| !queue_manager.is_registered(&id_clone).await
{
queue_manager.release_permit(&id_clone).await;
return;
}
if !queue_manager
@@ -3599,9 +3610,13 @@ async fn resume_download(
id_clone,
gid_clone
);
queue_manager.release_permit(&id_clone).await;
return;
}
if let Some(permit) = permit_candidate {
let _ = queue_manager
.park_aria2_permit_if_missing(&id_clone, permit)
.await;
}
let _ = app_handle_clone.emit(
"download-state",
crate::ipc::DownloadStateEvent::new(
@@ -3649,15 +3664,50 @@ async fn resume_download(
.await;
return;
}
Ok(status) => {
Ok(status) if status == "paused" => {
queue_manager.next_aria2_control_epoch(&id_clone).await;
queue_manager.cancel_aria2_retries(&id_clone).await;
queue_manager.release_permit(&id_clone).await;
log::error!(
"aria2 resume [{}]: {}; daemon reports gid {} as {}",
"aria2 resume [{}]: {}; daemon kept gid {} paused",
id_clone,
unpause_error,
gid_clone
);
let _ = app_handle_clone.emit(
"download-state",
crate::ipc::DownloadStateEvent::new(
&id_clone,
crate::ipc::DownloadStatus::Paused,
),
);
return;
}
Ok(status) if matches!(status.as_str(), "error" | "removed") => {
let terminal_error = format!(
"{unpause_error}; daemon reports gid {gid_clone} as {status}"
);
queue_manager
.apply_completion_locked(
&id_clone,
crate::queue::PendingOutcome::Error(terminal_error),
)
.await;
return;
}
Ok(status) => {
// An unrecognized daemon state is not proof that
// the transfer stopped. Keep its permit and
// mapping so a later reconciliation can observe
// the real terminal state.
log::error!(
"aria2 resume [{}]: {}; daemon reports gid {} as {}; retaining permit",
id_clone,
unpause_error,
gid_clone,
status
);
return;
}
Err(status_error) => {
log::error!(
@@ -3667,29 +3717,15 @@ async fn resume_download(
gid_clone,
status_error
);
let _ = app_handle_clone.emit(
"download-state",
crate::ipc::DownloadStateEvent::new(
&id_clone,
crate::ipc::DownloadStatus::Failed,
),
);
return;
}
}
let _ = app_handle_clone.emit(
"download-state",
crate::ipc::DownloadStateEvent::new(
&id_clone,
crate::ipc::DownloadStatus::Failed,
),
);
return;
}
let current_epoch = queue_manager
.is_aria2_control_epoch_current(&id_clone, control_epoch)
.await;
if queue_manager.is_aria2_retry_cancelled(&id_clone).await
|| !queue_manager
.is_aria2_control_epoch_current(&id_clone, control_epoch)
.await
|| !current_epoch
|| queue_manager.aria2_gid_for_download(&id_clone).as_deref()
!= Some(gid_clone.as_str())
{
@@ -3700,7 +3736,6 @@ async fn resume_download(
serde_json::json!([gid_clone]),
)
.await;
queue_manager.release_permit(&id_clone).await;
return;
}
log::info!("aria2 resume [{}]: unpaused gid {}", id_clone, gid_clone);
@@ -3710,7 +3745,16 @@ async fn resume_download(
"active" | "waiting" => {
let resume_epoch = state.queue_manager.current_aria2_control_epoch(&id).await;
drop(control_guard);
state.queue_manager.ensure_aria2_permit(&id).await;
state.queue_manager.allow_aria2_retries(&id).await;
let had_permit = state.queue_manager.has_active_permit(&id).await;
let permit_candidate = if had_permit {
None
} else {
state.queue_manager.acquire_aria2_permit_candidate().await
};
if permit_candidate.is_none() && !had_permit {
return Ok(true);
}
let _control_guard = state.queue_manager.acquire_aria2_control(&id).await;
let still_current = state.queue_manager.is_registered(&id).await
&& !state.queue_manager.is_aria2_retry_cancelled(&id).await
@@ -3720,6 +3764,12 @@ async fn resume_download(
.await
&& state.queue_manager.aria2_gid_for_download(&id).as_deref() == Some(gid.as_str());
if still_current {
if let Some(permit) = permit_candidate {
let _ = state
.queue_manager
.park_aria2_permit_if_missing(&id, permit)
.await;
}
log::info!(
"aria2 resume [{}]: gid {} already {}; no duplicate job created",
id,
@@ -4115,6 +4165,27 @@ async fn aria2_daemon_is_reachable(port: u16, secret: &str) -> bool {
false
}
/// Distinguish a temporary RPC/WebSocket outage from a daemon that actually
/// exited. The latter invalidates Aria2 GIDs and permits; the former must leave
/// them intact so reconnect reconciliation can recover the transfer.
fn aria2_daemon_process_exited(app_handle: &tauri::AppHandle) -> bool {
let guard = app_handle.state::<Aria2DaemonGuard>();
let Ok(mut child) = guard.child.lock() else {
return false;
};
match child.as_mut() {
Some(child) => match child.try_wait() {
Ok(Some(_)) => true,
Ok(None) => false,
Err(error) => {
log::warn!("could not verify aria2 process state after RPC outage: {error}");
false
}
},
None => true,
}
}
fn aria2_gid_not_found(error: &str) -> bool {
let lower = error.to_ascii_lowercase();
lower.contains("gid") && lower.contains("not found")
@@ -6540,7 +6611,9 @@ pub fn run() {
ws_retries += 1;
let state = app_handle_bg.state::<AppState>();
let port = state.aria2_port.load(std::sync::atomic::Ordering::Relaxed);
if !aria2_daemon_is_reachable(port, &state.aria2_secret).await {
if !aria2_daemon_is_reachable(port, &state.aria2_secret).await
&& aria2_daemon_process_exited(&app_handle_bg)
{
state.queue_manager.clear_aria2_permits().await;
}
tokio::time::sleep(std::time::Duration::from_secs(4)).await;
src-tauri/src/queue.rs
+27
View File
@@ -488,6 +488,13 @@ impl<R: tauri::Runtime> QueueManager<R> {
}
}
/// Acquire a permit without attaching it to a download yet. Resume
/// workers use this while they are outside the per-download control lock;
/// the permit is parked only after the worker revalidates its epoch.
pub async fn acquire_aria2_permit_candidate(&self) -> Option<OwnedSemaphorePermit> {
self.acquire_permit_after_retirement().await
}
fn retire_slot_if_needed(&self) -> bool {
let mut debt = self.slots_to_retire.load(Ordering::Relaxed);
while debt > 0 {
@@ -512,6 +519,26 @@ impl<R: tauri::Runtime> QueueManager<R> {
.insert(id.to_string(), permit);
}
/// Park a candidate only when no newer lifecycle has already claimed the
/// download. Dropping a duplicate candidate returns its slot safely.
pub async fn park_aria2_permit_if_missing(
&self,
id: &str,
permit: OwnedSemaphorePermit,
) -> bool {
let mut permits = self.active_permits.lock().await;
if permits.contains_key(id) {
return false;
}
permits.insert(id.to_string(), permit);
drop(permits);
self.active_kinds
.lock()
.await
.insert(id.to_string(), TaskKind::Aria2);
true
}
async fn tag_permit_generation(&self, id: &str, generation: u64) {
self.active_permit_generations
.lock()
src-tauri/src/retry.rs
+42 -13
View File
@@ -76,13 +76,14 @@ pub fn backoff_for(strike: usize) -> Duration {
/// that also mentions "timeout" in a URL) still fails fast.
pub fn is_permanent_network_error(message: &str) -> bool {
let m = message.to_ascii_lowercase();
const PERMANENT: [&str; 9] = [
"http 401",
"http 403",
"http 404",
"http 404.",
"http 410",
"http 451",
const PERMANENT_HTTP_STATUS: [&str; 5] = ["401", "403", "404", "410", "451"];
if PERMANENT_HTTP_STATUS
.iter()
.any(|status| contains_http_status(&m, status))
{
return true;
}
const PERMANENT: [&str; 3] = [
"404 not found",
"permission denied",
"no space left on device",
@@ -90,6 +91,32 @@ pub fn is_permanent_network_error(message: &str) -> bool {
PERMANENT.iter().any(|p| m.contains(p))
}
/// Match the HTTP status formats emitted by both clients, including
/// `HTTP/1.1 403` and `HTTP Error 403`, not only the shorthand `HTTP 403`.
fn contains_http_status(message: &str, status: &str) -> bool {
let tokens = message
.split(|character: char| {
character.is_ascii_whitespace()
|| matches!(character, '/' | '.' | ':' | '-' | '_')
})
.filter(|token| !token.is_empty())
.collect::<Vec<_>>();
tokens.windows(2).any(|window| window[0] == "http" && window[1] == status)
|| tokens.windows(3).any(|window| {
window[0] == "http"
&& (window[1] == "error"
|| window[1].chars().all(|character| character.is_ascii_digit()))
&& window[2] == status
})
|| tokens.windows(4).any(|window| {
window[0] == "http"
&& window[1].chars().all(|character| character.is_ascii_digit())
&& window[2].chars().all(|character| character.is_ascii_digit())
&& window[3] == status
})
}
pub fn is_transient_network_error(message: &str) -> bool {
if is_permanent_network_error(message) {
return false;
@@ -97,7 +124,7 @@ pub fn is_transient_network_error(message: &str) -> bool {
let m = message.to_ascii_lowercase();
const TRANSIENT: [&str; 38] = [
const TRANSIENT: [&str; 34] = [
// socket-layer / HTTP-client phrasing surfaced by aria2 and yt-dlp
"timed out",
"timeout",
@@ -117,12 +144,8 @@ pub fn is_transient_network_error(message: &str) -> bool {
"tls handshake failure",
"ssl/tls handshake failure",
// HTTP-level transient
"http 408",
"request timeout",
"http 503",
"503 service unavailable",
"http 429",
"http error 429",
"429 too many requests",
// aria2c HTTP error formats
"status=408",
@@ -141,7 +164,10 @@ pub fn is_transient_network_error(message: &str) -> bool {
"timeout.",
"invalid range header",
];
TRANSIENT.iter().any(|t| m.contains(t))
contains_http_status(&m, "408")
|| contains_http_status(&m, "429")
|| contains_http_status(&m, "503")
|| TRANSIENT.iter().any(|t| m.contains(t))
}
/// Outcome of a cancel-safe backoff sleep wrapped around a transient retry.
@@ -252,6 +278,7 @@ mod tests {
#[test]
fn classifies_http_503_as_transient() {
assert!(is_transient_network_error("HTTP 503 Service Unavailable"));
assert!(is_transient_network_error("HTTP/1.1 503 Service Unavailable"));
assert!(is_transient_network_error(
"http://127.0.0.1/file returned HTTP 503 Service Unavailable"
));
@@ -291,6 +318,8 @@ mod tests {
#[test]
fn refuses_to_retry_permanent_http_statuses() {
assert!(!is_transient_network_error("HTTP 404 Not Found"));
assert!(!is_transient_network_error("HTTP/1.1 403 Forbidden"));
assert!(!is_transient_network_error("HTTP Error 404: Not Found"));
assert!(!is_transient_network_error("HTTP 403 Forbidden"));
assert!(!is_transient_network_error("HTTP 410 Gone"));
assert!(!is_transient_network_error("HTTP 401 Unauthorized"));
src-tauri/tests/queue_manager.rs
+18
View File
@@ -227,6 +227,24 @@ async fn ensure_aria2_permit_does_not_double_acquire() {
assert_eq!(mgr.available_permits(), 2);
}
#[tokio::test]
async fn stale_aria2_permit_candidate_cannot_replace_current_permit() {
let (mgr, _spawner) = make_manager(2);
let existing = mgr.acquire_permit().await.unwrap();
assert!(mgr
.park_aria2_permit_if_missing("a", existing)
.await);
let candidate = mgr.acquire_aria2_permit_candidate().await.unwrap();
assert!(!mgr
.park_aria2_permit_if_missing("a", candidate)
.await);
assert_eq!(mgr.available_permits(), 1);
mgr.release_permit("a").await;
assert_eq!(mgr.available_permits(), 2);
}
#[tokio::test]
async fn aria2_control_epoch_invalidates_stale_resume_workers() {
let (mgr, _spawner) = make_manager(1);
src/App.tsx
+76 -64
View File
@@ -243,12 +243,79 @@ function App() {
useEffect(() => {
let active = true;
let cleanupListeners: (() => void) | null = null;
const initialize = async () => {
let unlistenDownload: (() => void) | null = null;
let unlistenTerminalState: (() => void) | null = null;
let unlistenExtension: (() => void) | null = null;
let unlistenDeepLink: (() => void) | null = null;
const disposeListeners = () => {
invoke('set_extension_frontend_ready', { ready: false }).catch(() => {});
unlistenTerminalState?.();
unlistenTerminalState = null;
unlistenExtension?.();
unlistenExtension = null;
unlistenDeepLink?.();
unlistenDeepLink = null;
unlistenDownload?.();
unlistenDownload = null;
};
try {
unlistenDownload = await initDownloadListener();
unlistenTerminalState = await listen('download-state', (event) => {
if (event.payload.status !== 'completed' && event.payload.status !== 'failed') return;
const settings = useSettingsStore.getState();
if (event.payload.status === 'completed' && settings.playCompletionSound) {
playCompletionChime().catch(error => {
console.error('Completion sound failed:', error);
});
}
if (!settings.showNotifications) return;
const item = useDownloadStore.getState().downloads.find(d => d.id === event.payload.id);
const fileName = item?.fileName || 'A file';
if (event.payload.status === 'completed') {
try {
sendNotification({
title: 'Download Complete',
body: `${fileName} has finished downloading.`
});
} catch (error) {
console.error('Completion notification failed:', error);
}
} else {
try {
sendNotification({
title: 'Download Failed',
body: `${fileName} failed to download.`,
});
} catch (error) {
console.error('Failure notification failed:', error);
}
}
});
unlistenExtension = await listen('extension-add-download', (event) => {
useDownloadStore.getState().handleExtensionDownload(event.payload).catch(error => {
console.error('Failed to handle browser extension download:', error);
});
});
unlistenDeepLink = await listen('deep-link-add-download', (event) => {
useDownloadStore.getState().openAddModalWithUrls(event.payload);
});
cleanupListeners = disposeListeners;
if (!active) {
disposeListeners();
cleanupListeners = null;
return;
}
await initializeDownloadState();
if (!active) return;
setCoreReady(true);
} catch (error) {
disposeListeners();
cleanupListeners = null;
if (!active) return;
console.error('Failed to initialize Firelink state:', error);
addToast({
@@ -315,10 +382,18 @@ function App() {
isActionable: true
});
}
if (!active) return;
setCoreReady(true);
invoke('set_extension_frontend_ready', { ready: true }).catch(error => {
console.error('Failed to activate browser extension integration:', error);
});
};
void initialize();
return () => {
active = false;
cleanupListeners?.();
cleanupListeners = null;
};
}, [addToast]);
@@ -604,69 +679,6 @@ function App() {
}
}, [theme]);
useEffect(() => {
if (!coreReady) return;
let disposed = false;
const unlistenDownload = initDownloadListener();
const unlistenTerminalState = listen('download-state', (event) => {
if (event.payload.status !== 'completed' && event.payload.status !== 'failed') return;
const settings = useSettingsStore.getState();
if (event.payload.status === 'completed' && settings.playCompletionSound) {
playCompletionChime().catch(error => {
console.error('Completion sound failed:', error);
});
}
if (!settings.showNotifications) return;
const item = useDownloadStore.getState().downloads.find(d => d.id === event.payload.id);
const fileName = item?.fileName || 'A file';
if (event.payload.status === 'completed') {
try {
sendNotification({
title: 'Download Complete',
body: `${fileName} has finished downloading.`
});
} catch (error) {
console.error('Completion notification failed:', error);
}
} else {
try {
sendNotification({
title: 'Download Failed',
body: `${fileName} failed to download.`,
});
} catch (error) {
console.error('Failure notification failed:', error);
}
}
});
const unlistenExtension = listen('extension-add-download', (event) => {
useDownloadStore.getState().handleExtensionDownload(event.payload).catch(error => {
console.error('Failed to handle browser extension download:', error);
});
});
const unlistenDeepLink = listen('deep-link-add-download', (event) => {
useDownloadStore.getState().openAddModalWithUrls(event.payload);
});
Promise.all([unlistenExtension, unlistenDeepLink])
.then(() => {
if (disposed) return;
return invoke('set_extension_frontend_ready', { ready: true });
})
.catch(error => console.error('Failed to activate browser extension integration:', error));
return () => {
disposed = true;
invoke('set_extension_frontend_ready', { ready: false }).catch(() => {});
unlistenTerminalState.then(f => f());
unlistenExtension.then(f => f());
unlistenDeepLink.then(f => f());
unlistenDownload.then(f => { if (f) f(); });
};
}, [coreReady]);
return (
<div className={`app-shell flex h-screen w-screen overflow-hidden text-text-primary ${
hasWindowChrome ? 'app-shell--window-chrome' : ''