fix(properties): harden resume lifecycle

This commit is contained in:
NimBold
2026-08-04 19:37:25 +03:30
parent 3905b3ed89
commit 135ba75a69
7 changed files with 318 additions and 24 deletions
+131 -8
View File
@@ -4873,6 +4873,9 @@ async fn resume_download(
)
.await;
if !parked {
queue_manager
.release_aria2_permit_candidate(&id_clone, lifecycle_generation)
.await;
log::warn!(
"aria2 resume [{}]: permit ownership was not established before unpause; leaving gid {} paused",
id_clone,
@@ -4881,13 +4884,6 @@ async fn resume_download(
return;
}
}
let _ = app_handle_clone.emit(
"download-state",
crate::ipc::DownloadStateEvent::new(
&id_clone,
crate::ipc::DownloadStatus::Downloading,
),
);
let unpause_error = match rpc_call(
aria2_port,
&aria2_secret,
@@ -4902,8 +4898,31 @@ async fn resume_download(
Err(error) => Some(format!("failed to resume aria2 gid {gid_clone}: {error}")),
};
if let Some(unpause_error) = unpause_error {
match aria2_download_status(aria2_port, &aria2_secret, &gid_clone).await {
match verify_aria2_resume_status(aria2_port, &aria2_secret, &gid_clone).await {
Ok(status) if matches!(status.as_str(), "active" | "waiting") => {
let still_current = queue_manager
.is_aria2_control_epoch_current(&id_clone, control_epoch)
.await
&& queue_manager.aria2_gid_for_download(&id_clone).as_deref()
== Some(gid_clone.as_str());
if !still_current {
let _ = rpc_call(
aria2_port,
&aria2_secret,
"aria2.forcePause",
serde_json::json!([gid_clone]),
)
.await;
return;
}
use tauri::Emitter;
let _ = app_handle_clone.emit(
"download-state",
crate::ipc::DownloadStateEvent::new(
&id_clone,
crate::ipc::DownloadStatus::Downloading,
),
);
log::warn!(
"aria2 resume [{}]: {} but daemon reports gid {} as {}; retaining permit",
id_clone,
@@ -4985,6 +5004,79 @@ async fn resume_download(
}
}
}
// A successful unpause RPC is not itself the postcondition:
// aria2 may still report the GID as paused, complete, or
// otherwise unavailable. Verify the daemon state before
// publishing Downloading to the renderer.
let status_after_unpause = match verify_aria2_resume_status(
aria2_port,
&aria2_secret,
&gid_clone,
)
.await
{
Ok(status) => status,
Err(error) => {
log::error!(
"aria2 resume [{}]: unpause succeeded but gid {} could not be verified: {}; retaining permit",
id_clone,
gid_clone,
error
);
return;
}
};
match status_after_unpause.as_str() {
"active" | "waiting" => {}
"complete" => {
queue_manager
.apply_completion_locked(
&id_clone,
crate::queue::PendingOutcome::Complete,
)
.await;
return;
}
"error" | "removed" => {
let terminal_error = format!(
"aria2 resume left gid {gid_clone} in terminal state {status_after_unpause}"
);
queue_manager
.apply_completion_locked(
&id_clone,
crate::queue::PendingOutcome::Error(terminal_error),
)
.await;
return;
}
"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;
let error = "aria2 kept the download paused after resume".to_string();
log::error!(
"aria2 resume [{}]: {}; gid {} remains paused",
id_clone,
error,
gid_clone
);
let _ = app_handle_clone.emit(
"download-state",
crate::ipc::DownloadStateEvent::paused_with_error(&id_clone, error),
);
return;
}
other => {
log::error!(
"aria2 resume [{}]: unpause left gid {} in unexpected state {}; retaining permit",
id_clone,
gid_clone,
other
);
return;
}
}
let current_epoch = queue_manager
.is_aria2_control_epoch_current(&id_clone, control_epoch)
.await;
@@ -5002,6 +5094,14 @@ async fn resume_download(
.await;
return;
}
use tauri::Emitter;
let _ = app_handle_clone.emit(
"download-state",
crate::ipc::DownloadStateEvent::new(
&id_clone,
crate::ipc::DownloadStatus::Downloading,
),
);
log::info!("aria2 resume [{}]: unpaused gid {}", id_clone, gid_clone);
});
Ok(true)
@@ -5067,6 +5167,9 @@ async fn resume_download(
)
.await;
if !parked && !queue_manager.has_active_permit(&id_clone).await {
queue_manager
.release_aria2_permit_candidate(&id_clone, lifecycle_generation)
.await;
return;
}
}
@@ -5521,6 +5624,26 @@ async fn aria2_download_status(port: u16, secret: &str, gid: &str) -> Result<Str
.ok_or_else(|| format!("aria2.tellStatus returned no status for gid {gid}"))
}
/// Verify a retained-GID resume against Aria2's actual state. `unpause` can
/// return before a paused GID becomes observable as active, and a transient
/// tellStatus failure must not turn a successful resume into a false failure.
/// Retry only that ambiguous observation; terminal and active states return
/// immediately and remain authoritative.
async fn verify_aria2_resume_status(port: u16, secret: &str, gid: &str) -> Result<String, String> {
let mut last_observation = Err(format!("aria2 resume status for gid {gid} was not observed"));
for attempt in 0..4u32 {
last_observation = match aria2_download_status(port, secret, gid).await {
Ok(status) if status != "paused" => return Ok(status),
Ok(status) => Ok(status),
Err(error) => Err(error),
};
if attempt < 3 {
tokio::time::sleep(Duration::from_millis(25 * (1_u64 << attempt))).await;
}
}
last_observation
}
async fn reconcile_aria2_downloads(app_handle: &tauri::AppHandle) {
let state = app_handle.state::<AppState>();
let port = state.aria2_port.load(std::sync::atomic::Ordering::Relaxed);
+13 -1
View File
@@ -3421,9 +3421,15 @@ impl<R: tauri::Runtime> QueueManager<R> {
if let Some(epoch) = aria2_lifecycle_epoch {
self.begin_aria2_dispatch(&id, epoch).await;
}
self.emit_state(&id, DownloadStatus::Downloading);
drop(control_guard);
// Media runners do not receive an Aria2 GID. Their permit is already
// active at this point, so publish their live state before spawning
// the runner; Aria2 tasks publish only after remember_gid below.
if matches!(&task.kind, TaskKind::Media) {
self.emit_state(&id, DownloadStatus::Downloading);
}
match task.kind {
TaskKind::Aria2 => {
let lifecycle_epoch = aria2_lifecycle_epoch
@@ -3477,7 +3483,13 @@ impl<R: tauri::Runtime> QueueManager<R> {
}
return;
}
// A queued task is not a live transfer until aria2 has
// accepted it and Firelink has installed the GID
// mapping. Emitting Downloading before this point
// 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.emit_state(&id, DownloadStatus::Downloading);
let install_web_seeds = buffered_outcome.is_none()
&& task.payload.is_torrent
&& !task.payload.torrent_verify_only
+57
View File
@@ -1782,6 +1782,7 @@ async fn media_terminal_error_emits_failed_without_completed() {
tokio::time::sleep(Duration::from_millis(100)).await;
let statuses = emitted_statuses(&event_rx);
assert!(statuses.iter().any(|status| status == "downloading"));
assert!(statuses.iter().any(|status| status == "failed"));
assert!(!statuses.iter().any(|status| status == "completed"));
assert_eq!(manager.available_permits(), 1);
@@ -1841,6 +1842,62 @@ async fn aria2_permit_survives_rpc_return() {
handle.abort();
}
#[tokio::test]
async fn aria2_does_not_emit_downloading_before_gid_mapping() {
let app = mock_builder()
.build(mock_context(noop_assets()))
.expect("mock app");
let (gid_started_tx, gid_started_rx) = tokio::sync::oneshot::channel();
let spawner = Arc::new(DelayedAria2Spawner::new(gid_started_tx));
let (event_tx, event_rx) = std::sync::mpsc::channel();
app.handle().listen("download-state", move |event| {
let _ = event_tx.send(event.payload().to_string());
});
let manager = Arc::new(QueueManager::test_new(app.handle().clone(), 1, spawner));
manager.push(aria2_task("delayed-start")).await.unwrap();
let dispatcher = {
let manager = Arc::clone(&manager);
tokio::spawn(async move { manager.run_dispatcher().await })
};
gid_started_rx
.await
.expect("add_uri should begin before the delayed GID is returned");
let early_statuses = emitted_statuses(&event_rx);
assert!(
!early_statuses.iter().any(|status| status == "downloading"),
"a queued task must not be reported as downloading before its GID is mapped"
);
timeout(Duration::from_secs(1), async {
loop {
if manager.aria2_gid_for_download("delayed-start").is_some() {
break;
}
tokio::task::yield_now().await;
}
})
.await
.expect("the delayed GID should eventually be mapped");
timeout(Duration::from_secs(1), async {
loop {
if emitted_statuses(&event_rx)
.iter()
.any(|status| status == "downloading")
{
break;
}
tokio::task::yield_now().await;
}
})
.await
.expect("the transfer should become downloading only after its GID is owned");
manager.release_permit("delayed-start").await;
dispatcher.abort();
}
#[tokio::test]
async fn failed_refresh_that_leaves_gid_paused_releases_permit_but_keeps_resume_mapping() {
let app = mock_builder()
+26 -6
View File
@@ -14,6 +14,7 @@ import {
PROPERTIES_WINDOW_ACTION_RESULT,
PROPERTIES_WINDOW_REMOVED,
PROPERTIES_WINDOW_SNAPSHOT,
attachAsyncPropertiesListener,
getPropertiesLifecycleAction,
sendPropertiesActionRequest,
sendPropertiesReady,
@@ -190,7 +191,7 @@ export const PropertiesWindowApp = () => {
const id = await invoke('get_properties_window_download_id');
if (cancelled) return;
setDownloadId(id);
unlistenSnapshot = await listen<PropertiesSnapshotEvent>(PROPERTIES_WINDOW_SNAPSHOT, async event => {
const snapshotListener = await listen<PropertiesSnapshotEvent>(PROPERTIES_WINDOW_SNAPSHOT, async event => {
if (event.payload.windowLabel !== windowLabel
|| event.payload.downloadId !== id
|| event.payload.sessionId !== sessionId) return;
@@ -220,7 +221,12 @@ export const PropertiesWindowApp = () => {
}
}
});
unlistenResult = await listen<PropertiesActionResult>(PROPERTIES_WINDOW_ACTION_RESULT, event => {
if (cancelled) {
snapshotListener();
return;
}
unlistenSnapshot = snapshotListener;
const resultListener = await listen<PropertiesActionResult>(PROPERTIES_WINDOW_ACTION_RESULT, event => {
if (event.payload.windowLabel !== windowLabel
|| event.payload.downloadId !== id
|| event.payload.sessionId !== sessionId) return;
@@ -248,7 +254,12 @@ export const PropertiesWindowApp = () => {
}
}
});
unlistenRemoved = await listen<{ windowLabel: string; downloadId: string }>(PROPERTIES_WINDOW_REMOVED, event => {
if (cancelled) {
resultListener();
return;
}
unlistenResult = resultListener;
const removedListener = await listen<{ windowLabel: string; downloadId: string }>(PROPERTIES_WINDOW_REMOVED, event => {
if (event.payload.windowLabel === windowLabel && event.payload.downloadId === id) {
if (readyRetryTimer !== undefined) {
window.clearInterval(readyRetryTimer);
@@ -258,6 +269,11 @@ export const PropertiesWindowApp = () => {
setNotice(t($ => $.downloadTable.noDownloads));
}
});
if (cancelled) {
removedListener();
return;
}
unlistenRemoved = removedListener;
await sendPropertiesReady(sessionId);
if (cancelled) return;
// Tauri event listeners are registered asynchronously. If the main
@@ -315,12 +331,16 @@ export const PropertiesWindowApp = () => {
useEffect(() => {
if (!isDirty) return;
let disposed = false;
let unlisten: UnlistenFn | undefined;
void currentWindow.onCloseRequested(event => {
attachAsyncPropertiesListener(currentWindow.onCloseRequested(event => {
event.preventDefault();
setClosePrompt(true);
}).then(value => { unlisten = value; });
return () => unlisten?.();
}), () => disposed, value => { unlisten = value; });
return () => {
disposed = true;
unlisten?.();
};
}, [currentWindow, isDirty]);
const requestAction = useCallback(async (
+34 -9
View File
@@ -14,6 +14,7 @@ import {
PROPERTIES_WINDOW_CLOSED,
PROPERTIES_WINDOW_READY,
applySecretPatch,
attachAsyncPropertiesListener,
beginExclusivePropertiesAction,
createFrameCoalescer,
enqueuePropertiesAction,
@@ -176,8 +177,10 @@ export const PropertiesWindowBridgeHost = () => {
};
const handleReady = async (payload: PropertiesWindowReady) => {
if (disposed) return;
try {
await invoke('validate_properties_window_request', payload);
if (disposed) return;
const item = useDownloadStore.getState().downloads.find(download => download.id === payload.downloadId);
if (!item) {
await sendPropertiesRemoved(payload.windowLabel, payload.downloadId);
@@ -192,12 +195,14 @@ export const PropertiesWindowBridgeHost = () => {
};
const processAction = async (request: PropertiesActionRequest) => {
if (disposed) return;
let ok = false;
let error: string | undefined;
const actionKey = `${request.windowLabel}:${request.downloadId}`;
let releaseAction: (() => void) | undefined;
try {
await invoke('validate_properties_window_request', request);
if (disposed) return;
const registration = windows.get(request.windowLabel);
if (!registration
|| registration.downloadId !== request.downloadId
@@ -302,6 +307,7 @@ export const PropertiesWindowBridgeHost = () => {
} finally {
releaseAction?.();
}
if (disposed) return;
if (ok) {
try {
await sendFor(request.windowLabel, request.downloadId);
@@ -325,12 +331,14 @@ export const PropertiesWindowBridgeHost = () => {
};
const handleAction = async (request: PropertiesActionRequest) => {
if (disposed) return;
const actionKey = `${request.windowLabel}:${request.downloadId}`;
try {
// The native command validates the caller, download binding, and
// renderer session. If a ready event is delayed or lost, this valid
// action can also establish the main-window registration.
await invoke('validate_properties_window_request', request);
if (disposed) return;
synchronizeRegistration(request.windowLabel, request.downloadId, request.sessionId);
} catch {
// Stale renderer actions are deliberately ignored. The current child
@@ -349,15 +357,32 @@ export const PropertiesWindowBridgeHost = () => {
await enqueuePropertiesAction(actionChains, actionKey, () => processAction(request));
};
void listen<PropertiesWindowReady>(PROPERTIES_WINDOW_READY, event => void handleReady(event.payload)).then(value => { unlistenReady = value; });
void listen<PropertiesActionRequest>(PROPERTIES_WINDOW_ACTION_REQUEST, event => void handleAction(event.payload)).then(value => { unlistenAction = value; });
void listen<string>(PROPERTIES_WINDOW_CLOSED, event => {
const registration = windows.get(event.payload);
windows.delete(event.payload);
snapshotRevisions.delete(event.payload);
snapshotCoalescer.cancel(event.payload);
if (registration) actionsInFlight.delete(`${event.payload}:${registration.downloadId}`);
}).then(value => { unlistenClosed = value; });
attachAsyncPropertiesListener(
listen<PropertiesWindowReady>(PROPERTIES_WINDOW_READY, event => {
if (!disposed) void handleReady(event.payload);
}),
() => disposed,
value => { unlistenReady = value; },
);
attachAsyncPropertiesListener(
listen<PropertiesActionRequest>(PROPERTIES_WINDOW_ACTION_REQUEST, event => {
if (!disposed) void handleAction(event.payload);
}),
() => disposed,
value => { unlistenAction = value; },
);
attachAsyncPropertiesListener(
listen<string>(PROPERTIES_WINDOW_CLOSED, event => {
if (disposed) return;
const registration = windows.get(event.payload);
windows.delete(event.payload);
snapshotRevisions.delete(event.payload);
snapshotCoalescer.cancel(event.payload);
if (registration) actionsInFlight.delete(`${event.payload}:${registration.downloadId}`);
}),
() => disposed,
value => { unlistenClosed = value; },
);
const unsubscribeStore = useDownloadStore.subscribe((state, previous) => {
for (const [windowLabel, registration] of windows) {
+37
View File
@@ -12,6 +12,7 @@ vi.mock('@tauri-apps/api/event', () => ({
import {
applySecretPatch,
attachAsyncPropertiesListener,
beginExclusivePropertiesAction,
createFrameCoalescer,
enqueuePropertiesAction,
@@ -130,6 +131,7 @@ describe('Properties window bridge', () => {
it('derives truthful lifecycle commands from the current status', () => {
expect(getPropertiesLifecycleAction('downloading')).toBe('pause');
expect(getPropertiesLifecycleAction('queued')).toBe('pause');
expect(getPropertiesLifecycleAction('retrying')).toBe('pause');
expect(getPropertiesLifecycleAction('paused')).toBe('resume');
expect(getPropertiesLifecycleAction('ready')).toBe('start');
@@ -255,4 +257,39 @@ describe('Properties window bridge', () => {
coalescer.cancelAll();
expect(frames.size).toBe(0);
});
it('unlistens a Tauri listener that resolves after bridge cleanup', async () => {
let resolveListener!: (unlisten: () => void) => void;
const listener = new Promise<() => void>(resolve => { resolveListener = resolve; });
let disposed = true;
let assigned = false;
let unlistened = false;
attachAsyncPropertiesListener(
listener,
() => disposed,
() => { assigned = true; },
);
resolveListener(() => { unlistened = true; });
await listener;
await Promise.resolve();
expect(assigned).toBe(false);
expect(unlistened).toBe(true);
});
it('assigns a live Tauri listener while the bridge is mounted', async () => {
let resolveListener!: (unlisten: () => void) => void;
const listener = new Promise<() => void>(resolve => { resolveListener = resolve; });
const unlisten = vi.fn();
let assigned: (() => void) | undefined;
attachAsyncPropertiesListener(listener, () => false, value => { assigned = value; });
resolveListener(unlisten);
await listener;
await Promise.resolve();
expect(assigned).toBe(unlisten);
expect(unlisten).not.toHaveBeenCalled();
});
});
+20
View File
@@ -1,4 +1,5 @@
import { emitTo } from '@tauri-apps/api/event';
import type { UnlistenFn } from '@tauri-apps/api/event';
import type { DownloadProgressEvent } from './bindings/DownloadProgressEvent';
import type { DownloadStatus } from './bindings/DownloadStatus';
import type { DownloadItem } from './store/useDownloadStore';
@@ -303,6 +304,25 @@ export const createFrameCoalescer = (
};
};
// Tauri listener registration is asynchronous. React StrictMode can unmount
// an effect before `listen()` resolves; in that case assigning the late
// unlisten callback after cleanup leaks a second bridge listener. A leaked
// Properties host can process one click twice, observe the queued state from
// the first action, and turn the intended resume into an immediate pause.
export const attachAsyncPropertiesListener = <T extends UnlistenFn>(
listener: Promise<T>,
isDisposed: () => boolean,
assign: (unlisten: T) => void,
): void => {
void listener.then(unlisten => {
if (isDisposed()) {
unlisten();
return;
}
assign(unlisten);
}).catch(() => undefined);
};
export const openPropertiesWindow = (downloadId: string): Promise<string> =>
invoke('open_download_properties_window', { id: downloadId });