From 2d6126396b97543e41956f1d67eb37065c9d0aaa Mon Sep 17 00:00:00 2001 From: Nathaniel Ledford Date: Fri, 23 Oct 2026 14:26:25 -0400 Subject: [PATCH] LRC-LIB-CATALOG-LOCK: quitting on macOS releases the library lock, so catalog.lock.owner is removed --- apps/lightcraft/src/main.rs | 6 ++++++ apps/lightcraft/src/tests.rs | 35 +++++++++++++++++++++++++++++++++++ crates/catalog/src/lock.rs | 6 ++++-- crates/engine/src/library.rs | 30 ++++++++++++++++++++++++++++++ docs/whats-new.md | 6 ++++++ 5 files changed, 81 insertions(+), 2 deletions(-) diff --git a/apps/lightcraft/src/main.rs b/apps/lightcraft/src/main.rs index 7b572fa0e..a15bac077 100644 --- a/apps/lightcraft/src/main.rs +++ b/apps/lightcraft/src/main.rs @@ -116,6 +116,12 @@ impl eframe::App for App { if !self.0.shutdown(EXIT_WAIT) { log::warn!("quit: background work was still running after {EXIT_WAIT:?}"); } + // macOS only: the process ends below without running destructors, and only the lock's + // `Drop` removes `catalog.lock.owner` (the OS frees the lock, not the note), so let go of + // the library's lock here, saved or not (we leave either way). Elsewhere eframe drops the + // app after this and the session's last writes (face data) still go out under the lock. + #[cfg(target_os = "macos")] + self.0.session.release_library_lock(); // macOS (#1331): end the process here, before eframe tears the window down, as Quit from // the menu bar already does (PR #444). AccessKit's macOS adapter subclasses the window's // content view and restores its class on teardown; AppKit's Touch Bar support observes diff --git a/apps/lightcraft/src/tests.rs b/apps/lightcraft/src/tests.rs index 0afbc4b2c..79b11904d 100644 --- a/apps/lightcraft/src/tests.rs +++ b/apps/lightcraft/src/tests.rs @@ -462,3 +462,38 @@ fn on_exit_ends_the_process_on_macos() { assert!(stdout.contains("calling on_exit"), "the child didn't run the test: {stdout}"); assert_eq!(stdout.contains("on_exit returned"), !cfg!(target_os = "macos"), "{stdout}"); } + +/// Quitting leaves no `catalog.lock.owner` behind, though the process ends without running +/// destructors (macOS): `on_exit` releases the library's lock itself. The child opens a library +/// and calls `on_exit` (on macOS that ends it); the parent looks at the folder afterwards. Other +/// systems drop the app after `on_exit`, which removes the note anyway. +#[test] +fn on_exit_removes_the_lock_owner_note() { + if let Some(dir) = std::env::var_os("LIGHTCRAFT_TEST_LOCK_DIR") { + let mut session = lightcraft_engine::Session::new(); + session.open_library(&dir, false).unwrap(); + let mut app = App( + LightcraftApp::new(session, Services::default()), + PrefsWriter::default(), + #[cfg(target_os = "macos")] + None, + ); + eframe::App::on_exit(&mut app, None); + return; + } + let dir = std::env::temp_dir().join(format!("lc-app-lock-owner-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + let out = std::process::Command::new(std::env::current_exe().unwrap()) + .args(["--exact", "tests::on_exit_removes_the_lock_owner_note", "--nocapture"]) + .env("LIGHTCRAFT_TEST_LOCK_DIR", &dir) + .env("LIGHTCRAFT_NO_PREFS", "1") + .output() + .unwrap(); + let stdout = String::from_utf8_lossy(&out.stdout); + assert!(out.status.success(), "{stdout}"); + assert!(dir.join("catalog.snap").exists(), "the child didn't open and save the library: {stdout}"); + assert!(!dir.join("catalog.lock.owner").exists(), "the owner note was left behind"); + assert!(lightcraft_engine::Session::new().open_library(&dir, false).is_ok(), "the library is still locked"); + let _ = std::fs::remove_dir_all(&dir); +} diff --git a/crates/catalog/src/lock.rs b/crates/catalog/src/lock.rs index f701087a6..5534e86cc 100644 --- a/crates/catalog/src/lock.rs +++ b/crates/catalog/src/lock.rs @@ -17,8 +17,10 @@ //! open" for a moment and a re-open would be refused (issue #680). `LOCK_UN` through our //! descriptor frees the lock whatever copies a child still holds. //! - Who holds it (process id, computer, program, version) is written to `catalog.lock.owner`, for -//! the error message. It's a separate file because a Windows lock also blocks reading the -//! locked file. It's only a hint: it may be missing or left over from a crash. +//! the error message. Only dropping the [`LibraryLock`] removes it, so a program that ends with +//! `std::process::exit` releases the lock first (`Session::release_library_lock`). It's a separate +//! file because a Windows lock also blocks reading the locked file. It's only a hint: it may be +//! missing or left over from a crash. //! - File systems without lock support (some network shares): the library opens unlocked, with a //! warning — refusing it would lock the user out of their own library. Locks over SMB/NFS are //! advisory and depend on the server; the owner file is then the only hint. diff --git a/crates/engine/src/library.rs b/crates/engine/src/library.rs index a5071b4b3..459eba713 100644 --- a/crates/engine/src/library.rs +++ b/crates/engine/src/library.rs @@ -733,6 +733,16 @@ impl Session { result } + /// Let go of the open library's lock (and with it the owner note, `catalog.lock.owner`) while + /// the library stays open in this session. For a host that ends the process without running + /// destructors (`std::process::exit`), after [`Session::close_library`] has saved: the OS would + /// free the lock anyway, but only the lock's `Drop` removes the note. Nothing else is + /// protected after this, so call it only when the process is about to end. A no-op without a + /// library or a lock. + pub fn release_library_lock(&mut self) { + drop(self.library.as_mut().and_then(|l| l.lock.take())); + } + fn view_json(&self) -> Vec { let view = ViewFile { source: self.source, @@ -900,6 +910,26 @@ mod resolve_tests { let _ = std::fs::remove_dir_all(&dir); } + /// A host that ends the process without destructors releases the lock by hand: the owner note + /// goes and another opener is let in, while the session keeps its library. + #[test] + fn release_library_lock_removes_the_owner_note() { + let dir = std::env::temp_dir().join(format!("lc-release-lock-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + let mut s = Session::new().with_fs(); + s.open_library(&dir, false).unwrap(); + s.close_library().unwrap(); + assert!(dir.join(lightcraft_catalog::lock::OWNER).exists(), "close_library keeps the lock"); + assert!(matches!(LibraryLock::acquire(&dir, "other"), Err(lightcraft_catalog::lock::LockError::InUse(_)))); + s.release_library_lock(); + assert!(!dir.join(lightcraft_catalog::lock::OWNER).exists()); + drop(LibraryLock::acquire(&dir, "other").unwrap()); + s.release_library_lock(); + assert!(s.library.is_some(), "the library stays open"); + let _ = std::fs::remove_dir_all(&dir); + } + /// `check_library` refuses what `open_library` would, without touching the open library. #[test] fn check_library_refuses_locked_libraries_files_and_lightroom_catalogs() { diff --git a/docs/whats-new.md b/docs/whats-new.md index c6c259200..a4c1dc53b 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -50,6 +50,12 @@ stay available in All Metadata; PSD edit copies keep their capture date. Original exports with Remove Location Info enabled are refused when a PSD or PSB embeds GPS, since LightCraft cannot strip metadata from a byte-for-byte original copy. +### Quitting on macOS leaves no lock note +- Quitting LightCraft on macOS no longer leaves `catalog.lock.owner` in the library folder. The note, which names the + program that has the library open, stayed behind after Quit and the window's close button, pointing at a process + that no longer existed. The library's lock is now released as soon as everything is saved, as it already was on + the other systems. + ### Choose where a Lightroom catalog goes - File ▸ Import Lightroom Catalog… now asks first: merge the catalog's photos, collections and edits into the library open now, or create a new library for them, named after the catalog and placed next to the open library