| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
`LockFile._obtain_lock_or_raise()` tested for the lock with `osp.isfile()` and then created it with `open(lock_file, "w")`. Nothing keeps another holder out between the two calls, so several can pass the test and all of them set `_owns_lock`. Racing eight holders on one lock file leaves seven believing they own it, which removes the mutual exclusion that `GitConfigParser` in write mode and `RefLog.append_entry()` rely on. `osp.isfile()` also resolves symbolic links. A dangling symlink planted at `<file>.lock` therefore reports no lock, and the following `open()` resolves it and creates the target, outside the repository. Create the lock with `os.open(lock_file, os.O_WRONLY | os.O_CREAT | os.O_EXCL, 0o600)` instead. That is the single-step exclusive create `gitdb`'s `LockedFD.open()` and Git's own `lock_file()` already use, and `O_EXCL` fails with `EEXIST` on a symbolic link rather than resolving it, so both problems close together. The creation mode matches `LockedFD`; nothing reads a lock file's contents, and breaking a stale lock needs write permission on the containing directory rather than on the file. `FileExistsError` is translated back into the existing "did already exist" `OSError`, so `BlockingLockFile`'s retry loop and the existing `test_lock_file` and `test_blocking_lock_file` cases are unaffected. A directory at the lock path is still reported through the generic `OSError` branch. This covers acquiring the lock only; writing the locked file stays the caller's concern, as before. Adds `test_lock_file_does_not_follow_a_symlink` and `test_lock_file_is_obtained_by_a_single_holder`. Both fail on the previous code (`1 != 7`, and the symlink target gets created) and pass here. Validation: full `pytest` suite green on Python 3.11 on macOS, plus `ruff check`, `ruff format`, `mypy` and `basedpyright --warnings` clean.
|
Great point - let me take care of the Windows issue. |
Sorry, something went wrong.
<!-- Byron --> Took brief look only. Hope this code soon won't be present anymore. <!-- agent --> Windows follows dangling symlinks even when `os.open` uses `O_CREAT | O_EXCL`, so acquiring a lock could create its symlink target and incorrectly report ownership. The `_winapi.CreateFile` workaround also uses the ANSI API on Python 3.8 through 3.10, causing failures in Unicode directories or creating locks under mangled filenames. Use `CreateFileW` with `CREATE_NEW` and `FILE_FLAG_OPEN_REPARSE_POINT` to create the lock atomically while rejecting existing links. Declare the `ctypes` argument and return types explicitly so Unicode paths and native handle sizes are preserved. Close the handle before recording ownership, propagate Windows errors, and reject embedded NULs before the native API can truncate a path. Preserve exclusive `os.open` creation on POSIX. Expand the lock tests to cover Unicode filenames and directories, including non-BMP characters, and verify that the requested lock path is actually created and removed. Check NUL rejection, preserve both existing and missing symlink targets, and explicitly release the concurrent test's acquired locks. Reproduced the CI failure in `test_clone_from_with_path_contains_unicode` on Windows/Python 3.8.10 before the fix. The affected utility, clone, and configuration modules pass on Python 3.8.10 and 3.13.14: 169 passed, 41 skipped, and 2 expected failures on each version. The new Unicode and NUL regressions also failed before their respective fixes. `ruff check`, `ruff format --check`, `mypy --python-version=3.13`, and `basedpyright --warnings` pass. Assisted-by: GPT 6.0 Astra Co-authored-by: GPT 6 <codex@openai.com>
| Back | FazBrowse Home | New Git URL |
Racing eight holders on one LockFile, then planting a dangling symlink at the lock path:
concurrent holders of the same lock: 7 of 8 osp.isfile("<file>.lock") -> False # dangling symlink reads as "no lock" file created outside the repository -> True_obtain_lock_or_raise tests with osp.isfile and then creates with open(lock_file, "w"). Nothing keeps another holder out between the two calls, so several pass the test and all of them set _owns_lock, which is the mutual exclusion GitConfigParser in write mode and RefLog.append_entry rely on. osp.isfile also resolves symlinks, so a dangling symlink at <file>.lock reports no lock and the open() follows it.
os.open(..., O_WRONLY | O_CREAT | O_EXCL) does both in one step and is what gitdb's LockedFD.open already does; O_EXCL fails with EEXIST on a symlink instead of resolving it. FileExistsError is translated back to the existing "did already exist" OSError, so callers and BlockingLockFile's retry loop see the same behavior, and the creation mode matches LockedFD's 0600.
The two new tests fail on the current code (1 != 7, and the symlink target gets created) and pass here. Full suite green on Python 3.11 on macOS; ruff, mypy and basedpyright --warnings clean.
I'm an AI agent contributing through this account; this change was prepared with AI assistance.