Skip to content

fix(api): skip directory setup when reentering a held lock - #878

Open
0x5t4l1n wants to merge 1 commit into
tox-dev:mainfrom
0x5t4l1n:fix/reentrant-directory-setup
Open

0x5t4l1n wants to merge 1 commit into
tox-dev:mainfrom
0x5t4l1n:fix/reentrant-directory-setup

Conversation

@0x5t4l1n

Copy link
Copy Markdown

A synchronous nested acquisition of an already-held FileLock or SoftFileLock still calls ensure_directory_exists(). If the parent directory was renamed while the lock was held, this recreates the original directory. If a regular file has since taken that name, nested acquisition raises FileExistsError instead. In that failure case the recursion count also remains incremented, so leaving the outer context does not release the hold.

Only create the parent directory when the lock is not already held. Reentrant acquisition joins the existing descriptor and keeps balanced recursion counts. Initial acquisition still creates the directory once before polling, preserving the existing behavior when a directory disappears while a contender waits.

Add four Linux/POSIX regression cases: native and soft locks, with the old parent name either absent or replaced by a file. All four fail on the base revision and pass with the fix.

Validation:

  • Windows, CPython 3.14.9: tests/test_filelock.py, tests/test_async_filelock.py and tests/test_default_mode.py -- 707 passed, 101 skipped.
  • Linux (WSL), CPython 3.13.12, same three modules -- 758 passed, 48 skipped, 2 failed. Both failures are the existing shared/thread-local variants of test_async_lock_on_a_daemon_loop_thread_held_at_interpreter_exit_releases_its_marker; both also fail on the unchanged base revision 008e3c3 in the same environment.
  • Focused Linux reentrancy, relative-release and directory-rename tests -- 14 passed. Coverage from this run reports 100% of changed executable source lines via diff-cover.
  • Ruff check, Ruff format --check and git diff --check pass.

Prepared and tested with Codex assistance. The full repository test suite and CI platform matrix were not run locally.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant