From 171ffc3acdb21c0fe1b49e13481ce7f7f83fbec1 Mon Sep 17 00:00:00 2001 From: clover caruso Date: Thu, 15 Oct 2026 08:42:50 -0700 Subject: [PATCH] fix(snowbound): quitting closes every section before the process ends Quitting dropped the window's state and returned, but dropping a section or a notebook's background only asks its thread to stop. The sync worker holds the replica until its current step ends, so the process could end first: the replica's connection never closed, leaving its write-ahead log beside it, and a step publishing to a share could be cut short. Quitting now stops each notebook's background, closes the section shown and the sections kept open, and waits for them within the three seconds it already gave the shown section to publish. A replay test edits two sections, quits, and finds both edits in their files and no log left. Assisted-by: claude-opus-5.5 --- crates/snowbound/src/main.rs | 52 ++++++++++++++++++++++---------- crates/snowbound/tests/replay.rs | 34 +++++++++++++++++++++ 2 files changed, 70 insertions(+), 16 deletions(-) diff --git a/crates/snowbound/src/main.rs b/crates/snowbound/src/main.rs index 9201203ff704050c79faf5df24f8a8d4583cdd6b..e08a1550ae6ffdb4d2bab035bced1a2ce62914a1 100644 --- a/crates/snowbound/src/main.rs +++ b/crates/snowbound/src/main.rs @@ -6565,7 +6565,7 @@ impl App { .map(|outline| (&outline.id, outline.document()))); state.session.is_none() && !unchanged }) else { - return self.exit(event_loop); + return event_loop.exit(); }; crate::confirm( "Discard this page?", @@ -6578,19 +6578,12 @@ impl App { }), ); } - - fn exit(&self, event_loop: &ActiveEventLoop) { - if let Some(state) = &self.state { - state.publish_now(QUIT_PUBLISH); - } - event_loop.exit(); - } } -/// How long quitting waits for the open section's edits to reach its file; they stay in the -/// replica and publish at the next launch otherwise. +/// How long quitting waits for the open section's edits to reach its file and for every +/// section to close; edits left stay in the replica and publish at the next launch. #[cfg(not(target_arch = "wasm32"))] -const QUIT_PUBLISH: std::time::Duration = std::time::Duration::from_secs(3); +const QUIT: std::time::Duration = std::time::Duration::from_secs(3); #[cfg(not(target_arch = "wasm32"))] impl ApplicationHandler for App { @@ -6610,7 +6603,7 @@ impl ApplicationHandler for App { }; match event { UserEvent::Quit | UserEvent::Replay(Replay::Quit) => self.close(event_loop), - UserEvent::Exit => self.exit(event_loop), + UserEvent::Exit => event_loop.exit(), UserEvent::Renderer(chosen) | UserEvent::Replay(Replay::Renderer(chosen)) => { let state = self.state.take().expect("The window is open"); match state.switch_renderer(chosen) { @@ -6831,10 +6824,37 @@ impl ApplicationHandler for App { /// Drops the window's state while the event loop still holds the Wayland connection the /// clipboard and the surface use; `run_app` closes it on returning. fn exiting(&mut self, _: &ActiveEventLoop) { - self.restart = self - .state - .take() - .and_then(|state| state.updates.restarting()); + let Some(mut state) = self.state.take() else { + return; + }; + let deadline = Instant::now() + QUIT; + state.publish_now(QUIT); + self.restart = state.updates.restarting(); + let session = state.session.take(); + let libraries: Vec<_> = (state.notebooks.iter()) + .chain(session.as_ref().map(|session| &session.library)) + .cloned() + .collect(); + // Dropped sections and backgrounds stop without waiting, so the process could end first. + let (closed, closing) = mpsc::channel::<()>(); + std::thread::spawn(move || { + // Backgrounds first, or a released section would be checked again. + for background in libraries + .iter() + .filter_map(|library| library.background.as_ref()) + { + background.stop(); + } + if let Some(session) = session + && let Err(error) = session.section.close() + { + eprintln!("Synchronization stopped: {error}"); + } + libraries.iter().for_each(|library| library.close_kept()); + drop(closed); + }); + drop(state); + let _ = closing.recv_timeout(deadline.saturating_duration_since(Instant::now())); } } diff --git a/crates/snowbound/tests/replay.rs b/crates/snowbound/tests/replay.rs index 07494a2b48a924cdaeb59310cdf3a3a357099934..75fa85b5e7d9bf4b1ab99abb0e154f0f6d481faf 100644 --- a/crates/snowbound/tests/replay.rs +++ b/crates/snowbound/tests/replay.rs @@ -589,6 +589,40 @@ fn renaming_a_section_from_its_tab_types_into_the_tab_at_once() { assert!(renamed.contains(r#"Tab "Renamed""#), "{renamed}"); } +/// Quitting publishes the edits of the section shown and the one left, and closes both +/// replicas, so SQLite leaves no write-ahead log beside them. +#[test] +fn quitting_publishes_and_closes_every_section() { + let scratch = Scratch::new("quit-closes"); + let notebook = + Path::new(env!("CARGO_MANIFEST_DIR")).join("../../corpus/cross-container/native/notebook"); + let type_into = |text| ["move 900 600", "press", "release", text, "settle"]; + let mut steps = type_into("type LEFT_QA").to_vec(); + steps.extend(["access click Cross", "settle"]); + steps.extend(type_into("type SHOWN_QA")); + replay(&scratch, Some(¬ebook), &steps); + for (file, text) in [("synthetic.one", "LEFT_QA"), ("Cross.one", "SHOWN_QA")] { + let pages = stored_pages(&scratch.0.join("notebook").join(file)); + assert!( + pages + .iter() + .any(|(_, page, _)| body_text(page).iter().any(|body| body == text)), + "{file} lacks {text}" + ); + } + let replicas = std::fs::read_dir(scratch.0.join("cache/replicas")) + .unwrap() + .flat_map(|folder| std::fs::read_dir(folder.unwrap().path()).unwrap()) + .map(|entry| entry.unwrap().file_name().into_string().unwrap()) + .filter(|name| name.contains(".sqlite")) + .collect::>(); + assert_eq!(replicas.len(), 2, "{replicas:?}"); + assert!( + replicas.iter().all(|name| name.ends_with(".sqlite")), + "{replicas:?}" + ); +} + fn stored_pages(path: &Path) -> Vec<(onestore::ExGuid, onestore::page::Page, u32)> { let bytes = std::fs::read(path).unwrap(); let store = onestore::Store::parse(&bytes).unwrap(); -- 2.54.0