Recovery: autosave every open design tab (#2884) (#2952)

* Fix recovery for unsaved background tabs (#2884)

* Recovery: scheduled autosaves of every tab write on the worker thread (#2884)

After #3139, Autosaver::save waits for the write and Autosaver::tick runs
it on a worker. The scheduled path now goes through tick (which skips a
tab whose previous write is still running), so several dirty tabs no
longer write on the window thread each interval (#3044). save stays for
Save, close and shutdown. Each tab's autosaver has no clock of its own:
the shared cadence decides when it is due. The tests wait for the
writes a tick started.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Recovery: closing a tab waits for its running autosave before removing the entry (#2884)

With the autosave on a worker thread, discard could remove the files and
the lock while a write was still running; the write then recreated the
entry without a lock, and the next launch offered a design closed with
Don't Save. discard now ends the running write first, as Drop does.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Christian Svensson
2026-10-20 23:58:05 +00:00
committed by GitHub
parent 94bd8719aa
commit 079a600481
5 changed files with 402 additions and 18 deletions
+17 -3
View File
@@ -1,6 +1,6 @@
//! Autosave and crash recovery.
//!
//! A running app keeps one recovery entry in the recovery folder: `<id>.solidcraft` (the design
//! Each open design keeps its own recovery entry in the recovery folder: `<id>.solidcraft` (the design
//! as of the last autosave), `<id>.json` (its file path, name and time) and `<id>.lock`, which
//! the app holds locked while it runs. Autosaves are written atomically, only when the design
//! has unsaved changes it hasn't written yet; a design with nothing unsaved has no entry. An
@@ -163,7 +163,7 @@ pub fn remove(dir: &Path, id: &str) -> Result<()> {
Ok(())
}
/// One app's autosaver.
/// One open design's autosaver. Keep it with that design when switching tabs.
pub struct Autosaver {
pub dir: PathBuf,
pub id: String,
@@ -232,7 +232,7 @@ impl Snapshot {
}
impl Autosaver {
/// Start autosaving into `dir` (created if needed), holding this app's lock.
/// Start autosaving into `dir` (created if needed), holding this design's lock.
pub fn new(dir: &Path, interval: Duration) -> Result<Autosaver> {
let io = |e: std::io::Error| EngineError::Other(format!("{}: {e}", dir.display()));
fs::create_dir_all(dir).map_err(io)?;
@@ -361,6 +361,20 @@ impl Autosaver {
Ok(())
}
/// The user closed this design (saving it or choosing Don't Save): discard only its entry.
pub fn discard(mut self) {
// A write still running would recreate the entry, without a lock, after it is removed:
// the next launch would offer a design the user closed.
if let Some(w) = self.writing.take() {
let _ = w.done.recv();
}
for ext in ["json", solidcraft_io::DESIGN_EXT] {
let _ = fs::remove_file(file(&self.dir, &self.id, ext));
}
self.lock = None;
let _ = fs::remove_file(file(&self.dir, &self.id, "lock"));
}
/// Closing the app: with nothing unsaved the entry goes; otherwise it stays for the next
/// launch to offer. The lock is released either way.
pub fn close(mut self, s: &Session) {
+32 -4
View File
@@ -18,7 +18,9 @@ pub const MAX_OPEN: usize = 32;
/// A design that is not the active one: everything that belongs to it alone.
pub struct DocSlot {
session: Session,
pub(crate) session: Session,
pub(crate) autosave: Option<solidcraft_engine::recovery::Autosaver>,
pub(crate) big_operation: bool,
cam: Camera,
pre_sketch_cam: Option<Camera>,
pre_sketch_marker: Option<Option<usize>>,
@@ -38,7 +40,16 @@ struct DocView {
impl DocSlot {
fn new(session: Session) -> DocSlot {
DocSlot { session, cam: Camera::default(), pre_sketch_cam: None, pre_sketch_marker: None, view: DocView::default(), occurrence_move: None }
DocSlot {
session,
autosave: None,
big_operation: false,
cam: Camera::default(),
pre_sketch_cam: None,
pre_sketch_marker: None,
view: DocView::default(),
occurrence_move: None,
}
}
}
@@ -58,6 +69,10 @@ pub struct Documents {
}
impl Documents {
pub(crate) fn background_mut(&mut self) -> impl Iterator<Item = &mut DocSlot> {
self.slots.iter_mut().filter_map(Option::as_mut)
}
/// Number of open designs (at least the active one).
pub fn count(&self) -> usize {
self.slots.len().max(1)
@@ -107,14 +122,19 @@ fn swap_in(app: &mut SolveApp, slot: DocSlot) -> DocSlot {
app.menu.props = None;
crate::sketch_attachment_ui::clear(app);
let out_rev = app.session.revision;
let out = DocSlot {
let mut out = DocSlot {
session: std::mem::replace(&mut app.session, slot.session),
autosave: app.autosave.as_mut().and_then(|a| std::mem::replace(&mut a.active, slot.autosave)),
big_operation: std::mem::replace(&mut app.big_operation, slot.big_operation),
cam: std::mem::replace(&mut app.cam, slot.cam),
pre_sketch_cam: std::mem::replace(&mut app.pre_sketch_cam, slot.pre_sketch_cam),
pre_sketch_marker: std::mem::replace(&mut app.pre_sketch_marker, slot.pre_sketch_marker),
view: take_view(app),
occurrence_move: std::mem::replace(&mut app.tree.occurrence_move, slot.occurrence_move),
};
// Only the displayed design owns the process-wide crash replay journal.
app.session.crash_journal = out.session.crash_journal;
out.session.crash_journal = false;
put_view(app, slot.view);
// Preferences the session holds go with the user, not the tab (#680).
app.session.auto_project = out.session.auto_project;
@@ -254,6 +274,10 @@ pub fn close(app: &mut SolveApp, i: usize, force: bool) {
app.docs.closing = None;
if n <= 1 {
// The last design: a blank one takes its place.
if let Some(a) = app.autosave.as_mut().and_then(|a| a.active.take()) {
a.discard();
}
app.big_operation = false;
crate::sketch_text_move::cancel_drag(app);
let rev = app.session.revision;
crate::dialogs::cancel(app);
@@ -273,7 +297,11 @@ pub fn close(app: &mut SolveApp, i: usize, force: bool) {
let next = if i + 1 < n { i + 1 } else { i - 1 };
switch(app, next);
}
app.docs.slots.remove(i);
if let Some(slot) = app.docs.slots.remove(i)
&& let Some(a) = slot.autosave
{
a.discard();
}
if app.docs.active > i {
app.docs.active -= 1;
}
+1 -1
View File
@@ -430,7 +430,7 @@ pub struct SolveApp {
pub status: Option<(String, f64, bool)>,
pub quit_requested: bool,
/// Autosave into the recovery folder (the desktop app turns it on).
pub autosave: Option<solidcraft_engine::recovery::Autosaver>,
pub autosave: Option<recovery_ui::Autosaves>,
/// Designs left by crashed apps, offered for recovery.
pub recoverable: Vec<solidcraft_engine::recovery::Entry>,
/// The bodies of a just-opened Y-up file, offered to be stood up (`stand_up`).
+349 -8
View File
@@ -2,6 +2,9 @@
use serde_json::json;
use solidcraft_engine::recovery::{self, Autosaver, Entry};
use solidcraft_engine::{Result, Session};
use std::path::{Path, PathBuf};
use std::time::{Duration, Instant};
use crate::SolveApp;
use crate::ui_kit;
@@ -9,11 +12,71 @@ use crate::ui_kit;
/// A command that ran at least this long counts as a big operation: autosave right after it.
pub const BIG_OPERATION_MS: f64 = 1500.0;
/// Autosave (when due, or after a big operation).
/// A tab's autosaver has no clock of its own: [`tick`] tells it when its turn has come.
const SHARED_CADENCE: Duration = Duration::MAX;
/// Shared settings and cadence; each tab owns its own entry and change tracking.
pub struct Autosaves {
pub dir: PathBuf,
pub interval: Duration,
pub(crate) active: Option<Autosaver>,
last_at: Instant,
}
impl Autosaves {
/// Save the active design immediately, including when taking over a recovered entry.
pub fn save(&mut self, session: &Session) -> Result<bool> {
save_slot(&self.dir, &mut self.active, session, true, true)
}
}
/// Bring one tab's autosave up to date. `due`: its turn has come. `wait`: write on this thread
/// and wait for it (Save, close, shutdown); otherwise the write runs on a worker thread, so a
/// scheduled autosave of several large designs never holds up the window (#3044).
fn save_slot(dir: &Path, slot: &mut Option<Autosaver>, session: &Session, due: bool, wait: bool) -> Result<bool> {
// Clean tabs need no serialization or entry, even between scheduled saves.
if !session.is_dirty() {
if let Some(mut a) = slot.take() {
// Rebase the active crash journal before deleting the autosave it may refer to.
a.save(session)?;
a.discard();
}
return Ok(false);
}
if !due {
return Ok(false);
}
if slot.is_none() {
*slot = Some(Autosaver::new(dir, SHARED_CADENCE)?);
}
match slot.as_mut() {
Some(a) if wait => a.save(session),
// Skips a tab whose previous write is still running.
Some(a) => a.tick(session, true),
None => Ok(false),
}
}
/// Autosave every open design on the shared cadence, or a tab after its big operation.
pub fn tick(app: &mut SolveApp) {
let big = std::mem::take(&mut app.big_operation);
let Some(a) = app.autosave.as_mut() else { return };
if let Err(e) = a.tick(&app.session, big) {
let due = a.last_at.elapsed() >= a.interval;
if due {
a.last_at = Instant::now();
}
let mut error = None;
for slot in app.docs.background_mut() {
let big = std::mem::take(&mut slot.big_operation);
if let Err(e) = save_slot(&a.dir, &mut slot.autosave, &slot.session, due || big, false) {
error.get_or_insert(e);
}
}
// Background writes must not replace the active design's crash-journal checkpoint.
if let Err(e) = save_slot(&a.dir, &mut a.active, &app.session, due || big, false) {
error.get_or_insert(e);
}
if let Some(e) = error {
let msg = format!("autosave failed: {e}");
if app.status.as_ref().is_none_or(|(s, _, _)| *s != msg) {
app.set_status(msg, true);
@@ -23,10 +86,10 @@ pub fn tick(app: &mut SolveApp) {
/// Start autosaving into `dir` and queue the designs that crashed apps left there.
pub fn start(app: &mut SolveApp, dir: &std::path::Path, interval: std::time::Duration) {
match Autosaver::new(dir, interval) {
match Autosaver::new(dir, SHARED_CADENCE) {
Ok(a) => {
app.recoverable = recovery::orphans(dir);
app.autosave = Some(a);
app.autosave = Some(Autosaves { dir: dir.to_path_buf(), interval, active: Some(a), last_at: Instant::now() });
}
Err(e) => app.set_status(format!("autosave is off: {e}"), true),
}
@@ -34,8 +97,17 @@ pub fn start(app: &mut SolveApp, dir: &std::path::Path, interval: std::time::Dur
/// The app is closing: keep unsaved work for the next launch, drop everything else.
pub fn close(app: &mut SolveApp) {
if let Some(a) = app.autosave.take() {
a.close(&app.session);
if let Some(mut a) = app.autosave.take() {
for slot in app.docs.background_mut() {
let _ = save_slot(&a.dir, &mut slot.autosave, &slot.session, true, true);
if let Some(saver) = slot.autosave.take() {
saver.close(&slot.session);
}
}
let _ = a.save(&app.session);
if let Some(saver) = a.active.take() {
saver.close(&app.session);
}
}
}
@@ -105,8 +177,277 @@ pub fn show(app: &mut SolveApp, ctx: &egui::Context) {
#[cfg(test)]
mod tests {
use serde_json::json;
use solidcraft_engine::recovery::Entry;
use super::*;
use crate::{Services, documents};
use std::fs;
fn app(tag: &str, interval: Duration) -> (SolveApp, PathBuf) {
let dir = std::env::temp_dir().join(format!("solidcraft-recovery-tabs-{tag}-{}", std::process::id()));
let _ = fs::remove_dir_all(&dir);
let mut app = SolveApp::new(Session::default(), Services::default());
start(&mut app, &dir, interval);
(app, dir)
}
fn edit(app: &mut SolveApp) {
app.run("solid.box", json!({"length": 10, "width": 10, "height": 10})).unwrap();
// Tests control the cadence without relying on how long the command took.
app.big_operation = false;
}
fn entries(dir: &Path) -> Vec<Entry> {
let mut entries: Vec<Entry> = fs::read_dir(dir)
.unwrap()
.map(|p| p.unwrap().path())
.filter(|p| p.extension().is_some_and(|e| e == "json"))
.map(|p| serde_json::from_slice(&fs::read(p).unwrap()).unwrap())
.collect();
entries.sort_by(|a, b| a.name.cmp(&b.name));
entries
}
/// A tick, then wait for the writes it started on worker threads. Waiting writes nothing
/// new: every design it would write was just written.
fn tick_and_wait(app: &mut SolveApp) {
tick(app);
let Some(a) = app.autosave.as_mut() else { return };
for slot in app.docs.background_mut() {
if let Some(saver) = slot.autosave.as_mut() {
let _ = saver.save(&slot.session);
}
}
if let Some(saver) = a.active.as_mut() {
let _ = saver.save(&app.session);
}
}
fn name(app: &mut SolveApp, name: &str) {
app.run("file.new", json!({"name": name})).unwrap();
}
#[test]
fn dirty_background_tab_is_saved_and_offered_after_a_crash() {
let (mut app, dir) = app("background", Duration::ZERO);
name(&mut app, "Background A");
edit(&mut app);
let expected = app.session.doc.clone();
documents::new_tab(&mut app);
assert!(!app.session.is_dirty());
tick_and_wait(&mut app);
assert_eq!(entries(&dir).len(), 1);
assert!(recovery::orphans(&dir).is_empty(), "all live entries stay locked");
drop(app); // Simulate a crash: release locks without a final save.
let restarted = restart(&dir);
assert_eq!(restarted.recoverable.len(), 1);
let entry = &restarted.recoverable[0];
assert_eq!(entry.name, "Background A");
assert_eq!(recovery::load(&dir, &entry.id).unwrap().0, *expected);
drop(restarted);
fs::remove_dir_all(dir).unwrap();
}
fn restart(dir: &Path) -> SolveApp {
let mut app = SolveApp::new(Session::default(), Services::default());
start(&mut app, dir, Duration::ZERO);
app
}
#[test]
fn switching_to_a_clean_tab_keeps_the_background_entry_and_its_id() {
let (mut app, dir) = app("clean-tab", Duration::ZERO);
edit(&mut app);
tick_and_wait(&mut app);
let before = entries(&dir);
documents::new_tab(&mut app);
tick_and_wait(&mut app);
assert_eq!(entries(&dir), before, "the clean tab cannot delete another tab's entry");
documents::switch(&mut app, 0);
tick_and_wait(&mut app);
assert_eq!(entries(&dir), before, "switching back preserves identity and skips unchanged content");
drop(app);
assert_eq!(recovery::orphans(&dir), before);
fs::remove_dir_all(dir).unwrap();
}
#[test]
fn several_dirty_tabs_keep_distinct_entries_and_recover_their_own_designs() {
let (mut app, dir) = app("several", Duration::ZERO);
let mut expected = Vec::new();
for i in 0..3 {
if i > 0 {
documents::new_tab(&mut app);
}
name(&mut app, &format!("Design {i}"));
for _ in 0..=i {
edit(&mut app);
}
expected.push(app.session.doc.clone());
}
tick_and_wait(&mut app);
let saved = entries(&dir);
assert_eq!(saved.len(), 3);
let ids: std::collections::HashSet<_> = saved.iter().map(|e| &e.id).collect();
assert_eq!(ids.len(), 3);
for i in [0, 2, 1, 0] {
documents::switch(&mut app, i);
tick_and_wait(&mut app);
assert_eq!(entries(&dir), saved);
}
drop(app);
let mut restarted = restart(&dir);
assert_eq!(restarted.recoverable.len(), 3);
for (entry, doc) in saved.iter().zip(expected) {
assert_eq!(recovery::load(&dir, &entry.id).unwrap().0, *doc);
restarted.run("doc.recover", json!({"dir": dir, "id": entry.id})).unwrap();
assert_eq!(*restarted.session.doc, *doc);
assert!(restarted.session.is_dirty());
}
drop(restarted);
fs::remove_dir_all(dir).unwrap();
}
#[test]
fn saving_and_closing_remove_only_the_affected_tabs_entry() {
let (mut app, dir) = app("cleanup", Duration::ZERO);
for i in 0..3 {
if i > 0 {
documents::new_tab(&mut app);
}
name(&mut app, &format!("Design {i}"));
edit(&mut app);
}
tick_and_wait(&mut app);
let saved = entries(&dir);
// Cleanup is immediate, even when the next autosave is far away.
app.autosave.as_mut().unwrap().interval = Duration::from_secs(3600);
app.run("file.save_as", json!({"path": dir.join("saved.solidcraft")})).unwrap();
tick_and_wait(&mut app);
assert_eq!(entries(&dir), saved[..2]);
documents::close(&mut app, 0, false);
documents::answer_prompt(&mut app, documents::Answer::Cancel);
assert_eq!(entries(&dir), saved[..2], "cancelling close keeps recovery");
documents::close(&mut app, 0, true); // Discard a background tab.
assert_eq!(entries(&dir), saved[1..2]);
documents::switch(&mut app, 0);
documents::close(&mut app, 0, true); // Discard the active tab.
assert!(entries(&dir).is_empty());
edit(&mut app);
app.autosave.as_mut().unwrap().save(&app.session).unwrap();
assert_eq!(entries(&dir).len(), 1);
documents::close(&mut app, 0, true); // Last tab becomes a fresh blank design.
assert!(entries(&dir).is_empty());
drop(app);
assert!(recovery::orphans(&dir).is_empty());
fs::remove_dir_all(dir).unwrap();
}
#[test]
fn shutdown_keeps_all_unsaved_tabs_but_confirmed_quit_discards_them() {
for discard in [false, true] {
let (mut app, dir) = app(if discard { "quit" } else { "shutdown" }, Duration::from_secs(3600));
edit(&mut app);
documents::new_tab(&mut app);
edit(&mut app);
if discard {
// Make entries first, then exercise Don't Save for every design.
app.autosave.as_mut().unwrap().interval = Duration::ZERO;
tick_and_wait(&mut app);
assert_eq!(entries(&dir).len(), 2);
documents::request_quit(&mut app);
documents::answer_prompt(&mut app, documents::Answer::DontSave);
documents::answer_prompt(&mut app, documents::Answer::DontSave);
assert!(app.quit_requested);
}
close(&mut app);
assert_eq!(recovery::orphans(&dir).len(), if discard { 0 } else { 2 });
assert!(app.docs.background_mut().all(|s| s.autosave.is_none()), "shutdown releases every lock");
fs::remove_dir_all(dir).unwrap();
}
}
#[test]
fn cadence_and_big_operations_follow_the_document_and_preferences() {
let (mut app, dir) = app("cadence", Duration::from_secs(3600));
app.session.crash_journal = true;
edit(&mut app);
app.big_operation = true;
documents::new_tab(&mut app);
assert!(app.session.crash_journal);
assert!(app.docs.background_mut().all(|s| !s.session.crash_journal));
assert!(!app.big_operation);
edit(&mut app);
tick_and_wait(&mut app);
assert_eq!(entries(&dir).len(), 1, "only the background big operation is due");
assert!(app.docs.background_mut().all(|s| !s.big_operation));
tick_and_wait(&mut app);
assert_eq!(entries(&dir).len(), 1, "the ordinary active edit waits for the interval");
app.autosave_minutes = 0.00001;
crate::prefs::set_autosave(&mut app);
app.autosave.as_mut().unwrap().last_at = Instant::now() - Duration::from_secs(1);
tick_and_wait(&mut app);
assert_eq!(entries(&dir).len(), 2, "changed preferences apply to every tab");
drop(app);
fs::remove_dir_all(dir).unwrap();
}
#[test]
fn one_failed_autosave_does_not_skip_the_other_tabs() {
let (mut app, dir) = app("failure", Duration::ZERO);
name(&mut app, "A");
edit(&mut app);
tick_and_wait(&mut app);
let id = entries(&dir)[0].id.clone();
let blocked = dir.join(format!("{id}.solidcraft"));
fs::remove_file(&blocked).unwrap();
fs::create_dir(&blocked).unwrap();
edit(&mut app);
documents::new_tab(&mut app);
name(&mut app, "B");
edit(&mut app);
// The writes run on worker threads: a failure is reported by a later tick.
let failed = |app: &SolveApp| app.status.as_ref().is_some_and(|(message, _, error)| *error && message.contains("autosave failed"));
let deadline = Instant::now() + Duration::from_secs(60);
tick(&mut app);
while !failed(&app) && Instant::now() < deadline {
std::thread::sleep(Duration::from_millis(5));
tick(&mut app);
}
assert!(failed(&app));
tick_and_wait(&mut app);
let saved = entries(&dir);
assert_eq!(saved.len(), 2);
assert_eq!(recovery::load(&dir, &saved[1].id).unwrap().0, *app.session.doc);
// Retrying at the next tick also recovers the previously failing tab.
fs::remove_dir(&blocked).unwrap();
tick_and_wait(&mut app);
assert_eq!(recovery::load(&dir, &id).unwrap().0.features.len(), 2);
drop(app);
fs::remove_dir_all(dir).unwrap();
}
#[test]
fn a_tab_closed_while_its_autosave_is_written_leaves_nothing_to_recover() {
let (mut app, dir) = app("discard-race", Duration::ZERO);
name(&mut app, "Closed");
// Enough of a design that its write is still running when the tab is closed.
for i in 0..40 {
app.run("solid.cylinder", json!({"radius": 3, "height": 5, "base": [i * 8, 0, 0], "operation": "new"})).unwrap();
}
app.big_operation = false;
documents::new_tab(&mut app);
tick(&mut app); // Starts writing the background tab on a worker thread.
documents::close(&mut app, 0, true); // Don't Save, right away.
let files = || fs::read_dir(&dir).unwrap().filter_map(|e| e.ok()).map(|e| e.file_name()).collect::<Vec<_>>();
// A write that outlived the close would recreate the entry; give it time to.
let deadline = Instant::now() + Duration::from_secs(2);
while Instant::now() < deadline {
assert!(entries(&dir).is_empty(), "the closed design came back: {:?}", files());
std::thread::sleep(Duration::from_millis(20));
}
drop(app);
assert!(recovery::orphans(&dir).is_empty());
fs::remove_dir_all(dir).unwrap();
}
fn entry(id: &str, name: &str) -> Entry {
Entry { id: id.into(), path: None, name: name.into(), saved_at: 0, features: 3 }
+3 -2
View File
@@ -62,8 +62,9 @@ written as STL files next to the `.urdf`. See [`docs/urdf.md`](https://github.co
Saves are atomic: a crash at any moment leaves either the old file or the new one, and the
replaced version is kept as `<file>.bak`. A save keeps the file's permissions, and a design
opened through a symbolic link (a synced folder linked into a project) is saved into the file the
link points to, so the link stays. Unsaved changes are autosaved to a recovery folder,
and the next launch offers to recover them. Older design files are upgraded on open, and files
link points to, so the link stays. Unsaved changes in every open tab are autosaved to separate
entries in a recovery folder, and the next launch offers to recover each design. Saving or
closing one tab removes only its recovery entry. Older design files are upgraded on open, and files
from a newer version are refused with a message.
**Save** writes a container by default; files named `.json` keep JSON on subsequent saves.