fix(marker): preserve live owners

docs/changelog/749.bugfix.rst

@@ -0,0 +1,2 @@
+Fix ``MarkerSoftFileLock`` acquisition and prevent contenders from evicting live protocol-2 owners after two seconds.
+Reclaim recognized records after owner death; preserve unknown contracts.

src/filelock/_marker.py

@@ -5,9 +5,9 @@
 from contextlib import suppress
 from typing import Final, Literal, NamedTuple
 
-from ._identity import host_name, process_start_token
+from ._identity import host_name, owner_is_stale, process_start_token
 from ._soft import SoftFileLock, _read_lock_file
-from ._util import write_all
+from ._util import break_lock_file, write_all
 
 #: Protocol 1 is the legacy ``<pid>\n<hostname>\n[<start_token>\n]`` marker that :class:`SoftFileLock` still writes.
 #: Protocol 2 carries the owner mode and the lease claim. A protocol 1 reader treats a protocol 2 marker as malformed
@@ -16,27 +16,15 @@
 
 _MAX_PID: Final[int] = 2**31 - 1
 
-#: ``unknown`` is never published: it names a mode some other filelock wrote that this version cannot interpret. Such a
-#: record still identifies a live owner, so it is parsed rather than read as malformed and aged out.
-OwnerMode = Literal["lease", "unknown"]
-
-
-class OwnerRecord(NamedTuple):
-    """The owner published in a protocol 2 marker."""
-
-    pid: int
-    hostname: str
-    mode: OwnerMode
-    token: str | None = None
-    lease_duration: float | None = None
-    start: int | None = None
+#: Preserve unknown contracts so contenders cannot mistake them for malformed, reclaimable markers.
+OwnerMode = Literal["lease", "exclusive", "unknown"]
 
 
 class MarkerSoftFileLock(SoftFileLock):
     """An existence lock whose marker carries a protocol 2 owner record."""
 
-    #: Filled in by each mode so the published record states the contract its holder acquired under.
-    _owner_mode: OwnerMode
+    #: Age-based expiry requires a lease contract; an exclusive holder grants none.
+    _owner_mode: OwnerMode = "exclusive"
 
     @property
     def owner(self) -> OwnerRecord | None:
