Merge fix/795

apps/figcraft/src/recovery.rs

@@ -120,8 +120,7 @@ impl Recovery {
         (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()?;
+            let lock = lock_folder(&dir).ok()?;
             Some(Self { dir, _lock: lock, written: HashMap::new(), next: 0.0, waiting: Vec::new() })
         })
     }
@@ -293,6 +292,17 @@ fn remove_kept(w: Waiting) {
 /// The extension of the note beside the copy of a document that is one of your online files.
 const ONLINE: &str = "online";
 
+/// How long the startup waits, once, for folder locks that may free themselves (#334).
+#[cfg(unix)]
+const LOCK_WAIT: std::time::Duration = std::time::Duration::from_millis(100);
+
+/// Lock the folder `dir` (`WouldBlock` while another process holds it).
+fn lock_folder(dir: &Path) -> Result<File, std::fs::TryLockError> {
+    let lock = File::options().write(true).create(true).truncate(false).open(dir.join(LOCK)).map_err(std::fs::TryLockError::Error)?;
+    lock.try_lock()?;
+    Ok(lock)
+}
+
 /// The copy at `file` is of one of your online files: join that file again as its owner, keeping
 /// the copy to compare with what arrives. The session, or `None` when the copy isn't one (or its
 /// file isn't recorded any more, or is open already): then it opens as any other copy.
@@ -362,6 +372,7 @@ pub fn recover_all(root: &Path, s: &mut Session) -> Recovered {
             }
         }
     };
+    let (mut free, mut held) = (Vec::new(), Vec::new());
     for e in entries.flatten() {
         let path = e.path();
         if !path.is_dir() {
@@ -369,10 +380,22 @@ pub fn recover_all(root: &Path, s: &mut Session) -> Recovered {
             continue;
         }
         // Still running (another window of the app), or not ours to take.
-        let Ok(lock) = File::options().write(true).create(true).truncate(false).open(path.join(LOCK)) else { continue };
-        if lock.try_lock().is_err() {
-            continue;
+        match lock_folder(&path) {
+            Ok(lock) => free.push((path, lock)),
+            Err(std::fs::TryLockError::WouldBlock) => held.push(path),
+            Err(_) => {}
         }
+    }
+    // A child between its `fork` and its `exec` shares the run's open descriptors, and a `flock`
+    // lasts while any of them is open, so an ended run's folder can look held for a moment. Wait
+    // once, however many folders are held, and try those again (#334). Windows doesn't pass the
+    // lock to child processes.
+    #[cfg(unix)]
+    if !held.is_empty() {
+        std::thread::sleep(LOCK_WAIT);
+        free.extend(held.into_iter().filter_map(|path| lock_folder(&path).ok().map(|lock| (path, lock))));
+    }
+    for (path, lock) in free {
         let lock = Arc::new(lock);
         let files: Vec<PathBuf> = std::fs::read_dir(&path).into_iter().flatten().flatten().map(|f| f.path()).collect();
         for f in &files {

apps/figcraft/src/recovery_tests.rs

@@ -203,6 +203,29 @@ fn a_run_whose_folder_name_is_still_held_takes_another() {
     let _ = std::fs::remove_dir_all(&root);
 }
 
+/// #334: a lock that frees itself is not a run that is still going. A child between its `fork` and
+/// its `exec` inherits the run's open descriptors and a `flock` lasts while any descriptor for the
+/// file is open, so a crashed run's copies must still come back once that child has exec'd.
+#[cfg(unix)]
+#[test]
+fn a_lock_freed_a_moment_later_is_not_a_run_that_is_still_going() {
+    let root = root("fork-window");
+    crashed(&edited(), &root);
+    // Hold the crashed run's folder lock on a second descriptor, the way a child forked before its
+    // exec holds it, and free it shortly after.
+    let lock = std::fs::File::options().write(true).create(true).truncate(false).open(root.join("crashed").join("lock")).unwrap();
+    lock.try_lock().unwrap();
+    let freed = std::thread::spawn(move || {
+        std::thread::sleep(std::time::Duration::from_millis(10));
+        drop(lock);
+    });
+    // The next start waits for that lock rather than reading the run as still going.
+    let mut next = Session::empty();
+    assert_eq!(recover(&root, &mut next), 1, "the copies come back once the lock frees itself");
+    freed.join().unwrap();
+    let _ = std::fs::remove_dir_all(&root);
+}
+
 /// The rectangles in `doc`.
 fn rectangles(doc: &figcraft_engine::doc::Document) -> usize {
     doc.nodes().filter(|n| matches!(n.kind, figcraft_engine::doc::NodeKind::Rectangle)).count()