Fix thumbnail lifecycle failures

crates/pdfs-gui/src/app/main.rs

@@ -407,6 +407,7 @@ fn build_window(app: &adw::Application) {
             thumbnail_status: browser_widgets.thumbnail_status.clone(),
             thumbnail_poll: RefCell::new(None),
             thumbnail_build_running: Cell::new(false),
+            thumbnail_cancel_pending: Cell::new(false),
             search_source: RefCell::new(None),
             load_generation: Cell::new(0),
             views: browser_widgets.views.clone(),

crates/pdfs-gui/src/app/pages/browser.rs

@@ -35,6 +35,7 @@ pub(crate) struct BrowserState {
     pub(crate) thumbnail_status: gtk4::Label,
     pub(crate) thumbnail_poll: RefCell<Option<glib::SourceId>>,
     pub(crate) thumbnail_build_running: Cell<bool>,
+    pub(crate) thumbnail_cancel_pending: Cell<bool>,
     /// Pending debounce timer for the search box; replaced on every keystroke so
     /// only the last pause actually fires a [`Request::Search`].
     pub(crate) search_source: RefCell<Option<glib::SourceId>>,
@@ -1469,7 +1470,13 @@ pub(crate) fn wire_browser_actions(
     let ui_uf = ui.clone();
     upload_folder.connect_clicked(move |_| prompt_upload_folder(&ui_uf));
     let ui_thumbs = ui.clone();
-    build_thumbnails.connect_clicked(move |_| start_thumbnail_build(&ui_thumbs));
+    build_thumbnails.connect_clicked(move |_| {
+        if ui_thumbs.browser.thumbnail_build_running.get() {
+            cancel_thumbnail_build(&ui_thumbs);
+        } else {
+            start_thumbnail_build(&ui_thumbs);
+        }
+    });
 }
 
 const THUMBNAIL_BUILD_POLL: Duration = Duration::from_millis(500);
@@ -1488,7 +1495,10 @@ fn start_thumbnail_build(ui: &Rc<Ui>) {
     }
     cancel_file_thumbnails(ui);
     ui.browser.thumbnail_build_running.set(true);
-    ui.browser.build_thumbnails.set_sensitive(false);
+    // Do not expose Cancel until the daemon has acknowledged Start; the two
+    // requests use separate control connections and could otherwise reorder.
+    ui.browser.thumbnail_cancel_pending.set(true);
+    repaint_thumbnail_build_action(ui, true);
     ui.browser.thumbnail_build_row.set_visible(true);
     ui.browser.thumbnail_progress.set_fraction(0.0);
     ui.browser
@@ -1504,21 +1514,61 @@ fn start_thumbnail_build(ui: &Rc<Ui>) {
     glib::spawn_future_local(async move {
         match rx.recv().await {
             Ok(Ok(Response::ThumbnailBuild { status })) => {
+                ui.browser.thumbnail_cancel_pending.set(false);
                 repaint_thumbnail_build(&ui, &status);
             }
             Ok(Ok(Response::Error { message, kind })) => {
-                thumbnail_build_failed(&ui, &message);
+                thumbnail_build_failed(&ui);
                 toast_failure(&ui, "Couldn't build thumbnails", &message, kind);
             }
             _ => {
                 let message = "The mount service didn't respond.";
-                thumbnail_build_failed(&ui, message);
+                thumbnail_build_failed(&ui);
                 toast_error(&ui, "Couldn't build thumbnails", message);
             }
         }
     });
 }
 
+fn cancel_thumbnail_build(ui: &Rc<Ui>) {
+    if ui.browser.thumbnail_cancel_pending.replace(true) {
+        return;
+    }
+    ui.browser.build_thumbnails.set_sensitive(false);
+    ui.browser
+        .thumbnail_status
+        .set_label("Stopping thumbnail build…");
+    let rx = spawn_request(ui.dirs.control_socket(), Request::CancelThumbnailBuild);
+    let ui = ui.clone();
+    glib::spawn_future_local(async move {
+        match rx.recv().await {
+            Ok(Ok(Response::ThumbnailBuild { status })) if !status.running => {
+                repaint_thumbnail_build(&ui, &status)
+            }
+            Ok(Ok(Response::ThumbnailBuild { .. })) => schedule_thumbnail_build_poll(&ui),
+            Ok(Ok(Response::Error { message, kind })) => {
+                thumbnail_cancel_failed(&ui);
+                toast_failure(&ui, "Couldn't cancel thumbnail build", &message, kind);
+            }
+            _ => {
+                thumbnail_cancel_failed(&ui);
+                toast_error(
+                    &ui,
+                    "Couldn't cancel thumbnail build",
+                    "The mount service didn't respond.",
+                );
+            }
+        }
+    });
+}
+
+fn thumbnail_cancel_failed(ui: &Rc<Ui>) {
+    ui.browser.thumbnail_cancel_pending.set(false);
+    ui.browser.thumbnail_build_running.set(true);
+    repaint_thumbnail_build_action(ui, true);
+    schedule_thumbnail_build_poll(ui);
+}
+
 fn schedule_thumbnail_build_poll(ui: &Rc<Ui>) {
     if let Some(source) = ui.browser.thumbnail_poll.borrow_mut().take() {
         source.remove();
@@ -1534,12 +1584,17 @@ fn schedule_thumbnail_build_poll(ui: &Rc<Ui>) {
                     repaint_thumbnail_build(&ui_result, &status)
                 }
                 Ok(Ok(Response::Error { message, .. })) => {
-                    thumbnail_build_failed(&ui_result, &message)
+                    thumbnail_build_failed(&ui_result);
+                    toast_error(&ui_result, "Thumbnail build stopped", &message);
+                }
+                _ => {
+                    thumbnail_build_failed(&ui_result);
+                    toast_error(
+                        &ui_result,
+                        "Thumbnail build stopped",
+                        "The mount service stopped reporting thumbnail progress.",
+                    );
                 }
-                _ => thumbnail_build_failed(
-                    &ui_result,
-                    "The mount service stopped reporting thumbnail progress.",
-                ),
             }
         });
     });
