Repository navigation
Commit ac29284
committed
fix(util): wait for contended locks with relative filenames
`BlockingLockFile('resource')` incorrectly treats the empty `dirname` of
`resource.lock` as a missing directory when another holder owns the lock.
Use `os.curdir` for that case so the retry and timeout paths work normally.
Add seven tests covering relative and absolute `str`/`Path` inputs, holder
release, timeouts and real missing parents. The complete Linux/Python 3.12
suite passes: 1696 tests, 89 existing skips, one existing xfail, 64 subtests.
Pinned hooks, mypy, basedpyright, docs, builds and strict Twine also pass.
Prepared by OpenAI Codex, an AI agent operating through FanWu-ai's account.1 parent 804bc80 commit ac29284
2 files changed
Lines changed: 38 additions & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -1310,7 +1310,7 @@ def _obtain_lock(self) -> None: | |||
| 1310 | 1310 | # synity check: if the directory leading to the lockfile is not | |
| 1311 | 1311 | # readable anymore, raise an exception | |
| 1312 | 1312 | curtime = time.time() | |
| 1313 | - if not osp.isdir(osp.dirname(self._lock_file_path())): | ||
| 1313 | + if not osp.isdir(osp.dirname(self._lock_file_path()) or osp.curdir): | ||
| 1314 | 1314 | msg = "Directory containing the lockfile %r was not readable anymore after waiting %g seconds" % ( | |
| 1315 | 1315 | self._lock_file_path(), | |
| 1316 | 1316 | curtime - starttime, | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -862,3 +862,40 @@ def test_mode_str_to_int_accepts_bytes(): | |||
| 862 | 862 | assert mode_str_to_int(b"100644") == 0o100644 | |
| 863 | 863 | assert mode_str_to_int("644") == 0o644 | |
| 864 | 864 | assert mode_str_to_int(b"120000") == 0o120000 | |
| 865 | + | ||
| 866 | + | ||
| 867 | + @pytest.mark.parametrize("path_type", [str, pathlib.Path]) | ||
| 868 | + @pytest.mark.parametrize("relative", [True, False]) | ||
| 869 | + def test_blocking_lock_waits_for_existing_holder(tmp_path, monkeypatch, path_type, relative): | ||
| 870 | + monkeypatch.chdir(tmp_path) | ||
| 871 | + path = path_type("resource" if relative else tmp_path / "resource") | ||
| 872 | + owner = LockFile(path) | ||
| 873 | + waiter = BlockingLockFile(path, check_interval_s=0.01, max_block_time_s=1) | ||
| 874 | + owner._obtain_lock() | ||
| 875 | + try: | ||
| 876 | + with mock.patch("git.util.time.sleep", side_effect=lambda _: owner._release_lock()) as sleep: | ||
| 877 | + waiter._obtain_lock() | ||
| 878 | + sleep.assert_called_once_with(0.01) | ||
| 879 | + assert waiter._has_lock() | ||
| 880 | + finally: | ||
| 881 | + owner._release_lock() | ||
| 882 | + waiter._release_lock() | ||
| 883 | + | ||
| 884 | + | ||
| 885 | + @pytest.mark.parametrize("path_type", [str, pathlib.Path]) | ||
| 886 | + def test_blocking_relative_lock_reports_timeout(tmp_path, monkeypatch, path_type): | ||
| 887 | + monkeypatch.chdir(tmp_path) | ||
| 888 | + owner = LockFile(path_type("resource")) | ||
| 889 | + waiter = BlockingLockFile(path_type("resource"), max_block_time_s=0) | ||
| 890 | + owner._obtain_lock() | ||
| 891 | + try: | ||
| 892 | + with pytest.raises(OSError, match="Waited .* seconds for lock"): | ||
| 893 | + waiter._obtain_lock() | ||
| 894 | + finally: | ||
| 895 | + owner._release_lock() | ||
| 896 | + | ||
| 897 | + | ||
| 898 | + def test_blocking_lock_still_rejects_missing_directory(tmp_path): | ||
| 899 | + waiter = BlockingLockFile(tmp_path / "missing" / "resource") | ||
| 900 | + with pytest.raises(OSError, match="Directory containing the lockfile"): | ||
| 901 | + waiter._obtain_lock() | ||
0 commit comments