Data Recovery: another app leaves a copy alone until its app writes it again

When a running app has to leave its recovery area (its lock file was
removed and another app locked the area), its copies stay in the old area
until it writes them again in its new area. Only that app knew this, so
once the other app let the old area go, any app could offer, restore or
discard those copies while their app still ran.

The app now writes an entry `<new area>/keeps-<old area>` in its own area
while copies of its documents are left in the old one, and removes it
when none are. Another app that finds an area unlocked checks for such an
entry, and treats the area as a running app's while the area that names
it is held. After a crash the new area is no longer held, so the copies
are offered as before. The heartbeat writes the entry again if it was
removed.

crates/engine/src/cmd/recovery.rs

@@ -19,7 +19,9 @@
 //! while their copies are restored or discarded the area is held, so two apps launched together
 //! never both take it. An area's lock file or folder can be removed while its app runs, so each
 //! recovery save and heartbeat checks that the lock still holds the area, and locks it again or
-//! takes a new area.
+//! takes a new area. The copies it had in the old area stay there until it has written them again
+//! in the new one, and an entry `<new area>/keeps-<old area>` ([`mark_left`]) tells other apps
+//! that those copies belong to a running app, so none of them offers or discards them meanwhile.
 //!
 //! A heartbeat alone can't tell a gone app from one whose timers stopped (a background tab), so
 //! when unsure an area counts as held, and its app repairs what another app did meanwhile: each
@@ -61,6 +63,9 @@ const BLOB_EXT: &str = ".blob";
 /// The heartbeat entry of an area in a store without locks: when its app last said it runs (Unix
 /// seconds, as text).
 const BEAT: &str = "heartbeat";
+/// The start of an entry `<area>/keeps-<left area>`: the app holding `<area>` still keeps the copies
+/// it left in `<left area>` ([`mark_left`]).
+const KEEPS: &str = "keeps-";
 /// The least time without a heartbeat after which an area's app counts as gone (seconds); else
 /// three Data Recovery intervals.
 const MIN_STALE: i64 = 180;
@@ -479,6 +484,8 @@ pub struct Recovery {
     /// store: each stays until its document has a copy in the new area, or needs none. They are
     /// still this session's: never offered or discarded as a crash's here.
     left: BTreeMap<u64, (Arc<dyn RecoveryStore>, String)>,
+    /// The `<own area>/keeps-<left area>` entries this session wrote in its area ([`mark_left`]).
+    marked: BTreeSet<String>,
 }
 
 impl Recovery {
@@ -553,6 +560,15 @@ fn beating(store: &dyn RecoveryStore, area: &str, stale: i64, mine: Option<i64>)
 /// beat only lets another app offer the copies once three intervals have passed.
 pub fn heartbeat(s: &mut Session) -> Vec<u64> {
     let mut lost = keep_hold(s);
+    // An entry that couldn't be written before, or that was removed since (by the user, a
+    // cleaner), is written now.
+    if let Some(own) = &s.recovery.own
+        && !s.recovery.marked.is_empty()
+        && let Ok(names) = own.store.list()
+    {
+        s.recovery.marked.retain(|n| names.contains(n));
+    }
+    mark_left(s);
     if let Some(own) = s.recovery.own.as_ref().filter(|o| o.beats) {
         let _ = beat(own.store.as_ref(), &own.area);
     }
@@ -622,6 +638,36 @@ fn drop_left(s: &mut Session, uid: u64) {
     let meta = store.remove(&format!("{file}{META_EXT}"));
     if doc.is_ok() && meta.is_ok() {
         s.recovery.left.remove(&uid);
+        mark_left(s);
+    }
+}
+
+/// Record in this session's area each area it left that still holds copies of its documents: an
+/// entry `<own area>/keeps-<left area>`, so another app leaves those copies alone while this one
+/// runs ([`keeper`]). The entries of areas that hold none of its copies any more go. Best effort:
+/// an entry not written or removed now is written or removed at the next heartbeat.
+fn mark_left(s: &mut Session) {
+    let Some(own) = &s.recovery.own else { return };
+    let (store, area) = (own.store.clone(), own.area.clone());
+    let want: BTreeSet<String> = s
+        .recovery
+        .left
+        .values()
+        .filter(|(st, _)| st.location() == store.location())
+        .filter_map(|(_, f)| f.split_once('/').map(|(a, _)| a))
+        .filter(|a| *a != area)
+        .map(|a| format!("{area}/{KEEPS}{a}"))
+        .collect();
+    let gone: Vec<String> = s.recovery.marked.difference(&want).cloned().collect();
+    for n in gone {
+        if store.remove(&n).is_ok() {
+            s.recovery.marked.remove(&n);
+        }
+    }
+    for n in want {
+        if !s.recovery.marked.contains(&n) && store.write(&n, b"").is_ok() {
+            s.recovery.marked.insert(n);
+        }
     }
 }
 
@@ -670,6 +716,9 @@ pub fn claim(s: &mut Session, store: &Arc<dyn RecoveryStore>) -> Result<String>
             }
         };
         s.recovery.own = Some(Own { store: store.clone(), area: area.clone(), hold, beats });