@@ -1593,13 +1648,12 @@ fn repaint_thumbnail_build(ui: &Rc<Ui>, status: &ThumbnailBuildStatus) {
 
     if status.running {
         ui.browser.thumbnail_build_running.set(true);
-        ui.browser.build_thumbnails.set_sensitive(false);
+        repaint_thumbnail_build_action(ui, true);
         schedule_thumbnail_build_poll(ui);
     } else {
         ui.browser.thumbnail_build_running.set(false);
-        ui.browser
-            .build_thumbnails
-            .set_sensitive(*ui.mounted.borrow());
+        ui.browser.thumbnail_cancel_pending.set(false);
+        repaint_thumbnail_build_action(ui, false);
         if ui.stack.visible_child_name().as_deref() == Some("browser") {
             reload_listing(ui);
         }
@@ -1612,15 +1666,32 @@ fn show_thumbnail_build_progress(status: &ThumbnailBuildStatus) -> bool {
     status.running
 }
 
-fn thumbnail_build_failed(ui: &Rc<Ui>, message: &str) {
-    ui.browser.thumbnail_build_running.set(false);
+fn repaint_thumbnail_build_action(ui: &Rc<Ui>, running: bool) {
+    ui.browser.build_thumbnails.set_icon_name(if running {
+        "process-stop-symbolic"
+    } else {
+        "pdfs-build-thumbnails-symbolic"
+    });
     ui.browser
         .build_thumbnails
-        .set_sensitive(*ui.mounted.borrow());
-    ui.browser.thumbnail_build_row.set_visible(true);
+        .set_tooltip_text(Some(if running {
+            "Cancel thumbnail build"
+        } else {
+            "Build thumbnails in this folder and its subfolders"
+        }));
+    ui.browser.build_thumbnails.set_sensitive(
+        *ui.mounted.borrow() && (!running || !ui.browser.thumbnail_cancel_pending.get()),
+    );
+}
+
+fn thumbnail_build_failed(ui: &Rc<Ui>) {
+    ui.browser.thumbnail_build_running.set(false);
+    ui.browser.thumbnail_cancel_pending.set(false);
+    repaint_thumbnail_build_action(ui, false);
+    ui.browser.thumbnail_build_row.set_visible(false);
     ui.browser.thumbnail_progress.set_fraction(0.0);
-    ui.browser.thumbnail_status.set_label(message);
-    ui.browser.thumbnail_status.set_tooltip_text(Some(message));
+    ui.browser.thumbnail_status.set_label("");
+    ui.browser.thumbnail_status.set_tooltip_text(None);
 }
 
 /// Send a mutating request (rename / move / delete / mkdir / upload, or a trash

crates/pdfs-gui/src/app/pages/status.rs

@@ -683,9 +683,11 @@ pub(crate) fn set_mounted(ui: &Rc<Ui>, mounted: bool) {
     ui.browser.new_folder.set_sensitive(mounted);
     ui.browser.upload.set_sensitive(mounted);
     ui.browser.upload_folder.set_sensitive(mounted);
-    ui.browser
-        .build_thumbnails
-        .set_sensitive(mounted && !ui.browser.thumbnail_build_running.get());
+    ui.browser.build_thumbnails.set_sensitive(
+        mounted
+            && (!ui.browser.thumbnail_build_running.get()
+                || !ui.browser.thumbnail_cancel_pending.get()),
+    );
     ui.gallery.upload.set_sensitive(mounted);
     ui.details.details.pin_row.set_sensitive(mounted);
     ui.details.details.rename_button.set_sensitive(mounted);

crates/pdfs-gui/src/app/widgets/thumbnails.rs

@@ -7,7 +7,6 @@
 
 use crate::*;
 use pdfs_core::control::{FileThumbRequest, is_thumbnail_image_name};
-use std::time::{SystemTime, UNIX_EPOCH};
 
 const THUMB_BATCH: usize = 32;
 const THUMB_CACHE_MAX: usize = 256;
@@ -25,6 +24,27 @@ pub(crate) struct FileThumbnailState {
     inflight: Cell<bool>,
     source: RefCell<Option<glib::SourceId>>,
     generation: Cell<u64>,
+    generation_loading: Cell<bool>,
+}
+
+#[derive(Debug, PartialEq, Eq)]
+enum ReplyDisposition {
+    Ignore,
+    Load,
+    Retry,
+    Missing,
+}
+
+fn reply_disposition(wanted: bool, has_path: bool, pending: bool) -> ReplyDisposition {
+    if !wanted {
+        ReplyDisposition::Ignore
+    } else if has_path {
+        ReplyDisposition::Load
+    } else if pending {
+        ReplyDisposition::Retry
+    } else {
+        ReplyDisposition::Missing
+    }
 }
 
 impl FileThumbnailState {
@@ -38,13 +58,8 @@ impl FileThumbnailState {
             queue: RefCell::new(VecDeque::new()),
             inflight: Cell::new(false),
             source: RefCell::new(None),
-            generation: Cell::new(
-                SystemTime::now()
-                    .duration_since(UNIX_EPOCH)
-                    .unwrap_or_default()
-                    .as_nanos()
-                    .min(u128::from(u64::MAX)) as u64,
-            ),
+            generation: Cell::new(0),
+            generation_loading: Cell::new(false),
         }
     }
 
@@ -84,14 +99,17 @@ pub(crate) fn cancel_file_thumbnails(ui: &Rc<Ui>) {
     }
     ui.file_thumbs.queue.borrow_mut().clear();
     ui.file_thumbs.wanted.borrow_mut().clear();
-    let generation = ui.file_thumbs.generation.get().saturating_add(1);
-    ui.file_thumbs.generation.set(generation);
+    let generation = ui.file_thumbs.generation.replace(0);
     // The request is useful even when its reply is not. `spawn_request` owns the
     // socket round-trip on a worker thread, so dropping the receiver is safe.
-    drop(spawn_request(
-        ui.dirs.control_socket(),
-        Request::CancelFileThumbs { generation },
-    ));
+    if generation != 0 {
+        drop(spawn_request(
+            ui.dirs.control_socket(),
+            Request::CancelFileThumbs {
+                generation: generation.saturating_add(1),
+            },
+        ));
+    }
 }
 
 /// A square thumbnail surface with a generic file icon underneath the picture.
@@ -244,6 +262,10 @@ fn schedule_file_thumbs(ui: &Rc<Ui>) {
     if ui.file_thumbs.queue.borrow().is_empty() || ui.file_thumbs.inflight.get() {
         return;
     }
+    if ui.file_thumbs.generation.get() == 0 {
+        reserve_file_thumb_generation(ui);
+        return;
+    }
     if let Some(source) = ui.file_thumbs.source.borrow_mut().take() {
         source.remove();
     }
@@ -255,6 +277,40 @@ fn schedule_file_thumbs(ui: &Rc<Ui>) {
     *ui.file_thumbs.source.borrow_mut() = Some(source);
 }
 
+fn reserve_file_thumb_generation(ui: &Rc<Ui>) {
+    if ui.file_thumbs.generation_loading.replace(true) {
+        return;
+    }
+    let rx = spawn_request(
+        ui.dirs.control_socket(),
+        Request::ReserveFileThumbGeneration,
+    );
+    let ui = ui.clone();
+    glib::spawn_future_local(async move {
+        let result = rx.recv().await;
+        ui.file_thumbs.generation_loading.set(false);
+        match result {
+            Ok(Ok(Response::FileThumbGeneration { generation })) if generation != 0 => {
+                ui.file_thumbs.generation.set(generation);
+                schedule_file_thumbs(&ui);
+            }
+            _ => retry_file_thumb_generation(&ui),
+        }
+    });
+}
+
+fn retry_file_thumb_generation(ui: &Rc<Ui>) {
+    if let Some(source) = ui.file_thumbs.source.borrow_mut().take() {
+        source.remove();
+    }
+    let ui_retry = ui.clone();
+    let source = glib::timeout_add_local_once(THUMB_RETRY, move || {
+        ui_retry.file_thumbs.source.borrow_mut().take();
+        schedule_file_thumbs(&ui_retry);
+    });
+    *ui.file_thumbs.source.borrow_mut() = Some(source);
+}
+
 fn flush_file_thumbs(ui: &Rc<Ui>) {
     if ui.file_thumbs.inflight.get() {
         return;
@@ -291,51 +347,63 @@ fn flush_file_thumbs(ui: &Rc<Ui>) {
         ui.file_thumbs.inflight.set(false);
         match result {
             Ok(Ok(Response::Thumbs { items: replies })) => {
-                let requested: HashMap<String, FileThumbRequest> = items
+                let mut requested: HashMap<String, FileThumbRequest> = items
                     .iter()
                     .cloned()
                     .map(|item| (item.uid.clone(), item))
                     .collect();
                 let mut pending = Vec::new();
                 for reply in replies {
-                    let Some(request) = requested.get(&reply.uid) else {
+                    let Some(request) = requested.remove(&reply.uid) else {
                         continue;
                     };
                     let key = thumbnail_key(&request.uid, request.modified);
-                    match reply.path {
-                        Some(path) => match gtk4::gdk::Texture::from_filename(&path) {
-                            Ok(texture) => {
-                                ui.file_thumbs.store_texture(&key, texture.clone());
-                                if let Some(pictures) =
-                                    ui.file_thumbs.wanted.borrow_mut().remove(&key)
-                                {
-                                    for picture in pictures {
-                                        paint(&picture, &key, &texture);
+                    let wanted = ui.file_thumbs.wanted.borrow().contains_key(&key);
+                    match reply_disposition(wanted, reply.path.is_some(), reply.pending) {
+                        ReplyDisposition::Ignore => continue,
+                        ReplyDisposition::Load => {
+                            let Some(path) = reply.path else { continue };
+                            match gtk4::gdk::Texture::from_filename(&path) {
+                                Ok(texture) => {
+                                    ui.file_thumbs.store_texture(&key, texture.clone());
+                                    if let Some(pictures) =
+                                        ui.file_thumbs.wanted.borrow_mut().remove(&key)
+                                    {
+                                        for picture in pictures {
+                                            paint(&picture, &key, &texture);
+                                        }
                                     }
                                 }
+                                Err(e) => {
+                                    tracing::debug!("cannot decode file thumbnail {path}: {e}");
+                                    ui.file_thumbs.store_missing(key.clone());
+                                    ui.file_thumbs.wanted.borrow_mut().remove(&key);
+                                }
                             }
-                            Err(e) => {
-                                tracing::debug!("cannot decode file thumbnail {path}: {e}");
-                                ui.file_thumbs.store_missing(key.clone());
-                                ui.file_thumbs.wanted.borrow_mut().remove(&key);
-                            }
-                        },
-                        None if reply.pending => pending.push(request.clone()),
-                        None => {
+                        }
+                        ReplyDisposition::Retry => pending.push(request),
+                        ReplyDisposition::Missing => {
                             ui.file_thumbs.store_missing(key.clone());
                             ui.file_thumbs.wanted.borrow_mut().remove(&key);
                         }
                     }
                 }
+                pending.extend(requested.into_values());
                 if !pending.is_empty() {
                     retry_file_thumbs(&ui, pending);
                 }
             }
+            Ok(Ok(Response::FileThumbsStale)) => {
+                ui.file_thumbs.generation.set(0);
+                retry_file_thumbs(&ui, items);
+            }
             Ok(Ok(Response::Error { message, .. })) => {
                 tracing::debug!("file thumbnails failed: {message}");
+                retry_file_thumbs(&ui, items);
             }
             Ok(Ok(_)) | Ok(Err(_)) | Err(_) => {
                 tracing::debug!("file thumbnails: no reply");
+                retry_file_thumbs(&ui, items);
             }
         }
         schedule_file_thumbs(&ui);
@@ -358,7 +426,7 @@ fn retry_file_thumbs(ui: &Rc<Ui>, items: Vec<FileThumbRequest>) {
 
 #[cfg(test)]
 mod tests {
-    use super::is_image_name;
+    use super::{ReplyDisposition, is_image_name, reply_disposition};
 
     #[test]
     fn image_extensions_are_case_insensitive_and_specific() {
@@ -369,6 +437,7 @@ mod tests {
             "archive.tiff",
             "camera.nef",
             "negative.CR3",
+            "camera.RAW",
         ] {
             assert!(is_image_name(name), "{name}");
         }
@@ -380,8 +449,21 @@ mod tests {
             "camera.heic",
             "frame.avif",
             "graphic.svg",
+            "camera.rw",
         ] {
             assert!(!is_image_name(name), "{name}");
         }
     }
+
+    #[test]
+    fn cancelled_listing_replies_cannot_become_permanent_misses() {
+        assert_eq!(
+            reply_disposition(false, false, false),
+            ReplyDisposition::Ignore
+        );
+        assert_eq!(
+            reply_disposition(true, false, false),
+            ReplyDisposition::Missing
+        );
+    }
 }