Merge PR #2725: LRC-LIB-CATALOG-LOCK: quitting on macOS removes the lock's owner note

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

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);
+}

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.

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<u8> {
         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() {

docs/whats-new.md

@@ -70,6 +70,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