diff --git a/crates/engine/src/link_watch.rs b/crates/engine/src/link_watch.rs index 51ffbd098..44b1bd86a 100644 --- a/crates/engine/src/link_watch.rs +++ b/crates/engine/src/link_watch.rs @@ -17,7 +17,8 @@ //! update, or answering No, leaves the link modified until the file changes again. use std::collections::{BTreeSet, HashMap, HashSet}; -use std::sync::{Arc, Mutex, PoisonError}; +use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::{Arc, Condvar, Mutex, PoisonError}; use serde_json::{Value, json}; @@ -27,8 +28,21 @@ use crate::{Result, Session}; pub type Stamp = (u64, u128); /// Stamps of a document's linked files taken on a worker thread: (document uid, [(file, -/// object ids, stamp)]). -pub type LinkScan = Arc, Option)>)>>>; +/// object ids, stamp)]), the condition the worker signals when it ends, and whether it has. +pub type LinkScan = Arc<(Mutex, Option)>)>>, Condvar, AtomicBool)>; + +/// Held by a look's worker: marks the look as ended and wakes [`Session::wait_link_scan`] when +/// dropped, whether the worker returned or unwound. +struct Ended(LinkScan); + +impl Drop for Ended { + fn drop(&mut self) { + let (done, finished, ended) = &*self.0; + let _held = done.lock().unwrap_or_else(PoisonError::into_inner); + ended.store(true, Ordering::SeqCst); + finished.notify_all(); + } +} /// The stamp of the linked file stored as `path` in the document saved at `doc_path`; `None` when /// there is no file, its path isn't followed ([`crate::cmd::fileio::document_path`]) or @@ -79,12 +93,14 @@ impl Session { let out = slot.clone(); let scope = crate::file_access::ScopeHandle::capture(); let spawned = std::thread::Builder::new().name("link-watch".into()).spawn(move || { + // Marks the look as ended when the worker returns or unwinds. + let _ended = Ended(out.clone()); let stamps = scope.run(|| { crate::file_access::confine(crate::file_access::current().or(opened_by).as_ref(), || { files.into_iter().map(|(p, ids)| (p.clone(), ids, stamp(&p, doc_path.as_deref()))).collect() }) }); - *out.lock().unwrap_or_else(PoisonError::into_inner) = Some((uid, stamps)); + *out.0.lock().unwrap_or_else(PoisonError::into_inner) = Some((uid, stamps)); }); if spawned.is_ok() { self.link_scan = Some(slot); @@ -94,11 +110,21 @@ impl Session { /// Apply a finished look (`None` while there is none, or it is still running): see /// [`Session::apply_link_stamps`]. pub fn poll_link_scan(&mut self) -> Option> { - let (uid, stamps) = self.link_scan.as_ref()?.lock().unwrap_or_else(PoisonError::into_inner).take()?; + let (uid, stamps) = self.link_scan.as_ref()?.0.lock().unwrap_or_else(PoisonError::into_inner).take()?; self.link_scan = None; Some(self.apply_link_stamps(uid, stamps, false)) } + /// Block until the look in flight has ended (at once when none is running), so that the next + /// [`Session::poll_link_scan`] applies its stamps. A look whose worker stopped without stamps + /// ends too, and the poll then returns `None`. For tests that drive the look step by step. + pub fn wait_link_scan(&self) { + let Some(scan) = &self.link_scan else { return }; + let (done, finished, ended) = &**scan; + let guard = done.lock().unwrap_or_else(PoisonError::into_inner); + drop(finished.wait_while(guard, |stamps| stamps.is_none() && !ended.load(Ordering::SeqCst)).unwrap_or_else(PoisonError::into_inner)); + } + /// Compare the stamps of document `uid`'s linked files with the ones last seen and act on /// the changed ones as Update Links says (`update`: read them again regardless). Returns /// `{updated: [ids], ask: [ids], modified: [ids]}`: read again, to offer (Ask When Modified), diff --git a/crates/engine/src/tests/link_paths.rs b/crates/engine/src/tests/link_paths.rs index 2c96be935..76bd7168a 100644 --- a/crates/engine/src/tests/link_paths.rs +++ b/crates/engine/src/tests/link_paths.rs @@ -77,11 +77,8 @@ fn linked_files_on_other_computers_are_never_read() { } s.execute("links.updateChanged", &json!({})).unwrap(); s.start_link_scan(); - let t0 = std::time::Instant::now(); - while s.poll_link_scan().is_none() { - assert!(t0.elapsed().as_secs() < 10, "the look never finished"); - std::thread::yield_now(); - } + s.wait_link_scan(); + assert!(s.poll_link_scan().is_some(), "no look ran"); }); assert_eq!(far_reads(&seen), Vec::<&String>::new()); // The watch kept no stamp of them (`//tmp/…` would have one had it been looked at). diff --git a/crates/engine/src/tests/linkwatch.rs b/crates/engine/src/tests/linkwatch.rs index 8c866b8d9..f1192e02d 100644 --- a/crates/engine/src/tests/linkwatch.rs +++ b/crates/engine/src/tests/linkwatch.rs @@ -6,10 +6,13 @@ use serde_json::{Value, json}; use crate::tests_links::{BLUE, Folder, RED, centre_colour, place, png, session}; use crate::*; -/// Write `bytes` to `path` so its modification time moves even on coarse file systems. +/// Write `bytes` to `path`, dated 2 s after the file it replaces so its modification time moves +/// even on coarse file systems. fn save(path: &str, bytes: &[u8]) { - std::thread::sleep(std::time::Duration::from_millis(30)); + let stamp = std::fs::metadata(path).unwrap().modified().unwrap(); std::fs::write(path, bytes).unwrap(); + let later = std::fs::FileTimes::new().set_modified(stamp + std::time::Duration::from_secs(2)); + std::fs::OpenOptions::new().write(true).open(path).unwrap().set_times(later).unwrap(); } fn colour(s: &Session) -> [u8; 3] { @@ -19,14 +22,8 @@ fn colour(s: &Session) -> [u8; 3] { /// One background look, waited for. fn look(s: &mut Session) -> Value { s.start_link_scan(); - let t0 = std::time::Instant::now(); - loop { - if let Some(r) = s.poll_link_scan() { - return r.unwrap(); - } - assert!(t0.elapsed().as_secs() < 10, "the look never finished"); - std::thread::yield_now(); - } + s.wait_link_scan(); + s.poll_link_scan().expect("a look ran").unwrap() } #[test] diff --git a/crates/ui-egui/src/tests/place.rs b/crates/ui-egui/src/tests/place.rs index 719baff96..e4c9d47ef 100644 --- a/crates/ui-egui/src/tests/place.rs +++ b/crates/ui-egui/src/tests/place.rs @@ -758,7 +758,8 @@ fn frame_at(app: &mut VectorcraftApp, ctx: &egui::Context, time: f64) -> std::ti /// for) or until `done` → whether it was. fn idle(app: &mut VectorcraftApp, ctx: &egui::Context, time: &mut f64, secs: f64, done: &dyn Fn(&VectorcraftApp) -> bool) -> bool { let end = *time + secs; - let real_end = std::time::Instant::now() + std::time::Duration::from_secs(20); + // The look last waited for: one that a frame after the wait still holds ended without stamps. + let mut waited: Option = None; loop { let wait = frame_at(app, ctx, *time); if done(app) { @@ -769,10 +770,13 @@ fn idle(app: &mut VectorcraftApp, ctx: &egui::Context, time: &mut f64, secs: f64 *time = end; return false; } - // The look runs on a worker thread, in real time: the clock waits for it, else a busy - // machine's slow look outlasts the simulated seconds and the test fails at random. - if app.session.link_scan.is_some() && std::time::Instant::now() < real_end { - std::thread::sleep(std::time::Duration::from_millis(5)); + // The look runs on a worker thread, in real time: the clock waits for it to finish, else a + // busy machine's slow look outlasts the simulated seconds and the test fails at random. + if let Some(scan) = app.session.link_scan.clone() + && !waited.as_ref().is_some_and(|w| std::sync::Arc::ptr_eq(w, &scan)) + { + app.session.wait_link_scan(); + waited = Some(scan); continue; } *time = next;