diff --git a/crates/snowbound/src/library.rs b/crates/snowbound/src/library.rs index 0c71c06099e3b9985ecda2493aecfd85bd284152..944eaa61f25b7112960b6dbf939fd06ad71e1320 100644 --- a/crates/snowbound/src/library.rs +++ b/crates/snowbound/src/library.rs @@ -324,8 +324,7 @@ pub struct Library { /// Sections opened ahead of being shown, or left open after, most recent first, which /// `open` hands out before opening any. kept: Mutex>, - /// The notebook's style themes, and when they were read. - themes: Mutex)>>, + themes: Mutex, /// The keys of its password-protected sections unlocked this run, kept as it is read again. keys: Arc, /// The share a notebook another computer shares by Live Share is reached through. @@ -338,6 +337,13 @@ pub struct Library { #[derive(Default)] pub struct Keys(Mutex>); +#[derive(Default)] +struct ThemeCache { + value: Arc, + checked: Option, + reading: bool, +} + impl Library { /// `location` named `name`, with no notebook read, server, sync or kept sections. fn new(location: &str, name: String, cache: &Path) -> Self { @@ -645,27 +651,53 @@ impl Library { }); } - /// The notebook's style themes, read again after a while so other machines' changes - /// reach pages opened later; a section opened on its own has none. - pub fn themes(&self) -> Arc { + /// The notebook's last-read themes, refreshed off the frame thread. + pub fn themes(self: &Arc) -> Arc { let mut kept = self .themes .lock() .unwrap_or_else(|poisoned| poisoned.into_inner()); - if let Some((read, themes)) = &*kept - && read.elapsed() < Duration::from_secs(30) + let themes = Arc::clone(&kept.value); + if !matches!(self.notebook, Ok(Some(_))) + || kept.reading + || kept + .checked + .is_some_and(|read| read.elapsed() < Duration::from_secs(30)) { - return Arc::clone(themes); + return themes; } - let themes = match &self.notebook { - Ok(Some(notebook)) => notebook.themes().unwrap_or_else(|error| { - eprintln!("{}: reading its themes failed: {error}", self.location); - Themes::default() - }), - _ => Themes::default(), - }; - let themes = Arc::new(themes); - *kept = Some((Instant::now(), Arc::clone(&themes))); + kept.reading = true; + drop(kept); + let (library, before) = (Arc::clone(self), Arc::clone(&themes)); + crate::spawn(move || { + let read = match &library.notebook { + Ok(Some(notebook)) => notebook.themes(), + _ => Ok(Themes::default()), + }; + let mut kept = library + .themes + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()); + kept.reading = false; + kept.checked = Some(Instant::now()); + match read { + Ok(mut read) => { + // A local edit may have landed while the file was being read. + if !Arc::ptr_eq(&before, &kept.value) { + notebook::sidecar::themes::merge(&mut read, (*kept.value).clone()); + } + let changed = *kept.value != read; + kept.value = Arc::new(read); + drop(kept); + if changed { + notify_background(); + } + } + Err(error) => { + eprintln!("{}: reading its themes failed: {error}", library.location); + } + } + }); themes } @@ -675,13 +707,15 @@ impl Library { if !matches!(self.notebook, Ok(Some(_))) { return; } - let mut themes = (*self.themes()).clone(); - notebook::sidecar::themes::merge(&mut themes, change.clone()); - *self - .themes - .lock() - .unwrap_or_else(|poisoned| poisoned.into_inner()) = - Some((Instant::now(), Arc::new(themes))); + self.themes(); + { + let mut kept = self + .themes + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()); + notebook::sidecar::themes::merge(Arc::make_mut(&mut kept.value), change.clone()); + kept.checked = Some(Instant::now()); + } let library = Arc::clone(self); crate::spawn(move || { let kept = library @@ -1471,6 +1505,89 @@ pub fn locate(path: &Path) -> Located { mod tests { use super::*; + #[cfg(unix)] + #[test] + fn stalled_theme_reads_leave_the_cache_usable_and_keep_local_edits() { + use notebook::sidecar::themes::{Assignment, Scope, merge}; + use std::{ + io::Write, + os::unix::{ffi::OsStrExt, fs::OpenOptionsExt}, + }; + + let root = + std::env::temp_dir().join(format!("snowbound-stalled-themes-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&root); + std::fs::create_dir_all(root.join("notebook/.snowbound")).unwrap(); + let path = root.join("notebook/.snowbound/themes.json"); + let filename = std::ffi::CString::new(path.as_os_str().as_bytes()).unwrap(); + assert_eq!(unsafe { libc::mkfifo(filename.as_ptr(), 0o600) }, 0); + let library = Arc::new(Library::notebook( + root.join("notebook").to_str().unwrap(), + &root.join("cache"), + )); + let cached = |library: Arc| { + let (send, receive) = std::sync::mpsc::channel(); + std::thread::spawn(move || send.send(library.themes()).unwrap()); + receive + .recv_timeout(Duration::from_secs(1)) + .expect("theme access must not wait for storage") + }; + assert_eq!(*cached(Arc::clone(&library)), Themes::default()); + let deadline = Instant::now() + Duration::from_secs(3); + let mut writer = loop { + match std::fs::OpenOptions::new() + .write(true) + .custom_flags(libc::O_NONBLOCK) + .open(&path) + { + Ok(writer) => break writer, + Err(error) + if error.raw_os_error() == Some(libc::ENXIO) && Instant::now() < deadline => + { + std::thread::sleep(Duration::from_millis(5)); + } + Err(error) => panic!("theme reader did not start: {error}"), + } + }; + assert_eq!(*cached(Arc::clone(&library)), Themes::default()); + let assignment = |theme: &str, assigned| Themes { + assignments: vec![Assignment { + scope: Scope::Notebook, + theme: Some(theme.to_owned()), + assigned, + }], + ..Themes::default() + }; + let local = assignment("local", 10); + { + let mut kept = library.themes.lock().unwrap(); + merge(Arc::make_mut(&mut kept.value), local.clone()); + } + assert_eq!(*cached(Arc::clone(&library)), local); + writer + .write_all(&serde_json::to_vec(&assignment("remote", 5)).unwrap()) + .unwrap(); + drop(writer); + while library.themes.lock().unwrap().reading { + assert!(Instant::now() < deadline, "theme read did not finish"); + std::thread::sleep(Duration::from_millis(5)); + } + assert_eq!(*cached(Arc::clone(&library)), local); + std::fs::remove_file(&path).unwrap(); + std::fs::create_dir(&path).unwrap(); + library.themes.lock().unwrap().checked = Some(Instant::now() - Duration::from_secs(31)); + assert_eq!(*cached(Arc::clone(&library)), local); + while library.themes.lock().unwrap().reading { + assert!( + Instant::now() < deadline, + "failed theme read did not finish" + ); + std::thread::sleep(Duration::from_millis(5)); + } + assert_eq!(*cached(library), local); + std::fs::remove_dir_all(root).unwrap(); + } + /// A section kept open is what `open` hands out next, without opening it again, and /// readying a section another holds leaves it to that one at once. #[test] diff --git a/crates/snowbound/src/main.rs b/crates/snowbound/src/main.rs index 333af46c7e9b037c3a22cdb9a7e4ef2d8c24ef97..7fcd736bbbab151d665760eb47247c9782db70fb 100644 --- a/crates/snowbound/src/main.rs +++ b/crates/snowbound/src/main.rs @@ -3960,7 +3960,7 @@ impl State { /// Follows a page shown in place of another. fn opened(&mut self) -> Result<(), Box> { - if let Err(error) = self.wear_theme() { + if let Err(error) = self.wear_theme(true) { eprintln!("Restyling the page failed: {error}"); } // A page a search result shows leaves the keys with the search. @@ -4243,6 +4243,7 @@ impl State { /// Applies what the section, and each notebook's closed sections, reported since the /// last poll. fn synced(&mut self) -> Result<(), Box> { + self.wear_theme(false)?; // Every notebook's changes are taken, so none is reported again. let reported: Vec<(Arc, Vec)> = (self.notebooks.iter()) .filter_map(|library| { diff --git a/crates/snowbound/src/themes.rs b/crates/snowbound/src/themes.rs index 359f3ebdd92089615af6ff5ee27c46939906490b..ee9128e2439a76db8c2bdad7ec1af17bdbef7b32 100644 --- a/crates/snowbound/src/themes.rs +++ b/crates/snowbound/src/themes.rs @@ -162,11 +162,14 @@ impl State { /// Dresses the open page in its theme: new text and Enter take the theme's styles, and /// style objects the theme gives otherwise are restyled, as one edit. A page with no theme /// keeps what it holds. - pub(crate) fn wear_theme(&mut self) -> Result<(), Box> { + pub(crate) fn wear_theme(&mut self, force: bool) -> Result<(), Box> { let theme = self.page_theme(); let sheet = (theme.as_ref()) .map(|theme| theme.sheet(self.section_color())) .unwrap_or_default(); + if !force && self.view.editor.styles == sheet { + return Ok(()); + } self.view.editor.styles = sheet.clone(); let Some(session) = &self.session else { return Ok(()); @@ -730,7 +733,7 @@ impl State { self.ui.close_popup(id()); self.themes = None; if chosen.is_some() - && let Err(error) = self.wear_theme() + && let Err(error) = self.wear_theme(true) { eprintln!("Restyling the page failed: {error}"); } diff --git a/crates/snowbound/src/undo.rs b/crates/snowbound/src/undo.rs index 7c54603f2ee5b61cacde8eafa8d35f137cceedf1..c040325a8d39d62d57e5a3a3af72c1314a41c2de 100644 --- a/crates/snowbound/src/undo.rs +++ b/crates/snowbound/src/undo.rs @@ -1021,7 +1021,7 @@ impl State { change: Change::Themes { from: to, to: from }, }; self.undo.note(taken, undo); - return self.wear_theme(); + return self.wear_theme(true); } let Some(section) = section else { let library = self