@@ -78,6 +66,15 @@ def force_break(self) -> None:
         """
         self.break_lock()
 
+    def _try_break_stale_lock(self) -> None:
+        with suppress(OSError, ValueError):
+            snapshot: Final[tuple[str | None, float, int]] = _read_lock_file(self.lock_file)
+            owner: Final[OwnerRecord | None]
+            if (owner := parse_marker(snapshot[0])) is None:
+                super()._try_break_stale_lock()
+            elif owner.mode != "unknown" and owner_is_stale(owner.pid, owner.hostname, owner.start):
+                break_lock_file(self.lock_file, *snapshot[1:])
+
     def _read_owner(self) -> OwnerRecord | None:
         with suppress(OSError, ValueError):
             return parse_marker(_read_lock_file(self.lock_file)[0])
@@ -95,6 +92,17 @@ def _published_record(self) -> OwnerRecord:
         )
 
 
+class OwnerRecord(NamedTuple):
+    """Keep optional metadata when reading unknown lock modes."""
+
+    pid: int
+    hostname: str
+    mode: OwnerMode
+    token: str | None = None
+    lease_duration: float | None = None
+    start: int | None = None
+
+
 def encode_marker(record: OwnerRecord) -> bytes:
     """Render an owner record as the bytes a protocol 2 marker holds."""
     lines = [_PROTOCOL, f"pid={record.pid}", f"host={record.hostname}", f"mode={record.mode}"]
@@ -127,7 +135,7 @@ def _build_record(fields: dict[str, str]) -> OwnerRecord | None:
     # A record naming no mode at all states no contract and stays malformed.
     if (published := fields.get("mode")) is None:
         return None
-    mode: OwnerMode = "lease" if published == "lease" else "unknown"
+    mode: Final[OwnerMode] = published if published in {"lease", "exclusive"} else "unknown"
     hostname = fields.get("host")
     if not hostname or "pid" not in fields:
         return None

tests/test_marker_records.py

@@ -2,16 +2,20 @@
 
 import os
 import time
+from multiprocessing import get_context
 from typing import TYPE_CHECKING
 
 import pytest
 
-from filelock import SoftFileLease, Timeout
+from filelock import MarkerSoftFileLock, OwnerRecord, SoftFileLease, Timeout
 from filelock._identity import process_start_token
-from filelock._marker import OwnerRecord, encode_marker
+from filelock._marker import encode_marker
+from tests.process_helpers import cleanup_processes
 
 if TYPE_CHECKING:
+    from multiprocessing.process import BaseProcess
     from pathlib import Path
+    from typing import Final
 
     from pytest_mock import MockerFixture
 
@@ -158,3 +162,68 @@ def test_marker_force_break_removes_the_marker(tmp_path: Path) -> None:
     SoftFileLease(marker).force_break()
 
     assert not marker.exists()
+
+
+@pytest.mark.parametrize("acquire", [pytest.param(True, id="acquire"), pytest.param(False, id="context")])
+def test_marker_lock_publishes_exclusive(marker: Path, *, acquire: bool) -> None:
+    lock: Final[MarkerSoftFileLock] = MarkerSoftFileLock(marker)
+    with lock.acquire() if acquire else lock:
+        owner: Final[OwnerRecord | None]
+        assert (owner := MarkerSoftFileLock(lock.lock_file).owner) is not None
+        assert (lock.is_locked, lock.is_lock_held_by_us, lock.pid, owner.mode) == (True, True, os.getpid(), "exclusive")
+    assert (lock.is_locked, lock.pid, lock.owner) == (False, None, None)
+
+
+@pytest.mark.parametrize(
+    "contender_type",
+    [pytest.param(MarkerSoftFileLock, id="marker"), pytest.param(SoftFileLease, id="lease")],
+)
+def test_marker_lock_excludes_contenders(marker: Path, contender_type: type[MarkerSoftFileLock]) -> None:
+    with MarkerSoftFileLock(marker):
+        os.utime(marker, (0, 0))
+        with pytest.raises(Timeout):
+            contender_type(marker, timeout=0.1).acquire()
+
+
+@pytest.mark.parametrize(
+    "mode",
+    [
+        pytest.param("exclusive", id="exclusive"),
+        pytest.param("lease", id="lease"),
+        pytest.param("future", id="unknown"),
+    ],
+)
+def test_marker_lock_preserves_live_records(marker: Path, mode: str) -> None:
+    with MarkerSoftFileLock(marker):
+        record: Final[str] = marker.read_text(encoding="utf-8").replace("mode=exclusive", f"mode={mode}")
+    marker.write_text(record + "token=t\nduration=1\n", encoding="utf-8")
+    os.utime(marker, (0, 0))
+    with pytest.raises(Timeout):
+        MarkerSoftFileLock(marker, timeout=0.1).acquire()
+
+
+def test_marker_lock_reclaims_dead_owner(marker: Path) -> None:
+    process: Final[BaseProcess] = get_context("spawn").Process(target=_leave_marker, args=(marker,))
+    with cleanup_processes([process]):
+        process.start()
+        process.join(timeout=5)
+        assert process.exitcode == 0
+    with MarkerSoftFileLock(marker, timeout=1) as lock:
+        assert lock.pid == os.getpid()
+
+
+def test_marker_lock_reclaims_malformed_record(marker: Path) -> None:
+    marker.write_text("broken", encoding="utf-8")
+    os.utime(marker, (0, 0))
+    with MarkerSoftFileLock(marker, timeout=1) as lock:
+        assert lock.pid == os.getpid()
+
+
+@pytest.fixture
+def marker(tmp_path: Path) -> Path:
+    return tmp_path / "a"
+
+
+def _leave_marker(marker: Path) -> None:
+    with MarkerSoftFileLock(marker):
+        os._exit(0)