From 2acc4f79789871adb0db5b2865ebd7a4ec635e94 Mon Sep 17 00:00:00 2001 From: Hakan Seven Date: Mon, 27 Jul 2026 13:13:58 +0300 Subject: [PATCH] fix(io): guard concurrent drawing saves Hold edit leases and compare disk fingerprints before replacement so another editor cannot be silently overwritten. Offer retry, reload, Save As, or explicit overwrite recovery. Closes #498 --- src/app/automation.rs | 9 +- src/app/commands/fileops.rs | 21 +- src/app/document.rs | 17 ++ src/app/mod.rs | 71 ++++- src/app/update/dialog.rs | 1 + src/app/update/file.rs | 544 +++++++++++++++++++++++++++++++++++- src/app/update/mod.rs | 40 +++ src/app/view/mod.rs | 4 + src/app/view/modal.rs | 300 +++++++++++++++++++- src/io/edit_lock.rs | 254 +++++++++++++++++ src/io/mod.rs | 158 ++++++++++- src/ui/command_line.rs | 28 +- 12 files changed, 1424 insertions(+), 23 deletions(-) create mode 100644 src/io/edit_lock.rs diff --git a/src/app/automation.rs b/src/app/automation.rs index 75aa7362d..ffe9a0b1b 100644 --- a/src/app/automation.rs +++ b/src/app/automation.rs @@ -350,9 +350,14 @@ impl OpenCADStudio { let Some(path) = path else { return err("save: no \"path\" and the document has none"); }; - match crate::io::save(&self.tabs[i].scene.document, &path) { + #[cfg(not(target_arch = "wasm32"))] + let result = + self.save_tab_synchronously_protected(i, path.clone(), true); + #[cfg(target_arch = "wasm32")] + let result = crate::io::save(&self.tabs[i].scene.document, &path) + .map_err(crate::io::SaveFailure::other); + match result { Ok(()) => { - self.tabs[i].current_path = Some(path.clone()); json!({ "ok": true, "saved": path.to_string_lossy() }) } Err(e) => err(format!("save: {e}")), diff --git a/src/app/commands/fileops.rs b/src/app/commands/fileops.rs index c60fc8207..00b95311d 100644 --- a/src/app/commands/fileops.rs +++ b/src/app/commands/fileops.rs @@ -22,13 +22,24 @@ impl OpenCADStudio { if self.tabs[t].is_start { continue; } - self.sync_vport_display(t); if let Some(path) = self.tabs[t].current_path.clone() { - self.tabs[t].scene.document.header.user_real1 = - self.tabs[t].scene.annotation_scale as f64; - match crate::io::save(&self.tabs[t].scene.document, &path) { + if self + .active_save_jobs + .contains_key(&self.tabs[t].id) + { + self.command_line.push_info(&format!( + "SAVEALL: {} already has a save running", + path.display() + )); + skipped += 1; + continue; + } + match self.save_tab_synchronously_protected( + t, + path.clone(), + false, + ) { Ok(()) => { - self.tabs[t].dirty = false; saved += 1; } Err(e) => self.command_line.push_error(&format!( diff --git a/src/app/document.rs b/src/app/document.rs index 091a7ba79..2c2997c1b 100644 --- a/src/app/document.rs +++ b/src/app/document.rs @@ -162,6 +162,17 @@ pub(super) struct DocumentTab { pub(super) id: u64, pub(super) scene: Scene, pub(super) current_path: Option, + /// Persistent native edit lease for the drawing currently at + /// `current_path`. The sidecar portion survives atomic file replacement. + #[cfg(not(target_arch = "wasm32"))] + pub(super) edit_lease: Option, + /// True when another editor owns the drawing lease. Direct Save stays + /// blocked until Retry acquires it or the user chooses Save As. + #[cfg(not(target_arch = "wasm32"))] + pub(super) edit_lock_conflict: bool, + /// Disk state observed after open or the latest successful save. + #[cfg(not(target_arch = "wasm32"))] + pub(super) disk_fingerprint: Option, pub(super) dirty: bool, /// Monotonic committed-edit/undo/redo revision. Background save completion /// uses it with scene epochs so an older snapshot never clears newer work. @@ -429,6 +440,12 @@ impl DocumentTab { id: NEXT_DOCUMENT_TAB_ID.fetch_add(1, Ordering::Relaxed), scene, current_path: None, + #[cfg(not(target_arch = "wasm32"))] + edit_lease: None, + #[cfg(not(target_arch = "wasm32"))] + edit_lock_conflict: false, + #[cfg(not(target_arch = "wasm32"))] + disk_fingerprint: None, dirty: false, edit_revision: 0, prev_selection: Vec::new(), diff --git a/src/app/mod.rs b/src/app/mod.rs index 71f90e574..a5edc5196 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -188,6 +188,10 @@ pub struct OpenProgress { pub size_bytes: u64, pub state: Arc, pub started: Instant, + /// Disk state captured before parsing starts. If another editor changes the + /// file while it loads, the first Save must not silently overwrite it. + #[cfg(not(target_arch = "wasm32"))] + pub fingerprint: Option, } // ── Application state ────────────────────────────────────────────────────── @@ -765,6 +769,16 @@ pub(super) struct OpenCADStudio { active_save_jobs: std::collections::HashMap, #[cfg(not(target_arch = "wasm32"))] save_job_serial: u64, + /// Destination leases held while Save As workers are active. + #[cfg(not(target_arch = "wasm32"))] + pending_save_leases: + std::collections::HashMap, + /// Locked-file failure currently shown in the recovery dialog. + #[cfg(not(target_arch = "wasm32"))] + pending_save_failure: Option, + /// Save stopped because the drawing changed outside this editor. + #[cfg(not(target_arch = "wasm32"))] + pending_external_change: Option, /// OS window for the unsaved-changes confirmation dialog. // ── Custom Save-As dialog ───────────────────────────────────────────── @@ -889,6 +903,29 @@ pub(super) enum SaveContinuation { Quit, } +#[derive(Debug, Clone)] +#[cfg(not(target_arch = "wasm32"))] +pub(super) struct PendingSaveFailure { + tab_id: u64, + path: PathBuf, + version: acadrust::DxfVersion, + purpose: SavePurpose, + continuation: SaveContinuation, + set_current_path: bool, + error: String, +} + +#[derive(Debug, Clone)] +#[cfg(not(target_arch = "wasm32"))] +pub(super) struct PendingExternalChange { + tab_id: u64, + path: PathBuf, + version: acadrust::DxfVersion, + purpose: SavePurpose, + continuation: SaveContinuation, + set_current_path: bool, +} + #[derive(Debug, Clone, Copy, PartialEq, Eq)] #[cfg(not(target_arch = "wasm32"))] pub(super) struct ThumbnailCacheKey { @@ -908,13 +945,14 @@ pub struct SaveOutcome { revision: u64, camera_generation: u64, path: PathBuf, + version: acadrust::DxfVersion, previous_autosave: Option, set_current_path: bool, purpose: SavePurpose, continuation: SaveContinuation, thumbnail_key: Option, refreshed_preview: Option>, - result: Result<(), String>, + result: Result<(), crate::io::SaveFailure>, } /// Where a colour chosen in the standalone palette window should be applied. @@ -1262,6 +1300,10 @@ pub enum ModalKind { Unsaved, SaveDialog, AecDropWarning, + #[cfg(not(target_arch = "wasm32"))] + FileInUse, + #[cfg(not(target_arch = "wasm32"))] + ExternalChange, #[cfg_attr(target_arch = "wasm32", allow(dead_code))] AssocPrompt, PointStyle, @@ -1490,6 +1532,27 @@ pub enum Message { /// Native background save/autosave completed. #[cfg(not(target_arch = "wasm32"))] SaveFinished(SaveOutcome), + /// Retry the failed save after the other application releases the file. + #[cfg(not(target_arch = "wasm32"))] + SaveFileInUseRetry, + /// Choose a different destination for the failed save. + #[cfg(not(target_arch = "wasm32"))] + SaveFileInUseSaveAs, + /// Cancel the failed save and any pending close/quit continuation. + #[cfg(not(target_arch = "wasm32"))] + SaveFileInUseCancel, + /// Reload the externally changed drawing and discard local edits. + #[cfg(not(target_arch = "wasm32"))] + ExternalChangeReload, + /// Keep local edits but choose a different destination. + #[cfg(not(target_arch = "wasm32"))] + ExternalChangeSaveAs, + /// Replace the externally changed disk copy with the local drawing. + #[cfg(not(target_arch = "wasm32"))] + ExternalChangeOverwrite, + /// Cancel the conflicted save and any pending close/quit continuation. + #[cfg(not(target_arch = "wasm32"))] + ExternalChangeCancel, // ───────────────────────────────────────────────────────────────────── CommandInput(String), CommandSubmit, @@ -2537,6 +2600,12 @@ impl OpenCADStudio { active_save_jobs: std::collections::HashMap::new(), #[cfg(not(target_arch = "wasm32"))] save_job_serial: 0, + #[cfg(not(target_arch = "wasm32"))] + pending_save_leases: std::collections::HashMap::new(), + #[cfg(not(target_arch = "wasm32"))] + pending_save_failure: None, + #[cfg(not(target_arch = "wasm32"))] + pending_external_change: None, save_dialog_format: "DWG 2018".to_string(), save_dialog_filename: "drawing.dwg".to_string(), save_dialog_for_unsaved: false, diff --git a/src/app/update/dialog.rs b/src/app/update/dialog.rs index 7e85c6d45..9792ecf58 100644 --- a/src/app/update/dialog.rs +++ b/src/app/update/dialog.rs @@ -272,6 +272,7 @@ pub(super) fn on_ribbon_tool_click(&mut self, tool_id: String, event: ModuleEven crate::app::SavePurpose::Manual, continuation, false, + true, ); return Task::batch([close, save]); } diff --git a/src/app/update/file.rs b/src/app/update/file.rs index 2f7b2001d..daa3516a5 100644 --- a/src/app/update/file.rs +++ b/src/app/update/file.rs @@ -43,6 +43,17 @@ where } } +#[cfg(not(target_arch = "wasm32"))] +fn native_paths_match(left: &std::path::Path, right: &std::path::Path) -> bool { + match ( + std::fs::canonicalize(left), + std::fs::canonicalize(right), + ) { + (Ok(left), Ok(right)) => left == right, + _ => left == right, + } +} + impl OpenCADStudio { /// Before a save, give every cached truck solid that still has no ACIS /// geometry (EXTRUDE/REVOLVE/SWEEP/LOFT/boolean results) an exact modeler @@ -390,7 +401,7 @@ pub(super) fn on_open_file(&mut self) -> Task { self.opening = Some(crate::app::OpenProgress { name: "Opening…".into(), size_bytes: 0, - state: state.clone(), + state: state.clone(), started: Instant::now(), }); Task::perform(crate::io::pick_and_load_web(state), Message::FileOpened) @@ -426,6 +437,233 @@ pub(super) fn on_open_file(&mut self) -> Task { } } + #[cfg(not(target_arch = "wasm32"))] + fn install_native_edit_guard( + &mut self, + i: usize, + path: &std::path::Path, + loaded_fingerprint: Option, + ) { + let current_fingerprint = + crate::io::edit_lock::FileFingerprint::capture(path).ok(); + if loaded_fingerprint + .as_ref() + .zip(current_fingerprint.as_ref()) + .is_some_and(|(loaded, current)| loaded != current) + { + self.command_line.push_error_once( + "Drawing changed on disk while it was opening; Save will require conflict resolution.", + ); + } + self.tabs[i].disk_fingerprint = + loaded_fingerprint.or(current_fingerprint); + match crate::io::edit_lock::EditLease::acquire(path) { + Ok(lease) => { + if let Some(warning) = lease.platform_warning() { + self.command_line.push_info(&format!( + "Edit lease active; {warning}. External-change checks remain active." + )); + } + self.tabs[i].edit_lease = Some(lease); + self.tabs[i].edit_lock_conflict = false; + } + Err(crate::io::edit_lock::EditLeaseError::Locked(error)) => { + self.tabs[i].edit_lease = None; + self.tabs[i].edit_lock_conflict = true; + self.command_line.push_error_once(&format!( + "Opened read-only against other editors: {error}" + )); + } + Err(crate::io::edit_lock::EditLeaseError::Unavailable(error)) => { + self.tabs[i].edit_lease = None; + self.tabs[i].edit_lock_conflict = false; + self.command_line.push_info(&format!( + "{error}. External-change checks remain active." + )); + } + } + } + + #[cfg(not(target_arch = "wasm32"))] + pub(in crate::app) fn refresh_native_edit_guard_after_save( + &mut self, + i: usize, + path: &std::path::Path, + path_changed: bool, + destination_lease: Option, + ) { + if path_changed { + self.tabs[i].edit_lease = None; + self.tabs[i].edit_lease = destination_lease; + self.tabs[i].edit_lock_conflict = false; + if self.tabs[i].edit_lease.is_none() { + self.install_native_edit_guard(i, path, None); + return; + } + } + + let refresh = self.tabs[i].edit_lease.as_mut().map(|lease| { + lease + .refresh_drawing_lock(path) + .map(|_| lease.platform_warning().map(str::to_owned)) + }); + match refresh { + Some(Ok(warning)) => { + self.tabs[i].edit_lock_conflict = false; + if let Some(warning) = warning { + self.command_line.push_info(&format!( + "{warning}. External-change checks remain active." + )); + } + } + Some(Err(crate::io::edit_lock::EditLeaseError::Locked(error))) => { + self.tabs[i].edit_lock_conflict = true; + self.command_line.push_error_once(&format!( + "Saved, but the refreshed drawing is locked by another editor: {error}" + )); + } + Some(Err(crate::io::edit_lock::EditLeaseError::Unavailable(error))) => { + self.tabs[i].edit_lock_conflict = false; + self.command_line.push_info(&format!( + "{error}. External-change checks remain active." + )); + } + None => self.install_native_edit_guard(i, path, None), + } + self.tabs[i].disk_fingerprint = + crate::io::edit_lock::FileFingerprint::capture(path).ok(); + } + + #[cfg(not(target_arch = "wasm32"))] + pub(in crate::app) fn save_tab_synchronously_protected( + &mut self, + i: usize, + path: std::path::PathBuf, + set_current_path: bool, + ) -> Result<(), crate::io::SaveFailure> { + let path_changed = self.tabs[i] + .current_path + .as_deref() + .is_none_or(|current| !native_paths_match(current, &path)); + if !path_changed && self.tabs[i].edit_lock_conflict { + return Err(crate::io::SaveFailure::file_in_use( + "drawing edit lock is held by another editor", + )); + } + + let destination_lease = if path_changed { + match crate::io::edit_lock::EditLease::acquire(&path) { + Ok(lease) => Some(lease), + Err(crate::io::edit_lock::EditLeaseError::Locked(error)) => { + return Err(crate::io::SaveFailure::file_in_use(error)); + } + Err(crate::io::edit_lock::EditLeaseError::Unavailable(error)) => { + self.command_line.push_info(&format!( + "{error}. External-change checks remain active." + )); + None + } + } + } else { + None + }; + + let expected_fingerprint = if path_changed { + match crate::io::edit_lock::FileFingerprint::capture(&path) { + Ok(fingerprint) => Some(fingerprint), + Err(error) if error.kind() == std::io::ErrorKind::NotFound => None, + Err(error) => { + return Err(crate::io::SaveFailure::other(format!( + "could not verify {} before saving: {error}", + path.display() + ))); + } + } + } else { + self.tabs[i].disk_fingerprint.clone().or_else(|| { + crate::io::edit_lock::FileFingerprint::capture(&path).ok() + }) + }; + + self.prepare_native_save(i); + let version = self.tabs[i].scene.document.version; + let snapshot = self.tabs[i].scene.document.clone(); + crate::io::save_owned_as_version_atomic( + snapshot, + &path, + version, + self.backup_on_save, + expected_fingerprint, + )?; + + if set_current_path { + self.tabs[i].current_path = Some(path.clone()); + } + self.refresh_native_edit_guard_after_save( + i, + &path, + path_changed, + destination_lease, + ); + self.tabs[i].dirty = false; + let _ = std::fs::remove_file(path.with_extension("sv$")); + Ok(()) + } + + #[cfg(not(target_arch = "wasm32"))] + fn retry_native_edit_guard( + &mut self, + i: usize, + path: &std::path::Path, + ) -> Result<(), String> { + if let Some(lease) = self.tabs[i].edit_lease.as_mut() { + match lease.refresh_drawing_lock(path) { + Ok(()) => { + let warning = lease.platform_warning().map(str::to_owned); + self.tabs[i].edit_lock_conflict = false; + if let Some(warning) = warning { + self.command_line.push_info(&format!( + "{warning}. External-change checks remain active." + )); + } + return Ok(()); + } + Err(crate::io::edit_lock::EditLeaseError::Locked(error)) => { + return Err(error); + } + Err(crate::io::edit_lock::EditLeaseError::Unavailable(error)) => { + self.tabs[i].edit_lock_conflict = false; + self.command_line.push_info(&format!( + "{error}. External-change checks remain active." + )); + return Ok(()); + } + } + } + + match crate::io::edit_lock::EditLease::acquire(path) { + Ok(lease) => { + let warning = lease.platform_warning().map(str::to_owned); + self.tabs[i].edit_lease = Some(lease); + self.tabs[i].edit_lock_conflict = false; + if let Some(warning) = warning { + self.command_line.push_info(&format!( + "{warning}. External-change checks remain active." + )); + } + Ok(()) + } + Err(crate::io::edit_lock::EditLeaseError::Locked(error)) => Err(error), + Err(crate::io::edit_lock::EditLeaseError::Unavailable(error)) => { + self.tabs[i].edit_lock_conflict = false; + self.command_line.push_info(&format!( + "{error}. External-change checks remain active." + )); + Ok(()) + } + } + } + pub(super) fn on_file_opened(&mut self, name: String, path: std::path::PathBuf, doc: acadrust::CadDocument, mut caches: crate::scene::DerivedCaches, ) -> Task { @@ -490,7 +728,14 @@ pub(super) fn on_open_file(&mut self) -> Task { idx }; + #[cfg(not(target_arch = "wasm32"))] + let opened_fingerprint = self + .opening + .as_ref() + .and_then(|opening| opening.fingerprint.clone()); self.tabs[i].current_path = Some(path.clone()); + #[cfg(not(target_arch = "wasm32"))] + self.install_native_edit_guard(i, &path, opened_fingerprint); self.tabs[i].scene.material_base_dir = path.parent().map(std::path::Path::to_path_buf); self.tabs[i].scene.document = doc; @@ -749,6 +994,7 @@ pub(super) fn on_open_file(&mut self) -> Task { purpose: crate::app::SavePurpose, continuation: crate::app::SaveContinuation, set_current_path: bool, + check_external_change: bool, ) -> Task { let tab_id = self.tabs[i].id; if self.active_save_jobs.contains_key(&tab_id) { @@ -758,6 +1004,67 @@ pub(super) fn on_open_file(&mut self) -> Task { } return Task::none(); } + if purpose != crate::app::SavePurpose::Autosave + && !set_current_path + && self.tabs[i].edit_lock_conflict + { + let error = "Drawing edit lock is held by another editor.".to_string(); + self.command_line.push_error_once(&format!( + "Unable to save \"{}\": {error}", + path.file_name() + .map(|name| name.to_string_lossy().into_owned()) + .unwrap_or_else(|| path.display().to_string()) + )); + self.pending_save_failure = Some(crate::app::PendingSaveFailure { + tab_id, + path, + version, + purpose, + continuation, + set_current_path, + error, + }); + self.restore_failed_save_continuation(continuation, i); + self.active_modal = Some(crate::app::ModalKind::FileInUse); + return Task::none(); + } + + let destination_is_current = self.tabs[i] + .current_path + .as_deref() + .is_some_and(|current| native_paths_match(current, &path)); + if set_current_path && !destination_is_current { + match crate::io::edit_lock::EditLease::acquire(&path) { + Ok(lease) => { + self.pending_save_leases.insert(tab_id, lease); + } + Err(crate::io::edit_lock::EditLeaseError::Locked(error)) => { + self.command_line.push_error_once(&format!( + "Unable to save \"{}\": {error}", + path.file_name() + .map(|name| name.to_string_lossy().into_owned()) + .unwrap_or_else(|| path.display().to_string()) + )); + self.pending_save_failure = Some(crate::app::PendingSaveFailure { + tab_id, + path, + version, + purpose, + continuation, + set_current_path, + error, + }); + self.restore_failed_save_continuation(continuation, i); + self.active_modal = Some(crate::app::ModalKind::FileInUse); + return Task::none(); + } + Err(crate::io::edit_lock::EditLeaseError::Unavailable(error)) => { + self.command_line.push_info(&format!( + "{error}. External-change checks remain active." + )); + } + } + } let epoch = self.tabs[i].scene.geometry_epoch; let revision = self.tabs[i].edit_revision; @@ -803,6 +1110,16 @@ pub(super) fn on_open_file(&mut self) -> Task { self.active_save_jobs.insert(tab_id, job_id); let previous_autosave = set_current_path.then(|| self.autosave_target(i)); let backup = purpose != crate::app::SavePurpose::Autosave && self.backup_on_save; + let expected_fingerprint = + if check_external_change && purpose != crate::app::SavePurpose::Autosave { + if set_current_path { + crate::io::edit_lock::FileFingerprint::capture(&path).ok() + } else { + self.tabs[i].disk_fingerprint.clone() + } + } else { + None + }; let worker_path = path.clone(); Task::perform( @@ -832,11 +1149,17 @@ pub(super) fn on_open_file(&mut self) -> Task { &worker_path, version, backup, + expected_fingerprint, ); (result, refreshed_preview) }) .join() - .unwrap_or_else(|_| (Err("save worker panicked".to_string()), None)); + .unwrap_or_else(|_| { + ( + Err(crate::io::SaveFailure::other("save worker panicked")), + None, + ) + }); crate::app::SaveOutcome { job_id, tab_id, @@ -844,6 +1167,7 @@ pub(super) fn on_open_file(&mut self) -> Task { revision, camera_generation, path, + version, previous_autosave, set_current_path, purpose, @@ -867,6 +1191,7 @@ pub(super) fn on_open_file(&mut self) -> Task { if latest { self.active_save_jobs.remove(&outcome.tab_id); } + let destination_lease = self.pending_save_leases.remove(&outcome.tab_id); let Some(i) = self.tabs.iter().position(|tab| tab.id == outcome.tab_id) else { if outcome.purpose == crate::app::SavePurpose::Autosave { @@ -881,7 +1206,60 @@ pub(super) fn on_open_file(&mut self) -> Task { return Task::none(); } - if let Err(error) = outcome.result { + if let Err(error) = &outcome.result { + if error.externally_modified + && outcome.purpose != crate::app::SavePurpose::Autosave + { + let file_name = outcome + .path + .file_name() + .map(|name| name.to_string_lossy().into_owned()) + .unwrap_or_else(|| outcome.path.display().to_string()); + self.command_line.push_error_once(&format!( + "Save stopped: \"{file_name}\" changed outside Open CAD Studio." + )); + self.pending_external_change = Some(crate::app::PendingExternalChange { + tab_id: outcome.tab_id, + path: outcome.path.clone(), + version: outcome.version, + purpose: outcome.purpose, + continuation: outcome.continuation, + set_current_path: outcome.set_current_path, + }); + self.restore_failed_save_continuation(outcome.continuation, i); + self.active_modal = Some(crate::app::ModalKind::ExternalChange); + return Task::none(); + } + if error.file_in_use && outcome.purpose != crate::app::SavePurpose::Autosave { + let file_name = outcome + .path + .file_name() + .map(|name| name.to_string_lossy().into_owned()) + .unwrap_or_else(|| outcome.path.display().to_string()); + self.command_line.push_error_once(&format!( + "Unable to save \"{file_name}\": file is in use by another application." + )); + self.pending_save_failure = Some(crate::app::PendingSaveFailure { + tab_id: outcome.tab_id, + path: outcome.path.clone(), + version: outcome.version, + purpose: outcome.purpose, + continuation: outcome.continuation, + set_current_path: outcome.set_current_path, + error: error.to_string(), + }); + match outcome.continuation { + crate::app::SaveContinuation::CloseTab => { + self.pending_close = Some(crate::app::PendingClose::Tab(i)); + } + crate::app::SaveContinuation::Quit => { + self.pending_close = Some(crate::app::PendingClose::Quit); + } + crate::app::SaveContinuation::None => {} + } + self.active_modal = Some(crate::app::ModalKind::FileInUse); + return Task::none(); + } self.command_line .push_error(&format!("Save failed: {error}")); return match outcome.continuation { @@ -912,6 +1290,13 @@ pub(super) fn on_open_file(&mut self) -> Task { self.command_line.push_output("Autosaved 1 drawing"); } crate::app::SavePurpose::Manual | crate::app::SavePurpose::SaveAs => { + let path_changed = outcome.set_current_path + && self.tabs[i] + .current_path + .as_deref() + .is_none_or(|current| { + !native_paths_match(current, &outcome.path) + }); self.command_line .push_output(&format!("Saved: {}", outcome.path.display())); self.recent_thumbs.remove(&outcome.path); @@ -924,6 +1309,12 @@ pub(super) fn on_open_file(&mut self) -> Task { self.tabs[i].current_path = Some(outcome.path.clone()); tasks.push(self.push_recent(outcome.path.clone())); } + self.refresh_native_edit_guard_after_save( + i, + &outcome.path, + path_changed, + destination_lease, + ); if snapshot_is_current { self.tabs[i].dirty = false; let _ = std::fs::remove_file(outcome.path.with_extension("sv$")); @@ -961,6 +1352,150 @@ pub(super) fn on_open_file(&mut self) -> Task { Task::batch(tasks) } + #[cfg(not(target_arch = "wasm32"))] + fn restore_failed_save_continuation( + &mut self, + continuation: crate::app::SaveContinuation, + tab_idx: usize, + ) { + self.pending_close = match continuation { + crate::app::SaveContinuation::None => None, + crate::app::SaveContinuation::CloseTab => { + Some(crate::app::PendingClose::Tab(tab_idx)) + } + crate::app::SaveContinuation::Quit => Some(crate::app::PendingClose::Quit), + }; + } + + #[cfg(not(target_arch = "wasm32"))] + pub(super) fn on_save_file_in_use_retry(&mut self) -> Task { + let Some(mut failure) = self.pending_save_failure.take() else { + self.close_active_modal(); + return Task::none(); + }; + let Some(i) = self.tabs.iter().position(|tab| tab.id == failure.tab_id) else { + self.close_active_modal(); + return Task::none(); + }; + if self.tabs[i].edit_lock_conflict { + if let Err(error) = self.retry_native_edit_guard(i, &failure.path) { + let continuation = failure.continuation; + failure.error = error.clone(); + self.pending_save_failure = Some(failure); + self.restore_failed_save_continuation(continuation, i); + self.command_line + .push_error_once(&format!("Unable to acquire edit lock: {error}")); + self.active_modal = Some(crate::app::ModalKind::FileInUse); + return Task::none(); + } + } + self.close_active_modal(); + self.restore_failed_save_continuation(failure.continuation, i); + self.active_tab = i; + self.prepare_native_save(i); + self.queue_native_save( + i, + failure.path, + failure.version, + failure.purpose, + failure.continuation, + failure.set_current_path, + true, + ) + } + + #[cfg(not(target_arch = "wasm32"))] + pub(super) fn on_save_file_in_use_save_as(&mut self) -> Task { + let Some(failure) = self.pending_save_failure.take() else { + self.close_active_modal(); + return Task::none(); + }; + let Some(i) = self.tabs.iter().position(|tab| tab.id == failure.tab_id) else { + self.close_active_modal(); + return Task::none(); + }; + self.close_active_modal(); + self.restore_failed_save_continuation(failure.continuation, i); + self.active_tab = i; + self.save_dialog_for_unsaved = + failure.continuation != crate::app::SaveContinuation::None; + self.open_save_dialog_window(i) + } + + #[cfg(not(target_arch = "wasm32"))] + pub(super) fn on_external_change_overwrite(&mut self) -> Task { + let Some(conflict) = self.pending_external_change.take() else { + self.close_active_modal(); + return Task::none(); + }; + let Some(i) = self.tabs.iter().position(|tab| tab.id == conflict.tab_id) else { + self.close_active_modal(); + return Task::none(); + }; + self.close_active_modal(); + self.restore_failed_save_continuation(conflict.continuation, i); + self.active_tab = i; + self.prepare_native_save(i); + self.queue_native_save( + i, + conflict.path, + conflict.version, + conflict.purpose, + conflict.continuation, + conflict.set_current_path, + false, + ) + } + + #[cfg(not(target_arch = "wasm32"))] + pub(super) fn on_external_change_save_as(&mut self) -> Task { + let Some(conflict) = self.pending_external_change.take() else { + self.close_active_modal(); + return Task::none(); + }; + let Some(i) = self.tabs.iter().position(|tab| tab.id == conflict.tab_id) else { + self.close_active_modal(); + return Task::none(); + }; + self.close_active_modal(); + self.restore_failed_save_continuation(conflict.continuation, i); + self.active_tab = i; + self.save_dialog_for_unsaved = + conflict.continuation != crate::app::SaveContinuation::None; + self.open_save_dialog_window(i) + } + + #[cfg(not(target_arch = "wasm32"))] + pub(super) fn on_external_change_reload(&mut self) -> Task { + let Some(conflict) = self.pending_external_change.take() else { + self.close_active_modal(); + return Task::none(); + }; + let Some(i) = self.tabs.iter().position(|tab| tab.id == conflict.tab_id) else { + self.close_active_modal(); + return Task::none(); + }; + let metadata = match std::fs::metadata(&conflict.path) { + Ok(metadata) => metadata, + Err(error) => { + self.close_active_modal(); + self.command_line + .push_error(&format!("Reload failed: {error}")); + return Task::none(); + } + }; + self.close_active_modal(); + self.pending_close = None; + self.tab_counter += 1; + self.tabs[i] = crate::app::document::DocumentTab::new_drawing(self.tab_counter); + self.active_tab = i; + self.apply_bg_default(i); + self.update(Message::OpenPathPicked(Some(( + conflict.path, + metadata.len(), + )))) + } + pub(super) fn on_save_file(&mut self) -> Task { if self.read_only { self.command_line @@ -989,6 +1524,7 @@ pub(super) fn on_open_file(&mut self) -> Task { crate::app::SavePurpose::Manual, crate::app::SaveContinuation::None, false, + true, ); } self.save_dialog_for_unsaved = false; @@ -1151,6 +1687,7 @@ pub(super) fn on_open_file(&mut self) -> Task { crate::app::SavePurpose::SaveAs, continuation, true, + true, ) } @@ -1219,6 +1756,7 @@ pub(super) fn on_open_file(&mut self) -> Task { crate::app::SavePurpose::Autosave, crate::app::SaveContinuation::None, false, + false, )); } Task::batch(tasks) diff --git a/src/app/update/mod.rs b/src/app/update/mod.rs index 4221eefb2..8dd36de91 100644 --- a/src/app/update/mod.rs +++ b/src/app/update/mod.rs @@ -80,6 +80,16 @@ impl OpenCADStudio { if self.active_modal == Some(ScaleManager) { self.scale_stage_discard(); } + #[cfg(not(target_arch = "wasm32"))] + if self.active_modal == Some(FileInUse) { + self.pending_save_failure = None; + self.pending_close = None; + } + #[cfg(not(target_arch = "wasm32"))] + if self.active_modal == Some(ExternalChange) { + self.pending_external_change = None; + self.pending_close = None; + } match self.active_modal { // Dismissing these via ✕ is the cancel/decline path. Some(Unsaved) => self.pending_close = None, @@ -457,6 +467,9 @@ impl OpenCADStudio { size_bytes, state: progress.clone(), started: Instant::now(), + #[cfg(not(target_arch = "wasm32"))] + fingerprint: + crate::io::edit_lock::FileFingerprint::capture(&path).ok(), }); let size_label = format_size(size_bytes); self.command_line @@ -4067,6 +4080,33 @@ impl OpenCADStudio { #[cfg(not(target_arch = "wasm32"))] Message::SaveFinished(outcome) => self.on_save_finished(outcome), + #[cfg(not(target_arch = "wasm32"))] + Message::SaveFileInUseRetry => self.on_save_file_in_use_retry(), + + #[cfg(not(target_arch = "wasm32"))] + Message::SaveFileInUseSaveAs => self.on_save_file_in_use_save_as(), + + #[cfg(not(target_arch = "wasm32"))] + Message::SaveFileInUseCancel => { + self.close_active_modal(); + Task::none() + } + + #[cfg(not(target_arch = "wasm32"))] + Message::ExternalChangeReload => self.on_external_change_reload(), + + #[cfg(not(target_arch = "wasm32"))] + Message::ExternalChangeSaveAs => self.on_external_change_save_as(), + + #[cfg(not(target_arch = "wasm32"))] + Message::ExternalChangeOverwrite => self.on_external_change_overwrite(), + + #[cfg(not(target_arch = "wasm32"))] + Message::ExternalChangeCancel => { + self.close_active_modal(); + Task::none() + } + // ── Page Setup ──────────────────────────────────────────────── Message::UpdateCheckResult(latest) => { let Some(info) = latest else { diff --git a/src/app/view/mod.rs b/src/app/view/mod.rs index 59a488c35..80f24233b 100644 --- a/src/app/view/mod.rs +++ b/src/app/view/mod.rs @@ -1722,6 +1722,10 @@ impl OpenCADStudio { AssocPrompt => (440, 210), Unsaved => (420, 160), AecDropWarning => (480, 230), + #[cfg(not(target_arch = "wasm32"))] + FileInUse => (560, 250), + #[cfg(not(target_arch = "wasm32"))] + ExternalChange => (620, 250), #[cfg(target_arch = "wasm32")] SaveDialog => (420, 200), #[cfg(not(target_arch = "wasm32"))] diff --git a/src/app/view/modal.rs b/src/app/view/modal.rs index 7d73b8e5d..db1345b05 100644 --- a/src/app/view/modal.rs +++ b/src/app/view/modal.rs @@ -26,6 +26,10 @@ impl OpenCADStudio { Some(K::DimStyle) => "Dimension Style Manager", Some(K::AssocPrompt) => "Default Application", Some(K::AecDropWarning) => "Save Warning", + #[cfg(not(target_arch = "wasm32"))] + Some(K::FileInUse) => "Unable to Save Drawing", + #[cfg(not(target_arch = "wasm32"))] + Some(K::ExternalChange) => "Drawing Changed on Disk", Some(K::LayerDeleteWarning) => "Delete Layer", Some(K::Unsaved) => "Unsaved Changes", Some(K::PointStyle) => "Point Style", @@ -583,6 +587,29 @@ impl OpenCADStudio { 230, ) } + #[cfg(not(target_arch = "wasm32"))] + super::super::ModalKind::FileInUse => { + let (path, error) = self + .pending_save_failure + .as_ref() + .map(|failure| { + ( + failure.path.display().to_string(), + failure.error.clone(), + ) + }) + .unwrap_or_default(); + sized(file_in_use_dialog_window(&path, &error), 560, 250) + } + #[cfg(not(target_arch = "wasm32"))] + super::super::ModalKind::ExternalChange => { + let path = self + .pending_external_change + .as_ref() + .map(|conflict| conflict.path.display().to_string()) + .unwrap_or_default(); + sized(external_change_dialog_window(&path), 620, 250) + } super::super::ModalKind::LayerDeleteWarning => { let (names, count) = self .layer_delete_pending @@ -922,6 +949,278 @@ fn unsaved_changes_dialog_window(name: &str) -> Element<'static, Message> { .into() } +#[cfg(not(target_arch = "wasm32"))] +fn file_in_use_dialog_window(path: &str, error: &str) -> Element<'static, Message> { + const BG: Color = Color { + r: 0.18, + g: 0.18, + b: 0.20, + a: 1.0, + }; + const BORDER_COL: Color = Color { + r: 0.38, + g: 0.38, + b: 0.42, + a: 1.0, + }; + const TEXT_COL: Color = Color { + r: 0.90, + g: 0.90, + b: 0.90, + a: 1.0, + }; + const DIM_COL: Color = Color { + r: 0.62, + g: 0.62, + b: 0.66, + a: 1.0, + }; + const BTN_PRIMARY: Color = Color { + r: 0.20, + g: 0.46, + b: 0.80, + a: 1.0, + }; + const BTN_PRIMARY_HOVER: Color = Color { + r: 0.26, + g: 0.55, + b: 0.92, + a: 1.0, + }; + const BTN_SECONDARY: Color = Color { + r: 0.28, + g: 0.28, + b: 0.30, + a: 1.0, + }; + const BTN_SECONDARY_HOVER: Color = Color { + r: 0.36, + g: 0.36, + b: 0.40, + a: 1.0, + }; + + let file_name = std::path::Path::new(path) + .file_name() + .map(|name| name.to_string_lossy().into_owned()) + .unwrap_or_else(|| "Drawing".to_string()); + let heading = format!("\"{file_name}\" could not be saved."); + let path_line = format!("Path: {path}"); + let details = format!("Details: {error}"); + let btn = |label: &'static str, msg: Message, base: Color, hover: Color| { + button(text(label).size(13).color(TEXT_COL)) + .on_press(msg) + .style(move |_: &Theme, status| button::Style { + background: Some(Background::Color(match status { + button::Status::Hovered | button::Status::Pressed => hover, + _ => base, + })), + text_color: TEXT_COL, + border: Border { + color: BORDER_COL, + width: 1.0, + radius: 4.0.into(), + }, + ..Default::default() + }) + .padding([6, 18]) + }; + + container( + column![ + text(heading).size(14).color(TEXT_COL), + Space::new().height(8), + text( + "The file is open or being used by another application. \ + Close it there and retry, or save this drawing under a different name." + ) + .size(13) + .color(TEXT_COL) + .width(Fill), + Space::new().height(12), + text(path_line).size(11).color(DIM_COL).width(Fill), + Space::new().height(4), + text(details).size(11).color(DIM_COL).width(Fill), + Space::new().height(18), + row![ + btn( + "Retry", + Message::SaveFileInUseRetry, + BTN_PRIMARY, + BTN_PRIMARY_HOVER + ), + Space::new().width(8), + btn( + "Save As", + Message::SaveFileInUseSaveAs, + BTN_SECONDARY, + BTN_SECONDARY_HOVER + ), + Space::new().width(8), + btn( + "Cancel", + Message::SaveFileInUseCancel, + BTN_SECONDARY, + BTN_SECONDARY_HOVER + ), + ], + ] + .spacing(0), + ) + .style(|_: &Theme| container::Style { + background: Some(Background::Color(BG)), + ..Default::default() + }) + .padding([18, 20]) + .width(Fill) + .height(Fill) + .into() +} + +#[cfg(not(target_arch = "wasm32"))] +fn external_change_dialog_window(path: &str) -> Element<'static, Message> { + const BG: Color = Color { + r: 0.18, + g: 0.18, + b: 0.20, + a: 1.0, + }; + const BORDER_COL: Color = Color { + r: 0.38, + g: 0.38, + b: 0.42, + a: 1.0, + }; + const TEXT_COL: Color = Color { + r: 0.90, + g: 0.90, + b: 0.90, + a: 1.0, + }; + const DIM_COL: Color = Color { + r: 0.62, + g: 0.62, + b: 0.66, + a: 1.0, + }; + const BTN_PRIMARY: Color = Color { + r: 0.20, + g: 0.46, + b: 0.80, + a: 1.0, + }; + const BTN_PRIMARY_HOVER: Color = Color { + r: 0.26, + g: 0.55, + b: 0.92, + a: 1.0, + }; + const BTN_SECONDARY: Color = Color { + r: 0.28, + g: 0.28, + b: 0.30, + a: 1.0, + }; + const BTN_SECONDARY_HOVER: Color = Color { + r: 0.36, + g: 0.36, + b: 0.40, + a: 1.0, + }; + const BTN_DANGER: Color = Color { + r: 0.62, + g: 0.31, + b: 0.14, + a: 1.0, + }; + const BTN_DANGER_HOVER: Color = Color { + r: 0.78, + g: 0.39, + b: 0.17, + a: 1.0, + }; + + let file_name = std::path::Path::new(path) + .file_name() + .map(|name| name.to_string_lossy().into_owned()) + .unwrap_or_else(|| "Drawing".to_string()); + let heading = format!("\"{file_name}\" was changed by another application."); + let path_line = format!("Path: {path}"); + let btn = |label: &'static str, msg: Message, base: Color, hover: Color| { + button(text(label).size(13).color(TEXT_COL)) + .on_press(msg) + .style(move |_: &Theme, status| button::Style { + background: Some(Background::Color(match status { + button::Status::Hovered | button::Status::Pressed => hover, + _ => base, + })), + text_color: TEXT_COL, + border: Border { + color: BORDER_COL, + width: 1.0, + radius: 4.0.into(), + }, + ..Default::default() + }) + .padding([6, 14]) + }; + + container( + column![ + text(heading).size(14).color(TEXT_COL), + Space::new().height(8), + text( + "Saving now could destroy those external changes. Reload the disk copy, \ + save your local work elsewhere, or explicitly overwrite it." + ) + .size(13) + .color(TEXT_COL) + .width(Fill), + Space::new().height(12), + text(path_line).size(11).color(DIM_COL).width(Fill), + Space::new().height(18), + row![ + btn( + "Reload from Disk", + Message::ExternalChangeReload, + BTN_PRIMARY, + BTN_PRIMARY_HOVER + ), + Space::new().width(8), + btn( + "Save As", + Message::ExternalChangeSaveAs, + BTN_SECONDARY, + BTN_SECONDARY_HOVER + ), + Space::new().width(8), + btn( + "Overwrite", + Message::ExternalChangeOverwrite, + BTN_DANGER, + BTN_DANGER_HOVER + ), + Space::new().width(8), + btn( + "Cancel", + Message::ExternalChangeCancel, + BTN_SECONDARY, + BTN_SECONDARY_HOVER + ), + ], + ] + .spacing(0), + ) + .style(|_: &Theme| container::Style { + background: Some(Background::Color(BG)), + ..Default::default() + }) + .padding([18, 20]) + .width(Fill) + .height(Fill) + .into() +} + /// Warning shown before a lossy Save-As: the drawing carries unsupported /// (AEC / application) objects that survive only as verbatim source-version /// bytes, so saving to a different version or to DXF would drop them. Offers to @@ -1148,4 +1447,3 @@ fn default_assoc_dialog_window() -> Element<'static, Message> { .padding([24, 28]) .into() } - diff --git a/src/io/edit_lock.rs b/src/io/edit_lock.rs new file mode 100644 index 000000000..512a7c66f --- /dev/null +++ b/src/io/edit_lock.rs @@ -0,0 +1,254 @@ +//! Native drawing edit leases and external-change fingerprints. +//! +//! The sidecar lock survives the editor's atomic temp-file replacement, while +//! the drawing lock interoperates with platform `flock` / `LockFileEx` users. +//! Neither mechanism can force every third-party Unix application to cooperate, +//! so saves also compare [`FileFingerprint`] immediately before replacement. + +use std::fs::{File, OpenOptions, TryLockError}; +use std::hash::{Hash, Hasher}; +use std::io::{Read, Seek, SeekFrom, Write}; +use std::path::{Path, PathBuf}; +use std::time::UNIX_EPOCH; + +const SAMPLE_BYTES: usize = 64 * 1024; + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct FileFingerprint { + len: u64, + modified_ns: Option, + sample_hash: u64, +} + +impl FileFingerprint { + pub fn capture(path: &Path) -> std::io::Result { + let mut file = File::open(path)?; + let metadata = file.metadata()?; + let len = metadata.len(); + let modified_ns = metadata + .modified() + .ok() + .and_then(|time| time.duration_since(UNIX_EPOCH).ok()) + .map(|duration| duration.as_nanos()); + + let mut offsets = vec![0]; + if len > SAMPLE_BYTES as u64 { + offsets.push((len / 2).saturating_sub((SAMPLE_BYTES / 2) as u64)); + offsets.push(len.saturating_sub(SAMPLE_BYTES as u64)); + } + offsets.sort_unstable(); + offsets.dedup(); + + let mut hasher = std::collections::hash_map::DefaultHasher::new(); + len.hash(&mut hasher); + for offset in offsets { + file.seek(SeekFrom::Start(offset))?; + let mut remaining = (len.saturating_sub(offset)).min(SAMPLE_BYTES as u64) as usize; + let mut buffer = vec![0_u8; remaining]; + let mut read = 0; + while remaining > 0 { + let count = file.read(&mut buffer[read..])?; + if count == 0 { + break; + } + read += count; + remaining -= count; + } + offset.hash(&mut hasher); + buffer[..read].hash(&mut hasher); + } + + Ok(Self { + len, + modified_ns, + sample_hash: hasher.finish(), + }) + } +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum EditLeaseError { + Locked(String), + Unavailable(String), +} + +impl std::fmt::Display for EditLeaseError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Locked(message) | Self::Unavailable(message) => f.write_str(message), + } + } +} + +#[derive(Debug)] +pub struct EditLease { + _sidecar: File, + drawing: Option, + lock_path: PathBuf, + platform_warning: Option, +} + +impl EditLease { + pub fn acquire(path: &Path) -> Result { + let canonical = std::fs::canonicalize(path).unwrap_or_else(|_| path.to_path_buf()); + let lock_path = sidecar_path(&canonical); + let mut sidecar = OpenOptions::new() + .read(true) + .write(true) + .create(true) + .open(&lock_path) + .map_err(|error| { + EditLeaseError::Unavailable(format!( + "Could not create edit lock {}: {error}", + lock_path.display() + )) + })?; + + match sidecar.try_lock() { + Ok(()) => {} + Err(TryLockError::WouldBlock) => { + let owner = std::fs::read_to_string(&lock_path) + .ok() + .map(|value| value.trim().to_string()) + .filter(|value| !value.is_empty()) + .unwrap_or_else(|| "another Open CAD Studio session".to_string()); + return Err(EditLeaseError::Locked(format!( + "Drawing edit lock is held by {owner}." + ))); + } + Err(TryLockError::Error(error)) => { + return Err(EditLeaseError::Unavailable(format!( + "Could not acquire edit lock {}: {error}", + lock_path.display() + ))); + } + } + + let _ = sidecar.set_len(0); + let _ = sidecar.seek(SeekFrom::Start(0)); + let _ = writeln!( + sidecar, + "Open CAD Studio pid={} path={}", + std::process::id(), + canonical.display() + ); + let _ = sidecar.flush(); + + let (drawing, platform_warning) = acquire_drawing_lock(&canonical)?; + Ok(Self { + _sidecar: sidecar, + drawing, + lock_path, + platform_warning, + }) + } + + pub fn platform_warning(&self) -> Option<&str> { + self.platform_warning.as_deref() + } + + /// Atomic save replaces the path with a new file object. Move the platform + /// lock to that new object; the sidecar remains locked throughout. + pub fn refresh_drawing_lock(&mut self, path: &Path) -> Result<(), EditLeaseError> { + let (drawing, warning) = acquire_drawing_lock(path)?; + self.drawing = drawing; + self.platform_warning = warning; + Ok(()) + } + + #[allow(dead_code)] + pub fn lock_path(&self) -> &Path { + &self.lock_path + } +} + +fn acquire_drawing_lock(path: &Path) -> Result<(Option, Option), EditLeaseError> { + let drawing = match OpenOptions::new().read(true).write(true).open(path) { + Ok(file) => file, + Err(error) if error_is_lock_conflict(&error) => { + return Err(EditLeaseError::Locked(format!( + "Drawing is open for writing in another application: {error}" + ))); + } + Err(error) => { + return Ok(( + None, + Some(format!("Platform drawing lock unavailable: {error}")), + )); + } + }; + + match drawing.try_lock() { + Ok(()) => Ok((Some(drawing), None)), + Err(TryLockError::WouldBlock) => Err(EditLeaseError::Locked( + "Drawing is locked by another application.".to_string(), + )), + Err(TryLockError::Error(error)) => Ok(( + None, + Some(format!("Platform drawing lock unavailable: {error}")), + )), + } +} + +#[cfg(target_os = "windows")] +fn error_is_lock_conflict(error: &std::io::Error) -> bool { + matches!(error.raw_os_error(), Some(32 | 33)) +} + +#[cfg(not(target_os = "windows"))] +fn error_is_lock_conflict(_error: &std::io::Error) -> bool { + false +} + +fn sidecar_path(path: &Path) -> PathBuf { + let name = path + .file_name() + .map(|value| value.to_string_lossy().into_owned()) + .unwrap_or_else(|| "drawing".to_string()); + path.parent() + .unwrap_or_else(|| Path::new(".")) + .join(format!(".{name}.ocs.lock")) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn unique_path(name: &str) -> PathBuf { + std::env::temp_dir().join(format!( + "ocs_edit_lock_{}_{}_{}", + std::process::id(), + name, + std::time::SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap() + .as_nanos() + )) + } + + #[test] + fn edit_lease_blocks_second_ocs_session() { + let path = unique_path("lease.dwg"); + std::fs::write(&path, b"dwg").unwrap(); + let first = EditLease::acquire(&path).unwrap(); + assert!(matches!( + EditLease::acquire(&path), + Err(EditLeaseError::Locked(_)) + )); + drop(first); + assert!(EditLease::acquire(&path).is_ok()); + let _ = std::fs::remove_file(sidecar_path(&path)); + let _ = std::fs::remove_file(path); + } + + #[test] + fn fingerprint_detects_same_size_content_change() { + let path = unique_path("fingerprint.dwg"); + std::fs::write(&path, b"abcd").unwrap(); + let before = FileFingerprint::capture(&path).unwrap(); + std::fs::write(&path, b"abce").unwrap(); + let after = FileFingerprint::capture(&path).unwrap(); + assert_ne!(before, after); + let _ = std::fs::remove_file(path); + } +} diff --git a/src/io/mod.rs b/src/io/mod.rs index 3c9d3da12..256a24a64 100644 --- a/src/io/mod.rs +++ b/src/io/mod.rs @@ -4,6 +4,8 @@ // Default save format: DWG (AC1032 / R2018+). pub mod file_association; +#[cfg(not(target_arch = "wasm32"))] +pub mod edit_lock; pub mod obj; #[cfg(not(target_arch = "wasm32"))] pub mod single_instance; @@ -550,6 +552,125 @@ pub fn write_backup(path: &std::path::Path) { } } +/// Structured native-save failure. The UI needs the OS error category after +/// the worker completes so file-sharing violations can offer recovery actions +/// instead of being flattened into an opaque command-line string. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct SaveFailure { + pub message: String, + pub file_in_use: bool, + pub externally_modified: bool, +} + +impl SaveFailure { + pub fn other(message: impl Into) -> Self { + Self { + message: message.into(), + file_in_use: false, + externally_modified: false, + } + } + + #[cfg(not(target_arch = "wasm32"))] + pub fn file_in_use(message: impl Into) -> Self { + Self { + message: message.into(), + file_in_use: true, + externally_modified: false, + } + } + + fn replacing(path: &Path, error: std::io::Error) -> Self { + Self { + message: format!("replace {}: {error}", path.display()), + file_in_use: replace_error_is_file_in_use(&error), + externally_modified: false, + } + } + + #[cfg(not(target_arch = "wasm32"))] + fn externally_modified(path: &Path) -> Self { + Self { + message: format!( + "{} changed on disk after it was opened", + path.display() + ), + file_in_use: false, + externally_modified: true, + } + } +} + +impl std::fmt::Display for SaveFailure { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(&self.message) + } +} + +impl std::error::Error for SaveFailure {} + +fn replace_error_is_file_in_use(error: &std::io::Error) -> bool { + #[cfg(target_os = "windows")] + { + // ERROR_SHARING_VIOLATION / ERROR_LOCK_VIOLATION. ReplaceFileW returns + // 32 for the common case where AutoCAD holds the DWG open (#498). + windows_replace_error_is_file_in_use(error.raw_os_error()) + } + #[cfg(not(target_os = "windows"))] + { + // Advisory locks normally do not block rename on Unix, but filesystems + // may still report EBUSY or ETXTBSY for an active destination. + matches!(error.raw_os_error(), Some(16 | 26)) + } +} + +#[cfg(any(target_os = "windows", test))] +fn windows_replace_error_is_file_in_use(raw_os_error: Option) -> bool { + matches!(raw_os_error, Some(32 | 33)) +} + +#[cfg(test)] +mod save_failure_tests { + use super::windows_replace_error_is_file_in_use; + + #[test] + fn issue_498_recognizes_windows_file_sharing_errors() { + assert!(windows_replace_error_is_file_in_use(Some(32))); + assert!(windows_replace_error_is_file_in_use(Some(33))); + assert!(!windows_replace_error_is_file_in_use(Some(5))); + assert!(!windows_replace_error_is_file_in_use(None)); + } + + #[cfg(not(target_arch = "wasm32"))] + #[test] + fn external_change_prevents_atomic_replace() { + let path = std::env::temp_dir().join(format!( + "ocs_external_change_{}_{}.dwg", + std::process::id(), + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_nanos() + )); + std::fs::write(&path, b"old").unwrap(); + let expected = super::edit_lock::FileFingerprint::capture(&path).unwrap(); + std::fs::write(&path, b"new").unwrap(); + + let error = super::save_owned_as_version_atomic( + acadrust::CadDocument::new(), + &path, + acadrust::DxfVersion::AC1032, + false, + Some(expected), + ) + .unwrap_err(); + + assert!(error.externally_modified); + assert_eq!(std::fs::read(&path).unwrap(), b"new"); + let _ = std::fs::remove_file(path); + } +} + // ── Plot Style Table ────────────────────────────────────────────────────── /// Show a file-open dialog and load the selected CTB or STB file. @@ -595,30 +716,44 @@ pub fn save_as_version( let clone_started = iced::time::Instant::now(); let snapshot = doc.clone(); let clone_ms = clone_started.elapsed().as_secs_f64() * 1000.0; - save_owned_as_version_inner(snapshot, path, version, false, clone_ms) + save_owned_as_version_inner(snapshot, path, version, false, clone_ms, |_| Ok(())) + .map_err(|error| error.to_string()) } /// Save an owned document snapshot. Preparation, serialization, compression and /// disk I/O can therefore run on a worker without borrowing live editor state. /// Output is written beside the destination and atomically renamed only after a /// complete file exists, so a failed save cannot truncate the previous drawing. -#[cfg_attr(target_arch = "wasm32", allow(dead_code))] +#[cfg(not(target_arch = "wasm32"))] pub fn save_owned_as_version_atomic( doc: CadDocument, path: &Path, version: acadrust::DxfVersion, backup: bool, -) -> Result<(), String> { - save_owned_as_version_inner(doc, path, version, backup, 0.0) + expected_fingerprint: Option, +) -> Result<(), SaveFailure> { + save_owned_as_version_inner(doc, path, version, backup, 0.0, move |path| { + let Some(expected) = expected_fingerprint else { + return Ok(()); + }; + match edit_lock::FileFingerprint::capture(path) { + Ok(current) if current == expected => Ok(()), + _ => Err(SaveFailure::externally_modified(path)), + } + }) } -fn save_owned_as_version_inner( +fn save_owned_as_version_inner( mut doc: CadDocument, path: &Path, version: acadrust::DxfVersion, backup: bool, clone_ms: f64, -) -> Result<(), String> { + before_replace: F, +) -> Result<(), SaveFailure> +where + F: FnOnce(&Path) -> Result<(), SaveFailure>, +{ let perf = crate::perf::enabled(); let total_started = iced::time::Instant::now(); doc.version = version; @@ -637,19 +772,24 @@ fn save_owned_as_version_inner( let result = match ext.as_str() { "dxf" => DxfWriter::new(&doc) .write_to_file(&temp_path) - .map_err(|e| e.to_string()), - _ => DwgWriter::write_to_file(&temp_path, &doc).map_err(|e| e.to_string()), + .map_err(|e| SaveFailure::other(e.to_string())), + _ => DwgWriter::write_to_file(&temp_path, &doc) + .map_err(|e| SaveFailure::other(e.to_string())), }; if let Err(error) = result { let _ = std::fs::remove_file(&temp_path); return Err(error); } + if let Err(error) = before_replace(path) { + let _ = std::fs::remove_file(&temp_path); + return Err(error); + } if backup { write_backup(path); } if let Err(error) = replace_save_file(&temp_path, path) { let _ = std::fs::remove_file(&temp_path); - return Err(format!("replace {}: {error}", path.display())); + return Err(SaveFailure::replacing(path, error)); } if perf { crate::perf_record!( diff --git a/src/ui/command_line.rs b/src/ui/command_line.rs index b41345f22..24a0b05e7 100644 --- a/src/ui/command_line.rs +++ b/src/ui/command_line.rs @@ -202,6 +202,22 @@ impl CommandLine { pub fn push_error(&mut self, msg: &str) { self.push(EntryKind::Error, format!("*Invalid* {msg}")); } + /// Append an error unless it is already the latest history line. Repeated + /// retry failures should refresh the concise message, not flood history + /// with identical copies (#498). + #[cfg(not(target_arch = "wasm32"))] + pub fn push_error_once(&mut self, msg: &str) { + let text = format!("*Invalid* {msg}"); + if let Some(last) = self + .history + .last_mut() + .filter(|entry| entry.kind == EntryKind::Error && entry.text == text) + { + last.created_at = Instant::now(); + return; + } + self.push(EntryKind::Error, text); + } pub fn push_info(&mut self, msg: &str) { self.push(EntryKind::Info, msg.to_string()); } @@ -850,7 +866,7 @@ const INFO_COLOR: Color = Color { #[cfg(test)] mod tests { - use super::ranked_matches; + use super::{ranked_matches, CommandLine}; use rustc_hash::FxHashMap; fn aliases(pairs: &[(&str, &str)]) -> FxHashMap { @@ -900,5 +916,13 @@ mod tests { let m = ranked_matches("L", &[], &a); assert_eq!(m.first().map(String::as_str), Some("LINE"), "got {m:?}"); } -} + #[test] + fn issue_498_repeated_save_error_is_not_duplicated() { + let mut line = CommandLine::new(); + let initial_len = line.history.len(); + line.push_error_once("Unable to save: file is in use."); + line.push_error_once("Unable to save: file is in use."); + assert_eq!(line.history.len(), initial_len + 1); + } +}