From e35f8eb1b78f7f30355509144e4e4f0d4a2f1905 Mon Sep 17 00:00:00 2001 From: Ashsajal Date: Sun, 18 Oct 2026 14:15:56 +0600 Subject: [PATCH 1/3] M8: wait for a recovery folder lock that frees itself (#334) A child process forked before its exec inherits the run's open descriptors, and a flock lasts while any descriptor for the file is open. A crashed run whose folder lock was still held by such a child read as a run that was still going, so recover_all skipped it and its unsaved documents were not opened at the next start. The desktop app's tests failed intermittently for the same reason whenever another test spawned a process in parallel (the reduced-motion read spawns gsettings on Linux and the BSDs): recovery_tests failed in 23 of 60 runs of the whole binary and never on its own. Taking a folder's lock now retries try_lock briefly, so a lock that frees itself is no longer read as a live run while a run that really is running still keeps its folder. Regression test: a_lock_freed_a_moment_later_is_not_a_run_that_is_still_going holds the crashed run's lock on a second descriptor, the way a forked child does, and frees it shortly after; the copies come back. The recovery tests that collide with a spawning test went from 8 failures in 10 runs to 0 in 10. --- apps/figcraft/src/recovery.rs | 29 +++++++++++++++++++++++------ apps/figcraft/src/recovery_tests.rs | 22 ++++++++++++++++++++++ docs/hardware-parity.md | 1 + 3 files changed, 46 insertions(+), 6 deletions(-) diff --git a/apps/figcraft/src/recovery.rs b/apps/figcraft/src/recovery.rs index 62243bc7..0ad97f0d 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_briefly(&dir.join(LOCK))?; Some(Self { dir, _lock: lock, written: HashMap::new(), next: 0.0, waiting: Vec::new() }) }) } @@ -293,6 +292,27 @@ 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 a folder's lock that frees itself is waited for, and between the tries. +const LOCK_WAIT: (usize, std::time::Duration) = (20, std::time::Duration::from_millis(5)); + +/// Lock the folder's `file`, waiting briefly for a lock that frees itself. 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 run that has ended can look like one that is still going +/// for the length of that window. A run that really is running holds the lock for as long as it +/// runs, and we give up on it after [`LOCK_WAIT`] (#334). +fn lock_briefly(file: &Path) -> Option { + let lock = File::options().write(true).create(true).truncate(false).open(file).ok()?; + let (tries, pause) = LOCK_WAIT; + for _ in 0..tries { + match lock.try_lock() { + Ok(()) => return Some(lock), + Err(std::fs::TryLockError::WouldBlock) => std::thread::sleep(pause), + Err(_) => return None, + } + } + None +} + /// 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. @@ -369,10 +389,7 @@ 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; - } + let Some(lock) = lock_briefly(&path.join(LOCK)) else { continue }; 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..c05e86d0 100644 --- a/apps/figcraft/src/recovery_tests.rs +++ b/apps/figcraft/src/recovery_tests.rs @@ -203,6 +203,28 @@ 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. +#[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(40)); + 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() diff --git a/docs/hardware-parity.md b/docs/hardware-parity.md index df822981..4780a0a8 100644 --- a/docs/hardware-parity.md +++ b/docs/hardware-parity.md @@ -50,6 +50,7 @@ Nothing has been timed against Figma on the same document (needs `.fig` import). | Date | Change | Summary | |---|---|---| +| 2026-10-18 | minor | Recovery copies and the fork/exec window (#334): a child process forked before its `exec` (the `gsettings` read of the reduced-motion setting, any spawn) inherits the running app's open recovery-folder lock, and a `flock` lasts while any descriptor for the file is open. A crashed run whose lock was still held by such a child therefore read as "still running" and its unsaved documents were not recovered at the next start, and the desktop app's tests failed intermittently whenever another test spawned a process in parallel. Taking a folder's lock now waits briefly for one that frees itself, so only a run that is really running keeps its folder. No numbers changed | | 2026-10-18 | minor | macOS quit crash (#781): on Macs with a Touch Bar the app could crash as it quit (an exception in AppKit's Touch Bar support while the window's content view, which the accessibility adapter subclasses, was torn down). Quitting now keeps everything (preferences, this run's recovery copies, live sessions ended) and then ends the process before that teardown. Quit from the menu bar ends the app without the unsaved-changes question, so unsaved files' recovery copies now stay and reopen at the next start instead of being removed. Numbers unchanged | | 2026-10-17 | minor | Linux row: clipboard reads on Linux and the BSDs time out after 2 s on a helper thread, images copied in other apps paste under Wayland (arboard's data-control feature, #693), a minimised window can't be detected under Wayland, so frame-only control requests may still wait there (#720), and Wayland compositors without server-side decorations give the window Adwaita controls (#592, checked by its author on GNOME, not by the maintainers; KDE not run); compiled in CI, not run on a Linux desktop. HiDPI row: native-density canvas images placed on whole device pixels (#359, geometry tests; not judged by eye). Numbers unchanged | | 2026-10-14 | minor | Copy and paste in the browser: the web build never wrote to the system clipboard, so Cmd+V after Cmd+C pasted whatever text the clipboard held before (as a text layer) instead of the copied layers. Copies (layers, Copy as SVG/CSS/text/PNG, links) now go out through egui's output, which eframe writes during the copy event (`web_clipboard_tests`). Numbers unchanged. | From d7a27c593ed15ad3ea8654bce15971fde7ac0c23 Mon Sep 17 00:00:00 2001 From: bflatastic Date: Sun, 18 Oct 2026 16:13:34 +0700 Subject: [PATCH 2/3] #795: wait once at startup for held recovery folders, only on Unix Collect the folders whose lock is held, wait 100 ms once and retry only those, instead of up to 100 ms per held folder. Recovery::start takes its folder without waiting again (a held name already falls through to another). Windows doesn't pass the lock to child processes, so the wait and its test are Unix-only; the test holds the lock 10 ms against the 100 ms wait. --- apps/figcraft/src/recovery.rs | 46 ++++++++++++++++------------- apps/figcraft/src/recovery_tests.rs | 3 +- 2 files changed, 28 insertions(+), 21 deletions(-) diff --git a/apps/figcraft/src/recovery.rs b/apps/figcraft/src/recovery.rs index 0ad97f0d..06b0cbeb 100644 --- a/apps/figcraft/src/recovery.rs +++ b/apps/figcraft/src/recovery.rs @@ -120,7 +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 = lock_briefly(&dir.join(LOCK))?; + let lock = lock_folder(&dir).ok()?; Some(Self { dir, _lock: lock, written: HashMap::new(), next: 0.0, waiting: Vec::new() }) }) } @@ -292,25 +292,15 @@ 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 a folder's lock that frees itself is waited for, and between the tries. -const LOCK_WAIT: (usize, std::time::Duration) = (20, std::time::Duration::from_millis(5)); +/// 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's `file`, waiting briefly for a lock that frees itself. 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 run that has ended can look like one that is still going -/// for the length of that window. A run that really is running holds the lock for as long as it -/// runs, and we give up on it after [`LOCK_WAIT`] (#334). -fn lock_briefly(file: &Path) -> Option { - let lock = File::options().write(true).create(true).truncate(false).open(file).ok()?; - let (tries, pause) = LOCK_WAIT; - for _ in 0..tries { - match lock.try_lock() { - Ok(()) => return Some(lock), - Err(std::fs::TryLockError::WouldBlock) => std::thread::sleep(pause), - Err(_) => return None, - } - } - None +/// 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 @@ -382,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() { @@ -389,7 +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 Some(lock) = lock_briefly(&path.join(LOCK)) else { 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 c05e86d0..0322e237 100644 --- a/apps/figcraft/src/recovery_tests.rs +++ b/apps/figcraft/src/recovery_tests.rs @@ -206,6 +206,7 @@ fn a_run_whose_folder_name_is_still_held_takes_another() { /// #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"); @@ -215,7 +216,7 @@ fn a_lock_freed_a_moment_later_is_not_a_run_that_is_still_going() { 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(40)); + 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. From 8d3a3e617167ce1ad5f7e2374065f252ed494d79 Mon Sep 17 00:00:00 2001 From: bflatastic Date: Sun, 18 Oct 2026 16:13:41 +0700 Subject: [PATCH 3/3] #795: drop the hardware-parity revision row (test-only effect; the maintainer records it) --- docs/hardware-parity.md | 1 - 1 file changed, 1 deletion(-) diff --git a/docs/hardware-parity.md b/docs/hardware-parity.md index 4780a0a8..df822981 100644 --- a/docs/hardware-parity.md +++ b/docs/hardware-parity.md @@ -50,7 +50,6 @@ Nothing has been timed against Figma on the same document (needs `.fig` import). | Date | Change | Summary | |---|---|---| -| 2026-10-18 | minor | Recovery copies and the fork/exec window (#334): a child process forked before its `exec` (the `gsettings` read of the reduced-motion setting, any spawn) inherits the running app's open recovery-folder lock, and a `flock` lasts while any descriptor for the file is open. A crashed run whose lock was still held by such a child therefore read as "still running" and its unsaved documents were not recovered at the next start, and the desktop app's tests failed intermittently whenever another test spawned a process in parallel. Taking a folder's lock now waits briefly for one that frees itself, so only a run that is really running keeps its folder. No numbers changed | | 2026-10-18 | minor | macOS quit crash (#781): on Macs with a Touch Bar the app could crash as it quit (an exception in AppKit's Touch Bar support while the window's content view, which the accessibility adapter subclasses, was torn down). Quitting now keeps everything (preferences, this run's recovery copies, live sessions ended) and then ends the process before that teardown. Quit from the menu bar ends the app without the unsaved-changes question, so unsaved files' recovery copies now stay and reopen at the next start instead of being removed. Numbers unchanged | | 2026-10-17 | minor | Linux row: clipboard reads on Linux and the BSDs time out after 2 s on a helper thread, images copied in other apps paste under Wayland (arboard's data-control feature, #693), a minimised window can't be detected under Wayland, so frame-only control requests may still wait there (#720), and Wayland compositors without server-side decorations give the window Adwaita controls (#592, checked by its author on GNOME, not by the maintainers; KDE not run); compiled in CI, not run on a Linux desktop. HiDPI row: native-density canvas images placed on whole device pixels (#359, geometry tests; not judged by eye). Numbers unchanged | | 2026-10-14 | minor | Copy and paste in the browser: the web build never wrote to the system clipboard, so Cmd+V after Cmd+C pasted whatever text the clipboard held before (as a text layer) instead of the copied layers. Copies (layers, Copy as SVG/CSS/text/PNG, links) now go out through egui's output, which eframe writes during the copy event (`web_clipboard_tests`). Numbers unchanged. |