From 03b9e443ac51053569ef6f4f9f02c3db45fac7ce Mon Sep 17 00:00:00 2001 From: houssemko Date: Wed, 28 Oct 2026 14:24:30 +0100 Subject: [PATCH 1/2] fix(ui): read and parse torrents off the GTK thread --- src/application.rs | 102 +++++++++++++++++++++++++-------------------- src/inline_add.rs | 32 ++++++++------ 2 files changed, 77 insertions(+), 57 deletions(-) diff --git a/src/application.rs b/src/application.rs index 540723ca..c7a041ce 100644 --- a/src/application.rs +++ b/src/application.rs @@ -148,28 +148,38 @@ async fn intake_remote_torrent( .map(|s| s.to_string_lossy().into_owned()) .unwrap_or_else(|| "download".to_string()); // Multi-file torrents offer per-file switches in the inline picker's - // right-sliding page. - match crate::torrent::torrent_file_list(&bytes) { - Ok((_, entries)) if entries.len() > 1 => { - add_card.open_torrent_picker(file_name, bytes, entries); - // Picker opened, not queued: return OpenedCard (202) so the - // extension keeps the browser copy until the user chooses. - Routed::OpenedCard + // right-sliding page. Parse off the GTK thread: bencode decode and the + // info-hash hash are CPU work on up to 10 MB. + let parsed = gio::spawn_blocking(move || { + let list = crate::torrent::torrent_file_list(&bytes)?; + Ok::<_, String>((bytes, list)) + }) + .await; + let (bytes, (_tname, entries)) = match parsed { + Ok(Ok(parsed)) => parsed, + Ok(Err(e)) => { + fail(e); + return Routed::Failed; } - Ok(_) => { - if let Err(e) = manager - .enqueue_torrent_file_async(bytes, &stem, None, None) - .await - { - fail(e); - return Routed::Failed; - } - Routed::Queued + Err(_) => { + fail(gettext("Could not read that .torrent file")); + return Routed::Failed; } - Err(e) => { + }; + if entries.len() > 1 { + add_card.open_torrent_picker(file_name, bytes, entries); + // Picker opened, not queued: return OpenedCard (202) so the + // extension keeps the browser copy until the user chooses. + Routed::OpenedCard + } else { + if let Err(e) = manager + .enqueue_torrent_file_async(bytes, &stem, None, None) + .await + { fail(e); - Routed::Failed + return Routed::Failed; } + Routed::Queued } } @@ -551,36 +561,38 @@ fn handle_open_files(s: Rc, files: Vec) { .map(|s| s.to_string_lossy().into_owned()) .unwrap_or_else(|| format!("{stem}.torrent")); glib::spawn_future_local(async move { - let bytes = - gio::spawn_blocking(move || crate::torrent::read_torrent_bytes(&path)) - .await - .ok() - .flatten(); - let Some(bytes) = bytes else { - toasts.add_toast(adw::Toast::new(&gettext( - "Could not read that .torrent file", - ))); - return; + // Read and parse off the GTK thread: a large multi-file + // torrent must not stall the main loop. + let loaded = gio::spawn_blocking(move || { + let Some(bytes) = crate::torrent::read_torrent_bytes(&path) else { + return Err(gettext("Could not read that .torrent file")); + }; + let list = crate::torrent::torrent_file_list(&bytes)?; + Ok::<_, String>((bytes, list)) + }) + .await; + let (bytes, (_tname, entries)) = match loaded { + Ok(Ok(loaded)) => loaded, + Ok(Err(e)) => { + toasts.add_toast(adw::Toast::new(&crate::ui_util::esc_markup(&e))); + return; + } + Err(_) => { + toasts.add_toast(adw::Toast::new(&gettext( + "Could not read that .torrent file", + ))); + return; + } }; // Multi-file torrents offer per-file switches in the // inline picker's right-sliding page. - match crate::torrent::torrent_file_list(&bytes) { - Ok((_, entries)) if entries.len() > 1 => { - add_card.open_torrent_picker(file_name, bytes, entries); - } - Ok(_) => { - if let Err(e) = manager - .enqueue_torrent_file_async(bytes, &stem, None, None) - .await - { - toasts.add_toast(adw::Toast::new(&crate::ui_util::esc_markup( - &e, - ))); - } - } - Err(e) => { - toasts.add_toast(adw::Toast::new(&crate::ui_util::esc_markup(&e))); - } + if entries.len() > 1 { + add_card.open_torrent_picker(file_name, bytes, entries); + } else if let Err(e) = manager + .enqueue_torrent_file_async(bytes, &stem, None, None) + .await + { + toasts.add_toast(adw::Toast::new(&crate::ui_util::esc_markup(&e))); } }); continue; diff --git a/src/inline_add.rs b/src/inline_add.rs index 1cc37488..f8828d04 100644 --- a/src/inline_add.rs +++ b/src/inline_add.rs @@ -1640,22 +1640,30 @@ fn wire_torrent_picker( .basename() .map(|p| p.to_string_lossy().into_owned()) .unwrap_or_else(|| "download.torrent".to_string()); - let bytes = match file - .path() - .and_then(|p| crate::torrent::read_torrent_bytes(&p)) - { - Some(b) => b, - None => { - set_field_error(&torrent_row, &gettext("Could not read that .torrent file")); - return; - } + let Some(path) = file.path() else { + set_field_error(&torrent_row, &gettext("Could not read that .torrent file")); + return; }; - let (_tname, entries) = match crate::torrent::torrent_file_list(&bytes) { - Ok(v) => v, - Err(e) => { + // Read and parse off the GTK thread: a large multi-file torrent + // must not stall the main loop. + let loaded = gio::spawn_blocking(move || { + let Some(bytes) = crate::torrent::read_torrent_bytes(&path) else { + return Err(gettext("Could not read that .torrent file")); + }; + let list = crate::torrent::torrent_file_list(&bytes)?; + Ok::<_, String>((bytes, list)) + }) + .await; + let (bytes, (_tname, entries)) = match loaded { + Ok(Ok(loaded)) => loaded, + Ok(Err(e)) => { set_field_error(&torrent_row, &e); return; } + Err(_) => { + set_field_error(&torrent_row, &gettext("Could not read that .torrent file")); + return; + } }; if entries.len() <= 1 { torrent_btn.set_sensitive(false); From 38d872ab8482ffc737ecc870508226fa25d32fea Mon Sep 17 00:00:00 2001 From: houssemko Date: Wed, 28 Oct 2026 14:40:17 +0100 Subject: [PATCH 2/2] refactor(ui): apply torrent off-thread review follow-ups Move torrent archive hashing into the restore blocking pool, dedupe the read+parse closure behind torrent::load_torrent_file, and log JoinError payloads instead of dropping them. --- src/application.rs | 30 ++++++++++++++---------------- src/download.rs | 16 +++++++++++----- src/inline_add.rs | 17 ++++++----------- src/torrent.rs | 12 ++++++++++++ 4 files changed, 43 insertions(+), 32 deletions(-) diff --git a/src/application.rs b/src/application.rs index c7a041ce..d9400226 100644 --- a/src/application.rs +++ b/src/application.rs @@ -147,25 +147,27 @@ async fn intake_remote_torrent( .file_stem() .map(|s| s.to_string_lossy().into_owned()) .unwrap_or_else(|| "download".to_string()); - // Multi-file torrents offer per-file switches in the inline picker's - // right-sliding page. Parse off the GTK thread: bencode decode and the - // info-hash hash are CPU work on up to 10 MB. + // Parse off the GTK thread: bencode decode and the info-hash hash are + // CPU work on up to 10 MB. let parsed = gio::spawn_blocking(move || { let list = crate::torrent::torrent_file_list(&bytes)?; Ok::<_, String>((bytes, list)) }) .await; let (bytes, (_tname, entries)) = match parsed { - Ok(Ok(parsed)) => parsed, + Ok(Ok(data)) => data, Ok(Err(e)) => { fail(e); return Routed::Failed; } - Err(_) => { + Err(e) => { + tracing::warn!(error = ?e, "torrent parse worker failed"); fail(gettext("Could not read that .torrent file")); return Routed::Failed; } }; + // Multi-file torrents offer per-file switches in the inline picker's + // right-sliding page. if entries.len() > 1 { add_card.open_torrent_picker(file_name, bytes, entries); // Picker opened, not queued: return OpenedCard (202) so the @@ -563,21 +565,17 @@ fn handle_open_files(s: Rc, files: Vec) { glib::spawn_future_local(async move { // Read and parse off the GTK thread: a large multi-file // torrent must not stall the main loop. - let loaded = gio::spawn_blocking(move || { - let Some(bytes) = crate::torrent::read_torrent_bytes(&path) else { - return Err(gettext("Could not read that .torrent file")); - }; - let list = crate::torrent::torrent_file_list(&bytes)?; - Ok::<_, String>((bytes, list)) - }) - .await; - let (bytes, (_tname, entries)) = match loaded { - Ok(Ok(loaded)) => loaded, + let loaded = + gio::spawn_blocking(move || crate::torrent::load_torrent_file(&path)) + .await; + let (bytes, _, entries) = match loaded { + Ok(Ok(data)) => data, Ok(Err(e)) => { toasts.add_toast(adw::Toast::new(&crate::ui_util::esc_markup(&e))); return; } - Err(_) => { + Err(e) => { + tracing::warn!(error = ?e, "torrent read/parse worker failed"); toasts.add_toast(adw::Toast::new(&gettext( "Could not read that .torrent file", ))); diff --git a/src/download.rs b/src/download.rs index e24b3540..988fd0de 100644 --- a/src/download.rs +++ b/src/download.rs @@ -435,7 +435,7 @@ struct RestoreSweepPlan { dest_dirs: HashSet, state_keep: HashSet<(String, u64)>, selections: Vec<(String, Vec)>, - session_keep: Option>, + session_urls: Option>, } /// One validated queue entry awaiting the restore apply phase (module level so a mid-loop failure never leaves half-spawned engines). @@ -4699,12 +4699,12 @@ impl DownloadManager { .filter(|it| it.status() != DownloadStatus::Done) .map(|it| (it.dest_dir(), it.id())) .collect(); - let session_keep = crate::torrent::session_handle().is_some().then(|| { + let session_urls = crate::torrent::session_handle().is_some().then(|| { self.items() .filter(|it| { it.status() != DownloadStatus::Done && crate::torrent::is_torrent(&it.url()) }) - .filter_map(|it| crate::torrent::info_hash_for_url(&it.url())) + .map(|it| it.url().to_string()) .collect() }); RestoreSweepPlan { @@ -4713,7 +4713,7 @@ impl DownloadManager { dest_dirs, state_keep, selections, - session_keep, + session_urls, } } @@ -4728,7 +4728,13 @@ impl DownloadManager { } crate::video::sweep_state_staging(&plan.state_keep); crate::cookies::sweep_cookie_staging(); - plan.session_keep + // Hashing reads each torrent archive: do it here, on the blocking pool, + // not while building the plan on the GTK thread. + plan.session_urls.map(|urls| { + urls.into_iter() + .filter_map(|url| crate::torrent::info_hash_for_url(&url)) + .collect() + }) } fn schedule_session_orphan_sweep(keep: Option>) { diff --git a/src/inline_add.rs b/src/inline_add.rs index f8828d04..e2d7250e 100644 --- a/src/inline_add.rs +++ b/src/inline_add.rs @@ -1646,21 +1646,16 @@ fn wire_torrent_picker( }; // Read and parse off the GTK thread: a large multi-file torrent // must not stall the main loop. - let loaded = gio::spawn_blocking(move || { - let Some(bytes) = crate::torrent::read_torrent_bytes(&path) else { - return Err(gettext("Could not read that .torrent file")); - }; - let list = crate::torrent::torrent_file_list(&bytes)?; - Ok::<_, String>((bytes, list)) - }) - .await; - let (bytes, (_tname, entries)) = match loaded { - Ok(Ok(loaded)) => loaded, + let loaded = + gio::spawn_blocking(move || crate::torrent::load_torrent_file(&path)).await; + let (bytes, _, entries) = match loaded { + Ok(Ok(data)) => data, Ok(Err(e)) => { set_field_error(&torrent_row, &e); return; } - Err(_) => { + Err(e) => { + tracing::warn!(error = ?e, "torrent read/parse worker failed"); set_field_error(&torrent_row, &gettext("Could not read that .torrent file")); return; } diff --git a/src/torrent.rs b/src/torrent.rs index ade46526..50ef5f20 100644 --- a/src/torrent.rs +++ b/src/torrent.rs @@ -230,6 +230,18 @@ pub(crate) fn read_torrent_bytes(path: &std::path::Path) -> Option> { (buf.len() as u64 <= MAX_TORRENT_BYTES).then_some(buf) } +/// Read and parse a `.torrent` file in one call, so UI paths can hand the +/// whole job to a blocking pool. +pub(crate) fn load_torrent_file( + path: &std::path::Path, +) -> Result<(Vec, String, Vec), String> { + let Some(bytes) = read_torrent_bytes(path) else { + return Err(gettext("Could not read that .torrent file")); + }; + let (name, entries) = torrent_file_list(&bytes)?; + Ok((bytes, name, entries)) +} + /// Bounded read of the archived `.torrent` behind a queue pseudo-URL (`None` when missing, unreadable, or over the ceiling). pub(crate) fn read_archive_bytes(url: &str) -> Option> { read_torrent_bytes(&archive_path_for_url(url)?)