US08-03: Compose the Runtime and Mount the Library Safely (#98)

photo_pipeline/services/app_lock.py

@@ -15,9 +15,17 @@ package:
      "started_at": "...", "library_roots": ["..."]}
 
 One holder per role: an API and a worker are designed to run together, a second
-worker is not. A lock whose process is gone is stale and is taken over with the
-takeover recorded — refusing to start because of a crashed predecessor would turn
-one outage into two.
+worker is not. A lock whose process is gone is stale and is taken over — refusing
+to start because of a crashed predecessor would turn one outage into two.
+
+Ownership is an advisory ``flock`` on that file, not the record inside it. The
+record says *who*; the kernel says *whether*. That distinction is what makes the
+lock work in containers (US08-03), where a PID and a hostname are namespaced: a
+lock left behind by a container that no longer exists names a pid that still
+"exists" in the new container and a host that cannot be probed, so believing the
+file would deadlock every restart. A flock is released when its holder dies however
+it dies, and is seen by every process that can open the file — which for a local
+data directory is every container of this deployment.
 
 Legacy detection is deliberately a heuristic, not a promise: the archived CLI has
 no lock of its own, so what can be observed is its state files being written right
@@ -27,6 +35,7 @@ mutating stage should refuse until it stops.
 
 from __future__ import annotations
 
+import fcntl
 import json
 import os
 import socket
@@ -109,6 +118,12 @@ def _now() -> datetime:
     return datetime.now(timezone.utc)
 
 
+def _in_container() -> bool:
+    from photo_pipeline import path_policy
+
+    return path_policy.in_container()
+
+
 def legacy_activity(config: Config) -> dict:
     """Legacy state files written within the activity window, if any."""
     seen: list[dict] = []
@@ -139,6 +154,7 @@ class LibraryLock:
         self.role = role
         self.path = Path(config.data_dir) / f"{role}{LOCK_SUFFIX}"
         self._acquired = False
+        self._handle = None
 
     # ── inspection ────────────────────────────────────────────────────────────
 
@@ -178,12 +194,31 @@ class LibraryLock:
                     "stop it before running the application"
                 )
 
+        self.path.parent.mkdir(parents=True, exist_ok=True)
+        # The kernel decides, because the file cannot: a container's PID and hostname
+        # are namespaced, so a lock left by a container that no longer exists names a
+        # pid that "exists" and a host that cannot be probed (US08-03). An advisory
+        # flock is held by a live process or by nobody, is released when that process
+        # dies however it dies, and is shared by every process that can open this
+        # file — which, for a local data directory, is every role in every container
+        # of this deployment.
+        handle = open(self.path, "a+", encoding="utf-8")
+        try:
+            fcntl.flock(handle.fileno(), fcntl.LOCK_EX | fcntl.LOCK_NB)
+        except OSError:
+            handle.close()
+            raise LockHeld(self.holder() or Holder(self.role, -1, "unknown", "unknown")) from None
+
         current = self.holder()
-        if current is not None:
-            if current.alive:
-                raise LockHeld(current)
-            # Stale: its process is gone. Take over, and say so.
-            self.path.unlink(missing_ok=True)
+        if current is not None and current.host != socket.gethostname() and not _in_container():
+            # We hold the kernel's lock, so nothing on *this* machine holds the file.
+            # On a host that still leaves one case open: a data directory shared with
+            # another machine, whose flock we cannot trust. Believe its record rather
+            # than run two writers. In a container the data volume is local by
+            # construction (US08-03), and a foreign hostname is only a dead container.
+            fcntl.flock(handle.fileno(), fcntl.LOCK_UN)
+            handle.close()
+            raise LockHeld(current)
 
         mine = Holder(
             role=self.role,
@@ -192,15 +227,14 @@ class LibraryLock:
             started_at=_now().isoformat(),
             library_roots=tuple(str(root) for root in self._config.library_roots),
         )
-        self.path.parent.mkdir(parents=True, exist_ok=True)
         payload = {k: v for k, v in mine.as_dict().items() if k != "alive"}
-        # Exclusive create, so two processes racing here cannot both believe they won.
-        try:
-            with open(self.path, "x", encoding="utf-8") as handle:
-                json.dump(payload, handle, indent=2)
-        except FileExistsError:
-            winner = self.holder()
-            raise LockHeld(winner or mine) from None
+        handle.seek(0)
+        handle.truncate()
+        json.dump(payload, handle, indent=2)
+        handle.flush()
+        # Held open on purpose: closing it is what releases the lock, and that must
+        # happen when this process ends, not when this method returns.
+        self._handle = handle
         self._acquired = True
         return mine
 
@@ -208,9 +242,10 @@ class LibraryLock:
         """Give up a lock this process owns. Another holder's lock is left alone."""
         if not self._acquired:
             return
-        current = self.holder()
-        if current is not None and current.pid == os.getpid():
-            self.path.unlink(missing_ok=True)
+        self.path.unlink(missing_ok=True)
+        if self._handle is not None:
+            self._handle.close()  # closing the descriptor releases the kernel lock
+            self._handle = None
         self._acquired = False
 
     def __enter__(self) -> "LibraryLock":