Autosave: a write that failed is tried again without another change (fixes #717) (#774)

The planner marked a drawing as autosaved when it dispatched the write, and the background
worker only logged a failure. An unchanged drawing whose write failed was then never written
again, even once the folder became writable. The worker now reports whether the file was
written; on a failure (or a thread that could not be spawned) the drawing is no longer counted as
autosaved, so the next plan restarts its SAVETIME clock and tries again.

crates/engine/src/autosave.rs

@@ -122,8 +122,9 @@ pub struct Autosaver {
     docs: HashMap<u64, Tracked>,
     /// Drawings whose autosave is being written right now.
     busy: HashSet<u64>,
+    /// Writes in progress; each tells whether its file was written.
     #[cfg(not(target_arch = "wasm32"))]
-    running: Vec<(u64, std::thread::JoinHandle<()>)>,
+    running: Vec<(u64, std::thread::JoinHandle<bool>)>,
     #[cfg(not(target_arch = "wasm32"))]
     start: std::time::Instant,
     #[cfg(not(target_arch = "wasm32"))]
@@ -341,14 +342,7 @@ mod native {
         /// Returns how long until the next autosave is due, so the host can wake up for it.
         pub fn tick(&mut self, s: &Session) -> Option<Duration> {
             let now = self.start.elapsed().as_secs_f64();
-            let busy = &mut self.busy;
-            self.running.retain(|(uid, h)| {
-                let running = !h.is_finished();
-                if !running {
-                    busy.remove(uid);
-                }
-                running
-            });
+            self.reap();
             let dir = savefilepath();
             let minutes = savetime();
             let jobs = self.plan(s, now, minutes, &dir);
@@ -371,6 +365,26 @@ mod native {
             writes
         }
 
+        /// Take in the writes that have finished ([`Autosaver::tick`] does this first). A drawing
+        /// whose write failed is no longer counted as autosaved: the next plan restarts its clock,
+        /// so it is tried again SAVETIME later even if it doesn't change.
+        pub fn reap(&mut self) {
+            let (done, running) = std::mem::take(&mut self.running).into_iter().partition::<Vec<_>, _>(|(_, h)| h.is_finished());
+            self.running = running;
+            for (uid, h) in done {
+                self.busy.remove(&uid);
+                if !h.join().unwrap_or(false) {
+                    self.unwritten(uid);
+                }
+            }
+        }
+
+        fn unwritten(&mut self, uid: u64) {
+            if let Some(t) = self.docs.get_mut(&uid) {
+                t.written = None;
+            }
+        }
+
         /// Carry out jobs from [`Autosaver::plan`]: writes on a background thread, deletes now.
         pub fn perform(&mut self, jobs: Vec<Job>) {
             for job in jobs {
@@ -382,15 +396,24 @@ mod native {
                             continue;
                         };
                         let spawned = std::thread::Builder::new().name("autosave".into()).spawn(move || match write_file(write, &doc, &path) {
-                            Ok(()) => log::info!("autosaved {path}"),
-                            Err(e) => log::warn!("autosave {path}: {e}"),
+                            Ok(()) => {
+                                log::info!("autosaved {path}");
+                                true
+                            }
+                            Err(e) => {
+                                log::warn!("autosave {path}: {e}");
+                                false
+                            }
                         });
                         match spawned {
                             Ok(h) => {
                                 self.busy.insert(uid);
                                 self.running.push((uid, h));
                             }
-                            Err(e) => log::warn!("autosave: {e}"),
+                            Err(e) => {
+                                log::warn!("autosave: {e}");
+                                self.unwritten(uid);
+                            }
                         }
                     }
                 }

crates/engine/tests/autosave.rs

@@ -211,6 +211,58 @@ fn autosave_files_are_written_in_the_background_and_removed_on_exit() {
     let _ = std::fs::remove_dir_all(&dir);
 }
 
+#[test]
+fn a_failed_autosave_is_tried_again_without_another_change() {
+    hooks();
+    let root = temp_dir("retry");
+    // The folder can't be created while a file holds its name: the write fails (#717).
+    let out = root.join("out");
+    std::fs::write(&out, b"not a folder").unwrap();
+    let dir = out.to_string_lossy().to_string();
+    let mut s = Session::new();
+    draw(&mut s);
+    let mut a = Autosaver::new("31-e");
+    a.plan(&s, 0.0, 1, &dir);
+    let jobs = a.plan(&s, 60.0, 1, &dir);
+    let path = writes(&jobs).pop().unwrap();
+    a.perform(jobs);
+
+    // Once the write has failed, the unchanged drawing waits SAVETIME again.
+    let mut rearmed = false;
+    for _ in 0..500 {
+        a.reap();
+        assert!(writes(&a.plan(&s, 120.0, 1, &dir)).is_empty());
+        if a.next_due(120.0, 1) == Some(60.0) {
+            rearmed = true;
+            break;
+        }
+        std::thread::sleep(Duration::from_millis(10));
+    }
+    assert!(rearmed, "a failed autosave must be tried again");
+
+    std::fs::remove_file(&out).unwrap();
+    let jobs = a.plan(&s, 180.0, 1, &dir);
+    assert_eq!(writes(&jobs), vec![path.clone()]);
+    a.perform(jobs);
+    for _ in 0..500 {
+        if std::path::Path::new(&path).exists() {
+            break;
+        }
+        std::thread::sleep(Duration::from_millis(10));
+    }
+    assert!(std::fs::read(&path).unwrap().ends_with(b"EOF\n"));
+
+    // A write that succeeded is not repeated.
+    for _ in 0..50 {
+        a.reap();
+        assert!(writes(&a.plan(&s, 1e4, 1, &dir)).is_empty());
+        std::thread::sleep(Duration::from_millis(10));
+    }
+    assert_eq!(a.next_due(1e4, 1), None);
+    a.shutdown();
+    let _ = std::fs::remove_dir_all(&root);
+}
+
 #[test]
 fn recover_as_copy_opens_an_unsaved_untitled_drawing() {
     hooks();