Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/filelock/_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -372,10 +372,10 @@ def _try_break_expired_lock(self) -> None:
# lock file with a symlink pointing at an old file, making stat() report the target's stale
# mtime so a waiter breaks a live lock and two processes hold it at once. lstat reads the
# symlink's own mtime, matching the O_NOFOLLOW reads elsewhere.
mtime = os.lstat(self.lock_file).st_mtime
if time.time() - mtime < lifetime:
st = os.lstat(self.lock_file)
if time.time() - st.st_mtime < lifetime:
return
break_lock_file(self.lock_file, mtime)
break_lock_file(self.lock_file, st.st_mtime, st.st_ino)

@abstractmethod
def _acquire(self) -> None:
Expand Down
18 changes: 9 additions & 9 deletions src/filelock/_soft.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,15 +57,15 @@ def _acquire(self) -> None:

def _try_break_stale_lock(self) -> None:
with suppress(OSError, ValueError):
content, mtime = _read_lock_file(self.lock_file)
content, mtime, ino = _read_lock_file(self.lock_file)
holder = _parse_lock_holder(content)

if holder is None:
# Unparsable: wrong line count, a non-integer PID or creation time, empty, oversized or not UTF-8.
# Self-heal only once the file is clearly not a half-written fresh lock (a peer between O_EXCL and
# _write_lock_info), so the brief create-then-write window is never mistaken for a stale lock.
if time.time() - mtime >= _MALFORMED_LOCK_AGE_THRESHOLD:
break_lock_file(self.lock_file, mtime)
break_lock_file(self.lock_file, mtime, ino)
return

pid, hostname, creation_time = holder
Expand All @@ -80,7 +80,7 @@ def _try_break_stale_lock(self) -> None:
return # same process or can't verify — don't evict
# else: PID alive but creation time differs — the PID was recycled, so the lock is stale.

break_lock_file(self.lock_file, mtime)
break_lock_file(self.lock_file, mtime, ino)

@staticmethod
def _is_process_alive(pid: int) -> bool:
Expand Down Expand Up @@ -201,21 +201,21 @@ def _windows_unlink_with_retry(self) -> None:
return


def _read_lock_file(path: str) -> tuple[str | None, float]:
def _read_lock_file(path: str) -> tuple[str | None, float, int]:
# The lock file is created with O_EXCL | O_NOFOLLOW, so a symlink here is a hostile replacement and must
# not be followed. O_NONBLOCK keeps an attacker-placed FIFO from stalling the open (O_NOFOLLOW alone only
# rejects a symlink, not a real FIFO at the path), and the capped read stops a huge file (e.g. /dev/zero)
# from exhausting memory. Content is None when the file is too large or not UTF-8, but the mtime still
# flows back so the caller can evict it as a stale, malformed lock.
# from exhausting memory. Content is None when the file is too large or not UTF-8, but the mtime and inode
# still flow back so the caller can evict it as a stale, malformed lock and verify identity before breaking.
fd = os.open(path, os.O_RDONLY | getattr(os, "O_NOFOLLOW", 0) | getattr(os, "O_NONBLOCK", 0))
try:
mtime, data = os.fstat(fd).st_mtime, os.read(fd, _MAX_LOCK_FILE_SIZE + 1)
st, data = os.fstat(fd), os.read(fd, _MAX_LOCK_FILE_SIZE + 1)
finally:
os.close(fd)
if len(data) <= _MAX_LOCK_FILE_SIZE:
with suppress(UnicodeDecodeError):
return data.decode("utf-8"), mtime
return None, mtime
return data.decode("utf-8"), st.st_mtime, st.st_ino
return None, st.st_mtime, st.st_ino


def _parse_lock_holder(content: str | None) -> tuple[int, str, int | None] | None:
Expand Down
18 changes: 11 additions & 7 deletions src/filelock/_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,30 +47,34 @@ def ensure_directory_exists(filename: Path | str) -> None:
Path(filename).parent.mkdir(parents=True, exist_ok=True)


