| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -1166,15 +1166,56 @@ def _obtain_lock_or_raise(self) -> None: | |||
| 1166 | 1166 | if self._has_lock(): | |
| 1167 | 1167 | return | |
| 1168 | 1168 | lock_file = self._lock_file_path() | |
| 1169 | - if osp.isfile(lock_file): | ||
| 1169 | + # Create the lock in one step. Checking for it first would allow another | ||
| 1170 | + # holder to create it between the check and the open. | ||
| 1171 | + try: | ||
| 1172 | + if sys.platform == "win32": | ||
| 1173 | + if "\0" in lock_file: | ||
| 1174 | + raise ValueError("embedded null character") | ||
| 1175 | + | ||
| 1176 | + import ctypes | ||
| 1177 | + from ctypes import wintypes | ||
| 1178 | + | ||
| 1179 | + # Unlike POSIX, Windows follows dangling symlinks even with O_EXCL. | ||
| 1180 | + # Open the reparse point itself so an existing link is rejected. | ||
| 1181 | + # Call the Unicode API directly: older _winapi.CreateFile wrappers | ||
| 1182 | + # use the ANSI API and can create a lock under the wrong filename. | ||
| 1183 | + kernel32 = ctypes.WinDLL("kernel32", use_last_error=True) | ||
| 1184 | + create_file = kernel32.CreateFileW | ||
| 1185 | + create_file.argtypes = ( | ||
| 1186 | + wintypes.LPCWSTR, | ||
| 1187 | + wintypes.DWORD, | ||
| 1188 | + wintypes.DWORD, | ||
| 1189 | + wintypes.LPVOID, | ||
| 1190 | + wintypes.DWORD, | ||
| 1191 | + wintypes.DWORD, | ||
| 1192 | + wintypes.HANDLE, | ||
| 1193 | + ) | ||
| 1194 | + create_file.restype = wintypes.HANDLE | ||
| 1195 | + close_handle = kernel32.CloseHandle | ||
| 1196 | + close_handle.argtypes = (wintypes.HANDLE,) | ||
| 1197 | + close_handle.restype = wintypes.BOOL | ||
| 1198 | + handle = create_file( | ||
| 1199 | + lock_file, | ||
| 1200 | + 0x40000000, # GENERIC_WRITE | ||
| 1201 | + 0, | ||
| 1202 | + None, | ||
| 1203 | + 1, # CREATE_NEW | ||
| 1204 | + 0x00200000, # FILE_FLAG_OPEN_REPARSE_POINT | ||
| 1205 | + None, | ||
| 1206 | + ) | ||
| 1207 | + if handle == wintypes.HANDLE(-1).value: | ||
| 1208 | + raise ctypes.WinError(ctypes.get_last_error()) | ||
| 1209 | + if not close_handle(handle): | ||
| 1210 | + raise ctypes.WinError(ctypes.get_last_error()) | ||
| 1211 | + else: | ||
| 1212 | + fd = os.open(lock_file, os.O_WRONLY | os.O_CREAT | os.O_EXCL, 0o600) | ||
| 1213 | + os.close(fd) | ||
| 1214 | + except FileExistsError as e: | ||
| 1170 | 1215 | raise OSError( | |
| 1171 | 1216 | "Lock for file %r did already exist, delete %r in case the lock is illegal" | |
| 1172 | 1217 | % (self._file_path, lock_file) | |
| 1173 | - ) | ||
| 1174 | - | ||
| 1175 | - try: | ||
| 1176 | - with open(lock_file, mode="w"): | ||
| 1177 | - pass | ||
| 1218 | + ) from e | ||
| 1178 | 1219 | except OSError as e: | |
| 1179 | 1220 | raise OSError(str(e)) from e | |
| 1180 | 1221 | ||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -12,6 +12,7 @@ | |||
| 12 | 12 | import subprocess | |
| 13 | 13 | import sys | |
| 14 | 14 | import tempfile | |
| 15 | + import threading | ||
| 15 | 16 | import time | |
| 16 | 17 | from unittest import SkipTest, mock | |
| 17 | 18 | ||
@@ -418,9 +419,11 @@ def test_it_should_dashify(self): | |||
| 418 | 419 | self.assertEqual("this-is-my-argument", dashify("this_is_my_argument")) | |
| 419 | 420 | self.assertEqual("foo", dashify("foo")) | |
| 420 | 421 | ||
| 421 | - def test_lock_file(self): | ||
| 422 | + @ddt.data("my-lock-file", "my-lock-file-\u0394", "\u0394/my-lock-file", "\U0001f680/my-lock-file") | ||
| 423 | + def test_lock_file(self, filename): | ||
| 422 | 424 | with tempfile.TemporaryDirectory() as tdir: | |
| 423 | - my_file = os.path.join(tdir, "my-lock-file") | ||
| 425 | + my_file = os.path.join(tdir, filename) | ||
| 426 | + os.makedirs(os.path.dirname(my_file), exist_ok=True) | ||
| 424 | 427 | lock_file = LockFile(my_file) | |
| 425 | 428 | assert not lock_file._has_lock() | |
| 426 | 429 | # Release lock we don't have - fine. | |
@@ -429,6 +432,7 @@ def test_lock_file(self): | |||
| 429 | 432 | # Get lock. | |
| 430 | 433 | lock_file._obtain_lock_or_raise() | |
| 431 | 434 | assert lock_file._has_lock() | |
| 435 | + assert os.path.isfile(my_file + ".lock") | ||
| 432 | 436 | ||
| 433 | 437 | # Concurrent access. | |
| 434 | 438 | other_lock_file = LockFile(my_file) | |
@@ -437,6 +441,7 @@ def test_lock_file(self): | |||
| 437 | 441 | ||
| 438 | 442 | lock_file._release_lock() | |
| 439 | 443 | assert not lock_file._has_lock() | |
| 444 | + assert not os.path.exists(my_file + ".lock") | ||
| 440 | 445 | ||
| 441 | 446 | other_lock_file._obtain_lock_or_raise() | |
| 442 | 447 | self.assertRaises(IOError, lock_file._obtain_lock_or_raise) | |
@@ -446,6 +451,67 @@ def test_lock_file(self): | |||
| 446 | 451 | lock_file._obtain_lock_or_raise() | |
| 447 | 452 | lock_file._release_lock() | |
| 448 | 453 | ||
| 454 | + def test_lock_file_rejects_embedded_nul(self): | ||
| 455 | + with tempfile.TemporaryDirectory() as tdir: | ||
| 456 | + my_file = os.path.join(tdir, "my-lock-file") | ||
| 457 | + lock_file = LockFile(my_file + "\0suffix") | ||
| 458 | + self.assertRaises(ValueError, lock_file._obtain_lock_or_raise) | ||
| 459 | + assert not lock_file._has_lock() | ||
| 460 | + assert not os.path.exists(my_file) | ||
| 461 | + | ||
| 462 | + @ddt.data(False, True) | ||
| 463 | + @requires_symlinks | ||
| 464 | + def test_lock_file_does_not_follow_a_symlink(self, target_exists): | ||
| 465 | + with tempfile.TemporaryDirectory() as tdir: | ||
| 466 | + my_file = os.path.join(tdir, "my-lock-file") | ||
| 467 | + outside = os.path.join(tdir, "outside-the-lock") | ||
| 468 | + content = b"Do not modify the symlink target." | ||
| 469 | + if target_exists: | ||
| 470 | + with open(outside, "wb") as stream: | ||
| 471 | + stream.write(content) | ||
| 472 | + os.symlink(outside, my_file + ".lock") | ||
| 473 | + | ||
| 474 | + lock_file = LockFile(my_file) | ||
| 475 | + self.assertRaises(IOError, lock_file._obtain_lock_or_raise) | ||
| 476 | + assert not lock_file._has_lock() | ||
| 477 | + lock_file._release_lock() | ||
| 478 | + assert os.path.islink(my_file + ".lock") | ||
| 479 | + if target_exists: | ||
| 480 | + with open(outside, "rb") as stream: | ||
| 481 | + self.assertEqual(stream.read(), content) | ||
| 482 | + else: | ||
| 483 | + assert not os.path.exists(outside) | ||
| 484 | + | ||
| 485 | + def test_lock_file_is_obtained_by_a_single_holder(self): | ||
| 486 | + with tempfile.TemporaryDirectory() as tdir: | ||
| 487 | + my_file = os.path.join(tdir, "my-lock-file") | ||
| 488 | + racers = 8 | ||
| 489 | + at_the_line = threading.Barrier(racers) | ||
| 490 | + holders = [] | ||
| 491 | + guard = threading.Lock() | ||
| 492 | + | ||
| 493 | + def obtain(): | ||
| 494 | + lock_file = LockFile(my_file) | ||
| 495 | + at_the_line.wait() | ||
| 496 | + try: | ||
| 497 | + lock_file._obtain_lock_or_raise() | ||
| 498 | + except OSError: | ||
| 499 | + return | ||
| 500 | + with guard: | ||
| 501 | + holders.append(lock_file) | ||
| 502 | + | ||
| 503 | + threads = [threading.Thread(target=obtain) for _ in range(racers)] | ||
| 504 | + for thread in threads: | ||
| 505 | + thread.start() | ||
| 506 | + for thread in threads: | ||
| 507 | + thread.join() | ||
| 508 | + | ||
| 509 | + try: | ||
| 510 | + self.assertEqual(1, len(holders)) | ||
| 511 | + finally: | ||
| 512 | + for lock_file in holders: | ||
| 513 | + lock_file._release_lock() | ||
| 514 | + | ||
| 449 | 515 | def test_blocking_lock_file(self): | |
| 450 | 516 | with tempfile.TemporaryDirectory() as tdir: | |
| 451 | 517 | my_file = os.path.join(tdir, "my-lock-file") | |
| Back | FazBrowse Home | New Git URL |
0 commit comments