diff --git a/crates/catalog/src/lock.rs b/crates/catalog/src/lock.rs index d699da049..f701087a6 100644 --- a/crates/catalog/src/lock.rs +++ b/crates/catalog/src/lock.rs @@ -25,7 +25,7 @@ //! - Browser storage (OPFS) has no file locks: see the web host for its own guard. use std::fs::{File, OpenOptions, TryLockError}; -use std::io; +use std::io::{self, Write}; use std::path::{Path, PathBuf}; /// The lock file in the library directory. @@ -116,9 +116,11 @@ impl LibraryLock { } Err(TryLockError::Error(e)) => return Ok(unlocked(e)), } + // A new file in place of whatever is at the name: a link planted there in a library folder + // someone else prepared is removed, never written through (as for the staging files, #1502). let owner = LockOwner::this_process(program); if let Ok(json) = serde_json::to_vec_pretty(&owner) - && let Err(e) = std::fs::write(dir.join(OWNER), json) + && let Err(e) = crate::store::create_staging(&dir.join(OWNER)).and_then(|mut f| f.write_all(&json)) { log::warn!("library {}: can't write {OWNER}: {e}", dir.display()); } diff --git a/crates/catalog/src/store.rs b/crates/catalog/src/store.rs index 553eb241f..ad008cf0c 100644 --- a/crates/catalog/src/store.rs +++ b/crates/catalog/src/store.rs @@ -75,8 +75,8 @@ impl FsStore { /// A new staging file at `tmp`: whatever is already there (a leftover from a crashed save, or a link) is removed first /// (a link itself, never what it points to), then the file is created exclusively, so a write never follows a link -/// planted at that name into another file (issue #1502). -fn create_staging(tmp: &std::path::Path) -> io::Result { +/// planted at that name into another file (issue #1502). Also writes the lock's owner note. +pub(crate) fn create_staging(tmp: &std::path::Path) -> io::Result { if std::fs::symlink_metadata(tmp).is_ok() { std::fs::remove_file(tmp)?; } diff --git a/crates/catalog/src/tests_lock.rs b/crates/catalog/src/tests_lock.rs index baa8da659..2472ba92a 100644 --- a/crates/catalog/src/tests_lock.rs +++ b/crates/catalog/src/tests_lock.rs @@ -62,6 +62,28 @@ fn leftover_lock_files_are_not_a_lock() { let _ = std::fs::remove_dir_all(&dir); } +/// A link planted at the owner note's name (`catalog.lock.owner`) in a library folder someone else +/// prepared is removed, never written through: the file it points to keeps its bytes, and the note +/// still lands in the library (as for the catalog's staging files, issue #1502). +#[cfg(unix)] +#[test] +fn a_link_at_the_owner_note_is_never_written_through() { + let _serial = serial(); + let dir = temp_dir("owner-link"); + let victim = dir.join("victim.txt"); + std::fs::write(&victim, b"keep").unwrap(); + std::os::unix::fs::symlink(&victim, dir.join(OWNER)).unwrap(); + let l = LibraryLock::acquire(&dir, "LightCraft").unwrap(); + assert!(l.held()); + assert_eq!(std::fs::read(&victim).unwrap(), b"keep"); + assert!(!std::fs::symlink_metadata(dir.join(OWNER)).unwrap().file_type().is_symlink(), "the link was replaced by the note"); + let owner: LockOwner = serde_json::from_slice(&std::fs::read(dir.join(OWNER)).unwrap()).unwrap(); + assert_eq!(owner.pid, std::process::id()); + drop(l); + assert_eq!(std::fs::read(&victim).unwrap(), b"keep"); + let _ = std::fs::remove_dir_all(&dir); +} + /// Another process holds the lock; when it is killed (a crash), the OS releases it. #[test] fn lock_held_by_another_process_is_released_when_it_dies() { diff --git a/docs/whats-new.md b/docs/whats-new.md index cffff83e2..cea298611 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -1,5 +1,12 @@ # What's new in LightCraft +## 2026-10-22 + +### Reliability +- Opening a library no longer writes through a link left at the lock's owner note (`catalog.lock.owner`): the link + is removed and a new note made in its place, so the file it pointed to is never changed (as for the catalog's + temporary files, issue #1502). + ## 2026-10-21 ### Sharper resampling in HDR and panorama merges