diff --git a/apps/figcraft/src/recovery.rs b/apps/figcraft/src/recovery.rs index 0885c68e..49178d44 100644 --- a/apps/figcraft/src/recovery.rs +++ b/apps/figcraft/src/recovery.rs @@ -61,8 +61,14 @@ struct Copy { impl Copy { /// Take in the result of the write in flight once it ended; a failed one leaves its revision uncopied. fn reap(&mut self) { - if self.flight.as_ref().is_some_and(|f| f.1.is_finished()) - && let Some((revision, h)) = self.flight.take() + if self.flight.as_ref().is_some_and(|f| f.1.is_finished()) { + self.join(); + } + } + + /// Wait for the write in flight and take in its result. + fn join(&mut self) { + if let Some((revision, h)) = self.flight.take() && h.join().unwrap_or(false) { self.done = Some(revision); @@ -157,14 +163,21 @@ impl Recovery { let path = self.dir.join(format!("{}.figcraft", d.uid)); let note = path.with_extension(ONLINE); if !needs_copy(d) { - if self.written.remove(&d.uid).is_some() { + if let Some(mut w) = self.written.remove(&d.uid) { + // A write still running would bring the copy back after it was removed. + w.join(); let _ = std::fs::remove_file(&path); let _ = std::fs::remove_file(¬e); } continue; } let entry = self.written.entry(d.uid).or_default(); - entry.reap(); + // An emergency save must copy the newest revision: let the write in flight finish first. + if wait { + entry.join(); + } else { + entry.reap(); + } // A write still in flight (for any revision) is waited for; a failed one is tried again. if entry.done == Some(d.revision) || entry.flight.is_some() { continue; @@ -197,9 +210,10 @@ impl Recovery { // Closed documents. let open: Vec = s.docs.iter().map(|d| d.uid).collect(); let dir = &self.dir; - self.written.retain(|uid, _| { + self.written.retain(|uid, w| { let keep = open.contains(uid); if !keep { + w.join(); let _ = std::fs::remove_file(dir.join(format!("{uid}.figcraft"))); let _ = std::fs::remove_file(dir.join(format!("{uid}.{ONLINE}"))); } @@ -207,6 +221,12 @@ impl Recovery { }); } + /// Test hook: pretend a write of `uid` at `revision` is in flight in `writer`. + #[cfg(test)] + pub fn hold(&mut self, uid: u64, revision: u64, writer: JoinHandle) { + self.written.insert(uid, Copy { done: None, flight: Some((revision, writer)) }); + } + /// A normal quit: the documents were saved or their changes discarded on purpose. (Earlier /// runs' copies still waiting for their outcome stay for the next start.) pub fn finish(self) { diff --git a/apps/figcraft/src/recovery_tests.rs b/apps/figcraft/src/recovery_tests.rs index bada7d98..415f337d 100644 --- a/apps/figcraft/src/recovery_tests.rs +++ b/apps/figcraft/src/recovery_tests.rs @@ -63,6 +63,51 @@ fn copies_follow_saves_and_a_normal_quit_leaves_nothing() { let _ = std::fs::remove_dir_all(&root); } +fn slow_write(path: std::path::PathBuf) -> std::thread::JoinHandle { + std::thread::spawn(move || { + std::thread::sleep(std::time::Duration::from_millis(150)); + std::fs::write(path, b"stale").is_ok() + }) +} + +#[test] +fn saving_or_closing_waits_for_a_slow_write_so_the_copy_does_not_come_back() { + let root = root("slow-cleanup"); + let mut s = edited(); + let mut r = Recovery::start(&root).unwrap(); + let dir = std::fs::read_dir(&root).unwrap().flatten().next().unwrap().path(); + let uid = s.docs[0].uid; + let copy = dir.join(format!("{uid}.figcraft")); + r.hold(uid, 1, slow_write(copy.clone())); + s.ds_mut().unwrap().mark_saved(); + r.write(&s, false); + std::thread::sleep(std::time::Duration::from_millis(400)); + assert!(!copy.exists(), "a saved document's copy stays removed"); + // Closed: the same. + let s = edited(); + let uid = s.docs[0].uid; + let copy = dir.join(format!("{uid}.figcraft")); + r.hold(uid, 1, slow_write(copy.clone())); + r.write(&Session::empty(), false); + std::thread::sleep(std::time::Duration::from_millis(400)); + assert!(!copy.exists(), "a closed document's copy stays removed"); + let _ = std::fs::remove_dir_all(&root); +} + +#[test] +fn an_emergency_save_waits_for_the_write_in_flight_and_copies_the_newest_revision() { + let root = root("slow-emergency"); + let s = edited(); + let mut r = Recovery::start(&root).unwrap(); + let dir = std::fs::read_dir(&root).unwrap().flatten().next().unwrap().path(); + let uid = s.docs[0].uid; + let copy = dir.join(format!("{uid}.figcraft")); + r.hold(uid, 0, slow_write(copy.clone())); + r.write(&s, true); + assert!(std::fs::read(©).is_ok_and(|b| b != b"stale"), "the newest revision replaced the slow write"); + let _ = std::fs::remove_dir_all(&root); +} + /// The files named `*.figcraft` in the run folder under `root`. fn copy_count(root: &std::path::Path) -> usize { let Some(dir) = std::fs::read_dir(root).unwrap().flatten().next().map(|e| e.path()) else { return 0 };