diff --git a/ROADMAP.md b/ROADMAP.md index b990a42dc..1e1e179d7 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -14,6 +14,12 @@ DesignCraft aims at full Adobe InDesign parity — and to be better: faster, ope - **2026-11-06** · Automation roots refuse `COM¹` to `LPT³` and a device name before a colon (`COM1:`, `NUL:x`). A batch-export layout of that name is written with a leading `_` on every platform. No readiness percentage change. - **2026-10-30** · Document Fonts lookup treats a bare document filename's empty parent as the current directory (#2014), preserving case-insensitive discovery and automation read-root checks. No readiness percentage change. +- **2026-10-27** · Automatic recovery writes off the interface thread, straight to disk: on a + 190-page catalogue with 4.2 GB of placed files an autosave was 3.6 GB deflated in RAM on the + interface thread, which stopped the window answering for 30-100 s after an edit. It is now a + background write (`file.recovery.save {background}`, `file.recovery.progress`), the caller + blocked for 109 ms instead, and Save, Close and Revert cancel a snapshot still being written. + No readiness percentage change. - **2026-10-28** · Inline table rows leave room for column and spanning footnotes, continuing in the next column or frame when needed (#1901). Cell-local footnotes contribute to row height and sit inside the cell rather than outside its measuring frame. Vertical justification keeps diff --git a/crates/engine/Cargo.toml b/crates/engine/Cargo.toml index c1c0f1d33..6b6a43a87 100644 --- a/crates/engine/Cargo.toml +++ b/crates/engine/Cargo.toml @@ -8,6 +8,10 @@ description = "DesignCraft engine: session, documents, history, selection, comma [lints] workspace = true +[features] +# Recovery-writer handshakes for other crates' tests (`designcraft_engine::recovery::testing`). +testing = [] + [dependencies] craft-print = { workspace = true } craft-docpath = { workspace = true } diff --git a/crates/engine/src/cmd/file/registry.rs b/crates/engine/src/cmd/file/registry.rs index c1f522956..df8709f10 100644 --- a/crates/engine/src/cmd/file/registry.rs +++ b/crates/engine/src/cmd/file/registry.rs @@ -67,10 +67,24 @@ pub fn specs() -> Vec { let dir = recovery_dir(s, p, FolderAccess::Read)?; Ok(Value::Array(crate::recovery::list(&dir).into_iter().map(|(uid, mut m)| { m["uid"] = json!(uid); m }).collect())) }), - cmd!(noundo "file.recovery.save", "Save Recovery Data", [], None, "{dir?} — write every unsaved document to the recovery folder (the app does this on a timer)", always, |s, p| { + cmd!(noundo "file.recovery.save", "Save Recovery Data", [], None, + "{dir?, background?} — write every unsaved document to the recovery folder (the app does this on a timer; `background` queues the writes and returns at once) → {saved} or {queued}", + always, |s, p| { + let background = match p.get("background") { + None | Some(Value::Null) => false, + Some(v) => v.as_bool().ok_or_else(|| bad("file.recovery.save", "`background` must be true or false"))?, + }; let dir = recovery_dir(s, p, FolderAccess::Write)?; + if background { + return Ok(json!({"queued": crate::recovery::start_tracked(s, &dir)?})); + } Ok(json!({"saved": crate::recovery::save_tracked(s, &dir)?})) }), + cmd!(query "file.recovery.progress", "Recovery Progress", [], None, + "{} → {written, busy, errors} recovery data queued in the background has been written", always, |s, _| { + let done = crate::recovery::finished(s); + Ok(json!({"written": done.written, "busy": done.busy, "errors": done.errors})) + }), cmd!(noundo "file.recovery.open", "Recover Documents", [], None, "{dir?} — reopen documents left in the recovery folder (unsaved)", always, |s, p| { // Reopening removes the entries it read. let dir = recovery_dir(s, p, FolderAccess::ReadWrite)?; diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 14a88bb68..ac899412e 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -414,6 +414,10 @@ pub struct Session { pub clipboard_origin: f64, /// Crash-recovery folder (the app sets it; saving or closing a document removes its entry). pub recovery_dir: Option, + /// This session's number for the background recovery writer, which returns each result to + /// the session that queued it. + #[cfg_attr(target_arch = "wasm32", allow(dead_code))] + pub(crate) recovery_owner: recovery::Owner, /// Copied text with its formatting (a frameless story slice), its plain text, and what pasting /// it needs from its document: a copy holding only the stories its anchored text frames show /// (and theirs) and the graphics the text and those stories place. @@ -479,6 +483,7 @@ impl Session { clipboard: None, clipboard_origin: 0.0, recovery_dir: None, + recovery_owner: recovery::Owner::new(), text_clipboard: None, link_checks: Default::default(), ui_requests: vec![], diff --git a/crates/engine/src/recovery.rs b/crates/engine/src/recovery.rs index 25256cbc5..16b1e0ee2 100644 --- a/crates/engine/src/recovery.rs +++ b/crates/engine/src/recovery.rs @@ -272,30 +272,39 @@ fn relock(dir: &Path, created: bool) -> Result { } /// Entries this process wrote or recovered in folders that are not its run's own: (canonical -/// folder, uid). Document uids start at 1 in every process, so a shared folder's entry under -/// one of this run's uids may be another run's. -static WRITTEN: std::sync::Mutex> = std::sync::Mutex::new(std::collections::BTreeSet::new()); +/// folder, uid), with the session that wrote or recovered each ([`Session::recovery_owner`]). +/// Document uids start at 1 in every process, so a shared folder's entry under one of this run's +/// uids may be another run's; and a recovered entry keeps the uid it was written under, which a +/// document of another session can have. +static WRITTEN: std::sync::Mutex> = std::sync::Mutex::new(std::collections::BTreeMap::new()); -/// What this run may do in a recovery folder. +/// What a session may do in a recovery folder. struct Access { /// This run's own folder (`run_dir`): every entry in it is this run's. own: bool, key: PathBuf, + /// The session asking ([`Session::recovery_owner`]). + owner: u64, } impl Access { fn owns(&self, uid: u64) -> bool { - self.own || WRITTEN.lock().unwrap_or_else(std::sync::PoisonError::into_inner).contains(&(self.key.clone(), uid)) + self.own || WRITTEN.lock().unwrap_or_else(std::sync::PoisonError::into_inner).get(&(self.key.clone(), uid)) == Some(&self.owner) } fn record(&self, uid: u64) { if !self.own { - WRITTEN.lock().unwrap_or_else(std::sync::PoisonError::into_inner).insert((self.key.clone(), uid)); + WRITTEN.lock().unwrap_or_else(std::sync::PoisonError::into_inner).insert((self.key.clone(), uid), self.owner); } } + /// Drop the record of an entry this session wrote or recovered; another session's stays. fn forget(&self, uid: u64) { - WRITTEN.lock().unwrap_or_else(std::sync::PoisonError::into_inner).remove(&(self.key.clone(), uid)); + let mut written = WRITTEN.lock().unwrap_or_else(std::sync::PoisonError::into_inner); + let key = (self.key.clone(), uid); + if written.get(&key) == Some(&self.owner) { + written.remove(&key); + } } /// Refuse an entry of another run before anything is written: this run never replaces it. @@ -319,9 +328,9 @@ impl Access { } } -/// Check what this run may do in `dir`. A folder that is not this run's own may be the folder -/// of another running app, whose lock that app holds: this run leaves it alone. -fn access(dir: &Path, created: bool) -> Result { +/// Check what session `owner` may do in `dir`. A folder that is not this run's own may be the +/// folder of another running app, whose lock that app holds: this run leaves it alone. +fn access(dir: &Path, created: bool, owner: u64) -> Result { #[cfg(not(target_arch = "wasm32"))] let own = relock(dir, created)?; #[cfg(not(target_arch = "wasm32"))] @@ -333,7 +342,7 @@ fn access(dir: &Path, created: bool) -> Result { let _ = created; false }; - Ok(Access { own, key: directory_key(dir)? }) + Ok(Access { own, key: directory_key(dir)?, owner }) } #[cfg(not(target_arch = "wasm32"))] @@ -485,13 +494,13 @@ fn read_entry_file(path: &Path) -> std::io::Result> { } /// Remove a document's recovery entry (it was saved or closed). In a folder that is not this -/// run's own, only an entry this run wrote or recovered is removed. -pub fn discard(dir: &Path, uid: u64) { +/// run's own, only an entry session `owner` wrote or recovered is removed. +pub fn discard(dir: &Path, uid: u64, owner: u64) { #[cfg(not(target_arch = "wasm32"))] if validate_recovery_dir(dir).is_err() { return; } - let Ok(access) = access(dir, false) else { return }; + let Ok(access) = access(dir, false, owner) else { return }; if !access.owns(uid) { return; } @@ -504,6 +513,7 @@ pub fn discard(dir: &Path, uid: u64) { /// Consume this document's checkpoint provenance after Save/Close/Revert. A default-directory /// uid may also be another live recovered document's retained source after a failed republish. pub(crate) fn discard_document(s: &mut Session, index: usize, default_dir: Option<&Path>) { + let owner = s.recovery_owner.0; let Some(doc) = s.docs.get_mut(index) else { return }; let uid = doc.uid; // A completed Save/Close/Revert consumes the provenance, even when removal fails. A stale @@ -514,13 +524,26 @@ pub(crate) fn discard_document(s: &mut Session, index: usize, default_dir: Optio { entries.push((dir.to_path_buf(), uid)); } + // Background writes still queued or running for it, in any folder (one named by + // `file.recovery.save {dir}` is not in its entries until the write's result is taken): none + // may publish after this point, and what one already published is removed below. + #[cfg(not(target_arch = "wasm32"))] + for dir in worker::cancel_document(uid, owner) { + if !entries.iter().any(|(entry_dir, entry_uid)| *entry_uid == uid && same_directory(entry_dir, &dir)) { + entries.push((dir, uid)); + } + } for (dir, uid) in entries { // Session ownership governs deletion as well as writes: never consume another live // document's only source merely because its old uid matches this unrelated document. if !s.docs.iter().enumerate().any(|(other_index, owner)| { other_index != index && owner.recovery_entries.iter().any(|(entry_dir, entry_uid)| *entry_uid == uid && same_directory(entry_dir, &dir)) }) { - discard(&dir, uid); + // A write of it this session still runs must not publish its snapshot after this + // point; another session's write of the same uid is its own. + #[cfg(not(target_arch = "wasm32"))] + worker::cancel_owned(&dir, uid, owner); + discard(&dir, uid, owner); } } } @@ -551,7 +574,7 @@ fn prepare_save(s: &Session, dir: &Path) -> Result { // coincides with an unrelated live document in a manually configured recovery directory. protect_retained_sources(s, dir)?; let created = secure_recovery_dir(dir)?; - let access = access(dir, created)?; + let access = access(dir, created, s.recovery_owner.0)?; // New Document's preview is never written. let preview = s.new_document_preview(); for document in s.docs.iter().filter(|d| d.is_dirty() && preview != Some(d.uid)) { @@ -560,18 +583,54 @@ fn prepare_save(s: &Session, dir: &Path) -> Result { Ok(access) } +/// Refuse a write now while another session's background write of one of the dirty documents' +/// uids to `dir` is queued or running. A write now cancels only this session's writes. +#[cfg(not(target_arch = "wasm32"))] +fn check_no_other_write(s: &Session, dir: &Path) -> Result<()> { + let preview = s.new_document_preview(); + for document in s.docs.iter().filter(|d| d.is_dirty() && preview != Some(d.uid)) { + if worker::queued_by_other(dir, document.uid, s.recovery_owner.0) { + return Err(still_being_written(dir)); + } + } + Ok(()) +} + +#[cfg(not(target_arch = "wasm32"))] +fn still_being_written(dir: &Path) -> EngineError { + EngineError::Other(format!( + "{}: another document's recovery data is still being written here; try again or choose another recovery folder", + dir.display() + )) +} + /// Write recovery entries for dirty documents and drop clean checkpoints. Returns how many /// were written. Command callers use the tracked variant without configuring automatic recovery. pub fn save(s: &Session, dir: &Path) -> Result { let access = prepare_save(s, dir)?; + #[cfg(not(target_arch = "wasm32"))] + check_no_other_write(s, dir)?; let mut n = 0; for d in &s.docs { // New Document's preview is not a document until Create makes it one. if !d.is_dirty() || s.new_document_preview() == Some(d.uid) { - discard(dir, d.uid); + // A background write this session queued of an older state must not publish after + // this discard, and what one already wrote in another folder is removed too. + #[cfg(not(target_arch = "wasm32"))] + for other in worker::cancel_document(d.uid, s.recovery_owner.0) { + discard(&other, d.uid, s.recovery_owner.0); + } + discard(dir, d.uid, s.recovery_owner.0); continue; } - write_entry(&access, dir, d)?; + // The caller asked for this write now: nothing has cancelled it. + // A background write of an older state this session queued must not land over this one, + // and no other session's write of the uid is queued until it is done. + #[cfg(not(target_arch = "wasm32"))] + let _now = worker::write_now(dir, d.uid, s.recovery_owner.0).ok_or_else(|| still_being_written(dir))?; + if let Some(e) = write_snapshot(&Snapshot::of(&access, dir, d), || false).error { + return Err(e); + } n += 1; } Ok(n) @@ -580,15 +639,10 @@ pub fn save(s: &Session, dir: &Path) -> Result { /// Command-path checkpointing also records explicitly selected directories, without configuring /// automatic recovery. The immutable public helper remains available for fixtures and callers. pub(crate) fn save_tracked(s: &mut Session, dir: &Path) -> Result { - // Reject the complete batch before writing rather than silently losing old ownership records. - for document in &s.docs { - if document.is_dirty() && !has_checkpoint(document, dir) && document.recovery_entries.len() >= MAX_RECOVERY_ENTRIES { - return Err(EngineError::Other( - "too many recovery checkpoints; save or close the document before choosing another recovery folder".into(), - )); - } - } + reject_overfull_checkpoints(s, dir)?; let access = prepare_save(s, dir)?; + #[cfg(not(target_arch = "wasm32"))] + check_no_other_write(s, dir)?; let mut n = 0; let preview = s.new_document_preview(); for index in 0..s.docs.len() { @@ -600,34 +654,841 @@ pub(crate) fn save_tracked(s: &mut Session, dir: &Path) -> Result { discard_document(s, index, Some(dir)); continue; } - let Some(document) = s.docs.get_mut(index) else { continue }; - let (doc, meta) = files(dir, document.uid); - let bytes = designcraft_format::save(document.preview_stash.as_deref().unwrap_or(document.doc.as_ref())) - .map_err(|e| EngineError::Other(e.to_string()))?; - // An entry this run did not write is never replaced, even one made since it checked. - let replace = access.owns(document.uid); - publish_recovery_file(&doc, &bytes, replace)?; - access.record(document.uid); - // The primary is useful even if optional metadata fails next. Track it immediately. - if !has_checkpoint(document, dir) { + let Some(document) = s.docs.get(index) else { continue }; + // A background write of an older state this session queued must not land over this one, + // and no other session's write of the uid is queued until it is done. + #[cfg(not(target_arch = "wasm32"))] + let _now = worker::write_now(dir, document.uid, s.recovery_owner.0).ok_or_else(|| still_being_written(dir))?; + let entry = Snapshot::of(&access, dir, document); + let written = write_snapshot(&entry, || false); + // The primary is the recovery data; its metadata is optional. Track the primary before + // returning a metadata error, so Save and Close remove the snapshot this run wrote. + if written.published + && let Some(document) = s.docs.get_mut(index) + && !has_checkpoint(document, dir) + { document.recovery_entries.push((dir.to_path_buf(), document.uid)); } - let metadata = json!({"path": document.path, "title": document.title(), "saved": designcraft_doc::vars::now()}); - publish_recovery_file(&meta, metadata.to_string().as_bytes(), replace)?; + if let Some(e) = written.error { + return Err(e); + } n += 1; } Ok(n) } -/// Publish a document's recovery entry before its optional metadata. -fn write_entry(access: &Access, dir: &Path, d: &DocState) -> Result<()> { - let (doc, meta) = files(dir, d.uid); - let bytes = designcraft_format::save(d.preview_stash.as_deref().unwrap_or(d.doc.as_ref())).map_err(|e| EngineError::Other(e.to_string()))?; - let replace = access.owns(d.uid); - publish_recovery_file(&doc, &bytes, replace)?; - access.record(d.uid); - let metadata = json!({"path": d.path, "title": d.title(), "saved": designcraft_doc::vars::now()}); - publish_recovery_file(&meta, metadata.to_string().as_bytes(), replace) +/// Reject a complete batch before writing rather than silently losing old ownership records. +fn reject_overfull_checkpoints(s: &Session, dir: &Path) -> Result<()> { + for document in &s.docs { + if !document.is_dirty() || has_checkpoint(document, dir) { + continue; + } + // Background writes whose results are not taken yet hold checkpoints the entries do not. + #[cfg(not(target_arch = "wasm32"))] + let pending = worker::pending_folders(document.uid, s.recovery_owner.0) + .into_iter() + .map(|pending| directory_key(&pending).unwrap_or(pending)) + .collect::>() + .iter() + .filter(|pending| !same_directory(pending, dir) && !has_checkpoint(document, pending)) + .count(); + #[cfg(target_arch = "wasm32")] + let pending = 0; + if document.recovery_entries.len() + pending >= MAX_RECOVERY_ENTRIES { + return Err(EngineError::Other( + "too many recovery checkpoints; save or close the document before choosing another recovery folder".into(), + )); + } + } + Ok(()) +} + +/// A document's recovery write, copied so a background thread can write it while the interface +/// stays responsive: no deflate, no copy of the placed files, happens on the interface thread. +struct Snapshot { + dir: PathBuf, + uid: u64, + /// The document as it was when the write started (data-merge previews save the unfilled one). + doc: std::sync::Arc, + path: Option, + title: String, + /// Whether this run may replace an entry already in the folder. + own: bool, + /// The folder is not this run's own: the entry, once written, is recorded as the session's. + shared: bool, + /// The folder's canonical key, for recording that a shared entry is this session's now. + key: PathBuf, + /// The session that queued a background write ([`Session::recovery_owner`]), which takes its + /// result. + #[cfg_attr(target_arch = "wasm32", allow(dead_code))] + owner: u64, +} + +/// A number for each session, so background results go back to the session that queued them. +/// Dropping it retires the number: results still queued for it are then dropped. +pub(crate) struct Owner(pub(crate) u64); + +/// The numbers of the sessions that exist. +static LIVE_OWNERS: std::sync::Mutex> = std::sync::Mutex::new(std::collections::BTreeSet::new()); + +impl Owner { + pub(crate) fn new() -> Self { + static NEXT_OWNER: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(1); + let id = NEXT_OWNER.fetch_add(1, std::sync::atomic::Ordering::Relaxed); + LIVE_OWNERS.lock().unwrap_or_else(std::sync::PoisonError::into_inner).insert(id); + Owner(id) + } +} + +impl Drop for Owner { + fn drop(&mut self) { + LIVE_OWNERS.lock().unwrap_or_else(std::sync::PoisonError::into_inner).remove(&self.0); + } +} + +impl Snapshot { + fn of(access: &Access, dir: &Path, d: &DocState) -> Self { + Snapshot { + dir: dir.to_path_buf(), + uid: d.uid, + doc: d.preview_stash.clone().unwrap_or_else(|| d.doc.clone()), + path: d.path.clone(), + title: d.title(), + // An entry this run did not write is never replaced, even one made since it checked. + own: access.owns(d.uid), + shared: !access.own, + key: access.key.clone(), + owner: access.owner, + } + } +} + +/// What one recovery write left in the folder: whether the primary document was published, and +/// the error that stopped it or the optional metadata written after it. The primary is the +/// recovery data (see the module docs: metadata may be missing or stale), so a metadata failure +/// must not lose the checkpoint that lets Save and Close remove what this run wrote. +struct Written { + published: bool, + error: Option, +} + +/// A writer that fails once `cancelled` says so, checked before each write of at most +/// [`STOP_CHECK_BYTES`]. +struct StopWhen<'a, W, C> { + out: &'a mut W, + cancelled: &'a C, +} + +/// The most [`StopWhen`] writes between two checks for a cancel. +const STOP_CHECK_BYTES: usize = 1 << 20; + +impl bool> std::io::Write for StopWhen<'_, W, C> { + fn write(&mut self, buf: &[u8]) -> std::io::Result { + if (self.cancelled)() { + return Err(std::io::Error::other("recovery write cancelled")); + } + // A large placed file is written in pieces, so a cancel does not wait for all of it. + self.out.write(&buf[..buf.len().min(STOP_CHECK_BYTES)]) + } + + fn flush(&mut self) -> std::io::Result<()> { + self.out.flush() + } +} + +impl std::io::Seek for StopWhen<'_, W, C> { + fn seek(&mut self, pos: std::io::SeekFrom) -> std::io::Result { + self.out.seek(pos) + } +} + +/// Publish a document's recovery entry before its optional metadata. `cancelled` says whether the +/// document was saved, reverted or closed since the write started: its snapshot is then dropped +/// instead of published, so a crash cannot offer back a copy older than the saved file. +fn write_snapshot(entry: &Snapshot, cancelled: impl Fn() -> bool) -> Written { + #[cfg(any(test, feature = "testing"))] + testing::gate(&entry.dir); + let (doc, meta) = files(&entry.dir, entry.uid); + // Set at the rename: an error after it (syncing the folder) leaves the entry on disk. + let landed = std::cell::Cell::new(false); + let published = publish_recovery_written( + &doc, + entry.own, + // A cancel stops the write at its next chunk instead of after the whole package. + |out| { + designcraft_format::write(&mut StopWhen { out, cancelled: &cancelled }, &entry.doc, None).map_err(|e| EngineError::Other(e.to_string())) + }, + &cancelled, + // Written: in a folder that is not this run's own, the entry is now this session's to + // replace and remove (Save and Close must clean up the snapshot they wrote). Recorded with + // the rename, so a cleanup after it can remove the entry, and recorded even when the + // entry was this session's as the write was queued: a cleanup since may have dropped that + // record. + || { + landed.set(true); + if entry.shared { + WRITTEN.lock().unwrap_or_else(std::sync::PoisonError::into_inner).insert((entry.key.clone(), entry.uid), entry.owner); + } + }, + ); + match published { + Ok(true) => {} + // Cancelled while writing: no snapshot and no metadata for it. A write stopped part way + // reports an error, which is the cancel's and not a failure. + Ok(false) => return Written { published: false, error: None }, + Err(_) if cancelled() => return Written { published: false, error: None }, + // Renamed into place, then the folder sync failed: the entry is there to track and clean up. + Err(e) if landed.get() => return Written { published: true, error: Some(e) }, + Err(e) => return Written { published: false, error: Some(e) }, + } + let metadata = json!({"path": entry.path, "title": entry.title, "saved": designcraft_doc::vars::now()}); + // A cancel after the primary landed removes it; the metadata must not be left without it. + let bytes = metadata.to_string(); + let error = publish_recovery_written( + &meta, + entry.own, + |out| std::io::Write::write_all(out, bytes.as_bytes()).map_err(|e| EngineError::Other(format!("{}: {e}", meta.display()))), + &cancelled, + || {}, + ) + .err(); + Written { published: true, error } +} + +/// Write recovery entries for dirty documents on a background thread and drop clean checkpoints +/// now. The interface thread only snapshots what to write: `designcraft_format::write` deflates +/// and copies placed files, which takes minutes on a catalogue with gigabytes of links. Returns +/// how many documents were queued. +pub(crate) fn start_tracked(s: &mut Session, dir: &Path) -> Result { + reject_overfull_checkpoints(s, dir)?; + let access = prepare_save(s, dir)?; + // Nothing runs on the web's single thread: it would freeze the page just the same. + #[cfg(target_arch = "wasm32")] + { + let _ = access; + return save_tracked(s, dir); + } + #[cfg(not(target_arch = "wasm32"))] + { + let mut queued = 0; + let preview = s.new_document_preview(); + for index in 0..s.docs.len() { + if s.docs.get(index).is_some_and(|document| preview == Some(document.uid)) { + continue; + } + if s.docs.get(index).is_some_and(|document| !document.is_dirty()) { + discard_document(s, index, Some(dir)); + continue; + } + let Some(document) = s.docs.get(index) else { continue }; + let mut entry = Snapshot::of(&access, dir, document); + entry.owner = s.recovery_owner.0; + // A write still running for this document already holds a snapshot of it; the next + // timer tick catches up with the edits made since. + match worker::submit(entry) { + Some(worker::Submit::Queued) => queued += 1, + Some(worker::Submit::AlreadyQueued) => {} + Some(worker::Submit::Conflict) => { + return Err(crate::EngineError::Other(format!( + "recovery data for document {} is already being written by another session", + document.uid + ))); + } + None => { + // No thread: write this one here rather than lose the recovery data. + let written = write_snapshot(&Snapshot::of(&access, dir, document), || false); + if written.published { + record_checkpoint(s, dir.to_path_buf(), document.uid); + } + if let Some(e) = written.error { + return Err(e); + } + queued += 1; + } + } + } + Ok(queued) + } +} + +/// Apply one finished background write: record the checkpoint of a document still open and +/// unsaved, or drop the snapshot of one saved, reverted or closed while its write ran. Returns +/// whether a checkpoint was recorded. +#[cfg(not(target_arch = "wasm32"))] +fn record_checkpoint(s: &mut Session, dir: PathBuf, uid: u64) -> bool { + let owner = s.recovery_owner.0; + match s.docs.iter_mut().find(|doc| doc.uid == uid) { + Some(doc) if doc.is_dirty() => { + if !has_checkpoint(doc, &dir) { + doc.recovery_entries.push((dir, uid)); + } + true + } + // Saved, reverted or closed while being written: its snapshot must not come back after a + // crash, and the timer writes only when a document is dirty, so nothing else would + // remove it. + Some(_) | None => { + discard(&dir, uid, owner); + false + } + } +} + +/// Apply finished background writes. Returns what was written and what failed. +pub(crate) struct Finished { + pub written: usize, + pub errors: Vec, + pub busy: bool, +} + +#[cfg(not(target_arch = "wasm32"))] +pub(crate) fn finished(s: &mut Session) -> Finished { + let mut out = Finished { written: 0, errors: Vec::new(), busy: false }; + let Some(w) = worker::started() else { return out }; + // Before draining: the worker sends a result before it clears the document as running, so a writer idle now has every result in the channel already. + let busy = w.busy(); + // Results go to the session that queued them: another session has none of those documents, + // and would remove their snapshots as if they had been closed. + let mine: Vec = { + let done = w.done.lock().unwrap_or_else(std::sync::PoisonError::into_inner); + let mut others = w.undelivered.lock().unwrap_or_else(std::sync::PoisonError::into_inner); + let mut all = std::mem::take(&mut *others); + all.extend(done.try_iter()); + let (mine, rest): (Vec<_>, Vec<_>) = all.into_iter().partition(|d| d.owner == s.recovery_owner.0); + // A session that is gone takes nothing: its results are dropped rather than kept for ever. + let live = LIVE_OWNERS.lock().unwrap_or_else(std::sync::PoisonError::into_inner); + *others = rest.into_iter().filter(|d| live.contains(&d.owner)).collect(); + mine + }; + for d in mine { + // A primary that landed belongs to this run even if its optional metadata failed: record + // it, so Save and Close remove the snapshot instead of leaving it in the folder for ever. + if d.published && record_checkpoint(s, d.dir, d.uid) { + out.written += 1; + } + if let Some(e) = d.error { + out.errors.push(e); + } + } + out.busy = busy; + out +} + +#[cfg(target_arch = "wasm32")] +pub(crate) fn finished(_s: &mut Session) -> Finished { + // Nothing is written in the background on the web. + Finished { written: 0, errors: Vec::new(), busy: false } +} + +/// Automatic recovery's writer: one thread with its own queue, so an unsaved document with +/// gigabytes of placed files is written without stopping the interface. +#[cfg(not(target_arch = "wasm32"))] +mod worker { + use std::collections::{BTreeMap, BTreeSet}; + use std::sync::mpsc::{Receiver, Sender}; + use std::sync::{Condvar, Mutex, OnceLock}; + + use super::{Path, PathBuf, Result, Snapshot, write_snapshot}; + + /// One document written, for the interface thread to record. + pub(super) struct Done { + pub(super) dir: PathBuf, + pub(super) uid: u64, + pub(super) error: Option, + /// The primary document reached the folder (its optional metadata may have failed): the + /// checkpoint must still be recorded, or Save and Close could not remove it. + pub(super) published: bool, + /// The session that queued the write. + pub(super) owner: u64, + } + + pub(super) struct Writer { + jobs: Sender, + pub(super) done: Mutex>, + /// Results taken from `done` by a session that did not queue them, kept for their own. + pub(super) undelivered: Mutex>, + /// Documents queued or being written, with the session that queued each: a second write + /// of one still holds nothing newer. + running: Mutex>, + /// A document saved, reverted or closed while its write was queued or running: the + /// snapshot it is producing must not be published. + cancelled: Mutex>, + /// Documents a session is writing now on its own thread, with that session: no background + /// write of one is queued until it is done (see [`write_now`]). + writing_now: Mutex>, + /// Notified each time a write finishes and leaves `running`. + idle: Condvar, + } + + /// A write in flight: the session that queued it, and its folder as that session named it. + struct Queued { + owner: u64, + dir: PathBuf, + /// A newer write of the same document and folder, queued after this one was cancelled: + /// it is written as soon as this one stops. + next: Option, + } + + static WRITER: OnceLock> = OnceLock::new(); + + /// A write's key: the folder's canonical path, so two spellings of one folder are one write. + fn key(dir: &Path, uid: u64) -> (PathBuf, u64) { + (super::directory_key(dir).unwrap_or_else(|_| dir.to_path_buf()), uid) + } + + impl Writer { + pub(super) fn busy(&self) -> bool { + self.running.lock().is_ok_and(|running| !running.is_empty()) + } + + /// `key` is the job's ([`Snapshot::key`], uid). Returns the write queued to follow it. + fn finish(&self, key: &(PathBuf, u64)) -> Option { + let next = match self.running.lock() { + Ok(mut running) => { + let next = running.get_mut(key).and_then(|q| q.next.take()); + match (&next, running.get_mut(key)) { + (Some(n), Some(q)) => { + q.owner = n.owner; + q.dir = n.dir.clone(); + } + _ => { + running.remove(key); + } + } + // The mark was for the write that stopped, not the one that follows it. + if let Ok(mut cancelled) = self.cancelled.lock() { + cancelled.remove(key); + } + next + } + Err(_) => None, + }; + self.idle.notify_all(); + next + } + + /// Block until no write is queued or running. + #[cfg(any(test, feature = "testing"))] + pub(super) fn wait_until_idle(&self) { + let mut running = self.running.lock().unwrap_or_else(std::sync::PoisonError::into_inner); + while !running.is_empty() { + running = self.idle.wait(running).unwrap_or_else(std::sync::PoisonError::into_inner); + } + } + + /// Do not publish the snapshot being written for `dir`/`uid`: the document was saved, + /// reverted or closed. Only a write that is queued or running is marked, so the set stays + /// as small as the work in flight. With `owner`, only writes that session queued are + /// marked: another session's write of the same uid stays. + fn cancel(&self, dir: &Path, uid: u64, owner: Option) { + let canonical = key(dir, uid); + let mine = |queued_by: u64| owner.is_none_or(|owner| owner == queued_by); + // A rename under way finishes first, so the caller's cleanup comes after it; a later + // one sees the mark. + let _publishing = super::PUBLISH.lock().unwrap_or_else(std::sync::PoisonError::into_inner); + // The folder's canonical path, or its name as given: a link in the path may lead + // elsewhere now than when the write was queued. + let Ok(mut running) = self.running.lock() else { return }; + let mut keys = Vec::new(); + for (k, q) in running.iter_mut().filter(|(k, q)| **k == canonical || (k.1 == uid && q.dir == dir)) { + // A write queued to follow it is older than what the caller has now, too. + if q.next.as_ref().is_some_and(|n| mine(n.owner)) { + q.next = None; + } + if mine(q.owner) { + keys.push(k.clone()); + } + } + if let Ok(mut cancelled) = self.cancelled.lock() { + cancelled.extend(keys); + } + } + + /// [`Writer::cancel`] for every folder a write of `uid` queued by session `owner` is queued + /// or running for. Returns those folders, and the folders of its finished writes whose + /// results nobody has taken. + fn cancel_document(&self, uid: u64, owner: u64) -> Vec { + // Held while results are read: the writer checks for a cancel and sends its result + // under it, so a result sent after this sees the cancel. + let _publishing = super::PUBLISH.lock().unwrap_or_else(std::sync::PoisonError::into_inner); + // The canonical keys to mark, and the folders as the session named them, which is how + // the cleanup finds this run's folder. + let mut dirs = Vec::new(); + if let Ok(mut running) = self.running.lock() { + let mut keys = Vec::new(); + for (key, q) in running.iter_mut().filter(|((_, u), _)| *u == uid) { + // A write another session queued to follow stays queued. + if q.next.as_ref().is_some_and(|n| n.owner == owner) { + q.next = None; + } + if q.owner == owner { + keys.push(key.clone()); + dirs.push(q.dir.clone()); + } + } + if let Ok(mut cancelled) = self.cancelled.lock() { + cancelled.extend(keys); + } + } + // Results stay queued for their session; only their folders are read here. + let done = self.done.lock().unwrap_or_else(std::sync::PoisonError::into_inner); + let mut undelivered = self.undelivered.lock().unwrap_or_else(std::sync::PoisonError::into_inner); + undelivered.extend(done.try_iter()); + for d in undelivered.iter_mut().filter(|d| d.uid == uid && d.owner == owner && d.published) { + dirs.push(d.dir.clone()); + // The caller removes that snapshot: its result must not record it as a checkpoint + // once the document has changed again. + d.published = false; + } + dirs + } + + /// The folders of writes of `uid` queued by session `owner` that are in flight, or finished + /// and published with their results not taken yet: checkpoints its entries do not hold yet. + fn pending_folders(&self, uid: u64, owner: u64) -> Vec { + // A cancelled write publishes nothing; a write queued to follow it does. + let mut dirs: Vec = self + .running + .lock() + .map(|running| { + running + .iter() + .filter(|(key, q)| { + key.1 == uid && ((q.owner == owner && !self.cancelled(key)) || q.next.as_ref().is_some_and(|n| n.owner == owner)) + }) + .map(|((dir, _), _)| dir.clone()) + .collect() + }) + .unwrap_or_default(); + let done = self.done.lock().unwrap_or_else(std::sync::PoisonError::into_inner); + let mut undelivered = self.undelivered.lock().unwrap_or_else(std::sync::PoisonError::into_inner); + undelivered.extend(done.try_iter()); + dirs.extend(undelivered.iter().filter(|d| d.uid == uid && d.owner == owner && d.published).map(|d| d.dir.clone())); + dirs + } + + /// Whether a session other than `owner` has a write of `dir`/`uid` queued or running that + /// is not cancelled, or queued to follow one. + fn queued_by_other(&self, dir: &Path, uid: u64, owner: u64) -> bool { + let running = self.running.lock().unwrap_or_else(std::sync::PoisonError::into_inner); + self.other_in_flight(&running, dir, uid, owner) + } + + /// [`Writer::queued_by_other`] with `running` held, counting writes now as well. + fn other_in_flight(&self, running: &BTreeMap<(PathBuf, u64), Queued>, dir: &Path, uid: u64, owner: u64) -> bool { + let canonical = key(dir, uid); + running + .iter() + .filter(|(k, q)| **k == canonical || (k.1 == uid && q.dir == dir)) + .any(|(k, q)| (q.owner != owner && !self.cancelled(k)) || q.next.as_ref().is_some_and(|n| n.owner != owner)) + || self.writing_now.lock().is_ok_and(|now| now.get(&canonical).is_some_and(|by| *by != owner)) + } + + /// Start a write now of `dir`/`uid` by session `owner`. Returns `None` while another + /// session's write of it is in flight. Otherwise cancels this session's writes of it (as + /// [`Writer::cancel`]) and records the write now under the returned key, all under one + /// hold of `running`. + fn write_now(&self, dir: &Path, uid: u64, owner: u64) -> Option<(PathBuf, u64)> { + let canonical = key(dir, uid); + let _publishing = super::PUBLISH.lock().unwrap_or_else(std::sync::PoisonError::into_inner); + let mut running = self.running.lock().unwrap_or_else(std::sync::PoisonError::into_inner); + if self.other_in_flight(&running, dir, uid, owner) { + return None; + } + let mut keys = Vec::new(); + for (k, q) in running.iter_mut().filter(|(k, q)| **k == canonical || (k.1 == uid && q.dir == dir)) { + q.next = None; + keys.push(k.clone()); + } + if let Ok(mut cancelled) = self.cancelled.lock() { + cancelled.extend(keys); + } + if let Ok(mut now) = self.writing_now.lock() { + now.insert(canonical.clone(), owner); + } + Some(canonical) + } + + /// `key` as in [`Writer::finish`]. + fn cancelled(&self, key: &(PathBuf, u64)) -> bool { + self.cancelled.lock().is_ok_and(|cancelled| cancelled.contains(key)) + } + } + + /// The writer, started on first use (a thread that can't start leaves recovery synchronous). + fn writer() -> Option<&'static Writer> { + WRITER + .get_or_init(|| { + let (jobs, job_rx) = std::sync::mpsc::channel::(); + let (done_tx, done_rx) = std::sync::mpsc::channel::(); + let spawned = std::thread::Builder::new().name("designcraft-recovery".into()).spawn(move || { + while let Ok(first) = job_rx.recv() { + let mut queued = Some(first); + while let Some(job) = queued.take() { + let (published, error) = guarded_write(&job); + let key = (job.key.clone(), job.uid); + let (dir, uid, owner) = (job.dir, job.uid, job.owner); + // The result before the document reads as idle: a writer that is not + // busy has already delivered it, so the interface never stops polling + // with an answer still on its way. + let sent = { + let _publishing = super::PUBLISH.lock().unwrap_or_else(std::sync::PoisonError::into_inner); + // Cancelled after it was published: the canceller removes the snapshot. + let published = published && !started().is_some_and(|w| w.cancelled(&key)); + done_tx.send(Done { dir, uid, error, published, owner }).is_ok() + }; + queued = WRITER.get().and_then(Option::as_ref).and_then(|w| w.finish(&key)); + if !sent { + return; + } + } + } + }); + spawned.ok().map(|_| Writer { + jobs, + done: Mutex::new(done_rx), + undelivered: Mutex::new(Vec::new()), + running: Mutex::new(BTreeMap::new()), + cancelled: Mutex::new(BTreeSet::new()), + writing_now: Mutex::new(BTreeMap::new()), + idle: Condvar::new(), + }) + }) + .as_ref() + } + + /// The writer if it has been started. + pub(super) fn started() -> Option<&'static Writer> { + WRITER.get().and_then(Option::as_ref) + } + + /// Do not publish the snapshot session `owner` is having written for `dir`/`uid` (see + /// [`Writer::cancel`]). + pub(super) fn cancel_owned(dir: &Path, uid: u64, owner: u64) { + if let Some(w) = started() { + w.cancel(dir, uid, Some(owner)); + } + } + + /// A write now in progress; dropping it lets background writes of its document queue again. + pub(super) struct WriteNow(Option<(PathBuf, u64)>); + + impl Drop for WriteNow { + fn drop(&mut self) { + if let (Some(key), Some(w)) = (self.0.take(), started()) + && let Ok(mut now) = w.writing_now.lock() + { + now.remove(&key); + } + } + } + + /// See [`Writer::write_now`]. `None` while another session's write of `dir`/`uid` is in + /// flight. Without a writer thread there are no background writes to wait for. + pub(super) fn write_now(dir: &Path, uid: u64, owner: u64) -> Option { + match writer() { + Some(w) => w.write_now(dir, uid, owner).map(|key| WriteNow(Some(key))), + None => Some(WriteNow(None)), + } + } + + /// See [`Writer::queued_by_other`]. + pub(super) fn queued_by_other(dir: &Path, uid: u64, owner: u64) -> bool { + started().is_some_and(|w| w.queued_by_other(dir, uid, owner)) + } + + /// See [`Writer::pending_folders`]. + pub(super) fn pending_folders(uid: u64, owner: u64) -> Vec { + started().map(|w| w.pending_folders(uid, owner)).unwrap_or_default() + } + + /// Do not publish any write of `uid` in flight (see [`Writer::cancel_document`]). + pub(super) fn cancel_document(uid: u64, owner: u64) -> Vec { + started().map(|w| w.cancel_document(uid, owner)).unwrap_or_default() + } + + /// Whether that write is already marked as cancelled (see [`Writer::cancelled`]). + #[cfg(test)] + pub(super) fn cancelled(dir: &Path, uid: u64) -> bool { + started().is_some_and(|w| w.cancelled(&key(dir, uid))) + } + + /// Write one snapshot, turning a panic into its error: a panic that ended this thread would + /// leave the document marked as running for ever (so it is never written again) and the + /// interface asking for frames for ever. A safety net: a writer still must not panic. + fn guarded_write(job: &Snapshot) -> (bool, Option) { + let key = (job.key.clone(), job.uid); + match guarded(|| Ok(write_snapshot(job, || started().is_some_and(|w| w.cancelled(&key))))) { + // A primary that landed is this run's to remove, even if its metadata failed. + Ok(written) => (written.published, written.error.map(|e| e.to_string())), + Err(e) => (false, Some(e.to_string())), + } + } + + /// [`guarded_write`]'s guard, for any write on this thread. + pub(super) fn guarded(write: impl FnOnce() -> Result) -> Result { + std::panic::catch_unwind(std::panic::AssertUnwindSafe(write)).unwrap_or_else(|payload| { + Err(crate::EngineError::Other(format!("recovery writer panicked: {}", crate::guard::panic_message(payload.as_ref())))) + }) + } + + pub(super) enum Submit { + Queued, + AlreadyQueued, + Conflict, + } + + /// Queue a document's write. `None`: no writer thread (the caller writes it itself). + pub(super) fn submit(entry: Snapshot) -> Option { + let w = writer()?; + let mut running = w.running.lock().ok()?; + let key = (entry.key.clone(), entry.uid); + // Another session's save or recovery write owns this destination. Check ownership while + // holding the same reservation locks used to queue or publish writes. + if let Some(owner) = w.writing_now.lock().ok().and_then(|now| now.get(&key).copied()) { + return Some(if owner == entry.owner { Submit::AlreadyQueued } else { Submit::Conflict }); + } + if let Some(q) = running.get_mut(&key) { + // The write in flight was cancelled (the document was saved since), so it holds + // nothing to recover: this one follows it, in place of any that was waiting. + if w.cancelled.lock().is_ok_and(|cancelled| cancelled.contains(&key)) { + if q.next.as_ref().is_some_and(|next| next.owner != entry.owner) { + return Some(Submit::Conflict); + } + q.next = Some(entry); + return Some(Submit::Queued); + } + return Some(if q.owner == entry.owner { Submit::AlreadyQueued } else { Submit::Conflict }); + } + running.insert(key.clone(), Queued { owner: entry.owner, dir: entry.dir.clone(), next: None }); + // A write is wanted again: whatever cancelled the last one does not apply to this one. + if let Ok(mut cancelled) = w.cancelled.lock() { + cancelled.remove(&key); + } + if w.jobs.send(entry).is_err() { + running.remove(&key); + return None; + } + Some(Submit::Queued) + } +} + +/// Handshakes for tests of the recovery writer, in this crate and, with the `testing` feature, in +/// the app's: hold a write at its start, and wait for the writer to finish. +#[cfg(any(test, feature = "testing"))] +pub mod testing { + use std::path::{Path, PathBuf}; + use std::sync::atomic::{AtomicBool, Ordering}; + use std::sync::mpsc::{Receiver, Sender}; + use std::sync::{Arc, Mutex, PoisonError}; + use std::thread::ThreadId; + + struct Gate { + dir: PathBuf, + holder: ThreadId, + entered: Sender<()>, + go: Receiver<()>, + ran_on_holder: Arc, + } + + static GATE: Mutex> = Mutex::new(None); + + /// The next recovery write into one folder, held at its start until this is released or + /// dropped. + pub struct HeldWrite { + entered: Receiver<()>, + go: Option>, + ran_on_holder: Arc, + } + + /// Hold the next recovery write into `dir`. A write on another thread waits before it writes + /// anything; a write on the calling thread does not wait, and [`HeldWrite::ran_here`] reports it. + pub fn hold_next_write(dir: &Path) -> HeldWrite { + let (entered_tx, entered) = std::sync::mpsc::channel(); + let (go, go_rx) = std::sync::mpsc::channel(); + let ran_on_holder = Arc::new(AtomicBool::new(false)); + let gate = Gate { + dir: dir.to_path_buf(), + holder: std::thread::current().id(), + entered: entered_tx, + go: go_rx, + ran_on_holder: ran_on_holder.clone(), + }; + *GATE.lock().unwrap_or_else(PoisonError::into_inner) = Some(gate); + HeldWrite { entered, go: Some(go), ran_on_holder } + } + + impl HeldWrite { + /// The held write ran on the thread that called [`hold_next_write`]. + pub fn ran_here(&self) -> bool { + self.ran_on_holder.load(Ordering::SeqCst) + } + + /// Block until the write has reached the gate. + pub fn wait_until_held(&self) { + let _ = self.entered.recv(); + } + + /// Let the held write go on. + pub fn release(mut self) { + if let Some(go) = self.go.take() { + let _ = go.send(()); + } + } + } + + impl Drop for HeldWrite { + fn drop(&mut self) { + // A gate no write reached is removed, so it cannot hold a later test's write. + let mut slot = GATE.lock().unwrap_or_else(PoisonError::into_inner); + if slot.as_ref().is_some_and(|gate| Arc::ptr_eq(&gate.ran_on_holder, &self.ran_on_holder)) { + *slot = None; + } + } + } + + /// Called at the start of every recovery write. + pub(super) fn gate(dir: &Path) { + let gate = { + let mut slot = GATE.lock().unwrap_or_else(PoisonError::into_inner); + if slot.as_ref().is_some_and(|gate| gate.dir == dir) { slot.take() } else { None } + }; + let Some(gate) = gate else { return }; + if std::thread::current().id() == gate.holder { + gate.ran_on_holder.store(true, Ordering::SeqCst); + let _ = gate.entered.send(()); + return; + } + let _ = gate.entered.send(()); + // A dropped `HeldWrite` closes the channel, which also lets the write go on. + let _ = gate.go.recv(); + } + + /// Block until the background writer has no write queued or running. + pub fn wait_until_idle() { + // Nothing is written in the background on the web. + #[cfg(not(target_arch = "wasm32"))] + if let Some(w) = super::worker::started() { + w.wait_until_idle(); + } + } + + static FAIL_SYNC: Mutex> = Mutex::new(None); + + /// Make the next folder sync after a recovery file is renamed into `dir` fail. + pub fn fail_next_sync(dir: &Path) { + *FAIL_SYNC.lock().unwrap_or_else(PoisonError::into_inner) = Some(dir.to_path_buf()); + } + + /// Whether this sync of `dir` is the one [`fail_next_sync`] asked to fail. + pub(super) fn sync_fails(dir: &Path) -> bool { + let mut slot = FAIL_SYNC.lock().unwrap_or_else(PoisonError::into_inner); + if slot.as_deref() == Some(dir) { + *slot = None; + return true; + } + false + } } #[cfg(not(target_arch = "wasm32"))] @@ -675,6 +1536,34 @@ fn secure_recovery_dir(_dir: &Path) -> Result { #[cfg(not(target_arch = "wasm32"))] fn publish_recovery_file(path: &Path, bytes: &[u8], replace: bool) -> Result<()> { + publish_recovery_written( + path, + replace, + |out| out.write_all(bytes).map_err(|e| EngineError::Other(format!("{}: {e}", path.display()))), + || false, + || {}, + ) + .map(|_| ()) +} + +/// Serializes the last check for a cancelled write with the rename that publishes it, and with +/// [`worker::cancel_owned`] and [`worker::cancel_document`]. +#[cfg(not(target_arch = "wasm32"))] +static PUBLISH: std::sync::Mutex<()> = std::sync::Mutex::new(()); + +/// [`publish_recovery_file`], with `write` filling the temporary file: a large document is written +/// straight to disk instead of being built in memory first. Returns whether it was published; +/// `cancelled` is asked after the bytes are written and before the rename, so a document saved or +/// closed meanwhile leaves nothing behind. `on_publish` runs right after the rename, before a +/// cancel can. +#[cfg(not(target_arch = "wasm32"))] +fn publish_recovery_written( + path: &Path, + replace: bool, + write: impl FnOnce(&mut std::io::BufWriter<&mut std::fs::File>) -> Result<()>, + cancelled: impl Fn() -> bool, + on_publish: impl FnOnce(), +) -> Result { let Some(dir) = path.parent() else { return Err(EngineError::Other(format!("{} has no parent folder", path.display()))); }; @@ -689,15 +1578,30 @@ fn publish_recovery_file(path: &Path, bytes: &[u8], replace: bool) -> Result<()> use std::os::unix::fs::PermissionsExt; tmp.as_file().set_permissions(std::fs::Permissions::from_mode(0o600)).map_err(|e| EngineError::Other(format!("{}: {e}", path.display())))?; } - tmp.as_file_mut().write_all(bytes).map_err(|e| EngineError::Other(format!("{}: {e}", path.display())))?; + { + let mut out = std::io::BufWriter::new(tmp.as_file_mut()); + write(&mut out)?; + out.flush().map_err(|e| EngineError::Other(format!("{}: {e}", path.display())))?; + } tmp.as_file_mut().sync_all().map_err(|e| EngineError::Other(format!("{}: {e}", path.display())))?; - // Validate the non-followed leaf again immediately before renaming the randomized temporary - // file to its fixed sibling destination. - validate_recovery_dir(dir)?; - let published = if replace { tmp.persist(path) } else { tmp.persist_noclobber(path) }; - published.map_err(|e| EngineError::Other(format!("{}: {}", path.display(), e.error)))?; + { + // Held from the check to the rename: a cancel waits for a rename already under way (the + // cleanup after it then removes the file), and a rename after a cancel sees it. + let _publishing = PUBLISH.lock().unwrap_or_else(std::sync::PoisonError::into_inner); + // Last moment: the document was saved, reverted or closed while the bytes were written. + // The temporary file is dropped, so nothing of the stale snapshot stays. + if cancelled() { + return Ok(false); + } + // Validate the non-followed leaf again immediately before renaming the randomized + // temporary file to its fixed sibling destination. + validate_recovery_dir(dir)?; + let published = if replace { tmp.persist(path) } else { tmp.persist_noclobber(path) }; + published.map_err(|e| EngineError::Other(format!("{}: {}", path.display(), e.error)))?; + on_publish(); + } sync_recovery_dir(dir)?; - Ok(()) + Ok(true) } #[cfg(not(target_arch = "wasm32"))] @@ -730,6 +1634,10 @@ fn is_link_like(metadata: &std::fs::Metadata) -> bool { #[cfg(not(target_arch = "wasm32"))] fn sync_recovery_dir(dir: &Path) -> Result<()> { validate_recovery_dir(dir)?; + #[cfg(any(test, feature = "testing"))] + if testing::sync_fails(dir) { + return Err(EngineError::Other(format!("{}: sync failed", dir.display()))); + } #[cfg(unix)] { @@ -755,6 +1663,17 @@ fn publish_recovery_file(_path: &Path, _bytes: &[u8], _replace: bool) -> Result< Err(EngineError::Other("recovery files are unavailable on the web".into())) } +#[cfg(target_arch = "wasm32")] +fn publish_recovery_written( + _path: &Path, + _replace: bool, + _write: impl FnOnce(&mut std::io::BufWriter<&mut std::fs::File>) -> Result<()>, + _cancelled: impl Fn() -> bool, + _on_publish: impl FnOnce(), +) -> Result { + Err(EngineError::Other("recovery files are unavailable on the web".into())) +} + /// Canonical keys are only for alias comparison; operational paths stay unchanged so run /// ownership checks still find the directory registered by the native profile. fn directory_key(dir: &Path) -> Result { @@ -826,10 +1745,11 @@ pub fn open(s: &mut Session, dir: &Path) -> Result> { fn open_with_publisher(s: &mut Session, dir: &Path, mut publish: impl FnMut(&Path, &[u8]) -> Result<()>) -> Result> { #[cfg(not(target_arch = "wasm32"))] validate_recovery_dir(dir)?; - let source = access(dir, false)?; + let owner = s.recovery_owner.0; + let source = access(dir, false, owner)?; let destination = s.recovery_dir.clone().unwrap_or_else(|| dir.to_path_buf()); let created = secure_recovery_dir(&destination)?; - let target = access(&destination, created)?; + let target = access(&destination, created, owner)?; let source_key = source.key.clone(); let destination_key = target.key.clone(); let mut opened = Vec::new(); @@ -850,6 +1770,12 @@ fn open_with_publisher(s: &mut Session, dir: &Path, mut publish: impl FnMut(&Pat }) { continue; } + // A background write of this session that is running, or finished with its result not + // taken yet, is a live document's checkpoint, not an entry to recover. + #[cfg(not(target_arch = "wasm32"))] + if worker::pending_folders(uid, owner).iter().any(|pending| directory_key(pending).is_ok_and(|key| key == source_key)) { + continue; + } #[cfg(not(target_arch = "wasm32"))] validate_recovery_dir(dir)?; let (doc, _) = files(dir, uid); @@ -887,7 +1813,7 @@ fn open_with_publisher(s: &mut Session, dir: &Path, mut publish: impl FnMut(&Pat let meta = json!({"path": state.path, "title": state.title(), "saved": designcraft_doc::vars::now()}); match publish(&metadata, meta.to_string().as_bytes()) { Ok(()) => { - discard(dir, uid); + discard(dir, uid, owner); state.recovery_entries = vec![(destination.clone(), state.uid)]; } Err(e) => log::warn!("recovery entry {uid} kept: {e}"), diff --git a/crates/engine/src/recovery/tests.rs b/crates/engine/src/recovery/tests.rs index c23a2c831..8d28dc8a9 100644 --- a/crates/engine/src/recovery/tests.rs +++ b/crates/engine/src/recovery/tests.rs @@ -1,5 +1,12 @@ use super::*; +/// [`super::discard`] as the session that wrote or recovered the entry, if one did. +fn discard(dir: &Path, uid: u64) { + let key = directory_key(dir).ok().map(|key| (key, uid)); + let owner = key.and_then(|key| WRITTEN.lock().unwrap_or_else(std::sync::PoisonError::into_inner).get(&key).copied()); + super::discard(dir, uid, owner.unwrap_or(0)); +} + fn dirty_session() -> Session { let mut session = Session::new(); session.execute("file.new", &json!({})).unwrap(); @@ -126,6 +133,727 @@ fn recovery_data_is_private_to_the_user() { } } +/// The recovery writer is one thread for the whole process, and its results go to whichever +/// session polls it: tests that submit to it take turns, so one test's write or error is never +/// drained by another. +static BACKGROUND: std::sync::Mutex<()> = std::sync::Mutex::new(()); + +fn background() -> std::sync::MutexGuard<'static, ()> { + BACKGROUND.lock().unwrap_or_else(std::sync::PoisonError::into_inner) +} + +/// Wait for the background writer to have nothing left, then read `file.recovery.progress` as the +/// app does at the end of a frame. +fn finished_writes(s: &mut Session) -> usize { + testing::wait_until_idle(); + let v = s.execute("file.recovery.progress", &json!({})).unwrap(); + assert_eq!(v["errors"], json!([]), "{v}"); + assert_eq!(v["busy"], false, "{v}"); + v["written"].as_u64().unwrap_or(0) as usize +} + +#[cfg(not(target_arch = "wasm32"))] +#[test] +fn a_background_save_returns_while_its_write_is_held() { + let _turn = background(); + let dir = tempfile::tempdir().unwrap(); + let mut s = dirty_session(); + s.recovery_dir = Some(dir.path().to_path_buf()); + let uid = s.docs[0].uid; + let (file, _) = files(dir.path(), uid); + let held = testing::hold_next_write(dir.path()); + let queued = s.execute("file.recovery.save", &json!({"background": true})).unwrap(); + assert!(!held.ran_here(), "the write ran on the calling thread"); + assert_eq!(queued["queued"], 1, "{queued}"); + // The writer thread is stopped at the start of the write, and the session goes on. + held.wait_until_held(); + let v = s.execute("file.recovery.progress", &json!({})).unwrap(); + assert_eq!(v["busy"], true, "{v}"); + s.execute("frame.create", &json!({"rect": [20, 20, 60, 60]})).unwrap(); + assert!(!file.exists(), "nothing is published while the write is held"); + held.release(); + assert_eq!(finished_writes(&mut s), 1); + assert!(file.exists()); + discard(dir.path(), uid); +} + +#[cfg(not(target_arch = "wasm32"))] +#[test] +fn a_document_saved_while_its_write_is_held_leaves_no_snapshot() { + let _turn = background(); + let dir = tempfile::tempdir().unwrap(); + let mut s = dirty_session(); + s.recovery_dir = Some(dir.path().to_path_buf()); + let uid = s.docs[0].uid; + let held = testing::hold_next_write(dir.path()); + s.execute("file.recovery.save", &json!({"background": true})).unwrap(); + held.wait_until_held(); + let saved = dir.path().join("saved.designcraft"); + s.execute("file.saveAs", &json!({"path": saved.to_string_lossy()})).unwrap(); + held.release(); + // Before the session takes the result: a crash now must not find the stale snapshot. + testing::wait_until_idle(); + assert!(!files(dir.path(), uid).0.exists(), "the stale snapshot was not published"); + assert!(!files(dir.path(), uid).1.exists(), "nor its metadata"); + assert_eq!(finished_writes(&mut s), 0); + assert!(list(dir.path()).is_empty()); +} + +#[cfg(not(target_arch = "wasm32"))] +#[test] +fn a_document_saved_while_its_write_to_a_named_folder_is_held_leaves_no_snapshot() { + // A folder given to `file.recovery.save` is not the session's, and is not in the document's + // entries until the write's result is taken. + let _turn = background(); + let named = tempfile::tempdir().unwrap(); + let mut s = dirty_session(); + let uid = s.docs[0].uid; + let held = testing::hold_next_write(named.path()); + let queued = s.execute("file.recovery.save", &json!({"background": true, "dir": named.path().to_string_lossy()})).unwrap(); + assert_eq!(queued["queued"], 1, "{queued}"); + held.wait_until_held(); + let saved = named.path().join("saved.designcraft"); + s.execute("file.saveAs", &json!({"path": saved.to_string_lossy()})).unwrap(); + held.release(); + // Before the session takes the result: a crash now must not find the stale snapshot. + testing::wait_until_idle(); + assert!(!files(named.path(), uid).0.exists(), "the stale snapshot was not published"); + assert_eq!(finished_writes(&mut s), 0); + assert!(list(named.path()).is_empty()); +} + +#[cfg(not(target_arch = "wasm32"))] +#[test] +fn saving_removes_a_finished_write_to_a_named_folder_whose_result_was_not_taken() { + let _turn = background(); + let named = tempfile::tempdir().unwrap(); + let mut s = dirty_session(); + let uid = s.docs[0].uid; + s.execute("file.recovery.save", &json!({"background": true, "dir": named.path().to_string_lossy()})).unwrap(); + testing::wait_until_idle(); + assert!(files(named.path(), uid).0.exists(), "the write finished"); + let saved = named.path().join("saved.designcraft"); + s.execute("file.saveAs", &json!({"path": saved.to_string_lossy()})).unwrap(); + assert!(!files(named.path(), uid).0.exists(), "Save removed the snapshot before its result was taken"); + // Edited again before the result is taken: the removed snapshot is not its checkpoint. + s.execute("frame.create", &json!({"rect": [20, 20, 60, 60]})).unwrap(); + assert_eq!(finished_writes(&mut s), 0); + assert!(!s.docs[0].recovery_entries.iter().any(|(d, _)| same_directory(d, named.path())), "no checkpoint for a removed snapshot"); + assert!(list(named.path()).is_empty()); +} + +#[cfg(not(target_arch = "wasm32"))] +#[test] +fn opening_recovery_leaves_a_finished_background_write_whose_result_was_not_taken() { + let _turn = background(); + let dir = tempfile::tempdir().unwrap(); + let mut s = dirty_session(); + s.recovery_dir = Some(dir.path().to_path_buf()); + let uid = s.docs[0].uid; + s.execute("file.recovery.save", &json!({"background": true})).unwrap(); + testing::wait_until_idle(); + assert!(files(dir.path(), uid).0.exists(), "the write finished"); + assert!(open(&mut s, dir.path()).unwrap().is_empty(), "the live document's own snapshot is not recovered"); + assert_eq!(s.docs.len(), 1); + assert_eq!(finished_writes(&mut s), 1); + assert_eq!(list(dir.path()).len(), 1); + discard(dir.path(), uid); +} + +#[cfg(not(target_arch = "wasm32"))] +#[test] +fn an_edit_after_a_save_is_written_after_the_cancelled_write_stops() { + let _turn = background(); + let dir = tempfile::tempdir().unwrap(); + let mut s = dirty_session(); + s.recovery_dir = Some(dir.path().to_path_buf()); + let uid = s.docs[0].uid; + let held = testing::hold_next_write(dir.path()); + s.execute("file.recovery.save", &json!({"background": true})).unwrap(); + held.wait_until_held(); + let saved = tempfile::tempdir().unwrap(); + s.execute("file.saveAs", &json!({"path": saved.path().join("saved.designcraft").to_string_lossy()})).unwrap(); + s.execute("frame.create", &json!({"rect": [20, 20, 60, 60]})).unwrap(); + // The held write was cancelled: the new edit is queued behind it instead of waiting for the + // next timer tick. + let queued = s.execute("file.recovery.save", &json!({"background": true})).unwrap(); + assert_eq!(queued["queued"], 1, "{queued}"); + held.release(); + assert_eq!(finished_writes(&mut s), 1); + let mut reopened = Session::new(); + assert_eq!(open(&mut reopened, dir.path()).unwrap().len(), 1); + assert_eq!(reopened.doc().unwrap().doc.spreads[0].items.len(), 2, "the snapshot after the edit was written"); + discard(dir.path(), reopened.doc().unwrap().uid); + discard(dir.path(), uid); +} + +#[cfg(not(target_arch = "wasm32"))] +#[test] +fn a_background_result_goes_to_the_session_that_queued_it() { + let _turn = background(); + let dir = tempfile::tempdir().unwrap(); + let mut queued_by = dirty_session(); + queued_by.recovery_dir = Some(dir.path().to_path_buf()); + let uid = queued_by.docs[0].uid; + let mut other = dirty_session(); + queued_by.execute("file.recovery.save", &json!({"background": true})).unwrap(); + testing::wait_until_idle(); + // Another session has none of these documents: it must neither take the result nor remove + // the snapshot as if its document had been closed. + let v = other.execute("file.recovery.progress", &json!({})).unwrap(); + assert_eq!(v["written"], 0, "{v}"); + assert!(files(dir.path(), uid).0.exists(), "the snapshot is still there"); + assert_eq!(finished_writes(&mut queued_by), 1); + assert!(queued_by.docs[0].recovery_entries.iter().any(|(d, u)| *u == uid && same_directory(d, dir.path())), "the checkpoint is recorded"); + discard(dir.path(), uid); +} + +#[cfg(not(target_arch = "wasm32"))] +#[test] +fn saving_in_one_session_leaves_another_sessions_write_alone() { + let _turn = background(); + let dir = tempfile::tempdir().unwrap(); + let mut queued_by = dirty_session(); + queued_by.recovery_dir = Some(dir.path().to_path_buf()); + let uid = queued_by.docs[0].uid; + // The same uid in another session (recovered documents keep the uids of their entries). + let mut other = dirty_session(); + other.docs[0].uid = uid; + let held = testing::hold_next_write(dir.path()); + queued_by.execute("file.recovery.save", &json!({"background": true})).unwrap(); + held.wait_until_held(); + let elsewhere = tempfile::tempdir().unwrap(); + other.execute("file.saveAs", &json!({"path": elsewhere.path().join("other.designcraft").to_string_lossy()})).unwrap(); + held.release(); + assert_eq!(finished_writes(&mut queued_by), 1); + assert!(files(dir.path(), uid).0.exists(), "the other session's save did not cancel this write"); + discard(dir.path(), uid); +} + +#[cfg(not(target_arch = "wasm32"))] +#[test] +fn saving_in_one_session_leaves_another_sessions_write_to_the_same_folder_alone() { + let _turn = background(); + let dir = tempfile::tempdir().unwrap(); + let mut queued_by = dirty_session(); + queued_by.recovery_dir = Some(dir.path().to_path_buf()); + let uid = queued_by.docs[0].uid; + // The same uid and recovery folder in another session. + let mut other = dirty_session(); + other.recovery_dir = Some(dir.path().to_path_buf()); + other.docs[0].uid = uid; + let held = testing::hold_next_write(dir.path()); + queued_by.execute("file.recovery.save", &json!({"background": true})).unwrap(); + held.wait_until_held(); + let elsewhere = tempfile::tempdir().unwrap(); + other.execute("file.saveAs", &json!({"path": elsewhere.path().join("other.designcraft").to_string_lossy()})).unwrap(); + assert!(!worker::cancelled(dir.path(), uid), "the other session's save cancelled this write"); + held.release(); + assert_eq!(finished_writes(&mut queued_by), 1); + assert!(files(dir.path(), uid).0.exists(), "this session's snapshot was kept"); + discard(dir.path(), uid); +} + +#[cfg(not(target_arch = "wasm32"))] +#[test] +fn a_write_now_in_one_session_leaves_another_sessions_write_of_the_same_uid_alone() { + let _turn = background(); + let dir = tempfile::tempdir().unwrap(); + let mut queued_by = dirty_session(); + queued_by.recovery_dir = Some(dir.path().to_path_buf()); + let uid = queued_by.docs[0].uid; + // The same uid in another session, with two objects instead of one. + let mut other = dirty_session(); + other.docs[0].uid = uid; + other.execute("frame.create", &json!({"rect": [20, 20, 60, 60]})).unwrap(); + let held = testing::hold_next_write(dir.path()); + queued_by.execute("file.recovery.save", &json!({"background": true})).unwrap(); + held.wait_until_held(); + let err = save(&other, dir.path()).unwrap_err(); + assert!(err.to_string().contains("still being written"), "{err}"); + let err = other.execute("file.recovery.save", &json!({"dir": dir.path().to_string_lossy()})).unwrap_err(); + assert!(err.to_string().contains("still being written"), "{err}"); + assert!(!worker::cancelled(dir.path(), uid), "the other session's write now cancelled this write"); + held.release(); + assert_eq!(finished_writes(&mut queued_by), 1); + let mut reopened = Session::new(); + assert_eq!(open(&mut reopened, dir.path()).unwrap().len(), 1); + assert_eq!(reopened.doc().unwrap().doc.spreads[0].items.len(), 1, "the queued session's snapshot was kept"); + discard(dir.path(), reopened.doc().unwrap().uid); + discard(dir.path(), uid); +} + +#[cfg(not(target_arch = "wasm32"))] +#[test] +fn no_other_sessions_write_of_the_uid_is_queued_during_a_write_now() { + let _turn = background(); + let dir = tempfile::tempdir().unwrap(); + let writing = dirty_session(); + let uid = writing.docs[0].uid; + let mut other = dirty_session(); + other.recovery_dir = Some(dir.path().to_path_buf()); + other.docs[0].uid = uid; + let now = worker::write_now(dir.path(), uid, writing.recovery_owner.0).expect("nothing else is in flight"); + let queued = other.execute("file.recovery.save", &json!({"background": true})).unwrap(); + assert_eq!(queued["queued"], 0, "another session's write was queued during the write now: {queued}"); + assert!(worker::write_now(dir.path(), uid, other.recovery_owner.0).is_none(), "a second write now of the uid started"); + drop(now); + let queued = other.execute("file.recovery.save", &json!({"background": true})).unwrap(); + assert_eq!(queued["queued"], 1, "{queued}"); + assert_eq!(finished_writes(&mut other), 1); + discard(dir.path(), uid); +} + +#[cfg(not(target_arch = "wasm32"))] +#[test] +fn a_write_now_cancels_a_background_write_to_the_same_folder_spelled_another_way() { + let _turn = background(); + let dir = tempfile::tempdir().unwrap(); + std::fs::create_dir(dir.path().join("sub")).unwrap(); + let spelled = dir.path().join("sub").join(".."); + let mut s = dirty_session(); + s.recovery_dir = Some(spelled.clone()); + let uid = s.docs[0].uid; + let held = testing::hold_next_write(&spelled); + s.execute("file.recovery.save", &json!({"background": true})).unwrap(); + held.wait_until_held(); + // The newer state, written now under the folder's plain name. + s.execute("frame.create", &json!({"rect": [20, 20, 60, 60]})).unwrap(); + s.execute("file.recovery.save", &json!({"dir": dir.path().to_string_lossy()})).unwrap(); + assert!(worker::cancelled(&spelled, uid), "the older write is cancelled"); + held.release(); + testing::wait_until_idle(); + let _ = s.execute("file.recovery.progress", &json!({})).unwrap(); + let mut reopened = Session::new(); + assert_eq!(open(&mut reopened, dir.path()).unwrap().len(), 1); + assert_eq!(reopened.doc().unwrap().doc.spreads[0].items.len(), 2, "the newer snapshot was kept"); + discard(dir.path(), reopened.doc().unwrap().uid); + discard(dir.path(), uid); +} + +#[cfg(not(target_arch = "wasm32"))] +#[test] +fn queued_background_writes_count_toward_the_checkpoint_limit() { + let _turn = background(); + let mut s = dirty_session(); + let folders: Vec = (0..=MAX_RECOVERY_ENTRIES).map(|_| tempfile::tempdir().unwrap()).collect(); + // The first write is held, so every later one is still queued when the next is asked for. + let held = testing::hold_next_write(folders[0].path()); + for folder in &folders[..MAX_RECOVERY_ENTRIES] { + let queued = s.execute("file.recovery.save", &json!({"background": true, "dir": folder.path().to_string_lossy()})).unwrap(); + assert_eq!(queued["queued"], 1, "{queued}"); + } + let over = folders[MAX_RECOVERY_ENTRIES].path(); + let err = s.execute("file.recovery.save", &json!({"background": true, "dir": over.to_string_lossy()})).unwrap_err(); + assert!(err.to_string().contains("too many recovery checkpoints"), "{err}"); + // Saving cancels the queued writes: they no longer count once the document changes again. + let saved = folders[0].path().join("saved.designcraft"); + s.execute("file.saveAs", &json!({"path": saved.to_string_lossy()})).unwrap(); + s.execute("frame.create", &json!({"rect": [20, 20, 60, 60]})).unwrap(); + let queued = s.execute("file.recovery.save", &json!({"background": true, "dir": over.to_string_lossy()})).unwrap(); + assert_eq!(queued["queued"], 1, "{queued}"); + held.release(); + assert_eq!(finished_writes(&mut s), 1, "only the write queued after the save is a checkpoint"); + assert_eq!(list(over).len(), 1); + let uid = s.docs[0].uid; + for folder in &folders { + discard(folder.path(), uid); + } +} + +#[cfg(not(target_arch = "wasm32"))] +#[test] +fn results_for_a_session_that_is_gone_are_not_kept() { + let _turn = background(); + let dir = tempfile::tempdir().unwrap(); + let mut gone = dirty_session(); + gone.recovery_dir = Some(dir.path().to_path_buf()); + let owner = gone.recovery_owner.0; + gone.execute("file.recovery.save", &json!({"background": true})).unwrap(); + testing::wait_until_idle(); + drop(gone); + let mut other = dirty_session(); + other.execute("file.recovery.progress", &json!({})).unwrap(); + let writer = worker::started().unwrap(); + let kept = writer.undelivered.lock().unwrap_or_else(std::sync::PoisonError::into_inner); + assert!(!kept.iter().any(|d| d.owner == owner), "a result nobody can take was kept"); +} + +#[test] +fn a_background_save_rejects_a_background_that_is_not_true_or_false() { + let dir = tempfile::tempdir().unwrap(); + let mut s = dirty_session(); + s.recovery_dir = Some(dir.path().to_path_buf()); + let err = s.execute("file.recovery.save", &json!({"background": "yes"})).unwrap_err(); + assert!(err.to_string().contains("`background` must be true or false"), "{err}"); + assert!(list(dir.path()).is_empty(), "nothing was written"); +} + +#[test] +fn a_background_save_writes_recovery_data_the_app_can_recover() { + let _turn = background(); + let dir = tempfile::tempdir().unwrap(); + let mut s = dirty_session(); + s.recovery_dir = Some(dir.path().to_path_buf()); + // The timer's call: the interface thread does not wait for the deflate or the file writes. + let queued = s.execute("file.recovery.save", &json!({"background": true})).unwrap(); + assert_eq!(queued["queued"], 1, "{queued}"); + assert_eq!(finished_writes(&mut s), 1); + assert_eq!(list(dir.path()).len(), 1); + assert!(s.docs[0].recovery_entries.iter().any(|(d, _)| same_directory(d, dir.path())), "the checkpoint is recorded"); + // Recovering it gives the unsaved edits back. + let mut reopened = Session::new(); + assert_eq!(open(&mut reopened, dir.path()).unwrap().len(), 1); + assert_eq!(reopened.doc().unwrap().doc.spreads[0].items.len(), 1); + assert!(reopened.doc().unwrap().is_dirty()); + discard(dir.path(), reopened.doc().unwrap().uid); +} + +#[test] +fn a_snapshot_that_finishes_after_the_save_is_dropped() { + // A write of a large document runs for seconds: the user can save while it does. The saved + // document's checkpoint is consumed then, and the write must not put a stale snapshot back. + let dir = tempfile::tempdir().unwrap(); + let mut s = dirty_session(); + s.recovery_dir = Some(dir.path().to_path_buf()); + let uid = s.docs[0].uid; + // The snapshot the writer took before the save. + let entry = Snapshot::of(&prepare_save(&s, dir.path()).unwrap(), dir.path(), &s.docs[0]); + let saved = dir.path().join("saved.designcraft"); + s.execute("file.saveAs", &json!({"path": saved.to_string_lossy()})).unwrap(); + assert!(!s.docs[0].is_dirty()); + // The write lands after the save. + let written = write_snapshot(&entry, || false); + assert!(written.published && written.error.is_none(), "the snapshot landed"); + let (file, _) = files(dir.path(), uid); + assert!(file.exists()); + assert!(!record_checkpoint(&mut s, dir.path().to_path_buf(), uid), "a saved document records no checkpoint"); + assert!(!file.exists(), "its snapshot is not left to be recovered"); + assert!(list(dir.path()).is_empty()); + // A document still unsaved does record one. + s.execute("frame.create", &json!({"rect": [10, 10, 50, 50]})).unwrap(); + let entry = Snapshot::of(&prepare_save(&s, dir.path()).unwrap(), dir.path(), &s.docs[0]); + let written = write_snapshot(&entry, || false); + assert!(written.published && written.error.is_none(), "the second snapshot landed"); + assert!(record_checkpoint(&mut s, dir.path().to_path_buf(), uid)); + assert!(s.docs[0].recovery_entries.iter().any(|(d, u)| *u == uid && d == dir.path())); +} + +#[test] +fn a_shared_folders_entry_is_only_its_writing_sessions() { + // A recovered entry keeps the uid it was written under, which a document of another session + // in the same process can have. + let dir = tempfile::tempdir().unwrap(); + let writer = dirty_session(); + let uid = writer.docs[0].uid; + assert_eq!(save(&writer, dir.path()).unwrap(), 1); + let (file, _) = files(dir.path(), uid); + let snapshot = std::fs::read(&file).unwrap(); + // Another session's clean document with that uid: its discard leaves the entry alone. + let mut clean = Session::new(); + clean.execute("file.new", &json!({})).unwrap(); + clean.docs[0].uid = uid; + assert_eq!(save(&clean, dir.path()).unwrap(), 0); + assert!(file.exists(), "another session's discard removed the entry"); + // Another session's edited document with that uid: it does not write over the entry. + let mut edited = dirty_session(); + edited.docs[0].uid = uid; + assert!(save(&edited, dir.path()).is_err()); + assert_eq!(std::fs::read(&file).unwrap(), snapshot, "another session wrote over the entry"); + super::discard(dir.path(), uid, writer.recovery_owner.0); + assert!(!file.exists(), "the writing session removes its entry"); +} + +#[test] +fn a_snapshot_queued_as_the_sessions_own_is_recorded_when_it_lands() { + // A write taken while the shared folder's entry was this session's, published after a cleanup + // dropped that record: Save and Close must still remove what it put there. + let dir = tempfile::tempdir().unwrap(); + let s = dirty_session(); + let uid = s.docs[0].uid; + let owner = s.recovery_owner.0; + assert_eq!(save(&s, dir.path()).unwrap(), 1); + let entry = Snapshot::of(&prepare_save(&s, dir.path()).unwrap(), dir.path(), &s.docs[0]); + assert!(entry.own); + super::discard(dir.path(), uid, owner); + let (file, _) = files(dir.path(), uid); + assert!(!file.exists()); + let written = write_snapshot(&entry, || false); + assert!(written.published && written.error.is_none(), "the snapshot landed"); + super::discard(dir.path(), uid, owner); + assert!(!file.exists(), "the snapshot it wrote is the session's to remove"); +} + +#[cfg(not(target_arch = "wasm32"))] +#[test] +fn a_snapshot_whose_metadata_failed_is_still_the_documents_to_remove() { + // The primary document is the recovery data; its metadata is optional. A metadata failure + // must not leave the published snapshot untracked, or Save and Close would not remove it and + // it would stay in the folder for ever. + let root = tempfile::tempdir().unwrap(); + let dir = run_dir(root.path()).unwrap(); + let mut s = dirty_session(); + s.recovery_dir = Some(dir.clone()); + let uid = s.docs[0].uid; + let (document, metadata) = files(&dir, uid); + // The folder is this run's own, so the entry is its own; a folder takes the metadata's name, + // which makes the optional write fail after the primary landed. + std::fs::create_dir(&metadata).unwrap(); + let err = save_tracked(&mut s, &dir).unwrap_err(); + assert!(err.to_string().contains(&metadata.display().to_string()), "{err}"); + assert!(document.exists(), "the primary landed"); + assert!(s.docs[0].recovery_entries.iter().any(|(d, u)| *u == uid && same_directory(d, &dir)), "and is tracked"); + // Saving the document removes the snapshot this run wrote. + let saved = root.path().join("saved.designcraft"); + s.execute("file.saveAs", &json!({"path": saved.to_string_lossy()})).unwrap(); + assert!(!document.exists(), "Save removed the snapshot"); + assert!(s.docs[0].recovery_entries.is_empty()); +} + +#[cfg(not(target_arch = "wasm32"))] +#[test] +fn a_snapshot_whose_folder_sync_failed_is_still_the_documents_to_remove() { + // The rename put the snapshot in place before the folder sync failed: it is tracked, so Save + // removes it. + let root = tempfile::tempdir().unwrap(); + let dir = run_dir(root.path()).unwrap(); + let mut s = dirty_session(); + s.recovery_dir = Some(dir.clone()); + let uid = s.docs[0].uid; + let (document, _) = files(&dir, uid); + testing::fail_next_sync(&dir); + let err = save_tracked(&mut s, &dir).unwrap_err(); + assert!(err.to_string().contains("sync failed"), "{err}"); + assert!(document.exists(), "the snapshot landed"); + assert!(s.docs[0].recovery_entries.iter().any(|(d, u)| *u == uid && same_directory(d, &dir)), "and is tracked"); + let saved = root.path().join("saved.designcraft"); + s.execute("file.saveAs", &json!({"path": saved.to_string_lossy()})).unwrap(); + assert!(!document.exists(), "Save removed the snapshot"); +} + +#[cfg(not(target_arch = "wasm32"))] +#[test] +fn a_document_undone_to_its_saved_state_while_its_write_is_held_leaves_no_snapshot() { + let _turn = background(); + let dir = tempfile::tempdir().unwrap(); + let mut s = dirty_session(); + s.recovery_dir = Some(dir.path().to_path_buf()); + let uid = s.docs[0].uid; + let held = testing::hold_next_write(dir.path()); + s.execute("file.recovery.save", &json!({"background": true})).unwrap(); + held.wait_until_held(); + s.execute("edit.undo", &json!({})).unwrap(); + assert!(!s.docs[0].is_dirty(), "back to the state it had"); + // A save of recovery data now drops the clean document's checkpoint, and the held write of + // the older state must not publish after it. + save(&s, dir.path()).unwrap(); + held.release(); + testing::wait_until_idle(); + assert!(!files(dir.path(), uid).0.exists(), "the older snapshot was not published"); + let _ = s.execute("file.recovery.progress", &json!({})); + assert!(list(dir.path()).is_empty()); +} + +#[cfg(not(target_arch = "wasm32"))] +#[test] +fn a_document_undone_to_its_saved_state_loses_the_snapshot_a_finished_write_left_in_another_folder() { + let _turn = background(); + let dir = tempfile::tempdir().unwrap(); + let other = tempfile::tempdir().unwrap(); + let mut s = dirty_session(); + s.recovery_dir = Some(other.path().to_path_buf()); + let uid = s.docs[0].uid; + s.execute("file.recovery.save", &json!({"background": true})).unwrap(); + // Written, but its result not yet taken by the session. + testing::wait_until_idle(); + assert!(files(other.path(), uid).0.exists()); + s.execute("edit.undo", &json!({})).unwrap(); + save(&s, dir.path()).unwrap(); + assert!(!files(other.path(), uid).0.exists(), "the snapshot of the older state is removed"); + let _ = s.execute("file.recovery.progress", &json!({})); +} + +#[cfg(not(target_arch = "wasm32"))] +#[test] +fn a_clean_document_leaves_another_sessions_write_queued_behind_its_own() { + let _turn = background(); + let dir = tempfile::tempdir().unwrap(); + let mut a = dirty_session(); + a.recovery_dir = Some(dir.path().to_path_buf()); + let uid = a.docs[0].uid; + let mut b = dirty_session(); + b.recovery_dir = Some(dir.path().to_path_buf()); + b.docs[0].uid = uid; + b.execute("frame.create", &json!({"rect": [20, 20, 60, 60]})).unwrap(); + let held = testing::hold_next_write(dir.path()); + a.execute("file.recovery.save", &json!({"background": true})).unwrap(); + held.wait_until_held(); + // A write now cancels the held one. The other session does not write over the entry it left. + a.execute("file.recovery.save", &json!({"dir": dir.path().to_string_lossy()})).unwrap(); + let err = b.execute("file.recovery.save", &json!({"background": true})).unwrap_err(); + assert!(err.to_string().contains("did not write"), "{err}"); + a.execute("edit.undo", &json!({})).unwrap(); + assert!(!a.docs[0].is_dirty()); + save(&a, dir.path()).unwrap(); + // Once that entry is gone, the other session's write is queued to follow the held one. + b.execute("file.recovery.save", &json!({"background": true})).unwrap(); + held.release(); + assert_eq!(finished_writes(&mut b), 1, "the other session's write ran"); + let mut reopened = Session::new(); + assert_eq!(open(&mut reopened, dir.path()).unwrap().len(), 1); + assert_eq!(reopened.doc().unwrap().doc.spreads[0].items.len(), 2, "its snapshot was kept"); + discard(dir.path(), uid); +} + +#[cfg(not(target_arch = "wasm32"))] +#[test] +fn a_background_recovery_write_reports_another_sessions_held_write() { + let _turn = background(); + let dir = tempfile::tempdir().unwrap(); + let mut first = dirty_session(); + first.recovery_dir = Some(dir.path().to_path_buf()); + let uid = first.docs[0].uid; + let mut second = dirty_session(); + second.recovery_dir = Some(dir.path().to_path_buf()); + second.docs[0].uid = uid; + second.execute("frame.create", &json!({"rect": [20, 20, 60, 60]})).unwrap(); + + let held = testing::hold_next_write(dir.path()); + first.execute("file.recovery.save", &json!({"background": true})).unwrap(); + held.wait_until_held(); + + let err = second.execute("file.recovery.save", &json!({"background": true})).unwrap_err(); + assert!(err.to_string().contains("another session"), "{err}"); + + held.release(); + assert_eq!(finished_writes(&mut first), 1); + assert!(files(dir.path(), uid).0.exists(), "the first session's recovery file remains intact"); + discard(dir.path(), uid); +} + +#[cfg(not(target_arch = "wasm32"))] +#[test] +fn a_clean_document_in_another_session_leaves_this_sessions_write_alone() { + let _turn = background(); + let dir = tempfile::tempdir().unwrap(); + let mut queued_by = dirty_session(); + queued_by.recovery_dir = Some(dir.path().to_path_buf()); + let uid = queued_by.docs[0].uid; + // The same uid, clean, in another session. + let mut other = Session::new(); + other.execute("file.new", &json!({})).unwrap(); + other.docs[0].uid = uid; + assert!(!other.docs[0].is_dirty()); + let held = testing::hold_next_write(dir.path()); + queued_by.execute("file.recovery.save", &json!({"background": true})).unwrap(); + held.wait_until_held(); + save(&other, dir.path()).unwrap(); + held.release(); + assert_eq!(finished_writes(&mut queued_by), 1); + assert!(files(dir.path(), uid).0.exists(), "the other session's save did not cancel this write"); + discard(dir.path(), uid); +} + +#[cfg(not(target_arch = "wasm32"))] +#[test] +fn a_background_snapshot_whose_metadata_failed_is_still_the_documents_to_remove() { + // The background writer's result must be applied the same way: a published primary is the + // document's snapshot even when its optional metadata fails. + let _turn = background(); + let root = tempfile::tempdir().unwrap(); + let dir = run_dir(root.path()).unwrap(); + let mut s = dirty_session(); + s.recovery_dir = Some(dir.clone()); + let uid = s.docs[0].uid; + let (document, metadata) = files(&dir, uid); + std::fs::create_dir(&metadata).unwrap(); + let queued = s.execute("file.recovery.save", &json!({"background": true})).unwrap(); + assert_eq!(queued["queued"], 1, "{queued}"); + testing::wait_until_idle(); + let v = s.execute("file.recovery.progress", &json!({})).unwrap(); + assert_eq!(v["busy"], false, "{v}"); + let reported = v["errors"].as_array().and_then(|errors| errors.first()).and_then(Value::as_str).map(str::to_string); + let reported = reported.expect("the metadata failure is reported"); + assert!(reported.contains(&metadata.display().to_string()), "{reported}"); + assert!(document.exists(), "the primary landed"); + assert!(s.docs[0].recovery_entries.iter().any(|(d, u)| *u == uid && same_directory(d, &dir)), "and is tracked"); + // Saving the document removes the snapshot this run wrote. + let saved = root.path().join("saved.designcraft"); + s.execute("file.saveAs", &json!({"path": saved.to_string_lossy()})).unwrap(); + assert!(!document.exists(), "Save removed the snapshot"); +} + +#[test] +fn a_write_cancelled_midway_publishes_nothing() { + // Save/Revert/Close during a write marks it cancelled; the snapshot must not land after it, + // even if the app crashes before the next interface frame discards it. + let dir = tempfile::tempdir().unwrap(); + let s = dirty_session(); + let entry = Snapshot::of(&prepare_save(&s, dir.path()).unwrap(), dir.path(), &s.docs[0]); + let written = write_snapshot(&entry, || true); + assert!(!written.published && written.error.is_none(), "a cancelled write is not an error"); + assert!(list(dir.path()).is_empty(), "a cancelled write leaves no snapshot"); + assert!(!files(dir.path(), entry.uid).0.exists(), "nor its document"); + assert!(!files(dir.path(), entry.uid).1.exists(), "nor its metadata"); + // The temporary file it was written to is dropped, not left behind. + assert!( + std::fs::read_dir(dir.path()).unwrap().all(|e| e.is_ok_and(|e| !e.file_name().to_string_lossy().starts_with(".designcraft-recovery-"))), + "no temporary file is left" + ); +} + +#[test] +fn a_cancel_stops_a_write_before_the_package_is_finished() { + let dir = tempfile::tempdir().unwrap(); + let s = dirty_session(); + let entry = Snapshot::of(&prepare_save(&s, dir.path()).unwrap(), dir.path(), &s.docs[0]); + // Cancelled once the write has started: the first check passes, every later one fails. + let checks = std::cell::Cell::new(0); + let written = write_snapshot(&entry, || { + checks.set(checks.get() + 1); + checks.get() > 1 + }); + assert!(!written.published && written.error.is_none(), "a stopped write is not an error"); + assert!(checks.get() >= 2, "the write checked again before it finished"); + assert!(!files(dir.path(), entry.uid).0.exists(), "nothing was published"); + assert!( + std::fs::read_dir(dir.path()).unwrap().all(|e| e.is_ok_and(|e| !e.file_name().to_string_lossy().starts_with(".designcraft-recovery-"))), + "no temporary file is left" + ); +} + +#[cfg(not(target_arch = "wasm32"))] +#[test] +fn only_a_queued_write_is_cancelled() { + // A mark for a document with no write in flight would stay in the writer's set for ever. + let dir = tempfile::tempdir().unwrap(); + worker::cancel_owned(dir.path(), 7, 1); + assert!(!worker::cancelled(dir.path(), 7), "nothing was queued for it"); +} + +#[cfg(not(target_arch = "wasm32"))] +#[test] +fn a_panicking_recovery_write_is_an_error_and_the_writer_keeps_going() { + let err = worker::guarded(|| -> Result<()> { panic!("boom") }).unwrap_err(); + assert!(err.to_string().contains("panicked") && err.to_string().contains("boom"), "{err}"); + assert_eq!(worker::guarded(|| Ok(7)).unwrap(), 7, "the next write still runs"); +} + +#[test] +fn a_saved_document_is_not_written_in_the_background() { + // The autosave writes unsaved documents only. + let _turn = background(); + let dir = tempfile::tempdir().unwrap(); + let mut s = dirty_session(); + s.recovery_dir = Some(dir.path().to_path_buf()); + let saved = dir.path().join("saved.designcraft"); + s.execute("file.saveAs", &json!({"path": saved.to_string_lossy()})).unwrap(); + let queued = s.execute("file.recovery.save", &json!({"background": true})).unwrap(); + assert_eq!(queued["queued"], 0, "{queued}"); + assert_eq!(finished_writes(&mut s), 0); + assert!(list(dir.path()).is_empty()); +} + #[test] fn existing_recovery_directory_is_used() { let root = tempfile::tempdir().unwrap(); @@ -440,6 +1168,11 @@ fn restart_recovery_child() { } } +/// A child process starts with a copy of every open file of this one until it runs its program, so +/// a lock file closed meanwhile stays locked: a test that closes a lock and expects it free does +/// not run while a child is started. +static SPAWN: std::sync::Mutex<()> = std::sync::Mutex::new(()); + fn run_restart_case(mode: &str) { let source = restart_fixture(); if mode == "max_uid" { @@ -448,12 +1181,20 @@ fn run_restart_case(mode: &str) { std::fs::rename(doc, max_doc).unwrap(); std::fs::rename(meta, max_meta).unwrap(); } - let output = std::process::Command::new(std::env::current_exe().unwrap()) - .args(["--exact", "recovery::tests::restart_recovery_child", "--ignored", "--nocapture"]) - .env("DESIGNCRAFT_TEST_RECOVERY_SOURCE", source.path()) - .env("DESIGNCRAFT_TEST_RECOVERY_MODE", mode) - .output() - .unwrap(); + // `spawn` returns once the child runs its program, and its copies of this process's files + // are closed by then. + let child = { + let _spawning = SPAWN.lock().unwrap_or_else(std::sync::PoisonError::into_inner); + std::process::Command::new(std::env::current_exe().unwrap()) + .args(["--exact", "recovery::tests::restart_recovery_child", "--ignored", "--nocapture"]) + .env("DESIGNCRAFT_TEST_RECOVERY_SOURCE", source.path()) + .env("DESIGNCRAFT_TEST_RECOVERY_MODE", mode) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .spawn() + .unwrap() + }; + let output = child.wait_with_output().unwrap(); assert!(output.status.success(), "fresh child failed: {}{}", String::from_utf8_lossy(&output.stdout), String::from_utf8_lossy(&output.stderr)); let child_stdout = String::from_utf8_lossy(&output.stdout); assert!(child_stdout.contains("test result: ok. 1 passed; 0 failed;"), "the fresh-process helper did not run exactly once: {child_stdout}"); @@ -684,6 +1425,8 @@ fn documents_of_two_apps_that_ended_are_both_recovered() { fn a_running_apps_recovery_data_is_left_alone() { let root = tempfile::tempdir().unwrap(); let running = run_left(root.path(), "run-7-0", "still open", 1); + // No child starts while the lock is open, so closing it frees it (see `SPAWN`). + let _no_spawn = SPAWN.lock().unwrap_or_else(std::sync::PoisonError::into_inner); let lock = std::fs::File::open(running.join(LOCK)).unwrap(); lock.try_lock().unwrap(); let own = run_dir(root.path()).unwrap(); diff --git a/crates/format/src/lib.rs b/crates/format/src/lib.rs index 52ce74860..0ac376b71 100644 --- a/crates/format/src/lib.rs +++ b/crates/format/src/lib.rs @@ -91,6 +91,24 @@ fn save_with_large_entries_from(doc: &Document, preview: Option<&[u8]>, large_en /// [`save_with_large_entries_from`], deflating only what a reader with `limits` inflates. fn save_within(doc: &Document, preview: Option<&[u8]>, large_entry: usize, limits: Limits) -> Result, FormatError> { + let mut buf = Cursor::new(Vec::new()); + write_with_large_entries_from(&mut buf, doc, preview, large_entry, limits)?; + Ok(buf.into_inner()) +} + +/// Write the package to `out` instead of returning it in memory: a document with gigabytes of +/// placed files needs no second copy in RAM (recovery snapshots, large saves). +pub fn write(out: &mut W, doc: &Document, preview: Option<&[u8]>) -> Result<(), FormatError> { + write_with_large_entries_from(out, doc, preview, LARGE_ENTRY, Limits::DEFAULT) +} + +fn write_with_large_entries_from( + out: &mut W, + doc: &Document, + preview: Option<&[u8]>, + large_entry: usize, + limits: Limits, +) -> Result<(), FormatError> { // JSON writes infinity and NaN as null, which no load accepts: the file would not open. if let Some(id) = doc.non_finite_item() { return Err(FormatError::Unsaveable(format!("object {} has a position or size that is not a finite number", id.0))); @@ -99,9 +117,8 @@ fn save_within(doc: &Document, preview: Option<&[u8]>, large_entry: usize, limit compact.compact_assets(); let doc = &compact; let io = |e: zip::result::ZipError| FormatError::Io(e.to_string()); - let mut buf = Cursor::new(Vec::new()); { - let mut z = zip::ZipWriter::new(&mut buf); + let mut z = zip::ZipWriter::new(out); let stored = zip::write::SimpleFileOptions::default().compression_method(zip::CompressionMethod::Stored); let deflate = zip::write::SimpleFileOptions::default().compression_method(zip::CompressionMethod::Deflated); z.start_file("mimetype", stored).map_err(io)?; @@ -140,7 +157,7 @@ fn save_within(doc: &Document, preview: Option<&[u8]>, large_entry: usize, limit } z.finish().map_err(io)?; } - Ok(buf.into_inner()) + Ok(()) } /// The file name extension of a DesignCraft document. diff --git a/crates/format/src/tests.rs b/crates/format/src/tests.rs index 5b73801ea..efbbba065 100644 --- a/crates/format/src/tests.rs +++ b/crates/format/src/tests.rs @@ -651,6 +651,24 @@ fn saved_tab_lists_load_sanitized() { assert_eq!(tabs.last().map(|t| t.position), Some(3960.0), "the first stops by position"); } +#[test] +fn writing_to_a_stream_makes_the_same_file_as_saving_in_memory() { + // A save that streams needs no copy of the archive in RAM; it must write the same package. + let mut d = Document::new(&NewDocument::default()); + let a = png_asset(&mut d, b"streamed bytes"); + place(&mut d, a, 0.0); + let png = b"\x89PNG streamed".to_vec(); + let saved = save_with_preview(&d, Some(&png)).unwrap(); + let mut out = Cursor::new(Vec::new()); + write(&mut out, &d, Some(&png)).unwrap(); + let streamed = out.into_inner(); + assert_eq!(saved.len(), streamed.len()); + assert_eq!(asset_entries(&streamed), 1); + let back = load(&streamed).unwrap(); + assert_eq!(back.assets.len(), 1); + assert_eq!(read_preview(Cursor::new(&streamed)), Some(png)); +} + #[test] fn the_preview_is_read_back_without_the_document() { let d = Document::new(&NewDocument::default()); diff --git a/crates/ui-egui/Cargo.toml b/crates/ui-egui/Cargo.toml index 6b08420a9..f7fe65d8e 100644 --- a/crates/ui-egui/Cargo.toml +++ b/crates/ui-egui/Cargo.toml @@ -44,6 +44,7 @@ zip = { workspace = true } # Headless (offscreen wgpu) rendering of the whole window for UI screenshots: `examples/ui_shot.rs`. egui_kittest = { workspace = true, features = ["wgpu", "snapshot"] } designcraft-fonts = { workspace = true, features = ["testing"] } +designcraft-engine = { workspace = true, features = ["testing"] } [build-dependencies] serde_json = { workspace = true } diff --git a/crates/ui-egui/src/app.rs b/crates/ui-egui/src/app.rs index 9f56e2206..431266c3c 100644 --- a/crates/ui-egui/src/app.rs +++ b/crates/ui-egui/src/app.rs @@ -111,6 +111,7 @@ impl DesignApp { shortcut_answers: Default::default(), last_time: now, last_recovery: now, + recovery_writing: false, egui_ctx: None, in_control: false, object_clip_text: None, diff --git a/crates/ui-egui/src/app/frame.rs b/crates/ui-egui/src/app/frame.rs index ceeb28743..985ede0f5 100644 --- a/crates/ui-egui/src/app/frame.rs +++ b/crates/ui-egui/src/app/frame.rs @@ -83,14 +83,28 @@ impl DesignApp { /// uses the monotonic clock, so a wall-clock/NTP jump cannot postpone or advance it. pub(super) fn ui_frame_end(&mut self, ctx: &egui::Context) { let now = now_ms() / 1000.0; - // Crash recovery: unsaved documents are written to the recovery folder every 30 s. + // Crash recovery: unsaved documents are written to the recovery folder every 30 s, on a + // background thread — writing a catalogue with gigabytes of placed files took minutes, and + // on the interface thread the window stopped answering for all of it. #[cfg(not(target_arch = "wasm32"))] - if self.session.recovery_dir.is_some() && now - self.last_recovery > (self.session.prefs.recovery_minutes * 60.0).max(5.0) { - self.last_recovery = now; - if self.session.documents().iter().any(|d| d.is_dirty()) - && let Err(e) = self.session.execute("file.recovery.save", &json!({})) - { - self.status(crate::i18n::tr(&self.ui.language, "Couldn't write recovery data: {error}").replace("{error}", &e.to_string())); + { + if self.session.recovery_dir.is_some() && now - self.last_recovery > (self.session.prefs.recovery_minutes * 60.0).max(5.0) { + self.last_recovery = now; + if self.session.documents().iter().any(|d| d.is_dirty()) { + match self.session.execute("file.recovery.save", &json!({"background": true})) { + Ok(v) if v["queued"].as_u64().unwrap_or(0) > 0 => self.recovery_writing = true, + Ok(_) => {} + Err(e) => self + .status(crate::i18n::tr(&self.ui.language, "Couldn't write recovery data: {error}").replace("{error}", &e.to_string())), + } + } + } + if self.recovery_writing { + self.poll_recovery(); + // Keep the frames coming while the writer works; it has no interface to ask. + if self.recovery_writing { + ctx.request_repaint_after(std::time::Duration::from_millis(150)); + } } } let dt = now - self.last_time; @@ -101,6 +115,18 @@ impl DesignApp { self.issue_screenshots(ctx); } + /// Take what the background recovery writer finished: its checkpoints, and what failed. + #[cfg(not(target_arch = "wasm32"))] + fn poll_recovery(&mut self) { + let Ok(v) = self.session.execute("file.recovery.progress", &json!({})) else { return }; + self.recovery_writing = v["busy"].as_bool().unwrap_or(false); + let errors: Vec = v["errors"].as_array().into_iter().flatten().filter_map(|e| e.as_str().map(str::to_string)).collect(); + for e in errors { + let message = crate::i18n::tr(&self.ui.language, "Couldn't write recovery data: {error}").replace("{error}", &e); + self.status(message); + } + } + /// The document area: the start screen, or the document tabs over one or two canvas panes. pub(crate) fn canvas_area(&mut self, ui: &mut egui::Ui, presenting: bool) { let t = theme::Tokens::get(ui.ctx()); diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 86cef0351..35c389846 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -255,6 +255,9 @@ pub struct DesignApp { last_time: f64, /// When recovery data was last written (seconds on the monotonic clock; see [`now_ms`]). pub last_recovery: f64, + /// Recovery data is being written in the background: poll for it, and keep repainting. + #[cfg_attr(target_arch = "wasm32", allow(dead_code))] + pub(crate) recovery_writing: bool, /// The egui context (set by the first UI pass): copied text goes to the system clipboard through it. egui_ctx: Option, /// A control-channel request is being handled: commands it runs never read or write the diff --git a/crates/ui-egui/src/tests.rs b/crates/ui-egui/src/tests.rs index 0fcd2f3f6..8f2eef43d 100644 --- a/crates/ui-egui/src/tests.rs +++ b/crates/ui-egui/src/tests.rs @@ -140,6 +140,47 @@ fn app_with_unsaved_document() -> DesignApp { app } +/// The recovery timer wrote the whole document on the interface thread: on a catalogue with +/// gigabytes of linked files the window stopped answering for minutes after every edit. +#[test] +fn the_recovery_timer_writes_off_the_interface_thread() { + let ctx = egui::Context::default(); + let mut app = app_with_unsaved_document(); + let dir = tempfile::tempdir().unwrap(); + app.session.recovery_dir = Some(dir.path().to_path_buf()); + app.last_recovery = -1.0e9; + let uid = app.session.documents()[0].uid; + let entry = dir.path().join(format!("{uid}.designcraft")); + // The writer stops at the start of the write until the test lets it go on, so a frame that + // waited for the write would never return. + let held = designcraft_engine::recovery::testing::hold_next_write(dir.path()); + // egui discards a pass that needs another layout; the timer runs in the first pass it keeps. + for _ in 0..10 { + shot_frame(&mut app, &ctx, egui::vec2(1400.0, 900.0)); + if app.last_recovery > 0.0 { + break; + } + } + assert!(app.last_recovery > 0.0, "the timer fired and restarted its interval"); + assert!(!held.ran_here(), "the timer wrote the recovery data on the interface thread"); + assert!(app.recovery_writing, "the timer queued the write"); + held.wait_until_held(); + // While the write is held, the interface keeps drawing frames and taking edits. + app.run("frame.create", json!({"rect": [20, 20, 80, 80]})).unwrap(); + shot_frame(&mut app, &ctx, egui::vec2(1400.0, 900.0)); + assert!(app.recovery_writing, "the write is still held"); + assert!(!entry.exists(), "nothing is published while the write is held"); + held.release(); + designcraft_engine::recovery::testing::wait_until_idle(); + shot_frame(&mut app, &ctx, egui::vec2(1400.0, 900.0)); + assert!(!app.recovery_writing, "the frame after the write took its result"); + assert!(entry.exists(), "the timer's background write published the document"); + // The checkpoint is the document's: saving it removes the recovery entry. + let saved = dir.path().join("saved.designcraft"); + app.run("file.saveAs", json!({"path": saved.to_string_lossy()})).unwrap(); + assert!(!dir.path().join(format!("{uid}.designcraft")).exists()); +} + /// File ▸ Close (and its shortcut) closed a document with unsaved changes without asking, and /// closing discards the document's recovery data too. #[test] diff --git a/docs/architecture.md b/docs/architecture.md index 29e9757ad..ab5afba2e 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -1,6 +1,6 @@ # DesignCraft architecture -> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-12 · **Change:** minor (shared PDF writing) · **Target:** Adobe InDesign 2026 +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-27 · **Change:** minor (recovery writes on a background thread, streamed to disk) · **Target:** Adobe InDesign 2026 How DesignCraft is built today. The design notes behind it are in `plan/architecture.md` (local, gitignored); this file describes what the code on `main` does. Agents: AGENTS.md has the @@ -58,7 +58,9 @@ panels, zoom, dialogs) are `UI_COMMANDS` in `crates/ui-egui/src/menus/ui_command Tool gestures emit Begin/Preview/Commit commands so a drag is one undo step. Every journaled command is replayable as a script. Escaped panics during a command or an import/export become an error (`engine/src/guard.rs`), never a lost document; native recovery is written periodically -into each running app's locked folder. Orderly native shutdown finalizes complete, current quit +into each running app's locked folder, on a background thread and straight to disk, so an +autosave of a catalogue with gigabytes of placed files no longer stops the window answering. +Orderly native shutdown finalizes complete, current quit approvals and writes the remaining documents' latest recovery state, including clean-document cleanup after Undo. Unavailable or failed recovery is reported. The desktop opts into a pinned winit 0.30 patch that cancels AppKit termination and queues ordinary window close requests, @@ -119,6 +121,7 @@ builds (on manifest changes), packaging lint and the release workflow; `xtask ci | Date | Change | Summary | |---|---|---| +| 2026-10-27 | minor | Recovery writes off the interface thread, streamed to disk; a snapshot still being written when its document is saved, reverted or closed is cancelled | | 2026-10-12 | minor | Documented cancelable Dock Quit through the native event-loop boundary | | 2026-10-12 | minor | Documented orderly native exit recovery and the remaining cancelable Dock Quit gap | | 2026-10-10 | major | Created from the code on main (92649ff) and `plan/architecture.md` | diff --git a/docs/gaps.md b/docs/gaps.md index 0042cc704..ed4ca9bec 100644 --- a/docs/gaps.md +++ b/docs/gaps.md @@ -1,6 +1,6 @@ # Where DesignCraft falls short of InDesign -> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-11-04 · **Change:** minor (gap 21 shrunk: pt-BR translates the whole interface; placed graphic layer-option preservation and explicit rendering warnings; gap 16 shrunk: File › File Info; gap 16: Convert Variable to Text; gap 28 added: placed-PDF font repair remains after visible unembedded font detection, #909; gap 26 shrunk: autoflow fills the following pages before it adds pages at the end; gap 6 shrunk: Type › Reload Fonts reads fonts installed, edited or removed while running (#328); gap 3: Package links are relative, #340; gap 27 closed: the loaded text cursor threads into a frame inside a group (#1075); .indd and .indt open through indd-utils; gap 0 shrunk: text threads by hand; threading gaps added; Optical kerning from outlines (#181); gap 6: macOS lists fonts that font services and managers activate; shared print spooler; image clipping paths from IDML; Tabs panel listed as present (#343); book chapter update safety; gap 16: Based on Parent leftovers; bounded control click/drag input; swatches on placed greyscale and bitmap images) · **Target:** Adobe InDesign 2026 (21.6) +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-11-04 · **Change:** minor (gap 20: automatic recovery moved off the interface thread; gap 21 shrunk: pt-BR translates the whole interface; placed graphic layer-option preservation and explicit rendering warnings; gap 16 shrunk: File › File Info; gap 16: Convert Variable to Text; gap 28 added: placed-PDF font repair remains after visible unembedded font detection, #909; gap 26 shrunk: autoflow fills the following pages before it adds pages at the end; gap 6 shrunk: Type › Reload Fonts reads fonts installed, edited or removed while running (#328); gap 3: Package links are relative, #340; gap 27 closed: the loaded text cursor threads into a frame inside a group (#1075); .indd and .indt open through indd-utils; gap 0 shrunk: text threads by hand; threading gaps added; Optical kerning from outlines (#181); gap 6: macOS lists fonts that font services and managers activate; shared print spooler; image clipping paths from IDML; Tabs panel listed as present (#343); book chapter update safety; gap 16: Based on Parent leftovers; bounded control click/drag input; swatches on placed greyscale and bitmap images) · **Target:** Adobe InDesign 2026 (21.6) Every known shortfall against InDesign 2026, one entry each, ranked by how much it stops an InDesign professional from doing real work. This is the work list: agents pick from the top. @@ -240,7 +240,11 @@ Kind: **F** feature · **U** UI/UX · **T** typography · **FF** file format · against InDesign on long books and image-heavy catalogues. - **Have:** compose 300k characters ~38 ms warm, damage-region repaint (< 1 ms while typing), parallel story composition, and an edit composes only the paragraphs it can change in a plain - story (240 paragraphs: 3 composed for a typed character). + story (240 paragraphs: 3 composed for a typed character). Automatic recovery writes on a + background thread and streams to disk, so an autosave of that catalogue no longer blocks the + interface thread for 30-100 s (`file.recovery.save {background}`, `file.recovery.progress`). A + snapshot still being written when its document is saved, reverted or closed is cancelled before + it is published, so a crash cannot offer back a copy older than the saved file. - **Estimate:** 40–80 h. ### 26. Autoflow flows into the primary text frame of the pages that follow (F)