Skip to content

🐛 fix(api): skip mkdir on a nested acquire - #878

Merged
gaborbernat merged 2 commits into
tox-dev:mainfrom
0x5t4l1n:fix/reentrant-directory-setup
Oct 28, 2026
Merged

gaborbernat merged 2 commits into
tox-dev:mainfrom
0x5t4l1n:fix/reentrant-directory-setup

Conversation

@0x5t4l1n

@0x5t4l1n 0x5t4l1n commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

A program that renames the lock file's directory while it holds a FileLock or SoftFileLock, then acquires the same lock again inside that hold, found the nested acquire recreating the directory under its old name. When a file had taken that name in the meantime, the nested acquire raised FileExistsError and left the hold's recursion count one step too high, so leaving the outer block no longer released the lock. 🔒

The lock now creates the parent directory only when it starts a fresh hold. A nested acquire joins the hold it already has, which keeps its original descriptor and path, so it has no reason to touch the filesystem. The first acquisition still creates the directory once before polling, so a directory that disappears while a contender waits fails that contender rather than building a second lock file beside the holder's, as before.

Programs that leave a held lock's directory in place see no change.

0x5t4l1n and others added 2 commits October 25, 2026 00:27
The regression test branched on whether a file took the renamed
directory's old name, so one case could mask the other and the ids read
as booleans. Each case now supplies the step that occupies the old name,
and the single directory check covers both outcomes.

Adds the news fragment the fix needs, and shortens the comment at the
guard to the two reasons it exists.
@gaborbernat gaborbernat changed the title fix(api): skip directory setup when reentering a held lock 🐛 fix(api): skip mkdir on a nested acquire Oct 28, 2026
@gaborbernat gaborbernat reopened this Oct 28, 2026
@gaborbernat
gaborbernat merged commit bc92a5b into tox-dev:main Oct 28, 2026
71 of 100 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants