diff --git a/crates/engine/src/autosave.rs b/crates/engine/src/autosave.rs index e1c56ef26..37127ac56 100644 --- a/crates/engine/src/autosave.rs +++ b/crates/engine/src/autosave.rs @@ -47,6 +47,11 @@ pub struct AutoSaveState { #[derive(Clone)] pub struct PendingWrite { pub path: PathBuf, + pub(crate) slot: u32, + pub(crate) revision: u64, + /// Originating project identity: the path changes on Save As, the epoch on Open/New. + pub(crate) project_path: Option, + pub(crate) project_epoch: u64, handle: std::sync::Arc>>>>, } @@ -57,9 +62,16 @@ impl std::fmt::Debug for PendingWrite { } impl PendingWrite { - pub fn spawn(path: PathBuf, work: impl FnOnce() -> std::io::Result<()> + Send + 'static) -> std::io::Result { + pub fn spawn( + path: PathBuf, + slot: u32, + revision: u64, + project_path: Option, + project_epoch: u64, + work: impl FnOnce() -> std::io::Result<()> + Send + 'static, + ) -> std::io::Result { let h = std::thread::Builder::new().name("autosave".into()).spawn(work)?; - Ok(PendingWrite { path, handle: std::sync::Arc::new(std::sync::Mutex::new(Some(h))) }) + Ok(PendingWrite { path, slot, revision, project_path, project_epoch, handle: std::sync::Arc::new(std::sync::Mutex::new(Some(h))) }) } pub fn is_finished(&self) -> bool { self.handle.lock().map(|h| h.as_ref().is_none_or(|h| h.is_finished())).unwrap_or(true) diff --git a/crates/engine/src/session_settings.rs b/crates/engine/src/session_settings.rs index 55c8e4154..ef3e748a6 100644 --- a/crates/engine/src/session_settings.rs +++ b/crates/engine/src/session_settings.rs @@ -225,36 +225,50 @@ impl Session { let prefs = self.autosave_prefs(); let fail = |e: std::io::Error| EngineError::Other(format!("auto-save failed: {e}")); let plan = autosave::plan_in(self.file_ops(), &prefs, self.path.as_deref(), root.as_deref(), self.autosave.last_slot).map_err(fail)?; + let path = plan.path.to_string_lossy().into_owned(); if self.autosave.background && !cfg!(target_arch = "wasm32") { let project = self.project.clone(); let config = self.config.clone(); let job = plan.clone(); - let pending = autosave::PendingWrite::spawn(plan.path.clone(), move || { - let json = project.to_json_compact(); - match &config { - Some(c) => job.write(c.files(), json.as_bytes()), - None => job.write(&crate::config::StdFiles, json.as_bytes()), - } - }) - .map_err(fail)?; + let pending = + autosave::PendingWrite::spawn(plan.path.clone(), plan.slot, self.revision, self.path.clone(), self.script_ui.project_epoch, move || { + let json = project.to_json_compact(); + match &config { + Some(c) => job.write(c.files(), json.as_bytes()), + None => job.write(&crate::config::StdFiles, json.as_bytes()), + } + }) + .map_err(fail)?; self.autosave.pending = Some(pending); } else { let json = self.project.to_json_compact(); plan.write(self.file_ops(), json.as_bytes()).map_err(fail)?; + self.commit_autosave(path.clone(), plan.slot, self.revision); } - let (path, slot) = (plan.path, plan.slot); - let p = path.to_string_lossy().to_string(); + Ok(path) + } + + /// Advance recovery bookkeeping only for a successfully written snapshot. + fn commit_autosave(&mut self, path: String, slot: u32, revision: u64) { self.autosave.last_slot = Some(slot); - self.autosave.saved_revision = Some(self.revision); - self.autosave.last_path = Some(p.clone()); + self.autosave.saved_revision = Some(revision); + self.autosave.last_path = Some(path); self.update_sentinel(); - Ok(p) } /// Wait for a background auto-save ([`autosave::AutoSaveState::background`]) to land. pub fn autosave_wait(&mut self) -> Result<()> { match self.autosave.pending.take() { - Some(w) => w.join().map_err(|e| EngineError::Other(format!("auto-save failed: {e}"))), + Some(w) => { + w.join().map_err(|e| EngineError::Other(format!("auto-save failed: {e}")))?; + // Open/New can replace the project before note_project_path collects this + // write. Save As changes only its path. Never publish the old snapshot as the + // new project's recovery record, even when that record's final reset fails. + if w.project_path == self.path && w.project_epoch == self.script_ui.project_epoch { + self.commit_autosave(w.path.to_string_lossy().into_owned(), w.slot, w.revision); + } + Ok(()) + } None => Ok(()), } } @@ -278,7 +292,7 @@ impl Session { return None; } self.autosave.last = Some(now); - if !self.is_dirty() || self.autosave.saved_revision == Some(self.revision) || self.render_job.is_some() { + if !self.is_dirty() || self.autosave.pending.is_some() || self.autosave.saved_revision == Some(self.revision) || self.render_job.is_some() { return None; } if self.path.is_none() && self.prefs.auto_save.location != "custom" && self.default_autosave_root().is_none() && self.autosave_folder_override.is_none() diff --git a/crates/engine/src/tests_settings.rs b/crates/engine/src/tests_settings.rs index 65e39b467..58819f55c 100644 --- a/crates/engine/src/tests_settings.rs +++ b/crates/engine/src/tests_settings.rs @@ -608,6 +608,216 @@ fn atomic_write_does_not_follow_a_link_at_the_old_temp_name() { let _ = std::fs::remove_dir_all(d); } +struct AutosaveTestStore { + config: DirConfig, + fail_next: std::sync::atomic::AtomicBool, + gate: std::sync::Mutex, std::sync::mpsc::Receiver<()>)>>, + release_on_prefs: std::sync::Mutex>>, + fail_reset_for: std::sync::Mutex>, + failed_resets: std::sync::atomic::AtomicUsize, +} + +impl ConfigStore for AutosaveTestStore { + fn read(&self, name: &str) -> Option { + self.config.read(name) + } + fn write(&self, name: &str, data: &str) -> std::io::Result<()> { + // note_project_path saves preferences after the project/path switch, before joining + // the pending autosave. Release that writer at this exact transition, without sleeps. + if name == PREFS_FILE + && let Some(release) = self.release_on_prefs.lock().unwrap().take() + { + release.send(()).unwrap(); + } + if name == autosave::SENTINEL + && let Some(path) = self.fail_reset_for.lock().unwrap().as_ref() + { + let record: autosave::Sentinel = serde_json::from_str(data).unwrap(); + if record.project.as_ref() == Some(path) && record.autosave.is_none() { + self.failed_resets.fetch_add(1, std::sync::atomic::Ordering::SeqCst); + return Err(std::io::Error::other("injected final recovery reset failure")); + } + } + self.config.write(name, data) + } + fn remove(&self, name: &str) -> std::io::Result<()> { + self.config.remove(name) + } + fn dir(&self) -> Option { + self.config.dir() + } + fn files(&self) -> &dyn crate::config::FileOps { + self + } +} + +impl crate::config::FileOps for AutosaveTestStore { + fn list(&self, dir: &std::path::Path) -> Vec<(String, std::time::SystemTime)> { + crate::config::StdFiles.list(dir) + } + fn write(&self, path: &std::path::Path, data: &[u8]) -> std::io::Result<()> { + if let Some((started, release)) = self.gate.lock().unwrap().take() { + started.send(()).unwrap(); + release.recv().unwrap(); + } + if self.fail_next.swap(false, std::sync::atomic::Ordering::SeqCst) { + return Err(std::io::Error::other("injected autosave write failure")); + } + crate::config::StdFiles.write(path, data) + } + fn remove(&self, path: &std::path::Path) -> std::io::Result<()> { + crate::config::StdFiles.remove(path) + } + fn is_file(&self, path: &std::path::Path) -> bool { + crate::config::StdFiles.is_file(path) + } +} + +fn background_autosave_session(dir: &std::path::Path) -> (Session, Arc) { + let store = Arc::new(AutosaveTestStore { + config: DirConfig::new(dir.join("config")), + fail_next: Default::default(), + gate: Default::default(), + release_on_prefs: Default::default(), + fail_reset_for: Default::default(), + failed_resets: Default::default(), + }); + let mut s = session_with_comp(); + s.config = Some(store.clone()); + s.prefs.auto_save.enabled = true; + s.prefs.auto_save.interval_minutes = 1; + s.prefs.auto_save.location = "custom".into(); + s.prefs.auto_save.folder = dir.join("saves").to_string_lossy().into_owned(); + s.autosave.background = true; + s.begin_recovery(); + (s, store) +} + +#[test] +fn background_autosave_failure_retries_the_same_revision_and_retains_recovery() { + let dir = tmp("autosave-retry"); + let (mut s, store) = background_autosave_session(&dir); + let first = s.autosave_now().unwrap(); + s.autosave_wait().unwrap(); + let committed = s.autosave.saved_revision; + s.execute("layer.newNull", json!({"name": "work to recover"})).unwrap(); + let revision = s.revision; + store.fail_next.store(true, std::sync::atomic::Ordering::SeqCst); + assert!(s.autosave_tick(1000.0).is_none()); + let failed_path = s.autosave_tick(1061.0).unwrap().unwrap(); + assert!(s.autosave_wait().unwrap_err().to_string().contains("injected autosave write failure")); + assert_eq!(s.autosave.saved_revision, committed); + assert_eq!(s.autosave.last_slot, Some(1)); + assert_eq!(s.autosave.last_path.as_deref(), Some(first.as_str())); + let sentinel: autosave::Sentinel = serde_json::from_str(&store.read(autosave::SENTINEL).unwrap()).unwrap(); + assert_eq!(sentinel.autosave.as_deref(), Some(first.as_str())); + assert!(!std::path::Path::new(&failed_path).is_file()); + let retried = s.autosave_tick(1122.0).expect("retry without another edit").unwrap(); + assert_eq!(retried, failed_path, "failed writes do not rotate the slot"); + s.autosave_wait().unwrap(); + assert_eq!(s.revision, revision); + assert_eq!(s.autosave.saved_revision, Some(revision)); + assert_eq!(s.autosave.last_slot, Some(2)); + assert!(std::fs::read_to_string(&retried).unwrap().contains("work to recover")); + let sentinel: autosave::Sentinel = serde_json::from_str(&store.read(autosave::SENTINEL).unwrap()).unwrap(); + assert_eq!(sentinel.autosave.as_deref(), Some(retried.as_str())); + assert!(s.autosave_tick(1183.0).is_none(), "committed revision needs no duplicate write"); + std::fs::remove_dir_all(dir).unwrap(); +} + +#[test] +fn background_autosave_commits_only_the_snapshot_that_finished() { + let dir = tmp("autosave-pending"); + let (mut s, store) = background_autosave_session(&dir); + let (started, started_rx) = std::sync::mpsc::channel(); + let (release, release_rx) = std::sync::mpsc::channel(); + *store.gate.lock().unwrap() = Some((started, release_rx)); + let revision = s.revision; + let path = s.autosave_now().unwrap(); + started_rx.recv_timeout(std::time::Duration::from_secs(5)).unwrap(); + assert!(s.autosave_tick(1000.0).is_none()); + let saved = s.autosave.saved_revision; + let last_path = s.autosave.last_path.clone(); + let sentinel: autosave::Sentinel = serde_json::from_str(&store.read(autosave::SENTINEL).unwrap()).unwrap(); + s.execute("layer.newNull", json!({"name": "edited during save"})).unwrap(); + assert!(s.autosave_tick(1061.0).is_none(), "a pending write must not block the next tick or launch a duplicate"); + release.send(()).unwrap(); + s.autosave_wait().unwrap(); + assert_eq!(saved, None, "an in-flight save is not committed"); + assert_eq!(last_path, None); + assert_eq!(sentinel.autosave, None); + assert_eq!(s.autosave.saved_revision, Some(revision), "completion must not mark newer edits as saved"); + assert_eq!(s.autosave.last_path.as_deref(), Some(path.as_str())); + assert!(!std::fs::read_to_string(&path).unwrap().contains("edited during save")); + let newer = s.autosave_tick(1122.0).unwrap().unwrap(); + s.autosave_wait().unwrap(); + assert_eq!(s.autosave.saved_revision, Some(s.revision)); + assert!(std::fs::read_to_string(newer).unwrap().contains("edited during save")); + std::fs::remove_dir_all(dir).unwrap(); +} + +fn autosave_project_transition_with_failed_reset(command: &str, reopen: bool) { + let dir = tmp("autosave-project-transition"); + let a = dir.join("A.ecproj").to_string_lossy().into_owned(); + let b = if reopen { a.clone() } else { dir.join("B.ecproj").to_string_lossy().into_owned() }; + let (mut s, store) = background_autosave_session(&dir); + s.execute("file.saveAs", json!({"path": a})).unwrap(); + if !reopen { + let mut other = session_with_comp(); + other.execute("layer.newNull", json!({"name": "project B"})).unwrap(); + other.execute("file.saveAs", json!({"path": b})).unwrap(); + } + let b_bytes = std::fs::read(&b).unwrap(); + s.execute("layer.newNull", json!({"name": "unsaved project A"})).unwrap(); + s.update_sentinel(); + let (started, started_rx) = std::sync::mpsc::channel(); + let (release, release_rx) = std::sync::mpsc::channel(); + *store.gate.lock().unwrap() = Some((started, release_rx)); + let snapshot = s.autosave_now().unwrap(); + started_rx.recv_timeout(std::time::Duration::from_secs(5)).unwrap(); + assert!(!s.autosave.pending.as_ref().unwrap().is_finished()); + let previous_record = store.read(autosave::SENTINEL).unwrap(); + *store.fail_reset_for.lock().unwrap() = Some(b.clone()); + *store.release_on_prefs.lock().unwrap() = Some(release); + s.execute(command, json!({"path": b})).unwrap(); + assert_eq!(s.path.as_deref(), Some(b.as_str())); + assert_eq!(store.failed_resets.load(std::sync::atomic::Ordering::SeqCst), 1, "the final recovery reset really failed"); + assert_eq!( + store.read(autosave::SENTINEL).as_deref(), + Some(previous_record.as_str()), + "completion must not publish A's snapshot under the switched project" + ); + assert_eq!(s.autosave.last_path, None); + assert_eq!(s.autosave.saved_revision, None); + if command == "file.open" { + assert_eq!(std::fs::read(&b).unwrap(), b_bytes, "opening B must not alter its file bytes"); + } + let prefs = s.prefs.clone(); + drop(s); // Simulate an unclean exit, leaving the failed-reset sentinel in place. + let mut restart = Session { config: Some(store), prefs, ..Default::default() }; + let recovery = restart.begin_recovery().expect("previous project remains recoverable"); + assert_eq!(recovery.project.as_deref(), Some(a.as_str())); + assert_eq!(recovery.autosave.as_deref(), Some(snapshot.as_str())); + assert!(recovery.dirty); + assert!(std::fs::read_to_string(snapshot).unwrap().contains("unsaved project A")); + std::fs::remove_dir_all(dir).unwrap(); +} + +#[test] +fn opening_another_project_during_autosave_keeps_recovery_consistent_if_the_reset_fails() { + autosave_project_transition_with_failed_reset("file.open", false); +} + +#[test] +fn reopening_the_same_path_during_autosave_keeps_recovery_consistent_if_the_reset_fails() { + autosave_project_transition_with_failed_reset("file.open", true); +} + +#[test] +fn save_as_during_autosave_keeps_recovery_consistent_if_the_reset_fails() { + autosave_project_transition_with_failed_reset("file.saveAs", false); +} + #[test] fn atomic_write_survives_a_crash_mid_write() { let d = tmp("atomic");