Fix launch guard ownership during concurrent startup (#1459)

* docs: track startup adapter recovery investigation (#477) * Fix launch guard ownership during concurrent startup * Launch guard (#477): fold the ownership tests into the tests module Review cleanup, no behaviour change: - Drop docs/issue-477-startup-recovery.md (investigation notes, not docs). - Move the investigation_477 and root_patch_477_tests cases into the existing tests module under behaviour names, without println! output; every assertion is kept. - Rewrap the module doc comment. * #477: preserve inaccessible launch markers on Windows --------- Co-authored-by: FredApps <FredApps@users.noreply.github.com>
2026-11-11 00:14:41 +00:00 · 2026-11-10 13:21:16 +01:00
parent d9fde92d83
commit 47e804f86f
1 changed files with 271 additions and 13 deletions
@@ -8,9 +8,9 @@
 //! until its window has kept drawing for [`SETTLE`] (or the app quits normally before then). A
 //! launch that finds the previous one's marker switches Window Graphics to OpenGL, which those
 //! drivers run, and says so once the window is up. `WGPU_BACKEND` (wgpu's own override) still
-//! wins over both. The marker is locked while its launch runs, so a second copy opened meanwhile
-//! isn't taken for a crash; and a window OpenGL can't open either puts Window Graphics back to
-//! Automatic ([`opengl_failed`]) rather than failing every launch.
+//! wins over both. A separate guard file is locked while its launch runs, so a second copy opened
+//! meanwhile isn't taken for a crash; and a window OpenGL can't open either puts Window Graphics
+//! back to Automatic ([`opengl_failed`]) rather than failing every launch.

 use std::fs::File;
 use std::path::{Path, PathBuf};
@@ -21,6 +21,46 @@ use eframe::wgpu::Backends;

 /// Left in the settings folder from a launch until its window has settled.
 const MARKER: &str = "launch-pending";
+const LOCK: &str = "launch.lock";
+
+/// Keep this file in place: unlinking a locked file lets a second inode acquire a new lock.
+fn acquire_guard(dir: &Path) -> Option<File> {
+    let file = match std::fs::create_dir_all(dir)
+        .and_then(|()| std::fs::OpenOptions::new().read(true).write(true).create(true).truncate(false).open(dir.join(LOCK)))
+    {
+        Ok(file) => file,
+        Err(e) => {
+            log::warn!("launch guard unavailable: {e}");
+            return None;
+        }
+    };
+    match file.try_lock() {
+        Ok(()) => {}
+        Err(std::fs::TryLockError::WouldBlock) => return None,
+        Err(e) => {
+            log::warn!("launch guard lock unavailable: {e}");
+            return None;
+        }
+    }
+    // Older versions lock the marker itself. Probe only the lock: Windows also prevents
+    // reads through another handle while it is held. An unreadable marker is not a stale one.
+    match File::open(dir.join(MARKER)) {
+        Ok(marker) => match marker.try_lock() {
+            Ok(()) => {}
+            Err(std::fs::TryLockError::WouldBlock) => return None,
+            Err(e) => {
+                log::warn!("launch marker lock unavailable: {e}");
+                return None;
+            }
+        },
+        Err(e) if e.kind() == std::io::ErrorKind::NotFound => {}
+        Err(e) => {
+            log::warn!("launch marker unavailable: {e}");
+            return None;
+        }
+    }
+    Some(file)
+}

 /// How long the window keeps drawing after its first frame before the launch counts as made.
 /// A driver that fails as the window is shown and its surface re-created does so within the
@@ -37,7 +77,7 @@ pub struct Launch {
    /// to show.
    pub notice: Option<String>,
    marker: Option<PathBuf>,
-    /// Holds the marker's lock until the launch settles (or the process ends).
+    /// Holds the separate guard file's lock until the launch settles (or the process ends).
    lock: Option<File>,
    /// When the window drew its first frame, until the launch settles.
    first_frame: Option<Instant>,
@@ -52,10 +92,13 @@ impl Launch {
        let prefs_path = dir.join(PREFS_FILE);
        let mut prefs = std::fs::read_to_string(&prefs_path).map(|t| Prefs::from_json(&t)).unwrap_or_default();
        let marker = dir.join(MARKER);
-        // Locked by another launch that is still opening its window: not a crash.
-        let held = File::open(&marker).is_ok_and(|f| matches!(f.try_lock(), Err(std::fs::TryLockError::WouldBlock)));
+        let Some(lock) = acquire_guard(dir) else {
+            return Launch { backends: (prefs.startup.window_graphics == "gl").then_some(Backends::GL), ..Launch::default() };
+        };
+        // Read again while owning the guard: another launch may have just changed preferences.
+        prefs = std::fs::read_to_string(&prefs_path).map(|t| Prefs::from_json(&t)).unwrap_or_default();
        let mut notice = None;
-        if marker.exists() && !held && prefs.startup.window_graphics == "auto" && cfg!(not(target_os = "macos")) {
+        if marker.exists() && prefs.startup.window_graphics == "auto" && cfg!(not(target_os = "macos")) {
            prefs.startup.window_graphics = "gl".into();
            match std::fs::create_dir_all(dir).and_then(|()| std::fs::write(&prefs_path, prefs.to_json())) {
                Ok(()) => log::warn!("the last launch stopped as its window opened: switching Window Graphics to OpenGL"),
@@ -69,15 +112,14 @@ impl Launch {
            );
        }
        let backends = (prefs.startup.window_graphics == "gl").then_some(Backends::GL);
-        let (marker, lock) = match std::fs::create_dir_all(dir).and_then(|()| File::create(&marker)) {
-            // Where locks aren't available the guard works as before.
-            Ok(f) => (Some(marker), f.try_lock().is_ok().then_some(f)),
+        let marker = match File::create(&marker) {
+            Ok(_) => Some(marker),
            Err(e) => {
                log::warn!("launch marker: {e}");
-                (None, None)
+                None
            }
        };
-        Launch { backends, notice, marker, lock, first_frame: None }
+        Launch { backends, notice, marker, lock: Some(lock), first_frame: None }
    }

    /// The window drew a frame. Until the launch has settled, returns how long that still takes
@@ -104,18 +146,19 @@ impl Launch {

    /// The launch made it: release and remove its marker.
    fn settled(&mut self) {
-        self.lock = None;
        if let Some(m) = self.marker.take()
            && let Err(e) = std::fs::remove_file(&m)
        {
            log::warn!("launch marker: {e}");
        }
+        self.lock = None;
    }
 }

 /// The window couldn't open with OpenGL (an error, not a crash): back to Automatic, and no marker.
 pub fn opengl_failed(dir: Option<&Path>) {
    let Some(dir) = dir else { return };
+    let Some(_lock) = acquire_guard(dir) else { return };
    let prefs_path = dir.join(PREFS_FILE);
    let mut prefs = std::fs::read_to_string(&prefs_path).map(|t| Prefs::from_json(&t)).unwrap_or_default();
    prefs.startup.window_graphics = "auto".into();
@@ -217,4 +260,219 @@ mod tests {
        }
        let _ = std::fs::remove_dir_all(&dir);
    }
+
+    /// A fresh settings folder for one test.
+    fn fresh_dir(case: &str) -> PathBuf {
+        let dir = std::env::temp_dir().join(format!("ec-launch-guard-{case}-{}", std::process::id()));
+        let _ = std::fs::remove_dir_all(&dir);
+        std::fs::create_dir_all(&dir).unwrap();
+        dir
+    }
+
+    /// Window Graphics saved as OpenGL.
+    fn save_opengl(dir: &Path) {
+        let mut prefs = Prefs::default();
+        prefs.startup.window_graphics = "gl".into();
+        std::fs::write(dir.join(PREFS_FILE), prefs.to_json()).unwrap();
+    }
+
+    /// A second copy that quits normally while the first is still opening doesn't remove the first
+    /// one's marker (#477).
+    #[test]
+    fn a_second_copy_quitting_leaves_the_running_launchs_marker() {
+        let dir = fresh_dir("second-quits");
+        let mut first = Launch::begin(Some(&dir), false);
+        let mut second = Launch::begin(Some(&dir), false);
+        second.exited();
+        let kept = dir.join(MARKER).exists();
+        first.exited();
+        let _ = std::fs::remove_dir_all(&dir);
+        assert!(kept, "the second copy removed the running launch's marker");
+    }
+
+    /// A second copy whose OpenGL window fails doesn't remove a running launch's marker.
+    #[test]
+    fn failed_opengl_in_a_second_copy_leaves_the_running_launchs_marker() {
+        let dir = fresh_dir("second-gl-fails");
+        let mut first = Launch::begin(Some(&dir), false);
+        let second = Launch::begin(Some(&dir), false);
+        opengl_failed(Some(&dir));
+        let kept = dir.join(MARKER).exists();
+        drop(second);
+        first.exited();
+        let _ = std::fs::remove_dir_all(&dir);
+        assert!(kept, "OpenGL cleanup removed another running launch's marker");
+    }
+
+    /// With `WGPU_BACKEND` set, a stale marker and a saved OpenGL choice are left as they are.
+    #[test]
+    fn wgpu_backend_leaves_a_stale_marker_and_saved_opengl_alone() {
+        let dir = fresh_dir("env-override");
+        save_opengl(&dir);
+        std::fs::write(dir.join(MARKER), b"stale-marker").unwrap();
+        let before = std::fs::read(dir.join(PREFS_FILE)).unwrap();
+        let mut l = Launch::begin(Some(&dir), true);
+        assert_eq!(l.backends, None);
+        assert!(l.notice.is_none());
+        l.exited();
+        assert!(dir.join(MARKER).exists());
+        assert_eq!(std::fs::read(dir.join(PREFS_FILE)).unwrap(), before);
+        let _ = std::fs::remove_dir_all(&dir);
+    }
+
+    /// After OpenGL fails to open a window, the next launch is back on Automatic, without a notice.
+    #[test]
+    fn failed_opengl_puts_the_next_launch_back_on_automatic() {
+        let dir = fresh_dir("gl-recovers");
+        save_opengl(&dir);
+        let l = Launch::begin(Some(&dir), false);
+        assert_eq!(l.backends, Some(Backends::GL));
+        drop(l);
+        opengl_failed(Some(&dir));
+        assert!(!dir.join(MARKER).exists());
+        assert_eq!(Prefs::from_json(&std::fs::read_to_string(dir.join(PREFS_FILE)).unwrap()).startup.window_graphics, "auto");
+        let mut l = Launch::begin(Some(&dir), false);
+        assert_eq!(l.backends, None);
+        assert!(l.notice.is_none());
+        l.exited();
+        let _ = std::fs::remove_dir_all(&dir);
+    }
+
+    /// The guard file is never removed or replaced: unlinking a locked file would let the next
+    /// launch lock a new one while the old lock is still held.
+    #[cfg(unix)]
+    #[test]
+    fn the_guard_file_stays_the_same_file_across_launches() {
+        use std::os::unix::fs::MetadataExt;
+        let dir = fresh_dir("guard-identity");
+        let mut a = Launch::begin(Some(&dir), false);
+        let before = std::fs::metadata(dir.join(LOCK)).unwrap();
+        let t0 = Instant::now();
+        assert_eq!(a.frame_drawn_at(t0), Some(SETTLE));
+        assert_eq!(a.frame_drawn_at(t0 + SETTLE), None);
+        assert!(!dir.join(MARKER).exists());
+        let after = std::fs::metadata(dir.join(LOCK)).unwrap();
+        assert_eq!((before.dev(), before.ino()), (after.dev(), after.ino()));
+        let mut b = Launch::begin(Some(&dir), false);
+        assert!(b.lock.is_some());
+        let reused = std::fs::metadata(dir.join(LOCK)).unwrap();
+        assert_eq!((before.dev(), before.ino()), (reused.dev(), reused.ino()));
+        b.exited();
+        let _ = std::fs::remove_dir_all(dir);
+    }
+
+    /// A second copy's frames and quit neither settle nor unlock the running launch.
+    #[test]
+    fn a_second_copys_frames_dont_settle_the_running_launch() {
+        let dir = fresh_dir("second-frames");
+        let mut a = Launch::begin(Some(&dir), false);
+        std::fs::write(dir.join(MARKER), b"owner-marker").unwrap();
+        let mut b = Launch::begin(Some(&dir), false);
+        assert!(b.lock.is_none());
+        assert!(b.marker.is_none());
+        let t0 = Instant::now();
+        assert_eq!(b.frame_drawn_at(t0), None);
+        assert_eq!(b.frame_drawn_at(t0 + SETTLE), None);
+        b.exited();
+        assert_eq!(std::fs::read(dir.join(MARKER)).unwrap(), b"owner-marker");
+        let probe = File::open(dir.join(LOCK)).unwrap();
+        assert!(matches!(probe.try_lock(), Err(std::fs::TryLockError::WouldBlock)));
+        a.exited();
+        let _ = std::fs::remove_dir_all(dir);
+    }
+
+    /// Older versions lock the marker itself: while one runs, a new launch changes nothing, and
+    /// once it has stopped without settling the next launch still switches to OpenGL.
+    #[test]
+    fn a_running_older_version_holding_the_marker_is_left_alone() {
+        let dir = fresh_dir("older-version");
+        std::fs::write(dir.join(MARKER), b"old-owner").unwrap();
+        std::fs::write(dir.join(PREFS_FILE), Prefs::default().to_json()).unwrap();
+        let old = File::open(dir.join(MARKER)).unwrap();
+        old.try_lock().unwrap();
+        let before = std::fs::read(dir.join(PREFS_FILE)).unwrap();
+        let mut b = Launch::begin(Some(&dir), false);
+        assert!(b.lock.is_none());
+        assert!(b.marker.is_none());
+        assert!(b.backends.is_none());
+        assert!(b.notice.is_none());
+        b.exited();
+        opengl_failed(Some(&dir));
+        assert_eq!(std::fs::read(dir.join(PREFS_FILE)).unwrap(), before);
+        // Windows locks forbid reading through another handle. Inspect the contents only after
+        // releasing the old owner's lock, but before recovery can truncate the marker.
+        drop(old);
+        assert_eq!(std::fs::read(dir.join(MARKER)).unwrap(), b"old-owner");
+        let mut recovered = Launch::begin(Some(&dir), false);
+        if cfg!(not(target_os = "macos")) {
+            assert_eq!(recovered.backends, Some(Backends::GL));
+        }
+        recovered.exited();
+        let _ = std::fs::remove_dir_all(dir);
+    }
+
+    /// A Windows sharing violation is not evidence that a marker is stale or absent.
+    #[cfg(windows)]
+    #[test]
+    fn an_unopenable_marker_leaves_settings_and_marker_alone() {
+        use std::os::windows::fs::OpenOptionsExt;
+
+        let dir = fresh_dir("marker-unopenable");
+        std::fs::write(dir.join(MARKER), b"old-owner").unwrap();
+        std::fs::write(dir.join(PREFS_FILE), Prefs::default().to_json()).unwrap();
+        let old = std::fs::OpenOptions::new().read(true).write(true).share_mode(0).open(dir.join(MARKER)).unwrap();
+        let before = std::fs::read(dir.join(PREFS_FILE)).unwrap();
+        let mut launch = Launch::begin(Some(&dir), false);
+        assert!(launch.lock.is_none());
+        assert!(launch.marker.is_none());
+        assert!(launch.backends.is_none());
+        assert!(launch.notice.is_none());
+        launch.exited();
+        opengl_failed(Some(&dir));
+        assert_eq!(std::fs::read(dir.join(PREFS_FILE)).unwrap(), before);
+        drop(old);
+        assert_eq!(std::fs::read(dir.join(MARKER)).unwrap(), b"old-owner");
+        let _ = std::fs::remove_dir_all(dir);
+    }
+
+    /// A guard file that can't be opened leaves Window Graphics and the marker as they are.
+    #[test]
+    fn an_unusable_guard_file_leaves_settings_and_marker_alone() {
+        let dir = fresh_dir("guard-unusable");
+        std::fs::create_dir(dir.join(LOCK)).unwrap();
+        std::fs::write(dir.join(MARKER), b"stale-marker").unwrap();
+        std::fs::write(dir.join(PREFS_FILE), Prefs::default().to_json()).unwrap();
+        let before = std::fs::read(dir.join(PREFS_FILE)).unwrap();
+        let mut l = Launch::begin(Some(&dir), false);
+        assert!(l.backends.is_none());
+        assert!(l.notice.is_none());
+        assert!(l.lock.is_none());
+        assert!(l.marker.is_none());
+        l.exited();
+        opengl_failed(Some(&dir));
+        assert_eq!(std::fs::read(dir.join(PREFS_FILE)).unwrap(), before);
+        assert_eq!(std::fs::read(dir.join(MARKER)).unwrap(), b"stale-marker");
+        let _ = std::fs::remove_dir_all(dir);
+    }
+
+    /// OpenGL cleanup from elsewhere waits until the running launch has let go; then it resets
+    /// Window Graphics to Automatic and removes the marker.
+    #[test]
+    fn failed_opengl_cleanup_waits_for_the_running_launch() {
+        let dir = fresh_dir("gl-cleanup-waits");
+        save_opengl(&dir);
+        let launch = Launch::begin(Some(&dir), false);
+        assert_eq!(launch.backends, Some(Backends::GL));
+        assert!(launch.lock.is_some());
+        let before = std::fs::read(dir.join(PREFS_FILE)).unwrap();
+        opengl_failed(Some(&dir));
+        assert_eq!(std::fs::read(dir.join(PREFS_FILE)).unwrap(), before);
+        assert!(dir.join(MARKER).exists(), "cleanup elsewhere can't remove a running launch's marker");
+        drop(launch);
+        opengl_failed(Some(&dir));
+        let after = Prefs::from_json(&std::fs::read_to_string(dir.join(PREFS_FILE)).unwrap()).startup.window_graphics;
+        assert_eq!(after, "auto", "once the launch has let go, failed OpenGL resets Window Graphics");
+        assert!(!dir.join(MARKER).exists());
+        let _ = std::fs::remove_dir_all(dir);
+    }
 }