def break_lock_file(lock_file: str, mtime_before: float) -> None:
def break_lock_file(lock_file: str, mtime_before: float, ino_before: int) -> None:
"""
Atomically break a stale lock file that was judged stale at modification time *mtime_before*.

The file is renamed to a process-private name before being unlinked, so two processes breaking the same lock
cannot delete each other's work (only one rename of a given inode succeeds; the loser gets ``OSError``). After the
rename the modification time is re-checked: a value newer than *mtime_before* means a peer recreated the lock
between the stale decision and the rename, so we grabbed a live file and must abort, leaving the renamed file in
place rather than rolling back (a rollback rename is itself racy — same trade-off as the soft read/write marker
break). ``lstat`` is used so a hostile symlink swapped in after the decision is not followed.
rename the file is re-checked: a newer modification time, or a different inode than *ino_before*, means a peer
recreated the lock between the stale decision and the rename, so we grabbed a live file and must abort, leaving the
renamed file in place rather than rolling back (a rollback rename is itself racy — same trade-off as the soft
read/write marker break). The inode check matters because filesystems with coarse modification-time granularity
(NFS, FAT) can give a same-second recreation the old mtime, so mtime alone would not catch it and a live lock would
be unlinked; the inode is the reliable identity, mirroring the token re-check in the soft read/write marker break.
``lstat`` is used so a hostile symlink swapped in after the decision is not followed.

:param lock_file: path to the lock file to break.
:param mtime_before: modification time observed when the lock was judged stale.
:param ino_before: inode number observed when the lock was judged stale.

:raises OSError: if the rename fails (e.g. the file vanished or is not owned in a sticky directory).

"""
break_path = f"{lock_file}.break.{os.getpid()}"
Path(lock_file).rename(break_path)
try:
mtime_after = os.lstat(break_path).st_mtime
st_after = os.lstat(break_path)
except OSError:
return
if mtime_after > mtime_before:
if st_after.st_mtime > mtime_before or st_after.st_ino != ino_before:
return
Path(break_path).unlink()

Expand Down
27 changes: 23 additions & 4 deletions tests/test_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@
def test_break_lock_file_unlinks_unchanged_file(tmp_path: Path) -> None:
lock = tmp_path / "test.lock"
lock.write_text("stale", encoding="utf-8")
break_lock_file(str(lock), os.lstat(lock).st_mtime)
st = os.lstat(lock)
break_lock_file(str(lock), st.st_mtime, st.st_ino)
assert not lock.exists()
assert list(tmp_path.glob("test.lock.break.*")) == []

Expand All @@ -26,22 +27,40 @@ def test_break_lock_file_preserves_file_when_mtime_advanced(tmp_path: Path) -> N
lock.write_text("live", encoding="utf-8")
# A mtime_before older than the file's real mtime models a peer recreating the lock after our stale read: the
# live file is renamed aside but must not be unlinked, so the holder's content survives instead of two holders.
break_lock_file(str(lock), mtime_before=0.0)
break_lock_file(str(lock), mtime_before=0.0, ino_before=os.lstat(lock).st_ino)
assert not lock.exists()
leftover = list(tmp_path.glob("test.lock.break.*"))
assert len(leftover) == 1
assert leftover[0].read_text(encoding="utf-8") == "live"


def test_break_lock_file_preserves_file_when_inode_changed(tmp_path: Path) -> None:
lock = tmp_path / "test.lock"
lock.write_text("stale", encoding="utf-8")
st = os.lstat(lock)
# Model a coarse-granularity filesystem (NFS, FAT) where a peer broke and recreated the lock with a new inode
# but the same mtime second. Creating the replacement while the original still exists guarantees a fresh inode.
other = tmp_path / "recreated"
other.write_text("live", encoding="utf-8")
os.utime(other, ns=(st.st_atime_ns, st.st_mtime_ns))
assert os.lstat(other).st_ino != st.st_ino
other.replace(lock)
break_lock_file(str(lock), st.st_mtime, st.st_ino)
leftover = list(tmp_path.glob("test.lock.break.*"))
assert len(leftover) == 1
assert leftover[0].read_text(encoding="utf-8") == "live"


def test_break_lock_file_aborts_if_break_path_vanishes(tmp_path: Path, mocker: MockerFixture) -> None:
lock = tmp_path / "test.lock"
lock.write_text("x", encoding="utf-8")
ino = os.lstat(lock).st_ino
mocker.patch("filelock._util.os.lstat", side_effect=FileNotFoundError)
break_lock_file(str(lock), 0.0)
break_lock_file(str(lock), 0.0, ino)
assert not lock.exists()
assert len(list(tmp_path.glob("test.lock.break.*"))) == 1


def test_break_lock_file_missing_source_raises(tmp_path: Path) -> None:
with pytest.raises(FileNotFoundError):
break_lock_file(str(tmp_path / "nope.lock"), 0.0)
break_lock_file(str(tmp_path / "nope.lock"), 0.0, 0)
Loading