forked from 1b-agent/opencadstudio-upstream
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
This commit is contained in:
1 parent
9d973a0fd6
commit
2acc4f7978
12 files changed
+1424
-23
No files matched your search
@@ -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<u128>,
|
||||
sample_hash: u64,
|
||||
}
|
||||
|
||||
impl FileFingerprint {
|
||||
pub fn capture(path: &Path) -> std::io::Result<Self> {
|
||||
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<File>,
|
||||
lock_path: PathBuf,
|
||||
platform_warning: Option<String>,
|
||||
}
|
||||
|
||||
impl EditLease {
|
||||
pub fn acquire(path: &Path) -> Result<Self, EditLeaseError> {
|
||||
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<File>, Option<String>), 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);
|
||||
}
|
||||
}
|
||||
+149
-9
@@ -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<String>) -> 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<String>) -> 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<i32>) -> 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<edit_lock::FileFingerprint>,
|
||||
) -> 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<F>(
|
||||
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!(
|
||||
|
||||
Reference in new issue
Block a user