Skip to content

Commit 109017e

Browse files
authored
Merge pull request #2284 from gitpython-developers/fix-relative-blocking-lock-20261011
fix relative blocking lock 20261011
2 parents d940ee7 + 3b9bb6d commit 109017e

3 files changed

Lines changed: 131 additions & 10 deletions

File tree

‎git/util.py‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1270,8 +1270,8 @@ class BlockingLockFile(LockFile):
12701270
specified timeout.
12711271
12721272
:note:
1273-
If the directory containing the lock was removed, an exception will be raised
1274-
during the blocking period, preventing hangs as the lock can never be obtained.
1273+
Only an existing lock causes retries. Other errors, including a removed or
1274+
unwritable parent directory, are raised immediately.
12751275
"""
12761276

12771277
__slots__ = ("_check_interval", "_max_block_time")
@@ -1297,7 +1297,7 @@ def __init__(
12971297

12981298
def _obtain_lock(self) -> None:
12991299
"""This method blocks until it obtained the lock, or raises :exc:`IOError` if it
1300-
ran out of time or if the parent directory was not available anymore.
1300+
ran out of time or lock creation failed for a reason other than an existing lock.
13011301
13021302
If this method returns, you are guaranteed to own the lock.
13031303
"""
@@ -1307,10 +1307,14 @@ def _obtain_lock(self) -> None:
13071307
try:
13081308
super()._obtain_lock()
13091309
except OSError as e:
1310-
# synity check: if the directory leading to the lockfile is not
1311-
# readable anymore, raise an exception
1310+
# LockFile retains the creation error as the cause of its OSError.
1311+
# Only an existing lock is contention; other errors must not be retried.
1312+
if not isinstance(e.__cause__, FileExistsError):
1313+
raise
1314+
1315+
# Abort if the directory containing the existing lock has disappeared.
13121316
curtime = time.time()
1313-
if not osp.isdir(osp.dirname(self._lock_file_path())):
1317+
if not osp.isdir(osp.dirname(self._lock_file_path()) or osp.curdir):
13141318
msg = "Directory containing the lockfile %r was not readable anymore after waiting %g seconds" % (
13151319
self._lock_file_path(),
13161320
curtime - starttime,

‎test/test_config.py‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
# This module is part of GitPython and is released under the
44
# 3-Clause BSD License: https://opensource.org/license/bsd-3-clause/
55

6+
import errno
67
import glob
78
import io
89
import os
@@ -17,7 +18,7 @@
1718
from git import GitConfigParser, Repo
1819
from git.compat import defenc
1920
from git.config import _OMD, cp
20-
from git.util import cwd, rmfile
21+
from git.util import BlockingLockFile, cwd, rmfile
2122
from test.lib import SkipTest, TestCase, fixture_path, with_rw_directory
2223

2324
_tc_lock_fpaths = osp.join(osp.dirname(__file__), "fixtures/*.lock")
@@ -28,6 +29,23 @@ def _rm_lock_files():
2829
rmfile(lfp)
2930

3031

32+
@pytest.mark.skipif(sys.platform == "win32", reason="Lock creation uses CreateFileW on Windows")
33+
@pytest.mark.parametrize("error_number", [errno.EACCES, errno.ENOENT])
34+
def test_blocking_config_lock_propagates_non_contention_errors(tmp_path, monkeypatch, error_number):
35+
monkeypatch.chdir(tmp_path)
36+
monkeypatch.setattr(GitConfigParser, "t_lock", BlockingLockFile)
37+
error = OSError(error_number, os.strerror(error_number), "config.lock")
38+
with mock.patch.object(os, "open", side_effect=error) as open_lock:
39+
with mock.patch.object(time, "sleep", side_effect=AssertionError("Must not retry")) as sleep:
40+
with pytest.raises(OSError) as exc_info:
41+
GitConfigParser("config", read_only=False)
42+
sleep.assert_not_called()
43+
open_lock.assert_called_once()
44+
assert exc_info.value.__cause__ is error
45+
assert str(exc_info.value) == str(error)
46+
assert not (tmp_path / "config.lock").exists()
47+
48+
3149
class TestBase(TestCase):
3250
def setUp(self):
3351
_rm_lock_files()

‎test/test_util.py‎

Lines changed: 102 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
# 3-Clause BSD License: https://opensource.org/license/bsd-3-clause/
55

66
import ast
7-
from datetime import datetime
7+
import errno
88
import os
99
import pathlib
1010
import pickle
@@ -14,6 +14,7 @@
1414
import tempfile
1515
import threading
1616
import time
17+
from datetime import datetime
1718
from unittest import SkipTest, mock
1819

1920
import ddt
@@ -37,12 +38,11 @@
3738
LockFile,
3839
cygpath,
3940
decygpath,
40-
is_cygwin_git,
4141
get_user_id,
42+
is_cygwin_git,
4243
remove_password_if_present,
4344
rmtree,
4445
)
45-
4646
from test.lib import TestBase, requires_symlinks, with_rw_repo
4747

4848

@@ -882,3 +882,102 @@ def test_iterable_list_name_lookup_ignores_list_attributes(name, prefix):
882882
assert name not in values
883883
with pytest.raises(IndexError, match="No item found with id"):
884884
values[name]
885+
886+
887+
@pytest.mark.parametrize("path_type", [str, pathlib.Path])
888+
@pytest.mark.parametrize("relative", [True, False])
889+
def test_blocking_lock_waits_for_existing_holder(tmp_path, monkeypatch, path_type, relative):
890+
monkeypatch.chdir(tmp_path)
891+
path = path_type("resource" if relative else tmp_path / "resource")
892+
owner = LockFile(path)
893+
waiter = BlockingLockFile(path, check_interval_s=0.01, max_block_time_s=1)
894+
owner._obtain_lock()
895+
try:
896+
with mock.patch.object(time, "sleep", side_effect=lambda _: owner._release_lock()) as sleep:
897+
waiter._obtain_lock()
898+
sleep.assert_called_once_with(0.01)
899+
assert waiter._has_lock()
900+
finally:
901+
owner._release_lock()
902+
waiter._release_lock()
903+
904+
905+
@pytest.mark.parametrize("path_type", [str, pathlib.Path])
906+
def test_blocking_relative_lock_reports_timeout(tmp_path, monkeypatch, path_type):
907+
monkeypatch.chdir(tmp_path)
908+
owner = LockFile(path_type("resource"))
909+
waiter = BlockingLockFile(path_type("resource"), max_block_time_s=0)
910+
owner._obtain_lock()
911+
try:
912+
with pytest.raises(OSError, match="Waited .* seconds for lock"):
913+
waiter._obtain_lock()
914+
finally:
915+
owner._release_lock()
916+
917+
918+
def test_blocking_lock_still_rejects_missing_directory(tmp_path):
919+
waiter = BlockingLockFile(tmp_path / "missing" / "resource")
920+
with mock.patch.object(time, "sleep", side_effect=AssertionError("Must not retry")) as sleep:
921+
with pytest.raises(OSError) as exc_info:
922+
waiter._obtain_lock()
923+
sleep.assert_not_called()
924+
assert isinstance(exc_info.value.__cause__, FileNotFoundError)
925+
assert not waiter._has_lock()
926+
927+
928+
@pytest.mark.skipif(sys.platform == "win32", reason="Lock creation uses CreateFileW on Windows")
929+
@pytest.mark.parametrize(
930+
"error_number", [errno.EACCES, errno.ENOENT, errno.ENOTDIR, errno.EROFS, errno.ENOSPC, errno.EIO]
931+
)
932+
@pytest.mark.parametrize("path_type", [str, pathlib.Path])
933+
@pytest.mark.parametrize("relative", [True, False])
934+
def test_blocking_lock_propagates_non_contention_errors(tmp_path, monkeypatch, error_number, path_type, relative):
935+
monkeypatch.chdir(tmp_path)
936+
path = path_type("resource" if relative else tmp_path / "resource")
937+
waiter = BlockingLockFile(path)
938+
error = OSError(error_number, os.strerror(error_number), waiter._lock_file_path())
939+
with mock.patch.object(os, "open", side_effect=error) as open_lock:
940+
with mock.patch.object(time, "sleep", side_effect=AssertionError("Must not retry")) as sleep:
941+
with pytest.raises(OSError) as exc_info:
942+
waiter._obtain_lock()
943+
sleep.assert_not_called()
944+
open_lock.assert_called_once()
945+
assert exc_info.value.__cause__ is error
946+
assert str(exc_info.value) == str(error)
947+
assert not waiter._has_lock()
948+
assert not os.path.lexists(waiter._lock_file_path())
949+
950+
951+
@pytest.mark.skipif(os.name != "posix", reason="Directory write permissions require POSIX")
952+
@pytest.mark.parametrize("path_type", [str, pathlib.Path])
953+
def test_blocking_relative_lock_rejects_unwritable_cwd(tmp_path, monkeypatch, path_type):
954+
monkeypatch.chdir(tmp_path)
955+
original_mode = tmp_path.stat().st_mode
956+
tmp_path.chmod(stat.S_IRUSR | stat.S_IXUSR)
957+
try:
958+
if os.access(".", os.W_OK):
959+
pytest.skip("Current user can write to read-only directories")
960+
assert os.path.isdir(".")
961+
waiter = BlockingLockFile(path_type("resource"))
962+
with mock.patch.object(time, "sleep", side_effect=AssertionError("Must not retry")) as sleep:
963+
with pytest.raises(OSError) as exc_info:
964+
waiter._obtain_lock()
965+
sleep.assert_not_called()
966+
assert isinstance(exc_info.value.__cause__, PermissionError)
967+
assert not waiter._has_lock()
968+
finally:
969+
tmp_path.chmod(original_mode)
970+
971+
972+
@pytest.mark.skipif(os.name != "posix", reason="Removing the current directory requires POSIX")
973+
@pytest.mark.parametrize("path_type", [str, pathlib.Path])
974+
def test_blocking_relative_lock_rejects_removed_cwd(tmp_path, monkeypatch, path_type):
975+
monkeypatch.chdir(tmp_path)
976+
tmp_path.rmdir()
977+
waiter = BlockingLockFile(path_type("resource"))
978+
with mock.patch.object(time, "sleep", side_effect=AssertionError("Must not retry")) as sleep:
979+
with pytest.raises(OSError) as exc_info:
980+
waiter._obtain_lock()
981+
sleep.assert_not_called()
982+
assert isinstance(exc_info.value.__cause__, FileNotFoundError)
983+
assert not waiter._has_lock()

0 commit comments

Comments
 (0)