diff --git a/apps/figcraft/src/recovery.rs b/apps/figcraft/src/recovery.rs index 62243bc7..06b0cbeb 100644 --- a/apps/figcraft/src/recovery.rs +++ b/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 { + 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 = std::fs::read_dir(&path).into_iter().flatten().flatten().map(|f| f.path()).collect(); for f in &files { diff --git a/apps/figcraft/src/recovery_tests.rs b/apps/figcraft/src/recovery_tests.rs index 8f384b87..0322e237 100644 --- a/apps/figcraft/src/recovery_tests.rs +++ b/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()