Skip to content

std.Build.Step.Compile: fix race condition in args file creation #23997

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ianprime0509
Copy link
Contributor

Fixes #23993

Previously, if multiple build processes tried to create the same args file, there was a race condition with the use of the non-atomic writeFile function which could cause a spawned compiler to read an empty or incomplete args file. This commit avoids the race condition by first writing to a temporary file with a random path and renaming it to the desired path.


This follows the same strategy as the package fetching code, including the convention of tmp/<random u64 hex> as the temporary file name.

I tested this with the original reproducer from the linked issue, and I'm no longer seeing any errors with this change applied. I also tested it with the .zig-cache directory deleted, to make sure it works properly on the first run, before the args file is created.

Fixes ziglang#23993

Previously, if multiple build processes tried to create the same args file, there was a race condition with the use of the non-atomic `writeFile` function which could cause a spawned compiler to read an empty or incomplete args file. This commit avoids the race condition by first writing to a temporary file with a random path and renaming it to the desired path.
// It's fine if the temporary file can't be cleaned up.
};
b.cache_root.handle.rename(tmp_path, args_file) catch |rename_err| switch (rename_err) {
error.PathAlreadyExists => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's worth noting that the package fetching code also handles error.AccessDenied:

error.PathAlreadyExists, error.AccessDenied => {
However, based on the documentation for RenameError, it looks like that case is only relevant for directories, not files as in this case:

zig/lib/std/posix.zig

Lines 2595 to 2597 in ef35c3d

/// On Windows, this error may be returned instead of PathAlreadyExists when
/// renaming a directory over an existing directory.
AccessDenied,
so this code doesn't handle it.

@alexrp alexrp requested a review from mlugg May 28, 2025 10:11
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.

Build fails sporadically with cached compile step argument file
1 participant