Merge #303: Recovery: wait for the write in flight before an emergency save and before removing copies (#202) (@MarekSpodBiedry)

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(&note);
                 }
                 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<u64> = 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<bool>) {
+        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) {

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<bool> {
+    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(&copy).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 };