authorgravatar for git@paperclover.netclover caruso <git@paperclover.net> 2026-10-05 01:58:00-07:00
committergravatar for git@paperclover.netclover caruso <git@paperclover.net> 2026-10-05 02:29:53-07:00
log85a3e4fb09fed22b4b1849cad623efd1cf7756fd
tree26bb51c5de724b250604c595a2413a42365432d7
parent12a7f783c4fa3c13083628ea28de8a96e65f3613
signature Signed by SSH key SHA256:52mNGHRsVFBDED9IAX5pe+LRWUefqTbxEReunq21QvU

fix: keep notebook switches responsive during cloud theme reads

Refresh notebook themes off the frame thread, retain the last good cache on read failures, and apply loaded themes when the worker reports back. Preserve newer local theme edits when a slower read completes. Verified with a stalled FIFO theme read: the previous release stops painting; the fixed app opens Options and switches appearance before the read completes. App tests, Clippy, and formatting pass. Assisted-by: gpt-6.1-sol

4 files changed, 149 insertions(+), 28 deletions(-)

crates/snowbound/src/library.rs+141-24
...@@ -324,8 +324,7 @@ pub struct Library {...@@ -324,8 +324,7 @@ pub struct Library {
324 /// Sections opened ahead of being shown, or left open after, most recent first, which324 /// Sections opened ahead of being shown, or left open after, most recent first, which
325 /// `open` hands out before opening any.325 /// `open` hands out before opening any.
326 kept: Mutex<crate::prefetch::Recent<String, Section>>,326 kept: Mutex<crate::prefetch::Recent<String, Section>>,
327 /// The notebook's style themes, and when they were read.327 themes: Mutex<ThemeCache>,
328 themes: Mutex<Option<(Instant, Arc<Themes>)>>,
329 /// The keys of its password-protected sections unlocked this run, kept as it is read again.328 /// The keys of its password-protected sections unlocked this run, kept as it is read again.
330 keys: Arc<Keys>,329 keys: Arc<Keys>,
331 /// The share a notebook another computer shares by Live Share is reached through.330 /// The share a notebook another computer shares by Live Share is reached through.
...@@ -338,6 +337,13 @@ pub struct Library {...@@ -338,6 +337,13 @@ pub struct Library {
338#[derive(Default)]337#[derive(Default)]
339pub struct Keys(Mutex<std::collections::HashMap<[u8; 16], (Key, Instant)>>);338pub struct Keys(Mutex<std::collections::HashMap<[u8; 16], (Key, Instant)>>);
340339
340#[derive(Default)]
341struct ThemeCache {
342 value: Arc<Themes>,
343 checked: Option<Instant>,
344 reading: bool,
345}
346
341impl Library {347impl Library {
342 /// `location` named `name`, with no notebook read, server, sync or kept sections.348 /// `location` named `name`, with no notebook read, server, sync or kept sections.
343 fn new(location: &str, name: String, cache: &Path) -> Self {349 fn new(location: &str, name: String, cache: &Path) -> Self {
...@@ -645,27 +651,53 @@ impl Library {...@@ -645,27 +651,53 @@ impl Library {
645 });651 });
646 }652 }
647653
648 /// The notebook's style themes, read again after a while so other machines' changes654 /// The notebook's last-read themes, refreshed off the frame thread.
649 /// reach pages opened later; a section opened on its own has none.655 pub fn themes(self: &Arc<Self>) -> Arc<Themes> {
650 pub fn themes(&self) -> Arc<Themes> {
651 let mut kept = self656 let mut kept = self
652 .themes657 .themes
653 .lock()658 .lock()
654 .unwrap_or_else(|poisoned| poisoned.into_inner());659 .unwrap_or_else(|poisoned| poisoned.into_inner());
655 if let Some((read, themes)) = &*kept660 let themes = Arc::clone(&kept.value);
656 && read.elapsed() < Duration::from_secs(30)661 if !matches!(self.notebook, Ok(Some(_)))
662 || kept.reading
663 || kept
664 .checked
665 .is_some_and(|read| read.elapsed() < Duration::from_secs(30))
657 {666 {
658 return Arc::clone(themes);667 return themes;
659 }668 }
660 let themes = match &self.notebook {669 kept.reading = true;
661 Ok(Some(notebook)) => notebook.themes().unwrap_or_else(|error| {670 drop(kept);
662 eprintln!("{}: reading its themes failed: {error}", self.location);671 let (library, before) = (Arc::clone(self), Arc::clone(&themes));
663 Themes::default()672 crate::spawn(move || {
664 }),673 let read = match &library.notebook {
665 _ => Themes::default(),674 Ok(Some(notebook)) => notebook.themes(),
666 };675 _ => Ok(Themes::default()),
667 let themes = Arc::new(themes);676 };
668 *kept = Some((Instant::now(), Arc::clone(&themes)));677 let mut kept = library
678 .themes
679 .lock()
680 .unwrap_or_else(|poisoned| poisoned.into_inner());
681 kept.reading = false;
682 kept.checked = Some(Instant::now());
683 match read {
684 Ok(mut read) => {
685 // A local edit may have landed while the file was being read.
686 if !Arc::ptr_eq(&before, &kept.value) {
687 notebook::sidecar::themes::merge(&mut read, (*kept.value).clone());
688 }
689 let changed = *kept.value != read;
690 kept.value = Arc::new(read);
691 drop(kept);
692 if changed {
693 notify_background();
694 }
695 }
696 Err(error) => {
697 eprintln!("{}: reading its themes failed: {error}", library.location);
698 }
699 }
700 });
669 themes701 themes
670 }702 }
671703
...@@ -675,13 +707,15 @@ impl Library {...@@ -675,13 +707,15 @@ impl Library {
675 if !matches!(self.notebook, Ok(Some(_))) {707 if !matches!(self.notebook, Ok(Some(_))) {
676 return;708 return;
677 }709 }
678 let mut themes = (*self.themes()).clone();710 self.themes();
679 notebook::sidecar::themes::merge(&mut themes, change.clone());711 {
680 *self712 let mut kept = self
681 .themes713 .themes
682 .lock()714 .lock()
683 .unwrap_or_else(|poisoned| poisoned.into_inner()) =715 .unwrap_or_else(|poisoned| poisoned.into_inner());
684 Some((Instant::now(), Arc::new(themes)));716 notebook::sidecar::themes::merge(Arc::make_mut(&mut kept.value), change.clone());
717 kept.checked = Some(Instant::now());
718 }
685 let library = Arc::clone(self);719 let library = Arc::clone(self);
686 crate::spawn(move || {720 crate::spawn(move || {
687 let kept = library721 let kept = library
...@@ -1471,6 +1505,89 @@ pub fn locate(path: &Path) -> Located {...@@ -1471,6 +1505,89 @@ pub fn locate(path: &Path) -> Located {
1471mod tests {1505mod tests {
1472 use super::*;1506 use super::*;
14731507
1508 #[cfg(unix)]
1509 #[test]
1510 fn stalled_theme_reads_leave_the_cache_usable_and_keep_local_edits() {
1511 use notebook::sidecar::themes::{Assignment, Scope, merge};
1512 use std::{
1513 io::Write,
1514 os::unix::{ffi::OsStrExt, fs::OpenOptionsExt},
1515 };
1516
1517 let root =
1518 std::env::temp_dir().join(format!("snowbound-stalled-themes-{}", std::process::id()));
1519 let _ = std::fs::remove_dir_all(&root);
1520 std::fs::create_dir_all(root.join("notebook/.snowbound")).unwrap();
1521 let path = root.join("notebook/.snowbound/themes.json");
1522 let filename = std::ffi::CString::new(path.as_os_str().as_bytes()).unwrap();
1523 assert_eq!(unsafe { libc::mkfifo(filename.as_ptr(), 0o600) }, 0);
1524 let library = Arc::new(Library::notebook(
1525 root.join("notebook").to_str().unwrap(),
1526 &root.join("cache"),
1527 ));
1528 let cached = |library: Arc<Library>| {
1529 let (send, receive) = std::sync::mpsc::channel();
1530 std::thread::spawn(move || send.send(library.themes()).unwrap());
1531 receive
1532 .recv_timeout(Duration::from_secs(1))
1533 .expect("theme access must not wait for storage")
1534 };
1535 assert_eq!(*cached(Arc::clone(&library)), Themes::default());
1536 let deadline = Instant::now() + Duration::from_secs(3);
1537 let mut writer = loop {
1538 match std::fs::OpenOptions::new()
1539 .write(true)
1540 .custom_flags(libc::O_NONBLOCK)
1541 .open(&path)
1542 {
1543 Ok(writer) => break writer,
1544 Err(error)
1545 if error.raw_os_error() == Some(libc::ENXIO) && Instant::now() < deadline =>
1546 {
1547 std::thread::sleep(Duration::from_millis(5));
1548 }
1549 Err(error) => panic!("theme reader did not start: {error}"),
1550 }
1551 };
1552 assert_eq!(*cached(Arc::clone(&library)), Themes::default());
1553 let assignment = |theme: &str, assigned| Themes {
1554 assignments: vec![Assignment {
1555 scope: Scope::Notebook,
1556 theme: Some(theme.to_owned()),
1557 assigned,
1558 }],
1559 ..Themes::default()
1560 };
1561 let local = assignment("local", 10);
1562 {
1563 let mut kept = library.themes.lock().unwrap();
1564 merge(Arc::make_mut(&mut kept.value), local.clone());
1565 }
1566 assert_eq!(*cached(Arc::clone(&library)), local);
1567 writer
1568 .write_all(&serde_json::to_vec(&assignment("remote", 5)).unwrap())
1569 .unwrap();
1570 drop(writer);
1571 while library.themes.lock().unwrap().reading {
1572 assert!(Instant::now() < deadline, "theme read did not finish");
1573 std::thread::sleep(Duration::from_millis(5));
1574 }
1575 assert_eq!(*cached(Arc::clone(&library)), local);
1576 std::fs::remove_file(&path).unwrap();
1577 std::fs::create_dir(&path).unwrap();
1578 library.themes.lock().unwrap().checked = Some(Instant::now() - Duration::from_secs(31));
1579 assert_eq!(*cached(Arc::clone(&library)), local);
1580 while library.themes.lock().unwrap().reading {
1581 assert!(
1582 Instant::now() < deadline,
1583 "failed theme read did not finish"
1584 );
1585 std::thread::sleep(Duration::from_millis(5));
1586 }
1587 assert_eq!(*cached(library), local);
1588 std::fs::remove_dir_all(root).unwrap();
1589 }
1590
1474 /// A section kept open is what `open` hands out next, without opening it again, and1591 /// A section kept open is what `open` hands out next, without opening it again, and
1475 /// readying a section another holds leaves it to that one at once.1592 /// readying a section another holds leaves it to that one at once.
1476 #[test]1593 #[test]
crates/snowbound/src/main.rs+2-1
...@@ -3960,7 +3960,7 @@ impl State {...@@ -3960,7 +3960,7 @@ impl State {
39603960
3961 /// Follows a page shown in place of another.3961 /// Follows a page shown in place of another.
3962 fn opened(&mut self) -> Result<(), Box<dyn Error>> {3962 fn opened(&mut self) -> Result<(), Box<dyn Error>> {
3963 if let Err(error) = self.wear_theme() {3963 if let Err(error) = self.wear_theme(true) {
3964 eprintln!("Restyling the page failed: {error}");3964 eprintln!("Restyling the page failed: {error}");
3965 }3965 }
3966 // A page a search result shows leaves the keys with the search.3966 // A page a search result shows leaves the keys with the search.
...@@ -4243,6 +4243,7 @@ impl State {...@@ -4243,6 +4243,7 @@ impl State {
4243 /// Applies what the section, and each notebook's closed sections, reported since the4243 /// Applies what the section, and each notebook's closed sections, reported since the
4244 /// last poll.4244 /// last poll.
4245 fn synced(&mut self) -> Result<(), Box<dyn Error>> {4245 fn synced(&mut self) -> Result<(), Box<dyn Error>> {
4246 self.wear_theme(false)?;
4246 // Every notebook's changes are taken, so none is reported again.4247 // Every notebook's changes are taken, so none is reported again.
4247 let reported: Vec<(Arc<Library>, Vec<String>)> = (self.notebooks.iter())4248 let reported: Vec<(Arc<Library>, Vec<String>)> = (self.notebooks.iter())
4248 .filter_map(|library| {4249 .filter_map(|library| {
crates/snowbound/src/themes.rs+5-2
...@@ -162,11 +162,14 @@ impl State {...@@ -162,11 +162,14 @@ impl State {
162 /// Dresses the open page in its theme: new text and Enter take the theme's styles, and162 /// Dresses the open page in its theme: new text and Enter take the theme's styles, and
163 /// style objects the theme gives otherwise are restyled, as one edit. A page with no theme163 /// style objects the theme gives otherwise are restyled, as one edit. A page with no theme
164 /// keeps what it holds.164 /// keeps what it holds.
165 pub(crate) fn wear_theme(&mut self) -> Result<(), Box<dyn Error>> {165 pub(crate) fn wear_theme(&mut self, force: bool) -> Result<(), Box<dyn Error>> {
166 let theme = self.page_theme();166 let theme = self.page_theme();
167 let sheet = (theme.as_ref())167 let sheet = (theme.as_ref())
168 .map(|theme| theme.sheet(self.section_color()))168 .map(|theme| theme.sheet(self.section_color()))
169 .unwrap_or_default();169 .unwrap_or_default();
170 if !force && self.view.editor.styles == sheet {
171 return Ok(());
172 }
170 self.view.editor.styles = sheet.clone();173 self.view.editor.styles = sheet.clone();
171 let Some(session) = &self.session else {174 let Some(session) = &self.session else {
172 return Ok(());175 return Ok(());
...@@ -730,7 +733,7 @@ impl State {...@@ -730,7 +733,7 @@ impl State {
730 self.ui.close_popup(id());733 self.ui.close_popup(id());
731 self.themes = None;734 self.themes = None;
732 if chosen.is_some()735 if chosen.is_some()
733 && let Err(error) = self.wear_theme()736 && let Err(error) = self.wear_theme(true)
734 {737 {
735 eprintln!("Restyling the page failed: {error}");738 eprintln!("Restyling the page failed: {error}");
736 }739 }
crates/snowbound/src/undo.rs+1-1
...@@ -1021,7 +1021,7 @@ impl State {...@@ -1021,7 +1021,7 @@ impl State {
1021 change: Change::Themes { from: to, to: from },1021 change: Change::Themes { from: to, to: from },
1022 };1022 };
1023 self.undo.note(taken, undo);1023 self.undo.note(taken, undo);
1024 return self.wear_theme();1024 return self.wear_theme(true);
1025 }1025 }
1026 let Some(section) = section else {1026 let Some(section) = section else {
1027 let library = self1027 let library = self