Skip to content

Fix race conditions in temporary file creation #13352

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Mic92
Copy link
Member

@Mic92 Mic92 commented Jun 12, 2025

Fix race conditions in temporary file creation

Replace weak random number generation with atomic counters and implement
retry loops to handle EEXIST errors in four locations:

  1. optimise-store.cc: Fix race in temporary hard link creation

    • Replace rand() with makeTempPath() for unique naming
    • Add infinite retry loop for handling concurrent processes
    • Use std::filesystem for portable path construction
  2. indirect-root-store.cc: Fix race in temporary symlink creation

    • Replace rand() with makeTempPath() for unique naming
    • Add infinite retry loop for makeSymlink()
    • Properly handle cleanup of failed temporary symlinks
  3. local-store.cc: Fix race in build log creation

    • Replace simple getpid() with makeTempPath() for unique naming
    • Add retry loop for handling concurrent log writes
    • Accept EEXIST as success when another process creates the log
  4. local-binary-cache-store.cc: Fix race in cache file creation

    • Replace getpid() + counter with makeTempPath() for unique naming
    • Ensures atomic file updates in binary cache operations

All fixes use the existing makeTempPath() utility which provides atomic
counter + getpid() based unique name generation. All locations ensure
proper SysError exceptions are thrown instead of propagating
std::filesystem errors.

Motivation

Context


Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

@Mic92 Mic92 requested a review from Ericson2314 as a code owner June 12, 2025 15:11
@github-actions github-actions bot added the store Issues and pull requests concerning the Nix store label Jun 12, 2025
@Mic92 Mic92 marked this pull request as draft June 12, 2025 15:13
Replace rand() with atomic counter for unique temp link names and add
retry loop to handle EEXIST errors when multiple processes optimize
the store simultaneously.
@Mic92 Mic92 force-pushed the fix-parallel branch 3 times, most recently from 977f63e to 4cc7eca Compare June 12, 2025 15:51
@Mic92 Mic92 marked this pull request as ready for review June 12, 2025 15:51
@Mic92 Mic92 added backport 2.28-maintenance Automatically creates a PR against the branch backport 2.29-maintenance Automatically creates a PR against the branch labels Jun 12, 2025
Mic92 added 3 commits June 12, 2025 17:58
Replace rand() with atomic counter and add retry loop to handle
race conditions when creating temporary symlinks for garbage
collector roots.
Add atomic counter to temporary log file names and retry loop to
handle EEXIST errors. If another process creates the log file first,
we gracefully exit since the log already exists.
Handle race conditions in binary cache file creation by retrying
with a new temporary name on EEXIST errors. Uses existing
makeTempPath which already has an atomic counter.
@Mic92 Mic92 requested review from edolstra and removed request for edolstra June 16, 2025 15:13
@Mic92 Mic92 marked this pull request as draft June 16, 2025 15:15
@Mic92
Copy link
Member Author

Mic92 commented Jun 16, 2025

needs more work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.28-maintenance Automatically creates a PR against the branch backport 2.29-maintenance Automatically creates a PR against the branch store Issues and pull requests concerning the Nix store
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant