M1: keep a recovery copy until this run has its own (#187)

apps/figcraft/src/recovery.rs

@@ -4,7 +4,10 @@
 //! in the config folder, in the background, at most every [`INTERVAL`] seconds, and the copy goes
 //! once the document is saved or closed. The app holds a lock on its folder. At startup, the folders
 //! whose lock is free belong to runs that ended without closing: their copies open again as
-//! unsaved documents (`file.recover`) and the folder goes. Closing the app normally removes its own.
+//! unsaved documents (`file.recover`). Each such copy **stays until this run has its own copy of
+//! the document** (or it is saved or closed): until then it is the only copy on disk, and a run
+//! that can't keep copies of its own leaves it for the next start. Then it goes, and its folder
+//! with its last copy. Closing the app normally removes its own.
 //!
 //! A document that is one of your online files has a note beside its copy (`<uid>.online`, written
 //! after the copy it describes): the server, the file's id, and who the app was in the file with
@@ -49,12 +52,21 @@ pub struct Recovery {
     waiting: Vec<Waiting>,
 }
 
-/// A copy of one of your online files whose file is being joined again as session `session`.
+/// A copy from an earlier run, kept until what it became is safe.
 pub struct Waiting {
-    session: u64,
+    until: Until,
     copy: PathBuf,
-    /// The lock of the folder it is in: no other start takes the copy meanwhile.
-    _lock: Arc<File>,
+    /// The lock of the folder it is in: no other start takes the copy meanwhile (none for a copy
+    /// left directly in the root by older versions).
+    _lock: Option<Arc<File>>,
+}
+
+/// What a kept copy waits for.
+enum Until {
+    /// One of your online files, joined again as this session: its outcome.
+    Session(u64),
+    /// Opened as document `uid`: this run's own copy of it (or its save or close).
+    Copied(u64),
 }
 
 /// What [`recover_all`] brought back.
@@ -62,21 +74,26 @@ pub struct Waiting {
 pub struct Recovered {
     /// Documents opened from their copies.
     pub copies: usize,
-    /// Copies of online files waiting for their file ([`Recovery::adopt`]).
+    /// The copies kept until what they became is safe ([`Recovery::adopt`]).
     pub waiting: Vec<Waiting>,
 }
 
 impl Recovery {
     /// Take this run's folder under `root` (`None` when it can't be created or locked: no copies).
+    /// A crashed run with the same process id may still hold its folder (its copies kept): then
+    /// another name is taken.
     pub fn start(root: &Path) -> Option<Self> {
-        let dir = root.join(std::process::id().to_string());
-        std::fs::create_dir_all(&dir).ok()?;
-        let lock = File::create(dir.join(LOCK)).ok()?;
-        lock.try_lock().ok()?;
-        Some(Self { dir, _lock: lock, written: HashMap::new(), next: 0.0, waiting: Vec::new() })
+        let pid = std::process::id();
+        (0..10).find_map(|n| {
+            let dir = root.join(if n == 0 { pid.to_string() } else { format!("{pid}-{n}") });
+            std::fs::create_dir_all(&dir).ok()?;
+            let lock = File::options().write(true).create(true).truncate(false).open(dir.join(LOCK)).ok()?;
+            lock.try_lock().ok()?;
+            Some(Self { dir, _lock: lock, written: HashMap::new(), next: 0.0, waiting: Vec::new() })
+        })
     }
 
-    /// Keep earlier runs' copies of online files until their outcome is known.
+    /// Keep earlier runs' copies until what they became is safe.
     pub fn adopt(&mut self, waiting: Vec<Waiting>) {
         self.waiting.extend(waiting);
     }
@@ -86,8 +103,10 @@ impl Recovery {
     /// an idle window can sleep).
     pub fn tick(&mut self, s: &Session, now: f64) -> Option<f64> {
         self.settle(s);
-        // A join on its way settles within its timeouts: look again soon.
-        let settling = (!self.waiting.is_empty()).then_some(0.5);
+        // A join on its way settles within its timeouts: look again soon. A copy waiting for this
+        // run's own is looked at again with the next copies.
+        let joining = self.waiting.iter().any(|w| matches!(w.until, Until::Session(_)));
+        let settling = if joining { Some(0.5) } else { (!self.waiting.is_empty()).then_some(INTERVAL) };
         let pending = s.docs.iter().any(|d| needs_copy(d) && self.written.get(&d.uid).is_none_or(|w| w.0 != d.revision));
         if now < self.next {
             return pending.then_some(self.next - now).or(settling);
@@ -97,26 +116,28 @@ impl Recovery {
         settling
     }
 
-    /// Remove earlier runs' copies whose outcome is known, once what they became is copied here.
-    fn settle(&mut self, s: &Session) {
-        if self.waiting.iter().all(|w| s.live.recovering(w.session)) {
-            return;
+    /// Remove earlier runs' copies once what they became is safe: copied here (the write
+    /// succeeded), saved or closed. An online file's copy goes once its outcome is known and every
+    /// document is safe, since which of them it became isn't known.
+    pub fn settle(&mut self, s: &Session) {
+        let joined = |w: &Waiting| matches!(w.until, Until::Session(id) if !s.live.recovering(id));
+        if self.waiting.iter().any(joined) {
+            self.write(s, true);
         }
-        self.write(s, true);
-        let (done, waiting): (Vec<Waiting>, Vec<Waiting>) =
-            std::mem::take(&mut self.waiting).into_iter().partition(|w| !s.live.recovering(w.session));
+        let all_safe = s.docs.iter().all(|d| self.safe(d));
+        let (done, waiting): (Vec<Waiting>, Vec<Waiting>) = std::mem::take(&mut self.waiting).into_iter().partition(|w| match w.until {
+            Until::Session(_) => joined(w) && all_safe,
+            Until::Copied(uid) => s.docs.iter().find(|d| d.uid == uid).is_none_or(|d| self.safe(d)),
+        });
         self.waiting = waiting;
-        for w in done {
-            let _ = std::fs::remove_file(&w.copy);
-            let _ = std::fs::remove_file(w.copy.with_extension(ONLINE));
-            // The folder goes with its last copy (while another waits there its lock is held).
-            let dir = w.copy.parent().map(Path::to_path_buf).filter(|_| Arc::strong_count(&w._lock) == 1);
-            drop(w);
-            if let Some(dir) = dir {
-                let _ = std::fs::remove_file(dir.join(LOCK));
-                let _ = std::fs::remove_dir(dir);
-            }
-        }
+        done.into_iter().for_each(remove_kept);
+    }
+
+    /// Whether document `d`'s work is on disk: saved (or nothing to keep), or copied here with no
+    /// write in flight.
+    fn safe(&self, d: &figcraft_engine::DocState) -> bool {
+        let written = self.written.get(&d.uid).is_some_and(|w| w.1.as_ref().is_none_or(JoinHandle::is_finished));
+        !needs_copy(d) || (written && self.dir.join(format!("{}.figcraft", d.uid)).is_file())
     }
 
     /// Copy every document with unsaved changes; `wait`: before returning (after a panic).
@@ -175,7 +196,9 @@ impl Recovery {
     /// 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) {
-        let Self { dir, _lock, written, .. } = self;
+        let Self { dir, _lock, written, waiting, .. } = self;
+        // The documents earlier runs' copies became were saved or discarded too.
+        waiting.into_iter().filter(|w| matches!(w.until, Until::Copied(_))).for_each(remove_kept);
         // Windows removes a folder only once nothing in it is open: copies being written first.
         for h in written.into_values().filter_map(|w| w.1) {
             let _ = h.join();
@@ -185,6 +208,20 @@ impl Recovery {
     }
 }
 
+/// Remove a kept copy and its note, and its folder with its last copy (while another is kept
+/// there, its lock is held).
+fn remove_kept(w: Waiting) {
+    let _ = std::fs::remove_file(&w.copy);
+    let _ = std::fs::remove_file(w.copy.with_extension(ONLINE));
+    let last = w._lock.as_ref().is_some_and(|l| Arc::strong_count(l) == 1);
+    let dir = w.copy.parent().map(Path::to_path_buf).filter(|_| last);
+    drop(w);
+    if let Some(dir) = dir {
+        let _ = std::fs::remove_file(dir.join(LOCK));
+        let _ = std::fs::remove_dir(dir);
+    }
+}
+
 /// The extension of the note beside the copy of a document that is one of your online files.
 const ONLINE: &str = "online";
 
@@ -222,9 +259,9 @@ pub fn recover(root: &Path, s: &mut Session) -> usize {
 }
 
 /// Open the copies left by runs that ended without closing (folders under `root` whose lock is
-/// free, and copies directly in `root` from older versions) as unsaved documents, then remove them.
-/// A copy that can't be read stays where it is. A copy of one of your online files is kept
-/// instead while its file is joined again ([`Recovered::waiting`]).
+/// free, and copies directly in `root` from older versions) as unsaved documents. Each is kept
+/// until this run has its own copy of the document ([`Recovered::waiting`]); a copy of one of your
+/// online files, while its file is joined again. A copy that can't be read stays where it is.
 pub fn recover_all(root: &Path, s: &mut Session) -> Recovered {
     let mut out = Recovered::default();
     let Ok(entries) = std::fs::read_dir(root) else { return out };
@@ -234,10 +271,15 @@ pub fn recover_all(root: &Path, s: &mut Session) -> Recovered {
         }
         // One of your online files comes back from its server, its copy kept until then.
         if let Some((session, lock)) = lock.and_then(|l| Some((rejoin_online(file, s)?, l))) {
-            out.waiting.push(Waiting { session, copy: file.to_path_buf(), _lock: lock.clone() });
-        } else if s.execute("file.recover", &json!({"path": file.to_string_lossy()})).is_ok() {
+            out.waiting.push(Waiting { until: Until::Session(session), copy: file.to_path_buf(), _lock: Some(lock.clone()) });
+            return;
+        }
+        let before: Vec<u64> = s.docs.iter().map(|d| d.uid).collect();
+        if s.execute("file.recover", &json!({"path": file.to_string_lossy()})).is_ok() {
             out.copies += 1;
-            let _ = std::fs::remove_file(file);
+            if let Some(uid) = s.docs.iter().map(|d| d.uid).find(|u| !before.contains(u)) {
+                out.waiting.push(Waiting { until: Until::Copied(uid), copy: file.to_path_buf(), _lock: lock.cloned() });
+            }
         }
     };
     for e in entries.flatten() {
@@ -265,7 +307,7 @@ pub fn recover_all(root: &Path, s: &mut Session) -> Recovered {
         for f in files.iter().filter(|f| f.extension().is_some_and(|e| e == ONLINE) && !f.with_extension("figcraft").exists()) {
             let _ = std::fs::remove_file(f);
         }
-        // Empty now unless a copy couldn't be read, or waits for its file (kept, its lock held).
+        // Empty now unless a copy couldn't be read, or is kept (its lock held).
         if Arc::strong_count(&lock) == 1 {
             drop(lock);
             let _ = std::fs::remove_file(path.join(LOCK));

apps/figcraft/src/recovery_online_tests.rs

@@ -223,6 +223,30 @@ fn a_copy_ahead_of_the_server_has_its_edits_applied_and_sent() {
     let _ = std::fs::remove_dir_all(&root);
 }
 
+#[test]
+fn an_online_copy_stays_while_what_it_became_cant_be_copied() {
+    let root = root("ahead-blocked");
+    let (mut s, script, mut server, _) = shared();
+    s.execute("shape.ellipse", &json!({"x": 30, "y": 0, "width": 10, "height": 10})).unwrap();
+    script.lock().unwrap().sent.clear();
+    let left = crash(&s, &root);
+    let (mut next, script, mut r, _) = restart(&s, &root);
+    arrive(&mut next, &script, &mut server);
+    assert!(next.docs[1].is_dirty(), "the copy's edit is on top, not saved anywhere yet");
+    // Something is in the way of this run's copy: writing it fails.
+    let mine = root.join(std::process::id().to_string()).join(format!("{}.figcraft", next.docs[1].uid));
+    std::fs::create_dir_all(&mine).unwrap();
+    r.tick(&next, 1.0);
+    assert_eq!((copies(&left, "figcraft"), copies(&left, "online")), (1, 1), "kept: the work is nowhere else on disk");
+    // Once it can be written, the old copy goes.
+    std::fs::remove_dir(&mine).unwrap();
+    next.execute("shape.rectangle", &json!({"x": 50, "y": 0, "width": 4, "height": 4})).unwrap();
+    r.tick(&next, 2.0);
+    assert!(!left.exists());
+    drop(r);
+    let _ = std::fs::remove_dir_all(&root);
+}
+
 #[test]
 fn a_copy_the_server_already_has_is_discarded() {
     let root = root("equal");
@@ -332,7 +356,7 @@ fn without_its_record_the_copy_opens_as_any_other() {
     assert_eq!(recover(&root, &mut next), 1);
     assert!(script.lock().unwrap().joined.is_empty());
     assert!(next.docs.first().is_some_and(|d| d.is_dirty() && d.path.is_none()));
-    assert!(std::fs::read_dir(&root).unwrap().next().is_none(), "the folder is gone");
+    assert!(std::fs::read_dir(&root).unwrap().next().is_some(), "the copy stays until a run has its own");
     let _ = std::fs::remove_dir_all(&root);
 }
 

apps/figcraft/src/recovery_tests.rs

@@ -4,7 +4,7 @@
 use figcraft_engine::Session;
 use serde_json::json;
 
-use crate::recovery::{Recovery, recover};
+use crate::recovery::{Recovery, recover, recover_all};
 
 fn root(name: &str) -> std::path::PathBuf {
     let r = std::env::temp_dir().join(format!("figcraft-recovery-{name}-{}", std::process::id()));
@@ -34,7 +34,7 @@ fn a_crashed_runs_unsaved_documents_come_back() {
     let ds = next.docs.first().unwrap();
     assert!(ds.is_dirty() && ds.path.is_none());
     assert_eq!(next.doc().unwrap().node(next.doc().unwrap().pages[0]).map(|p| p.children.len()), Some(1));
-    assert!(std::fs::read_dir(&root).unwrap().next().is_none(), "the folder is gone");
+    assert!(std::fs::read_dir(&root).unwrap().next().is_some(), "the copy stays until a run has its own");
     let _ = std::fs::remove_dir_all(&root);
 }
 
@@ -62,3 +62,126 @@ 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);
 }
+
+/// A run that ended without closing, leaving a copy of `s`'s unsaved document: its folder (as
+/// another process's).
+fn crashed(s: &Session, root: &std::path::Path) -> std::path::PathBuf {
+    let mut r = Recovery::start(root).unwrap();
+    r.write(s, true);
+    drop(r);
+    let left = root.join("crashed");
+    std::fs::rename(root.join(std::process::id().to_string()), &left).unwrap();
+    left
+}
+
+fn copies_in(d: &std::path::Path) -> usize {
+    std::fs::read_dir(d).map_or(0, |d| d.flatten().filter(|f| f.path().extension().is_some_and(|e| e == "figcraft")).count())
+}
+
+/// The next start: the copies come back, and this run's recovery keeps them until it covers them.
+fn restart(root: &std::path::Path) -> (Session, Recovery) {
+    let mut next = Session::empty();
+    let recovered = recover_all(root, &mut next);
+    assert_eq!(recovered.copies, 1);
+    let mut r = Recovery::start(root).unwrap();
+    r.adopt(recovered.waiting);
+    (next, r)
+}
+
+#[test]
+fn a_recovered_copy_stays_until_this_run_has_its_own_copy() {
+    let root = root("handoff");
+    let left = crashed(&edited(), &root);
+    let (next, mut r) = restart(&root);
+    assert_eq!(copies_in(&left), 1, "the only copy on disk until this run writes one");
+    r.write(&next, true);
+    r.tick(&next, 100.0);
+    assert!(!left.exists(), "covered by this run's copy, the old one goes");
+    assert_eq!(copies_in(&root.join(std::process::id().to_string())), 1);
+    r.finish();
+    let _ = std::fs::remove_dir_all(&root);
+}
+
+#[test]
+fn a_recovered_copy_stays_while_its_replacement_cant_be_written() {
+    let root = root("blocked");
+    let left = crashed(&edited(), &root);
+    let (next, mut r) = restart(&root);
+    // Something is in the way of this run's copy: writing it fails.
+    let mine = root.join(std::process::id().to_string()).join(format!("{}.figcraft", next.docs[0].uid));
+    std::fs::create_dir_all(&mine).unwrap();
+    r.write(&next, true);
+    r.tick(&next, 100.0);
+    assert_eq!(copies_in(&left), 1, "kept: the work is nowhere else on disk");
+    // Once it can be written, the old copy goes.
+    std::fs::remove_dir(&mine).unwrap();
+    let mut next = next;
+    next.execute("shape.rectangle", &json!({"x": 1, "y": 1, "width": 2, "height": 2})).unwrap();
+    r.write(&next, true);
+    r.tick(&next, 200.0);
+    assert!(!left.exists());
+    r.finish();
+    let _ = std::fs::remove_dir_all(&root);
+}
+
+#[test]
+fn without_a_recovery_of_its_own_the_copy_stays_for_the_next_start() {
+    let root = root("nocopies");
+    crashed(&edited(), &root);
+    let mut next = Session::empty();
+    let recovered = recover_all(&root, &mut next);
+    assert_eq!(recovered.copies, 1);
+    // This run couldn't take a folder of its own: it ends (or crashes) with nothing copied.
+    drop(recovered);
+    let mut after = Session::empty();
+    assert_eq!(recover(&root, &mut after), 1, "the work comes back again");
+    let _ = std::fs::remove_dir_all(&root);
+}
+
+#[test]
+fn a_recovered_copy_goes_once_its_document_is_saved_or_closed() {
+    for close in [false, true] {
+        let root = root(if close { "closed" } else { "saved" });
+        let left = crashed(&edited(), &root);
+        let (mut next, mut r) = restart(&root);
+        if close {
+            next.docs.clear();
+        } else {
+            next.ds_mut().unwrap().mark_saved();
+        }
+        r.tick(&next, 0.0);
+        assert!(!left.exists(), "close {close}");
+        r.finish();
+        let _ = std::fs::remove_dir_all(&root);
+    }
+}
+
+#[test]
+fn a_normal_quit_removes_the_recovered_copies_it_took_over() {
+    let root = root("quitafter");
+    let left = crashed(&edited(), &root);
+    let (_, r) = restart(&root);
+    // Quitting asked to save or discard every document.
+    r.finish();
+    assert!(!left.exists());
+    let _ = std::fs::remove_dir_all(&root);
+}
+
+#[test]
+fn a_run_whose_folder_name_is_still_held_takes_another() {
+    let root = root("pidreuse");
+    // A crashed run with this process's id whose folder the start still holds (its copy waits).
+    let held = root.join(std::process::id().to_string());
+    std::fs::create_dir_all(&held).unwrap();
+    let lock = std::fs::File::create(held.join("lock")).unwrap();
+    lock.try_lock().unwrap();
+    let mut r = Recovery::start(&root).expect("a folder of its own");
+    let s = edited();
+    r.write(&s, true);
+    assert_eq!(copies_in(&held), 0);
+    let dirs = std::fs::read_dir(&root).unwrap().flatten().filter(|e| e.path() != held).count();
+    assert_eq!(dirs, 1);
+    r.finish();
+    drop(lock);
+    let _ = std::fs::remove_dir_all(&root);
+}

apps/figcraft/src/services.rs

@@ -242,6 +242,8 @@ impl Services for Desktop {
     fn emergency_save(&mut self, session: &Session) {
         if let Some(r) = crate::recovery::lock(&self.recovery).as_mut() {
             r.write(session, true);
+            // Earlier runs' copies now covered go, so the next start doesn't open them twice.
+            r.settle(session);
         }
     }
     fn reduce_motion(&self) -> bool {