diff --git a/apps/wordcraft/src/main.rs b/apps/wordcraft/src/main.rs index db5b2488a..a0ed03249 100644 --- a/apps/wordcraft/src/main.rs +++ b/apps/wordcraft/src/main.rs @@ -77,7 +77,7 @@ impl eframe::App for App { { ctx.send_viewport_cmd(egui::ViewportCommand::CancelClose); self.0.quit_requested = false; - schedule_macos_terminate(&self.0, self.2.as_ref()); + schedule_macos_terminate(&mut self.0, self.2.as_ref()); } #[cfg(not(target_os = "macos"))] if self.0.quit_requested { @@ -94,6 +94,7 @@ impl eframe::App for App { } fn on_exit(&mut self) { print::finish_pending_jobs(); + self.0.session.finish_recovery(); save_prefs(&self.0); if let Some(key_file) = &self.2 { key_file.remove(); @@ -109,7 +110,7 @@ impl eframe::App for App { /// removed first, as `on_exit` does, because a fallback [`std::process::exit`] /// skips `on_exit`. A second close while a quit is already scheduled is ignored. #[cfg(target_os = "macos")] -fn schedule_macos_terminate(app: &WordApp, key_file: Option<&control_server::KeyFile>) { +fn schedule_macos_terminate(app: &mut WordApp, key_file: Option<&control_server::KeyFile>) { use std::sync::atomic::{AtomicBool, Ordering}; if app.printing.busy() { return; @@ -121,6 +122,7 @@ fn schedule_macos_terminate(app: &WordApp, key_file: Option<&control_server::Key log::info!("window close: quitting"); // This path can bypass on_exit. Join the owned workers before termination and cleanup. print::finish_pending_jobs(); + app.session.finish_recovery(); save_prefs(app); if let Some(key_file) = key_file { key_file.remove(); diff --git a/crates/engine/src/recovery.rs b/crates/engine/src/recovery.rs index cf03a45a4..3306a9d61 100644 --- a/crates/engine/src/recovery.rs +++ b/crates/engine/src/recovery.rs @@ -88,6 +88,21 @@ impl Session { } None } + /// Waits for recovery to finish anything already under way, discarding this session's own + /// copy first if it no longer needs one (the document was saved, say). Desktop hosts call + /// this right before the process exits, the same as they wait out pending print jobs: + /// otherwise a save just before close can race the process exiting before a now-redundant + /// recovery copy actually gets removed, so the next launch wrongly offers to recover a + /// document that was in fact saved cleanly. + pub fn finish_recovery(&mut self) { + #[cfg(not(target_arch = "wasm32"))] + if let Some(mut r) = self.recovery.take() { + if let Err(e) = r.finish(self) { + log::warn!("recovery: {e}"); + } + self.recovery = Some(r); + } + } /// Only an explicit protect/save action waits for the worker and removes plaintext copies. /// A failure prevents applying the password; typing never waits. pub(crate) fn clear_recovery_before_password(&mut self) -> Result<(), CmdError> { diff --git a/crates/engine/src/recovery/desktop.rs b/crates/engine/src/recovery/desktop.rs index 676a6ed0c..040a24138 100644 --- a/crates/engine/src/recovery/desktop.rs +++ b/crates/engine/src/recovery/desktop.rs @@ -549,6 +549,34 @@ impl Recovery { self.revision = None; Ok(()) } + /// Waits out any job already in flight (an autosave queued just before shutdown, say), then, + /// if the document turns out to need no recovery copy kept (not dirty, or password-protected + /// — [`tick`](Self::tick)'s own condition), discards it and waits for that too. Desktop hosts + /// call this before the process exits, the same as the print spooler's own pending-job wait + /// (`finish_pending_jobs`): otherwise a save that lands right before close can race the + /// process exiting before the now-redundant recovery copy is actually removed, so the *next* + /// launch wrongly offers to recover a document that was in fact saved cleanly. + pub fn finish(&mut self, s: &Session) -> Result<(), String> { + if self.busy { + let e = self.rx.recv().map_err(|e| e.to_string())?; + self.documents = e.documents; + self.busy = false; + self.ready = true; + e.outcome?; + } + if let Some(id) = self.id.clone() + && (!s.dirty || s.password.is_some()) + { + self.queue(Job::Discard(id))?; + let e = self.rx.recv().map_err(|e| e.to_string())?; + self.busy = false; + self.documents = e.documents; + e.outcome?; + self.id = None; + self.revision = None; + } + Ok(()) + } } #[cfg(test)] @@ -628,6 +656,31 @@ mod tests { settle(s); } + /// #3140: a save that lands right before the app closes can race the process exiting before + /// the now-redundant recovery copy is actually removed, so the next launch wrongly offers to + /// recover a document that was in fact saved cleanly. `finish_recovery` (desktop hosts call it + /// on exit, the same as they wait for pending print jobs) must discard it synchronously + /// instead, with no regular tick in between — simulating closing right after Save, not after + /// however long it'd take the next normal autosave tick to notice and clean up. + #[test] + fn finish_recovery_discards_its_own_copy_for_a_document_saved_right_before_exit() { + let profile = Temp::new(); + let mut s = session(&profile.0); + s.run("document.setText", &json!({"text": "About to be saved"})).unwrap(); + save_now(&mut s); + assert_eq!(s.recovery_documents().len(), 1, "a recovery copy exists for the dirty document"); + // A real Save does this, then the app can close on the very next frame, with no regular + // recovery_tick() in between to notice `!s.dirty` and queue the discard itself. + s.dirty = false; + assert_eq!( + s.recovery_documents().len(), + 1, + "confirms the race: setting dirty=false alone (what Save does) doesn't discard anything by itself" + ); + s.finish_recovery(); + assert!(s.recovery_documents().is_empty(), "finish_recovery should have discarded the now-redundant copy before returning"); + } + #[test] fn recovery_commands_only_address_known_profile_records() { use crate::file_access::{AutomationRoots, confine};