Merge pull request #1519 from mechpanos/fix/gpu-sentinel-false-positive

fix(gpu): only blame the backend for a crash that happened early

src/gpu_backend.rs

@@ -20,7 +20,9 @@
 //!     pipeline creation (catches shader-compile failures), offscreen render
 //!     and readback. The first fully-passing backend wins;
 //!   - backends implicated in the previous run's crash (crash sentinel) are
-//!     skipped;
+//!     skipped, but only when that run died young: one that drew for
+//!     `SENTINEL_PROOF_DELAY` marks the sentinel survived, so a later kill
+//!     (Task Manager, OOM, power cut) costs the user nothing;
 //!   - if no hardware backend passes, one software-adapter probe (`--gpu-probe
 //!     sw`, e.g. WARP / llvmpipe / SwiftShader) is tried: slow but alive;
 //!   - the winning probe also reports adapter limits, so the packed
@@ -292,14 +294,35 @@ pub fn previous_run_crashed() -> bool {
     sentinel_path().is_some_and(|p| p.exists())
 }
 
+/// How long a backend has to keep the app alive before it stops being a
+/// suspect. The crashes this guard exists for kill the process at GPU init
+/// or on the first drawing viewport, well inside this window; a process that
+/// lived longer has already proved the backend works, so whatever ended it
+/// later was something else.
+pub const SENTINEL_PROOF_DELAY: Duration = Duration::from_secs(30);
+
+/// Marker line added to the sentinel once the backend has proved itself.
+const SURVIVED_MARKER: &str = "survived=1";
+
 /// Backends implicated in the previous run's crash, from the stale sentinel.
+///
+/// A sentinel left behind by a run that had already been drawing for
+/// [`SENTINEL_PROOF_DELAY`] names no suspect. Disarming only happens on a
+/// clean return from the event loop, so without this every abnormal end —
+/// the user killing the app from Task Manager, an OOM kill, a power cut, a
+/// panic with nothing to do with the GPU — used to blacklist whichever
+/// backend happened to be in use. Backends were then given up one by one
+/// until the app was left on a software rasterizer, telling the user to
+/// update a driver that was never at fault.
 pub fn crashed_backends() -> Vec<String> {
     let Some(p) = sentinel_path() else {
         return Vec::new();
     };
-    std::fs::read_to_string(p)
+    let text = std::fs::read_to_string(p).unwrap_or_default();
-        .unwrap_or_default()
+    if text.lines().any(|l| l.trim() == SURVIVED_MARKER) {
-        .lines()
+        return Vec::new();
+    }
+    text.lines()
         .find_map(|l| l.strip_prefix("backend="))
         .unwrap_or("")
         .split(',')
@@ -309,6 +332,23 @@ pub fn crashed_backends() -> Vec<String> {
         .collect()
 }
 
+/// Record that the armed backend has kept the app alive long enough to be
+/// trusted. Appends to the sentinel rather than removing it, so a crash is
+/// still visible to anyone reading the file.
+pub fn mark_sentinel_survived() {
+    let Some(p) = sentinel_path() else {
+        return;
+    };
+    let Ok(text) = std::fs::read_to_string(&p) else {
+        return;
+    };
+    if text.lines().any(|l| l.trim() == SURVIVED_MARKER) {
+        return;
+    }
+    let _ = std::fs::write(&p, format!("{text}{SURVIVED_MARKER}
+"));
+}
+
 pub fn arm_sentinel(backend: &str) {
     if let Some(p) = sentinel_path() {
         if let Some(parent) = p.parent() {
@@ -994,8 +1034,13 @@ mod tests {
         assert!(ProbeCaps::from_json_line("{\"backend\":\"gl\"}").is_none());
     }
 
+    /// `SENTINEL_DIR_ENV` is process-global, so the sentinel tests cannot
+    /// run beside each other.
+    static SENTINEL_TESTS: std::sync::Mutex<()> = std::sync::Mutex::new(());
+
     #[test]
     fn sentinel_round_trip_records_backends() {
+        let _guard = SENTINEL_TESTS.lock().unwrap_or_else(|e| e.into_inner());
         let dir = std::env::temp_dir().join(format!("ocs-gpu-test-{}", std::process::id()));
         let _ = std::fs::create_dir_all(&dir);
         // SAFETY: single-threaded test mutating a process-local test hook.
@@ -1011,6 +1056,37 @@ mod tests {
         let _ = std::fs::remove_dir_all(&dir);
     }
 
+    /// A run that drew long enough to prove the backend leaves a sentinel
+    /// behind when it is killed, but that sentinel must accuse nobody —
+    /// otherwise closing the app from Task Manager once costs the user a
+    /// working backend for good.
+    #[test]
+    fn a_backend_that_survived_is_not_blamed_for_a_later_kill() {
+        let _guard = SENTINEL_TESTS.lock().unwrap_or_else(|e| e.into_inner());
+        let dir = std::env::temp_dir().join(format!("ocs-gpu-survived-{}", std::process::id()));
+        let _ = std::fs::create_dir_all(&dir);
+        // SAFETY: single-threaded test mutating a process-local test hook.
+        unsafe { std::env::set_var(SENTINEL_DIR_ENV, &dir) };
+
+        arm_sentinel("dx12");
+        assert_eq!(crashed_backends(), vec!["dx12".to_string()], "a young run blames its backend");
+
+        mark_sentinel_survived();
+        assert!(previous_run_crashed(), "the sentinel still records that the run ended badly");
+        assert!(
+            crashed_backends().is_empty(),
+            "a backend that kept the app alive must not be skipped at the next launch"
+        );
+
+        // Marking twice must not corrupt the file or resurrect the accusation.
+        mark_sentinel_survived();
+        assert!(crashed_backends().is_empty());
+
+        disarm_sentinel();
+        unsafe { std::env::remove_var(SENTINEL_DIR_ENV) };
+        let _ = std::fs::remove_dir_all(&dir);
+    }
+
     #[test]
     fn force_hooks_drive_probes_without_spawning() {
         // SAFETY: single-threaded test mutating process-local test hooks.

src/main.rs

@@ -203,6 +203,14 @@ fn main() -> iced::Result {
             OpenCADStudio::gpu_backend::arm_sentinel(
                 gpu.backend_value.as_deref().unwrap_or("auto"),
             );
+            // The backend is only a suspect while it is young: a run that
+            // keeps drawing past this mark has proved it works, so a later
+            // end — Task Manager, an OOM kill, a power cut — must not cost
+            // the user that backend at the next launch.
+            std::thread::spawn(|| {
+                std::thread::sleep(OpenCADStudio::gpu_backend::SENTINEL_PROOF_DELAY);
+                OpenCADStudio::gpu_backend::mark_sentinel_survived();
+            });
             let result = app::run();
             OpenCADStudio::gpu_backend::disarm_sentinel();
             result