From ecb4be19ba4a43e82c9ca4a74a7d408f0758f391 Mon Sep 17 00:00:00 2001 From: bflatastic Date: Mon, 12 Oct 2026 10:36:26 +0700 Subject: [PATCH] #1244: a crash between a recovery copy's document and its details keeps every image The details name the copy's images, so the new document written before them paired with the old details lost the images placed since the last copy (and a first copy lost all of them). Until the new document is in, the details name the old and the new images. --- crates/engine/src/cmd/recovery.rs | 14 ++++++- crates/engine/src/tests_recovery.rs | 60 +++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 1 deletion(-) diff --git a/crates/engine/src/cmd/recovery.rs b/crates/engine/src/cmd/recovery.rs index f466355c1..ce4ce7a53 100644 --- a/crates/engine/src/cmd/recovery.rs +++ b/crates/engine/src/cmd/recovery.rs @@ -654,8 +654,20 @@ impl RecoveryJob { image_bytes += data.len(); } } + let meta_name = format!("{}{META_EXT}", self.file); + // The details name the images, so a crash between writing the document and its details + // would pair the new document with the old copy's images: when the new document has + // images the old details don't name, they name both until the new document is in. + let old = read_meta(store, &self.file).images; + if meta.images.iter().any(|(k, r)| old.get(k).is_none_or(|o| o.blob != r.blob)) { + let mut both = meta.clone(); + for (k, r) in old { + both.images.entry(k).or_insert(r); + } + store.write(&meta_name, &serde_json::to_vec_pretty(&both).map_err(|e| e.to_string())?)?; + } store.write(&format!("{}{DOC_EXT}", self.file), bytes)?; - store.write(&format!("{}{META_EXT}", self.file), &meta_bytes)?; + store.write(&meta_name, &meta_bytes)?; // Images this copy no longer shows, nor any other. sweep(store, area); Ok(json!({ "file": self.file, "title": self.title, "bytes": bytes.len(), "imageBytes": image_bytes })) diff --git a/crates/engine/src/tests_recovery.rs b/crates/engine/src/tests_recovery.rs index cf46b91dc..37f5ae81c 100644 --- a/crates/engine/src/tests_recovery.rs +++ b/crates/engine/src/tests_recovery.rs @@ -574,3 +574,63 @@ fn copies_from_before_images_were_kept_apart_still_restore() { assert_eq!(images_of(&s), images); assert_eq!(s.doc().unwrap().title(), "Untitled-1 [Recovered]"); } + +/// A store whose app dies right after writing a copy's document entry: later writes fail, so the +/// copy's details stay as the last interval left them. +struct DiesAfterDocument { + inner: Arc, + dead: std::sync::atomic::AtomicBool, +} + +impl RecoveryStore for DiesAfterDocument { + fn list(&self) -> std::result::Result, String> { + self.inner.list() + } + fn read(&self, name: &str) -> std::result::Result, String> { + self.inner.read(name) + } + fn write(&self, name: &str, bytes: &[u8]) -> std::result::Result<(), String> { + if self.dead.load(std::sync::atomic::Ordering::Relaxed) { + return Err("the app is gone".into()); + } + self.inner.write(name, bytes)?; + if name.ends_with(".vectorcraft") { + self.dead.store(true, std::sync::atomic::Ordering::Relaxed); + } + Ok(()) + } + fn remove(&self, name: &str) -> std::result::Result<(), String> { + self.inner.remove(name) + } + fn location(&self) -> String { + self.inner.location() + } + fn lock(&self, area: &str) -> std::result::Result { + self.inner.lock(area) + } + fn now(&self) -> Option { + self.inner.now() + } +} + +/// A crash between writing a copy's document and its details restores the new document with +/// every image, the one placed since the last copy too (and the first copy likewise). +#[test] +fn a_crash_between_a_copys_document_and_its_details_keeps_every_image() { + for first in [true, false] { + let store = Arc::new(MemoryStore::default()); + let mut s = session_in(&store); + if !first { + place_png(&mut s, "red.png", crate::tests_links::RED); + s.execute("file.recovery.save", &json!({})).unwrap(); + } + place_png(&mut s, "blue.png", crate::tests_links::BLUE); + let images = images_of(&s); + s.recovery.set_store(Arc::new(DiesAfterDocument { inner: store.clone(), dead: Default::default() })); + assert!(s.execute("file.recovery.save", &json!({})).is_err(), "the details weren't written"); + drop(s); + let mut s = relaunch(&store); + assert_eq!(restored(s.execute("file.recovery.restore", &json!({})).unwrap()), 1, "first copy: {first}"); + assert_eq!(images_of(&s), images, "first copy: {first}"); + } +}