From 57278aa264f7bbda73a30923db9b52a5541ae034 Mon Sep 17 00:00:00 2001 From: clover caruso Date: Sat, 10 Oct 2026 19:03:10 -0700 Subject: [PATCH] fix: release section cache before waiting for sync shutdown Keep section creation and other structure changes from holding the kept-section cache lock while a stalled network worker finishes. Cover timeout failure with a gated remote and assert that the cache remains usable during shutdown. Fixes #108 Assisted-by: gpt-6.1-sol --- crates/snowbound/src/library.rs | 106 +++++++++++++++++++++++++++++++- 1 file changed, 103 insertions(+), 3 deletions(-) diff --git a/crates/snowbound/src/library.rs b/crates/snowbound/src/library.rs index 3545d6381e303f6b651a9aa462b999d28da068aa..7678482809052655e2d117e301393237f3c6837a 100644 --- a/crates/snowbound/src/library.rs +++ b/crates/snowbound/src/library.rs @@ -1016,9 +1016,12 @@ impl Library { /// Closes the sections kept open, before the notebook's files change. pub fn close_kept(&self) { - if let Ok(mut kept) = self.kept.lock() { - close(kept.trim(0, |_| 1)); - } + let sections = self + .kept + .lock() + .map(|mut kept| kept.trim(0, |_| 1)) + .unwrap_or_default(); + close(sections); } pub async fn close_kept_async(&self) -> Result<(), Box> { @@ -1598,6 +1601,103 @@ pub fn locate(path: &Path) -> Located { mod tests { use super::*; + #[test] + fn closing_a_stalled_section_leaves_the_kept_cache_accessible() { + use std::sync::{Condvar, mpsc}; + + struct Stalled { + file: PathBuf, + entered: Arc>>>, + release: Arc<(Mutex, Condvar)>, + } + + impl notebook::Remote for Stalled { + fn read(&mut self) -> io::Result> { + onestore::read_file(&self.file) + } + + fn stamp(&mut self) -> io::Result { + if let Some(entered) = self.entered.lock().unwrap().take() { + entered.send(()).unwrap(); + let (lock, changed) = &*self.release; + let _ = changed + .wait_timeout_while(lock.lock().unwrap(), Duration::from_secs(5), |done| { + !*done + }) + .unwrap(); + } + Err(io::ErrorKind::TimedOut.into()) + } + + fn publish( + &mut self, + transaction: &onestore::Transaction, + ) -> Result<(), onestore::CommitError> { + transaction.commit_file(&self.file) + } + + fn confirm(&mut self, base: &onestore::Stamp) -> Result<(), onestore::CommitError> { + onestore::confirm_file(&self.file, base) + } + } + + let root = + std::env::temp_dir().join(format!("snowbound-stalled-close-{}", std::process::id())); + let _ = notebook::fs::remove_dir_all(&root); + notebook::fs::create_dir_all(&root).unwrap(); + let file = root.join("Stalled.one"); + notebook::fs::write( + &file, + &onestore::create_section("Stalled.one", "", "Author").unwrap(), + ) + .unwrap(); + let (entered, waiting) = mpsc::channel(); + let release = Arc::new((Mutex::new(false), Condvar::new())); + let gate = Arc::clone(&release); + let entered = Arc::new(Mutex::new(Some(entered))); + let section = Section::open_with( + &file, + root.join("cache"), + move |file| { + Ok(Stalled { + file: file.to_owned(), + entered: Arc::clone(&entered), + release: Arc::clone(&gate), + }) + }, + || {}, + ) + .unwrap(); + waiting.recv_timeout(Duration::from_secs(3)).unwrap(); + let library = Arc::new(Library::section(&file, &root.join("cache"))); + library.keep("Stalled.one", section); + let closing = Arc::clone(&library); + let close = std::thread::spawn(move || closing.close_kept()); + let deadline = Instant::now() + Duration::from_secs(1); + let available = loop { + if library + .kept + .try_lock() + .is_ok_and(|kept| !kept.contains(&"Stalled.one".to_owned())) + { + break true; + } + if Instant::now() >= deadline { + break false; + } + std::thread::yield_now(); + }; + *release.0.lock().unwrap() = true; + release.1.notify_all(); + close.join().unwrap(); + assert!( + available, + "stopping a network worker held the UI's cache lock" + ); + drop(library); + notebook::fs::remove_dir_all(root).unwrap(); + } + #[cfg(unix)] #[test] fn stalled_theme_reads_leave_the_cache_usable_and_keep_local_edits() { -- 2.54.0