Skip to content

Fix race conditions in temporary file creation#13352

Draft
Mic92 wants to merge 4 commits intoNixOS:masterfrom
Mic92:fix-parallel
Draft

Fix race conditions in temporary file creation#13352
Mic92 wants to merge 4 commits intoNixOS:masterfrom
Mic92:fix-parallel

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