From dcbedbd805c7cdb37fd4e80b169bd1e40dc8553c Mon Sep 17 00:00:00 2001 From: Dahak Builds Date: Wed, 4 Nov 2026 17:16:15 +0000 Subject: [PATCH] Don't blindly reopen a design that stopped the last session (#5064) (#10443) A design that crashes or hangs the app while it is being reopened was reopened blindly every launch, bricking the app each time. A launch that reopens designs now marks them in the recovery folder; the marker is cleared as each design finishes opening. A launch that finds the marker left behind skips those designs once and says which one the last session stopped on, so the app starts usable and the loop breaks. Co-authored-by: Dahak Builds --- apps/solidcraft/src/main.rs | 27 +++++++-- crates/engine/src/recovery.rs | 79 ++++++++++++++++++++++++++ crates/ui-egui/src/documents.rs | 48 ++++++++++++++++ crates/ui-egui/src/file_load.rs | 7 +++ crates/ui-egui/src/lib.rs | 4 ++ docs/book/src/guide/troubleshooting.md | 5 ++ 6 files changed, 166 insertions(+), 4 deletions(-) diff --git a/apps/solidcraft/src/main.rs b/apps/solidcraft/src/main.rs index e034b948c8..e78182203a 100644 --- a/apps/solidcraft/src/main.rs +++ b/apps/solidcraft/src/main.rs @@ -1025,6 +1025,8 @@ fn main() -> eframe::Result { solidcraft_engine::crash::install(dir, build) }) .unwrap_or_default(); + // The recovery folder also holds the reopen guard marker (#5064); read once, reused below. + let recovery_dir = solidcraft_engine::recovery::default_dir(); // A crash in native code (a graphics driver) says where it happened: the exception or signal // and the module on Windows and macOS, and on every system the startup step the app was in. #[cfg(target_os = "windows")] @@ -1213,8 +1215,25 @@ fn main() -> eframe::Result { }; // No design given: the designs open at the last quit, when the preference asks (#5542). // Not when driven over the control channel, whose clients expect a fresh design. - let files = - if files.is_empty() && !sample && control_port.is_none() { solidcraft_ui_egui::documents::designs_to_reopen(&app) } else { files }; + // A design that stopped the last session while it was being reopened is skipped this + // time, so one bad file cannot brick every launch (#5064). + app.reopen_guard_dir = recovery_dir.clone(); + let files = if files.is_empty() && !sample && control_port.is_none() { + let (reopen, skipped) = solidcraft_ui_egui::documents::reopen_plan(&app, recovery_dir.as_deref()); + if !skipped.is_empty() { + let names: Vec = skipped + .iter() + .map(|p| std::path::Path::new(p).file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_else(|| p.clone())) + .collect(); + notes.push(format!( + "The last session stopped while reopening {}. It was not reopened this time; open it yourself if you need it.", + names.join(", ") + )); + } + reopen + } else { + files + }; for f in &files { app.open_path_async(f); } @@ -1230,9 +1249,9 @@ fn main() -> eframe::Result { app.home.show(rev); } // Autosave, and offer what crashed sessions left behind. - if let Some(dir) = solidcraft_engine::recovery::default_dir() { + if let Some(dir) = recovery_dir.as_ref() { let every = if app.autosave_minutes > 0.0 { app.autosave_minutes * 60.0 } else { f64::MAX / 4.0 }; - solidcraft_ui_egui::recovery_ui::start(&mut app, &dir, std::time::Duration::from_secs_f64(every.min(1e9))); + solidcraft_ui_egui::recovery_ui::start(&mut app, dir, std::time::Duration::from_secs_f64(every.min(1e9))); // Safe mode keeps autosaving but offers nothing: the entries wait for a normal start. if safe_mode { app.recoverable.clear(); diff --git a/crates/engine/src/recovery.rs b/crates/engine/src/recovery.rs index 7aefc4c158..e40236ac7f 100644 --- a/crates/engine/src/recovery.rs +++ b/crates/engine/src/recovery.rs @@ -180,6 +180,54 @@ pub fn remove(dir: &Path, id: &str) -> Result<()> { Ok(()) } +/// #5064: marker file in the recovery folder listing the designs a launch is reopening. +const REOPEN_MARKER: &str = "reopening.json"; + +/// A launch that reopens designs marks them here. The marker is removed as each design +/// finishes opening; one still there at the next launch means the last session died (crashed, +/// hung, or was force-quit) while opening those designs — blindly reopening them would brick +/// the app the same way, so the next launch skips them instead (#5064). +pub fn begin_reopen(dir: &Path, paths: &[String]) { + if paths.is_empty() || fs::create_dir_all(dir).is_err() { + return; + } + if let Ok(bytes) = serde_json::to_vec(paths) { + let _ = fs::write(dir.join(REOPEN_MARKER), bytes); + } +} + +/// The designs the previous session was reopening when it died (empty: nothing is stuck). +pub fn stuck_reopen(dir: &Path) -> Vec { + read_reopen_marker(dir).unwrap_or_default() +} + +/// The skip has been said: forget the marker so the launch after the skip reopens normally. +pub fn clear_reopen(dir: &Path) { + let _ = fs::remove_file(dir.join(REOPEN_MARKER)); +} + +/// One reopened design finished (opened or failed to): take it off the marker. When the last +/// one is gone the marker goes with it, so a healthy session leaves nothing behind. +pub fn reopen_finished(dir: &Path, path: &str) { + let Some(mut left) = read_reopen_marker(dir) else { return }; + left.retain(|p| p != path); + if left.is_empty() { + clear_reopen(dir); + } else if let Ok(bytes) = serde_json::to_vec(&left) { + let _ = fs::write(dir.join(REOPEN_MARKER), bytes); + } +} + +/// The marker's paths, bounded like every other file read from the folder. +fn read_reopen_marker(dir: &Path) -> Option> { + let path = dir.join(REOPEN_MARKER); + let meta = fs::metadata(&path).ok()?; + if !meta.is_file() || meta.len() > MAX_ENTRY_BYTES as u64 { + return None; + } + serde_json::from_slice(&fs::read(&path).ok()?).ok() +} + /// One open design's autosaver. Keep it with that design when switching tabs. pub struct Autosaver { pub dir: PathBuf, @@ -677,4 +725,35 @@ mod tests { assert!(remove(&d, "device").is_err()); fs::remove_dir_all(&d).unwrap(); } + + /// #5064: a design that kills the app while it is being reopened must not be blindly + /// reopened every launch. The marker survives the death, the next launch sees what the + /// last one died on, and a healthy session clears it by itself. + #[test] + fn reopen_guard_survives_a_session_that_died_mid_reopen() { + let d = dir("reopen-guard"); + assert!(stuck_reopen(&d).is_empty(), "no marker: nothing is stuck"); + begin_reopen(&d, &[]); + assert!(!d.join(REOPEN_MARKER).exists(), "nothing to reopen writes no marker"); + + let paths = vec!["/d/one.solidcraft".to_string(), "/d/two.solidcraft".to_string()]; + begin_reopen(&d, &paths); + assert_eq!(stuck_reopen(&d), paths); + // The first design opens; the session dies on the second. + reopen_finished(&d, "/d/one.solidcraft"); + assert_eq!(stuck_reopen(&d), vec!["/d/two.solidcraft".to_string()]); + // The skip clears the marker so the launch after it reopens normally. + clear_reopen(&d); + assert!(stuck_reopen(&d).is_empty()); + // A healthy session: every design finishes and the marker goes with the last one. + begin_reopen(&d, &paths); + reopen_finished(&d, "/d/one.solidcraft"); + reopen_finished(&d, "/d/two.solidcraft"); + assert!(stuck_reopen(&d).is_empty() && !d.join(REOPEN_MARKER).exists()); + // A damaged marker is ignored, not fatal. + fs::write(d.join(REOPEN_MARKER), b"not json").unwrap(); + assert!(stuck_reopen(&d).is_empty()); + reopen_finished(&d, "/d/one.solidcraft"); + fs::remove_dir_all(&d).unwrap(); + } } diff --git a/crates/ui-egui/src/documents.rs b/crates/ui-egui/src/documents.rs index aa6877c195..82c21ef2b1 100644 --- a/crates/ui-egui/src/documents.rs +++ b/crates/ui-egui/src/documents.rs @@ -769,6 +769,25 @@ pub fn designs_to_reopen(app: &SolveApp) -> Vec { app.preferences.open_designs.iter().filter(|p| std::path::Path::new(p.as_str()).is_file()).take(MAX_OPEN).cloned().collect() } +/// What this launch reopens (#5542), with the crash guard applied (#5064): when the recovery +/// folder still holds the marker of a session that died mid-reopen, nothing is reopened this +/// time and the designs it died on come back as the reason, so the app starts usable instead +/// of hanging on the same design every launch. The marker is written here and cleared as each +/// design finishes opening (`file_load`), so a healthy session leaves nothing behind. +pub fn reopen_plan(app: &SolveApp, dir: Option<&std::path::Path>) -> (Vec, Vec) { + let Some(dir) = dir else { + return (designs_to_reopen(app), Vec::new()); + }; + let stuck = solidcraft_engine::recovery::stuck_reopen(dir); + if !stuck.is_empty() { + solidcraft_engine::recovery::clear_reopen(dir); + return (Vec::new(), stuck); + } + let reopen = designs_to_reopen(app); + solidcraft_engine::recovery::begin_reopen(dir, &reopen); + (reopen, Vec::new()) +} + /// The control channel's `ui.documents`: list, `new`, `switch`, `close` (by index; `force` /// closes without asking), `close_others` (all but `index`), `close_all`, `move` (`from`, `to`) /// or `quit`; `answer`: save | dont_save | cancel answers "save changes?"; `open_elsewhere`: @@ -1751,6 +1770,35 @@ mod tests { assert!(designs_to_reopen(&a).is_empty(), "a file that is gone is skipped"); } + /// #5064: a design that kills the app while it is being reopened is skipped once, with the + /// reason said, and the launch after that reopens normally again. + #[test] + fn reopen_plan_skips_what_killed_the_last_session() { + let dir = std::env::temp_dir().join(format!("solidcraft-reopen-plan-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + let file = dir.join("part.solidcraft"); + std::fs::write(&file, []).unwrap(); + let path = file.to_string_lossy().to_string(); + let mut a = app(); + a.preferences.open_designs = vec![path.clone()]; + + // A normal launch: it reopens the design and marks that it is doing so. + assert_eq!(reopen_plan(&a, Some(&dir)), (vec![path.clone()], Vec::new())); + // The session dies while opening it: the marker survives, so the next launch skips it + // and says what it skipped. + assert_eq!( + reopen_plan(&a, Some(&dir)), + (Vec::new(), vec![path.clone()]), + "the design that killed the last session is skipped, with the reason" + ); + // The skip cleared the marker: the launch after it works normally again. + assert_eq!(reopen_plan(&a, Some(&dir)), (vec![path.clone()], Vec::new())); + // No recovery folder on this system: the guard is off, the plain list is used. + assert_eq!(reopen_plan(&a, None).0, vec![path]); + let _ = std::fs::remove_dir_all(&dir); + } + /// Cancel, Esc's answer, or a Save that saves nothing (a never-saved design whose Save As /// picker is cancelled) stay open with the changes. #[test] diff --git a/crates/ui-egui/src/file_load.rs b/crates/ui-egui/src/file_load.rs index a99359878e..5b5468a5e0 100644 --- a/crates/ui-egui/src/file_load.rs +++ b/crates/ui-egui/src/file_load.rs @@ -1127,6 +1127,13 @@ pub fn tick(app: &mut SolveApp, ctx: &egui::Context) { } } } + // #5064: opened or not, this design is no longer a reopen the session could die on; + // it comes off the marker the launch wrote before reopening anything. + if request.kind == Kind::Open + && let Some(dir) = &app.reopen_guard_dir + { + solidcraft_engine::recovery::reopen_finished(dir, &request.path); + } } #[cfg(not(target_arch = "wasm32"))] start(app); diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 2500c7892d..231937e59a 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -677,6 +677,9 @@ pub struct SolveApp { pub network_prompt: Option, /// Designs left by crashed apps, offered for recovery. pub recoverable: Vec, + /// The recovery folder holding this launch's reopen-guard marker (#5064); `None` when + /// there is no recovery folder. A reopened design that finishes opening is taken off it. + pub reopen_guard_dir: Option, /// Discard all was clicked in the recovery window: its confirmation is showing (#9299). pub recovery_discard_all: bool, /// The selection's measurement in the view's corner (#7000). @@ -885,6 +888,7 @@ impl SolveApp { disk_prompt: false, network_prompt: None, recoverable: Vec::new(), + reopen_guard_dir: None, recovery_discard_all: false, quick_measure: Default::default(), stand_up: None, diff --git a/docs/book/src/guide/troubleshooting.md b/docs/book/src/guide/troubleshooting.md index 685ee7ca21..8bfd43ae85 100644 --- a/docs/book/src/guide/troubleshooting.md +++ b/docs/book/src/guide/troubleshooting.md @@ -23,6 +23,11 @@ uses only the software renderer and keeps your settings. ## A design won't open or hangs while opening +- **A design that stopped the app while it was being reopened is skipped once.** If SolidCraft + crashes or hangs while reopening the designs from the last session, the next start does not + reopen them: it says which design the last session stopped on and opens the start page + instead, so the app stays usable. Open the design yourself if you want it; the skip clears + itself either way. - **Open the previous save.** Every save keeps the version it replaced next to it as `.bak` (see [Files](files.md)). Copy it to a new name ending in `.sc`, for example `bracket-previous.sc`, and open that. Work saved after the previous save is