fix(io): stop the drawing lock blocking our own reads on Windows
Saving to a path that already holds a drawing fails on Windows with "could not verify <path> before saving: ... (os error 33)". Save As over any existing drawing is affected; saving to a new path is not. EditLease::acquire() opens the destination and takes the platform drawing lock on it, then the save fingerprints the same path to check nothing changed underneath. LockFileEx is mandatory and scoped to the handle that took it, so that second open is refused against our own lock, lands in the non-NotFound arm and aborts the save. flock on Unix is advisory and the second open succeeds, so neither CI nor a Linux desktop sees it. Read through the handle the lease already holds instead. A duplicate shares the lock ownership, so the pre-replace re-check on the save worker can use one too rather than re-opening the path. The same collision reached the headless open op, which read a path this session may hold leased: automation's save_then_open_round_trips fails on main for that reason, one line after the save it no longer reaches.
This commit is contained in:
parent
0b5d4bd311
commit
c88f07d219
4 changed files with 173 additions and 6 deletions
|
|
@ -212,7 +212,7 @@ impl OpenCADStudio {
|
|||
let Some(path) = req["path"].as_str() else {
|
||||
return err("open: missing \"path\"");
|
||||
};
|
||||
let bytes = match std::fs::read(path) {
|
||||
let bytes = match self.read_drawing(std::path::Path::new(path)) {
|
||||
Ok(b) => b,
|
||||
Err(e) => return err(format!("open: {e}")),
|
||||
};
|
||||
|
|
@ -981,6 +981,40 @@ mod tests {
|
|||
let _ = std::fs::remove_file(&b);
|
||||
}
|
||||
|
||||
/// Saving to a path that already holds a drawing leases the destination and then
|
||||
/// fingerprints it, and on Windows the lease's lock is mandatory: re-opening the
|
||||
/// path to read it was refused against our own lock, so every Save As over an
|
||||
/// existing drawing failed with os error 33. Saving to a new path was unaffected,
|
||||
/// which is the pair this test keeps.
|
||||
#[test]
|
||||
fn saving_over_an_existing_drawing_succeeds() {
|
||||
for (label, pre_existing) in [("new path", false), ("existing drawing", true)] {
|
||||
let path = std::env::temp_dir().join(format!(
|
||||
"ocs_save_over_{}_{}.dxf",
|
||||
std::process::id(),
|
||||
pre_existing,
|
||||
));
|
||||
let _ = std::fs::remove_file(&path);
|
||||
if pre_existing {
|
||||
std::fs::write(&path, b"a previous drawing").unwrap();
|
||||
}
|
||||
|
||||
let mut app = OpenCADStudio::new_for_test();
|
||||
app.automation_op(r#"{"op":"new"}"#);
|
||||
let p = path.to_string_lossy().replace('\\', "\\\\");
|
||||
let saved = app.automation_op(&format!(r#"{{"op":"save","path":"{p}"}}"#));
|
||||
assert_eq!(saved["ok"], true, "{label}: {}", saved["error"]);
|
||||
|
||||
drop(app);
|
||||
let sidecar = path.with_file_name(format!(
|
||||
".{}.ocs.lock",
|
||||
path.file_name().unwrap().to_string_lossy()
|
||||
));
|
||||
let _ = std::fs::remove_file(sidecar);
|
||||
let _ = std::fs::remove_file(&path);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn save_then_open_round_trips() {
|
||||
let mut app = OpenCADStudio::new_for_test();
|
||||
|
|
@ -998,3 +1032,4 @@ mod tests {
|
|||
let _ = std::fs::remove_file(&path);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue