diff --git a/crates/ui-egui/src/clipboard_probe.rs b/crates/ui-egui/src/clipboard_probe.rs index 902024bac2..d492893ce0 100644 --- a/crates/ui-egui/src/clipboard_probe.rs +++ b/crates/ui-egui/src/clipboard_probe.rs @@ -14,8 +14,11 @@ use crate::sysclip::ClipboardProbeFactory; /// How often the worker looks at the clipboard while the UI wants to know. const PROBE_INTERVAL: Duration = Duration::from_millis(250); -/// How long dropping the probe waits for the worker to let go of its clipboard handle. -const STOP_WAIT: Duration = Duration::from_millis(250); +/// How long dropping the probe (when the app quits) waits for the worker to let go of its clipboard +/// handle (VectorCraft's own bound). arboard's X11 reads wait up to 4 s for each text format they +/// ask for; when a look is still waiting after this bound, the app quits without waiting longer, +/// and what it copied may not reach the clipboard manager. +const STOP_WAIT: Duration = Duration::from_secs(5); /// The background clipboard check ([`crate::VectorcraftApp`]'s `clipboard_probe`). pub(crate) struct Probe { @@ -89,8 +92,8 @@ impl Probe { } impl Drop for Probe { - /// Wake the worker to stop and wait (a moment at most: it may be stuck on an owner that never - /// answers) until it has let go of its clipboard handle. + /// Wake the worker to stop and wait ([`STOP_WAIT`] at most: it may be stuck on an owner that + /// never answers) until it has let go of its clipboard handle. fn drop(&mut self) { self.wants = None; let end = Instant::now() + STOP_WAIT; diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 644125b545..e636f0520a 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -133,6 +133,12 @@ pub struct VectorcraftApp { /// [`VectorcraftApp::sync_views`], which puts the views back with their documents after /// `document.move` reorders them (#5663). pub(crate) view_uids: Vec, + /// The thread that checks the system clipboard for `system_paste` instead + /// ([`Services::clipboard_probe`]). It is declared before `services` so that it is dropped + /// first: on X11 the last clipboard handle alive hands what the app copied to the clipboard + /// manager, and that must be the app's own handle in `services`, dropped on the UI thread + /// before the process exits. + clipboard_probe: Option, pub services: Services, pub canvas: CanvasCache, pub perf: Perf, @@ -234,9 +240,6 @@ pub struct VectorcraftApp { pub(crate) clipboard_image_request: Option, /// When `system_paste` was last checked in line (app time, s; at most once per frame). system_paste_at: f64, - /// The thread that checks the system clipboard for `system_paste` instead - /// ([`Services::clipboard_probe`]). - clipboard_probe: Option, /// File dialogs shown off the UI thread and what runs again with their answers. pub(crate) picks: picks::Picks, /// The screen color sampler shown and not answered yet, and where its color goes. diff --git a/crates/ui-egui/src/tests/sysclip_probe.rs b/crates/ui-egui/src/tests/sysclip_probe.rs index 48e4f2e348..05cab31bf7 100644 --- a/crates/ui-egui/src/tests/sysclip_probe.rs +++ b/crates/ui-egui/src/tests/sysclip_probe.rs @@ -14,11 +14,13 @@ use vectorcraft_engine::cmd::clipboard::Flavour; use crate::tests_sysclip::{copy_rect, run}; use crate::{Services, SystemClipboard, VectorcraftApp, menus}; -/// What the clipboard handles saw: the thread each `has()` ran on, and how many handles are alive. +/// What the clipboard handles saw: the thread each `has()` ran on, how many handles are alive, and +/// the thread each handle was dropped on, in order. #[derive(Default)] struct Recorder { threads: Mutex>>, alive: AtomicUsize, + drops: Mutex>>, } impl Recorder { @@ -44,10 +46,42 @@ impl Slow { impl Drop for Slow { fn drop(&mut self) { + self.rec.drops.lock().unwrap().push(std::thread::current().name().map(str::to_owned)); self.rec.alive.fetch_sub(1, Ordering::SeqCst); } } +/// The probe's clipboard: its `has()` tells `looking` it started, then waits until `answer` sends +/// (an owner that answers only once the app is quitting). +struct Held { + rec: Arc, + looking: std::sync::mpsc::Sender<()>, + answer: std::sync::mpsc::Receiver<()>, +} + +impl Drop for Held { + fn drop(&mut self) { + self.rec.drops.lock().unwrap().push(std::thread::current().name().map(str::to_owned)); + } +} + +impl SystemClipboard for Held { + fn write(&mut self, _flavours: &[Flavour]) -> Result<(), String> { + Ok(()) + } + fn holds_ours(&mut self) -> bool { + false + } + fn read(&mut self, _mimes: &[&'static str]) -> Option { + None + } + fn has(&mut self, _mimes: &[&'static str]) -> bool { + let _ = self.looking.send(()); + let _ = self.answer.recv(); + false + } +} + impl SystemClipboard for Slow { fn write(&mut self, _flavours: &[Flavour]) -> Result<(), String> { Ok(()) @@ -143,6 +177,35 @@ fn the_probe_rests_after_a_copy_and_stops_with_the_app() { assert_eq!(rec.alive.load(Ordering::SeqCst), 0, "the probe's thread kept its clipboard"); } +#[test] +fn quitting_drops_the_probes_clipboard_before_the_apps() { + let rec = Arc::new(Recorder::default()); + let (looking, looked) = std::sync::mpsc::channel(); + let (answer, answered) = std::sync::mpsc::channel::<()>(); + let make = { + let rec = Arc::clone(&rec); + move || -> Box { Box::new(Held { rec, looking, answer: answered }) } + }; + let services = Services { + system_clipboard: Some(Slow::boxed(&rec, Duration::ZERO, Some(false))), + clipboard_probe: Some(Box::new(make)), + ..Default::default() + }; + let mut app = VectorcraftApp::new(Session::new(), services); + run(&mut app, "file.new", json!({"width": 100, "height": 100})); + frame(&mut app); + frame(&mut app); + // The probe's thread is waiting on the clipboard's owner when the user quits. + looked.recv_timeout(Duration::from_secs(60)).expect("the probe never looked"); + drop(answer); + drop(app); + // On X11 the last handle dropped hands what the app copied to the clipboard manager: it must be + // the app's own, dropped on the UI thread before the process exits. + let drops = rec.drops.lock().unwrap().clone(); + let ui = std::thread::current().name().map(str::to_owned); + assert_eq!(drops, vec![Some("vectorcraft-clip".to_owned()), ui], "handles dropped on: {drops:?}"); +} + #[test] fn without_its_thread_the_check_runs_in_line_again() { let rec = Arc::new(Recorder::default());