diff --git a/crates/ui-egui/src/ocr_ui.rs b/crates/ui-egui/src/ocr_ui.rs index a267aa000e..977f90f2f9 100644 --- a/crates/ui-egui/src/ocr_ui.rs +++ b/crates/ui-egui/src/ocr_ui.rs @@ -270,13 +270,18 @@ impl PdfCraftApp { let Some(job) = self.session.ocr_job(id, pages, settings) else { return }; let progress = Arc::new(Mutex::new(OcrProgress { total: job.pages.len(), ..Default::default() })); let p = progress.clone(); + // A failure is written on the worker thread: in the UI's language. + let lang = crate::i18n::current(); let work = move || { - let result = pdfcraft_engine::ocr::engine().map(|ocr| { - job.run(&ocr, |done, total| { - let Ok(mut s) = p.lock() else { return false }; - s.done = done; - s.total = total; - !s.cancel + crate::i18n::set_current(lang); + let result = recognize_guarded(|| { + pdfcraft_engine::ocr::engine().map(|ocr| { + job.run(&ocr, |done, total| { + let Ok(mut s) = p.lock() else { return false }; + s.done = done; + s.total = total; + !s.cancel + }) }) }); if let Ok(mut s) = p.lock() { @@ -329,44 +334,15 @@ impl PdfCraftApp { let lang = crate::i18n::current(); let work = move || { crate::i18n::set_current(lang); - let (mut ok, mut words, mut failed) = (0, 0, Vec::new()); - match pdfcraft_engine::ocr::engine() { - Err(e) => failed.push(e), - Ok(ocr) => { - for (i, (name, bytes)) in files.into_iter().enumerate() { - if let Ok(mut s) = p.lock() { - s.done = i; - } - let r = pdfcraft_engine::ocr::recognize_file(&name, Arc::new(bytes), None, settings.clone(), &ocr, |_, _| true); - let saved = r.and_then(|r| { - #[cfg(not(target_arch = "wasm32"))] - crate::editing::write_atomically(&dir.join(&name).to_string_lossy(), &r.bytes).map_err(|e| e.to_string())?; - #[cfg(target_arch = "wasm32")] - crate::editing::download(&name, &r.bytes)?; - Ok(r.words()) - }); - match saved { - Ok(n) => { - ok += 1; - words += n; - } - Err(e) => failed.push(format!("{name}: {}", e)), - } - } - } - } - let done = if ok == 1 { - crate::i18n::fmt(tl!("Recognized {w} words in 1 file"), &[("w", &words.to_string())]) - } else { - crate::i18n::fmt(tl!("Recognized {w} words in {n} files"), &[("w", &words.to_string()), ("n", &ok.to_string())]) - }; - let mut msg = done; - if !failed.is_empty() { - msg.push_str(&crate::i18n::fmt(tl!("; failed: {list}"), &[("list", &failed.join("; "))])); - } + // A panic in the recognizer still ends the batch with a summary (#3919). + #[cfg(not(target_arch = "wasm32"))] + let summary = pdfcraft_engine::guard(|| recognize_files(files, &settings, &p, &dir)); + #[cfg(target_arch = "wasm32")] + let summary = pdfcraft_engine::guard(|| recognize_files(files, &settings, &p)); + let summary = summary.unwrap_or_else(|panic| failed(&panic)); if let Ok(mut s) = p.lock() { s.done = s.total; - s.message = Some(msg); + s.message = Some(summary); } }; #[cfg(not(target_arch = "wasm32"))] @@ -380,7 +356,54 @@ impl PdfCraftApp { self.ocr_batch = Some(progress); self.poll_ocr(); } +} +/// Recognize each file and write its searchable copy into `dir`; returns the summary to show. +fn recognize_files( + files: Vec<(String, Vec)>, + settings: &OcrSettings, + p: &Mutex, + #[cfg(not(target_arch = "wasm32"))] dir: &std::path::Path, +) -> String { + let (mut ok, mut words, mut failed) = (0, 0, Vec::new()); + match pdfcraft_engine::ocr::engine() { + Err(e) => failed.push(e), + Ok(ocr) => { + for (i, (name, bytes)) in files.into_iter().enumerate() { + if let Ok(mut s) = p.lock() { + s.done = i; + } + let r = pdfcraft_engine::ocr::recognize_file(&name, Arc::new(bytes), None, settings.clone(), &ocr, |_, _| true); + let saved = r.and_then(|r| { + #[cfg(not(target_arch = "wasm32"))] + crate::editing::write_atomically(&dir.join(&name).to_string_lossy(), &r.bytes).map_err(|e| e.to_string())?; + #[cfg(target_arch = "wasm32")] + crate::editing::download(&name, &r.bytes)?; + Ok(r.words()) + }); + match saved { + Ok(n) => { + ok += 1; + words += n; + } + Err(e) => failed.push(format!("{name}: {}", e)), + } + } + } + } + let done = if ok == 1 { + crate::i18n::fmt(tl!("Recognized {w} words in 1 file"), &[("w", &words.to_string())]) + } else { + crate::i18n::fmt(tl!("Recognized {w} words in {n} files"), &[("w", &words.to_string()), ("n", &ok.to_string())]) + }; + let mut msg = done; + if !failed.is_empty() { + msg.push_str(&crate::i18n::fmt(tl!("; failed: {list}"), &[("list", &failed.join("; "))])); + } + msg +} + +impl PdfCraftApp { /// Stop a running recognition (the pages read so far are kept). pub fn cancel_ocr(&mut self) { if let Some(r) = &self.ocr_run @@ -393,13 +416,13 @@ impl PdfCraftApp { /// Show progress; apply the result once the worker is done. pub(crate) fn poll_ocr(&mut self) { if let Some(b) = self.ocr_batch.clone() { - let msg = b.lock().ok().map(|mut s| s.message.take().ok_or((s.done, s.total))); + let msg = b.lock().map(|mut s| s.message.take().ok_or((s.done, s.total))).map_err(|e| e.to_string()); match msg { - Some(Ok(m)) => { + Ok(Ok(m)) => { self.ocr_batch = None; self.notify(m); } - Some(Err((done, total))) => { + Ok(Err((done, total))) => { let m = crate::i18n::fmt( tl!("Recognizing text… file {d} of {t}"), &[("d", &(done + 1).min(total.max(1)).to_string()), ("t", &total.max(1).to_string())], @@ -411,13 +434,26 @@ impl PdfCraftApp { ctx.request_repaint_after(std::time::Duration::from_millis(200)); } } - None => self.ocr_batch = None, + // The worker stopped while holding the lock: it will never report back (#3919). + Err(e) => { + self.ocr_batch = None; + self.notify(failed(&e)); + } } } let Some(run) = self.ocr_run.as_ref() else { return }; let (doc, progress, auto) = (run.doc, run.progress.clone(), run.automatic); let automatic = auto.is_some(); - let Ok(mut s) = progress.lock() else { return }; + let mut s = match progress.lock() { + Ok(s) => s, + Err(e) => { + // As above: without this the run never clears and refuses every later one. + let e = e.to_string(); + self.ocr_run = None; + self.notify(failed(&e)); + return; + } + }; // The result of a run Edit started waits while Read mode or a dialog is up: nothing may change // the document beneath them, and the run may have been started before they appeared. A run // the user asked for completes as before. @@ -496,6 +532,17 @@ impl PdfCraftApp { } } +/// "Recognize text failed: {e}", in the current language. +fn failed(e: &str) -> String { + crate::i18n::fmt(tl!("{label} failed: {e}"), &[("label", tl!("Recognize text")), ("e", e)]) +} + +/// `recognize`, with a panic in it (a damaged model, a page the recognizer can't take) turned into +/// the run's error: the run has to end either way, or it refuses every later one (#3919). +fn recognize_guarded(recognize: impl FnOnce() -> Result) -> Result { + pdfcraft_engine::guard(recognize).unwrap_or_else(|panic| Err(failed(&panic))) +} + #[cfg(test)] mod tests { use super::*; @@ -642,4 +689,43 @@ mod tests { assert_eq!(app.session.get(id).unwrap().can_undo(), Some("Add image"), "its result was not applied"); assert!(!app.views[0].auto_ocr_tried.contains(&0), "the page may be judged again"); } + + #[test] + fn a_panicking_recognizer_ends_the_run_with_an_error() { + let error = recognize_guarded(|| -> Result<(), String> { panic!("the model file is damaged") }).unwrap_err(); + assert!(error.starts_with("Recognize text failed") && error.contains("the model file is damaged"), "{error}"); + assert_eq!(recognize_guarded(|| Ok::<_, String>(7)), Ok(7)); + } + + /// Poison `m` the way a worker that panics while it holds the lock does. + fn poison(m: &Arc>) { + let worker = m.clone(); + let _ = std::thread::spawn(move || { + let _held = worker.lock(); + panic!("the worker died"); + }) + .join(); + assert!(m.is_poisoned()); + } + + #[test] + fn a_run_whose_worker_died_ends_instead_of_waiting_forever() { + let mut app = PdfCraftApp::new(); + app.set_option("language", "en").unwrap(); + let progress = Arc::new(Mutex::new(OcrProgress { total: 3, ..Default::default() })); + poison(&progress); + app.ocr_run = Some(OcrRun { doc: DocId(1), progress, automatic: None }); + app.poll_ocr(); + assert!(app.ocr_run.is_none(), "the next run isn't refused as already running"); + let toast = app.toast.take().map(|t| t.0).unwrap_or_default(); + assert!(toast.starts_with("Recognize text failed"), "{toast}"); + + let batch = Arc::new(Mutex::new(BatchProgress { total: 2, ..Default::default() })); + poison(&batch); + app.ocr_batch = Some(batch); + app.poll_ocr(); + assert!(app.ocr_batch.is_none()); + let toast = app.toast.take().map(|t| t.0).unwrap_or_default(); + assert!(toast.starts_with("Recognize text failed"), "{toast}"); + } }