Skip to content

Commit ea594a5

Browse files
authored
Keep Unix lock files after release (#577)
## Problem Since `03b0ab7` (`feat(unix): delete lock file on release`), `UnixFileLock._release()` unlinks the path before unlocking and closing the fd. That lets a process that already opened the old inode keep waiting on that deleted file while another process recreates the filename and locks a new inode, so both can believe they hold the same lock. Fixes #574. ## Solution - stop unlinking the Unix lock file on release - keep the Unix-specific expectations aligned with the persistent-path behavior - add a regression that proves a waiter fd cannot acquire a different inode after another lock instance reacquires the same path ## Testing - `.venv-ci/bin/python -m pytest tests/test_filelock.py tests/test_unix_fallback.py tests/test_self_deadlock.py -q` - `.venv-ci/bin/python -m ruff check src/filelock/_unix.py tests/test_filelock.py tests/test_unix_fallback.py tests/test_self_deadlock.py` - `.venv-ci/bin/python -m ruff format --check src/filelock/_unix.py tests/test_filelock.py tests/test_unix_fallback.py tests/test_self_deadlock.py`
1 parent 7595a7b commit ea594a5

2 files changed

Lines changed: 37 additions & 12 deletions

File tree

src/filelock/_unix.py

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,9 @@ class UnixFileLock(BaseFileLock):
3838
"""
3939
Uses the :func:`fcntl.flock` to hard lock the lock file on unix systems.
4040
41-
Lock file cleanup: Unix and macOS delete the lock file reliably after release, even in
42-
multi-threaded scenarios. Unlike Windows, Unix allows unlinking files that other processes
43-
have open.
41+
The lock file is intentionally left in place after release. Unlinking a locked file on Unix
42+
can split waiters across different inodes and break mutual exclusion for processes that
43+
coordinate via the same path.
4444
"""
4545

4646
def _acquire(self) -> None: # noqa: C901, PLR0912
@@ -103,10 +103,8 @@ def _fallback_to_soft_lock(self) -> None:
103103
def _release(self) -> None:
104104
fd = cast("int", self._context.lock_file_fd)
105105
self._context.lock_file_fd = None
106-
with suppress(OSError):
107-
Path(self.lock_file).unlink()
108106
fcntl.flock(fd, fcntl.LOCK_UN)
109-
with suppress(OSError): # close can raise EIO on FUSE/Docker bind-mount filesystems after unlink
107+
with suppress(OSError): # close can raise EIO on FUSE/Docker bind-mount filesystems
110108
os.close(fd)
111109

112110

tests/test_filelock.py

Lines changed: 33 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -945,7 +945,7 @@ def test_mtime_zero_exit_branch(
945945
lock.acquire(timeout=0)
946946

947947

948-
@pytest.mark.parametrize("lock_type", [FileLock, SoftFileLock])
948+
@pytest.mark.parametrize("lock_type", [SoftFileLock])
949949
def test_lock_file_removed_after_release(tmp_path: Path, lock_type: type[BaseFileLock]) -> None:
950950
lock_path = tmp_path / "test.lock"
951951
lock = lock_type(str(lock_path))
@@ -955,7 +955,7 @@ def test_lock_file_removed_after_release(tmp_path: Path, lock_type: type[BaseFil
955955

956956

957957
@pytest.mark.skipif(sys.platform == "win32", reason="Unix flock semantics")
958-
def test_concurrent_acquire_release_removes_lock_file(tmp_path: Path) -> None:
958+
def test_concurrent_acquire_release_keeps_lock_file(tmp_path: Path) -> None:
959959
lock_path = tmp_path / "test.lock"
960960
errors: list[Exception] = []
961961

@@ -974,24 +974,51 @@ def worker() -> None:
974974
for thread in threads:
975975
thread.join(timeout=30)
976976
assert not errors, errors
977-
assert not lock_path.exists()
977+
assert lock_path.exists()
978978

979979

980980
@pytest.mark.skipif(sys.platform == "win32", reason="Unix flock semantics")
981-
def test_lock_acquired_after_release_unlinks(tmp_path: Path) -> None:
981+
def test_lock_acquired_after_release_keeps_path(tmp_path: Path) -> None:
982982
lock_path = tmp_path / "test.lock"
983983
first = FileLock(str(lock_path), is_singleton=False)
984984
second = FileLock(str(lock_path), is_singleton=False)
985985

986986
first.acquire()
987987
assert lock_path.exists()
988988
first.release()
989-
assert not lock_path.exists()
989+
assert lock_path.exists()
990990

991991
second.acquire()
992992
assert lock_path.exists()
993993
second.release()
994-
assert not lock_path.exists()
994+
assert lock_path.exists()
995+
996+
997+
@pytest.mark.skipif(sys.platform == "win32", reason="Unix flock semantics")
998+
def test_waiter_fd_cannot_split_lock_after_release(tmp_path: Path) -> None:
999+
import fcntl
1000+
from errno import EAGAIN, EWOULDBLOCK
1001+
1002+
lock_path = tmp_path / "test.lock"
1003+
first = FileLock(str(lock_path), is_singleton=False)
1004+
replacement = FileLock(str(lock_path), is_singleton=False)
1005+
1006+
first.acquire()
1007+
waiter_fd = os.open(lock_path, os.O_RDWR)
1008+
1009+
try:
1010+
first.release()
1011+
assert lock_path.exists()
1012+
1013+
replacement.acquire()
1014+
try:
1015+
with pytest.raises(BlockingIOError) as exc_info:
1016+
fcntl.flock(waiter_fd, fcntl.LOCK_EX | fcntl.LOCK_NB)
1017+
assert exc_info.value.errno in {EAGAIN, EWOULDBLOCK}
1018+
finally:
1019+
replacement.release()
1020+
finally:
1021+
os.close(waiter_fd)
9951022

9961023

9971024
@pytest.mark.skipif(sys.platform == "win32", reason="Unix flock semantics")

0 commit comments

Comments
 (0)