fix(print): the options read ends at its deadline and a dead read wakes the dialog (#1818)

Review follow-up on #2108:
- output_within waited on the reader thread with join, so a process the
  command started could hold the output pipe, and the wait, long past the
  limit after the command exited or was killed. The output now comes back
  over a channel with recv_timeout; such a reader finishes on its own.
- the printer-list and options workers ask for a frame from a guard
  dropped on exit, also when the source panics, so an idle dialog sees
  the closed channel without waiting for another event.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RUNQSPVXx3dDBabRZHAnjz

crates/print/src/spool.rs

@@ -48,32 +48,41 @@ pub(crate) fn output_within(command: &mut std::process::Command, limit: std::tim
     };
     // Read on a thread of its own, so a full pipe never holds the command while the deadline is
     // kept here. Past the cap the rest is drained and dropped, so the command still finishes.
+    // The output comes back over a channel rather than through `join`: a process the command
+    // started can keep the pipe open after the command itself has exited or been killed, and
+    // the wait for the output must end at the deadline too. Such a reader is left to finish on
+    // its own when that process closes the pipe.
+    let (tx, rx) = std::sync::mpsc::channel();
     let reader = std::thread::Builder::new().name("command output".into()).spawn(move || {
         let mut out = Vec::new();
         let read = (&mut stdout).take(OUTPUT_CAP).read_to_end(&mut out);
         let _ = std::io::copy(&mut stdout, &mut std::io::sink());
-        read.map(|_| out)
+        let _ = tx.send(read.map(|_| out));
     });
-    let reader = match reader {
-        Ok(reader) => reader,
-        Err(e) => {
-            stop(&mut child);
-            return Err(e);
-        }
-    };
+    if let Err(e) = reader {
+        stop(&mut child);
+        return Err(e);
+    }
+    let timed_out = || std::io::Error::new(std::io::ErrorKind::TimedOut, format!("the command did not answer within {limit:?}"));
     let deadline = std::time::Instant::now() + limit;
     loop {
         match child.try_wait() {
-            Ok(Some(_)) => return reader.join().map_err(|_| std::io::Error::other("the command's output was lost"))?,
+            Ok(Some(_)) => {
+                // A moment's grace, so output written just before an exit at the deadline counts.
+                let left = deadline.saturating_duration_since(std::time::Instant::now()).max(std::time::Duration::from_millis(100));
+                return match rx.recv_timeout(left) {
+                    Ok(read) => read,
+                    Err(std::sync::mpsc::RecvTimeoutError::Timeout) => Err(timed_out()),
+                    Err(std::sync::mpsc::RecvTimeoutError::Disconnected) => Err(std::io::Error::other("the command's output was lost")),
+                };
+            }
             Ok(None) if std::time::Instant::now() >= deadline => {
                 stop(&mut child);
-                let _ = reader.join();
-                return Err(std::io::Error::new(std::io::ErrorKind::TimedOut, format!("the command did not answer within {limit:?}")));
+                return Err(timed_out());
             }
             Ok(None) => std::thread::sleep(std::time::Duration::from_millis(10)),
             Err(e) => {
                 stop(&mut child);
-                let _ = reader.join();
                 return Err(e);
             }
         }

crates/print/src/tests.rs

@@ -781,10 +781,22 @@ mod bounded_commands {
         match std::env::var(MODE).as_deref() {
             Ok("answer") => println!("hello from the command"),
             Ok("hang") => std::thread::sleep(std::time::Duration::from_secs(60)),
+            // Holds the output pipe it inherited for a while, as a daemon a command starts can.
+            Ok("linger") => std::thread::sleep(LINGER),
+            // Starts a lingering process on its own output, then exits or hangs itself.
+            Ok(mode @ ("orphan-exit" | "orphan-hang")) => {
+                let _ = command("linger").spawn();
+                println!("started");
+                if mode == "orphan-hang" {
+                    std::thread::sleep(std::time::Duration::from_secs(60));
+                }
+            }
             _ => {}
         }
     }
 
+    const LINGER: std::time::Duration = std::time::Duration::from_secs(8);
+
     fn command(mode: &str) -> std::process::Command {
         let mut c = std::process::Command::new(std::env::current_exe().unwrap());
         c.args(["--exact", "tests::bounded_commands::stand_in_for_a_command", "--nocapture", "--"]).env(MODE, mode);
@@ -804,4 +816,17 @@ mod bounded_commands {
         assert_eq!(err.kind(), std::io::ErrorKind::TimedOut);
         assert!(started.elapsed() < std::time::Duration::from_secs(20), "stopped soon after the deadline, not when the command gives up");
     }
+
+    /// A process the command started keeps the output pipe open after the command has exited,
+    /// or after it was killed at the deadline: the wait still ends at the deadline (#2108
+    /// review), not when that process lets go of the pipe.
+    #[test]
+    fn a_process_left_holding_the_output_does_not_extend_the_wait() {
+        for mode in ["orphan-exit", "orphan-hang"] {
+            let started = std::time::Instant::now();
+            let err = crate::spool::output_within(&mut command(mode), std::time::Duration::from_millis(1500)).unwrap_err();
+            assert_eq!(err.kind(), std::io::ErrorKind::TimedOut, "{mode}");
+            assert!(started.elapsed() < LINGER / 2, "{mode}: returned after {:?}, when the lingering process let go", started.elapsed());
+        }
+    }
 }

crates/ui-egui/src/print_ui.rs

@@ -303,6 +303,22 @@ fn default_printer(printers: &[spool::Printer]) -> Option<String> {
     printers.iter().find(|p| p.default).or(printers.first()).map(|p| p.name.clone())
 }
 
+/// Asks for a frame when a worker thread ends, also when it panics: [`PdfCraftApp::poll_print`]
+/// then sees the answer, or the disconnected channel of a worker that died, without waiting
+/// for some other event to draw (#2108 review). Declared before the worker's sender, so the
+/// sender is dropped first and the frame finds the channel closed.
+#[cfg(not(target_arch = "wasm32"))]
+struct WakeOnExit(Option<egui::Context>);
+
+#[cfg(not(target_arch = "wasm32"))]
+impl Drop for WakeOnExit {
+    fn drop(&mut self) {
+        if let Some(ctx) = &self.0 {
+            ctx.request_repaint();
+        }
+    }
+}
+
 impl PdfCraftApp {
     pub fn open_print(&mut self) {
         let Some((i, _)) = self.active_ids() else { return };
@@ -394,13 +410,12 @@ impl PdfCraftApp {
             return;
         }
         let (tx, rx) = std::sync::mpsc::channel();
-        let ctx = self.ctx.clone();
+        let wake = WakeOnExit(self.ctx.clone());
         std::thread::spawn(move || {
+            let _wake = wake;
+            let tx = tx;
             // The receiver may be gone (the app quit): nothing to report to then.
             let _ = tx.send(source());
-            if let Some(ctx) = ctx {
-                ctx.request_repaint();
-            }
         });
         self.print_tasks.printers = Some(rx);
     }
@@ -414,14 +429,13 @@ impl PdfCraftApp {
         }
         let source: OptionsSource = self.options_source.clone().unwrap_or_else(|| std::sync::Arc::new(|p: &str| spool::printer_options(p)));
         let (tx, rx) = std::sync::mpsc::channel();
-        let ctx = self.ctx.clone();
+        let wake = WakeOnExit(self.ctx.clone());
         let name = printer.clone();
         let spawned = std::thread::Builder::new().name("printer options".into()).spawn(move || {
+            let _wake = wake;
+            let tx = tx;
             // The receiver may be gone (the app quit): nothing to report to then.
             let _ = tx.send(source(&name));
-            if let Some(ctx) = ctx {
-                ctx.request_repaint();
-            }
         });
         match spawned {
             Ok(_) => self.print_tasks.options.push((printer, rx)),

crates/ui-egui/tests/print.rs

@@ -486,3 +486,40 @@ fn printer_options_are_read_off_the_gui_thread_and_a_dead_read_offers_nothing()
     assert_eq!(h.state().print_draft.driver_options, Some(("Dead".into(), Vec::new())), "a dead read offers nothing, and the panel says so");
     h.get_by_label("This printer offers no settings of its own.");
 }
+
+/// A read of the printer's options that dies still asks for a frame, so an idle dialog stops
+/// saying it is reading without waiting for the mouse to move (#2108 review).
+#[cfg(not(target_arch = "wasm32"))]
+#[test]
+fn a_dead_options_read_wakes_an_idle_dialog() {
+    use std::sync::atomic::{AtomicBool, Ordering};
+    use std::sync::{Arc, Mutex};
+    let mut h = harness();
+    let gate = Arc::new(Mutex::new(()));
+    let hold = gate.lock().unwrap();
+    let wait = gate.clone();
+    let dying = Arc::new(AtomicBool::new(false));
+    let flag = dying.clone();
+    h.state_mut().options_source = Some(Arc::new(move |_: &str| {
+        let _open = wait.lock();
+        flag.store(true, Ordering::SeqCst);
+        panic!("the read died");
+    }));
+    assert!(h.state_mut().execute("print.dialog"));
+    h.run_steps(2);
+    let d = &mut h.state_mut().print_draft;
+    d.driver_options = None;
+    d.driver_options_loading = Some("Office".into());
+    d.read_options_for = Some("Office".into());
+    d.show_driver_options = true;
+    h.run();
+    assert!(!h.ctx.has_requested_repaint(), "the dialog is idle while the read waits");
+    drop(hold);
+    let deadline = std::time::Instant::now() + std::time::Duration::from_secs(10);
+    while !(dying.load(Ordering::SeqCst) && h.ctx.has_requested_repaint()) && std::time::Instant::now() < deadline {
+        std::thread::sleep(std::time::Duration::from_millis(5));
+    }
+    assert!(h.ctx.has_requested_repaint(), "the dead read asked for a frame");
+    h.run();
+    assert_eq!(h.state().print_draft.driver_options, Some(("Office".into(), Vec::new())));
+}