From 8b961b980ad9bd9376e81aa3bf11d16bbfccadf5 Mon Sep 17 00:00:00 2001
From: MarekSpodBiedry <73122968+MarekSpodBiedry@users.noreply.github.com>
Date: Wed, 14 Oct 2026 16:12:40 +0200
Subject: [PATCH] Recovery: wait for the write in flight before an emergency
 save and before removing copies (#202)

---
 apps/figcraft/src/recovery.rs       | 21 ++++++++++++--
 apps/figcraft/src/recovery_tests.rs | 45 +++++++++++++++++++++++++++++
 2 files changed, 64 insertions(+), 2 deletions(-)

diff --git a/apps/figcraft/src/recovery.rs b/apps/figcraft/src/recovery.rs
index 93335394..8e2c719f 100644
--- a/apps/figcraft/src/recovery.rs
+++ b/apps/figcraft/src/recovery.rs
@@ -125,13 +125,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((_, writer)) = self.written.remove(&d.uid) {
+                    // A write still running would bring the copy back after it was removed.
+                    if let Some(h) = writer {
+                        let _ = h.join();
+                    }
                     let _ = std::fs::remove_file(&path);
                     let _ = std::fs::remove_file(&note);
                 }
                 continue;
             }
             let entry = self.written.entry(d.uid).or_insert((0, None));
+            // An emergency save must copy the newest revision: let the write in flight finish first.
+            if wait && let Some(h) = entry.1.take() {
+                let _ = h.join();
+            }
             let busy = entry.1.as_ref().is_some_and(|h| !h.is_finished());
             if entry.0 == d.revision || busy {
                 continue;
@@ -162,9 +170,12 @@ 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 {
+                if let Some(h) = w.1.take() {
+                    let _ = h.join();
+                }
                 let _ = std::fs::remove_file(dir.join(format!("{uid}.figcraft")));
                 let _ = std::fs::remove_file(dir.join(format!("{uid}.{ONLINE}")));
             }
@@ -172,6 +183,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, (revision, Some(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 95560319..13faaef5 100644
--- a/apps/figcraft/src/recovery_tests.rs
+++ b/apps/figcraft/src/recovery_tests.rs
@@ -62,3 +62,48 @@ fn copies_follow_saves_and_a_normal_quit_leaves_nothing() {
     assert!(!dir.exists(), "a normal quit removes the copies");
     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));
+        let _ = std::fs::write(path, b"stale");
+    })
+}
+
+#[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);
+}