+        // The entries written before are in the area this session had (released or left).
+        s.recovery.marked.clear();
+        mark_left(s);
         return Ok(area);
     }
     Err(EngineError::Other("no free recovery area".into()))
@@ -686,6 +735,8 @@ pub fn release(s: &mut Session) {
         drop_left(s, uid);
     }
     let Some(Own { store, area, hold, .. }) = s.recovery.own.take() else { return };
+    // Its `keeps-` entries go with the area's other entries.
+    s.recovery.marked.clear();
     let mine = |n: &String| n.split_once('/').is_some_and(|(a, _)| a == area);
     for name in store.list().unwrap_or_default().iter().filter(|n| mine(n)) {
         let _ = store.remove(name);
@@ -931,6 +982,7 @@ impl RecoveryJob {
                     let _ = store.remove(&format!("{file}{DOC_EXT}"));
                     let _ = store.remove(&format!("{file}{META_EXT}"));
                 }
+                mark_left(s);
             }
             // Best effort, as in `remove_copy`.
             _ => {
@@ -1152,6 +1204,10 @@ fn owner(s: &mut Session, store: &dyn RecoveryStore, area: &str, hold: &mut Vec<
     }
     match store.lock(area) {
         Ok(Lock::Held(h)) => {
+            // Copies an app left here when it had to leave the area stay its own while it runs.
+            if let Some(who) = keeper(s, store, area) {
+                return who;
+            }
             hold.push(h);
             Owner::Gone
         }
@@ -1160,6 +1216,9 @@ fn owner(s: &mut Session, store: &dyn RecoveryStore, area: &str, hold: &mut Vec<
             if store.announced(area).unwrap_or(false) || beating(store, area, stale_after(s), s.recovery.adopted.get(area).copied()) {
                 return Owner::Running;
             }
+            if let Some(who) = keeper(s, store, area) {
+                return who;
+            }
             // Best effort: without it another app might offer the copies too, which loses nothing.
             if take && let Ok(t) = beat(store, area) {
                 s.recovery.adopted.insert(area.to_string(), t);
@@ -1169,6 +1228,40 @@ fn owner(s: &mut Session, store: &dyn RecoveryStore, area: &str, hold: &mut Vec<
     }
 }
 
+/// Who keeps the copies left in `area` by an app that had to leave it, from the entries
+/// `<its area>/keeps-<area>` ([`mark_left`]): this session, or another app that is running. `None`:
+/// no app that runs keeps them. A store that can't be listed now keeps them (a copy kept loses
+/// nothing).
+fn keeper(s: &Session, store: &dyn RecoveryStore, area: &str) -> Option<Owner> {
+    let Ok(names) = store.list() else { return Some(Owner::Running) };
+    let mark = format!("/{KEEPS}{area}");
+    let mut who = None;
+    for k in names.iter().filter_map(|n| n.strip_suffix(mark.as_str())).filter(|k| *k != area) {
+        match holder(s, store, k) {
+            Owner::Mine => return Some(Owner::Mine),
+            Owner::Running => who = Some(Owner::Running),
+            Owner::Gone => {}
+        }
+    }
+    who
+}
+
+/// Whose area `area` is, without taking it over: this session's, a running app's, or nobody's (a
+/// lock taken to tell goes at once).
+fn holder(s: &Session, store: &dyn RecoveryStore, area: &str) -> Owner {
+    if s.recovery.own.as_ref().is_some_and(|o| o.area == area && o.store.location() == store.location()) {
+        return Owner::Mine;
+    }
+    match store.lock(area) {
+        Ok(Lock::Held(_)) => Owner::Gone,
+        Ok(Lock::Busy) | Err(_) => Owner::Running,
+        Ok(Lock::Unsupported) => {
+            let runs = store.announced(area).unwrap_or(false) || beating(store, area, stale_after(s), s.recovery.adopted.get(area).copied());
+            if runs { Owner::Running } else { Owner::Gone }
+        }
+    }
+}
+
 /// Tidy away a gone app's area `area` once no copies are left in it, then let it go (`hold`). The
 /// area is removed while it is still held, so an app taking a new area of that name meanwhile
 /// finds it held or gone and takes the next name.

crates/engine/src/tests/recovery.rs

@@ -958,6 +958,54 @@ fn a_copy_left_behind_is_not_offered_by_its_own_app() {
     let _ = std::fs::remove_dir_all(&dir);
 }
 
+/// A copy left in an area the session had to leave is not another app's to offer, restore or
+/// discard while the session runs, even after the app that locked that area let it go: an entry in
+/// the session's new area says it keeps it. Once the copy is written again in the new area, the
+/// entry goes; after a crash before that, the copy is offered.
+#[cfg(unix)]
+#[test]
+fn a_copy_left_behind_is_not_offered_by_another_app_while_its_app_runs() {
+    use cmd::recovery::Lock;
+    let dir = std::env::temp_dir().join(format!("vc-recovery-leftother-{}", std::process::id()));
+    let _ = std::fs::remove_dir_all(&dir);
+    let left_behind = |area: &str| dir.join(area).join("Untitled-1-1.vectorcraft");
+    let leave = |a: &mut Session, area: &str| {
+        std::fs::remove_file(dir.join(area).join(".lock")).unwrap();
+        let Ok(Lock::Held(other)) = FolderStore::new(&dir).lock(area) else { panic!("the other app locks the area") };
+        cmd::recovery::heartbeat(a);
+        drop(other);
+        a.recovery.own_area().unwrap().to_string()
+    };
+    let (mut a, area) = saved_in(&dir);
+    let mine = leave(&mut a, &area);
+    assert_ne!(mine, area);
+    assert!(dir.join(&mine).join(format!("keeps-{area}")).is_file());
+    // Removed meanwhile: the next heartbeat writes it again.
+    std::fs::remove_file(dir.join(&mine).join(format!("keeps-{area}"))).unwrap();
+    cmd::recovery::heartbeat(&mut a);
+    assert!(dir.join(&mine).join(format!("keeps-{area}")).is_file());
+    let mut b = other_app(&dir);
+    assert!(offered(&mut b).is_empty());
+    assert_eq!(restored(b.execute("file.recovery.restore", &json!({})).unwrap()), 0);
+    assert!(b.execute("file.recovery.discard", &json!({})).unwrap()["discarded"].as_array().unwrap().is_empty());
+    assert!(b.execute("file.recovery.discard", &json!({"file": format!("{area}/Untitled-1-1")})).is_err());
+    assert!(left_behind(&area).is_file());
+    // Written again in the new area: the copy left behind and the entry go.
+    a.execute("file.recovery.save", &json!({})).unwrap();
+    assert!(!left_behind(&area).exists());
+    assert!(!dir.join(&mine).join(format!("keeps-{area}")).exists());
+    assert!(offered(&mut b).is_empty());
+    drop(a);
+    let _ = std::fs::remove_dir_all(&dir);
+    // A crash before the copy is written again: the copy left behind is offered.
+    let (mut a, area) = saved_in(&dir);
+    leave(&mut a, &area);
+    drop(a);
+    assert_eq!(offered(&mut other_app(&dir)).len(), 1);
+    assert!(left_behind(&area).is_file());
+    let _ = std::fs::remove_dir_all(&dir);
+}
+
 /// A complex document (Data Recovery off for those) gets no new copy after the session left its
 /// area: it keeps the copy it left there, and the heartbeat asks for it only once.
 #[cfg(unix)]