diff --git a/crates/engine/src/autosave.rs b/crates/engine/src/autosave.rs index 8e1c2b1..44cc3b8 100644 --- a/crates/engine/src/autosave.rs +++ b/crates/engine/src/autosave.rs @@ -122,8 +122,9 @@ pub struct Autosaver { docs: HashMap, /// Drawings whose autosave is being written right now. busy: HashSet, + /// 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)>, #[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 { 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::, _>(|(_, 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) { 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); + } } } } diff --git a/crates/engine/tests/autosave.rs b/crates/engine/tests/autosave.rs index ea25c63..699f04e 100644 --- a/crates/engine/tests/autosave.rs +++ b/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();