Catcrafts/Crafter.Build
2
0
Fork 0
Code Issues Pull requests Projects Releases 1 Packages Wiki Activity Actions

Race condition: concurrent builds clobber each other's external dep clones #3

Merged
jorijnvdgraaf merged 1 commit from claude/issue-2 into master 2026-05-27 04:06:53 +02:00
Conversation 0 Commits 1 Files changed 1 +10 -2
1 changed files with 10 additions and 2 deletions
Download patch file Download diff file Expand all files Collapse all files
Showing only changes of commit 5ac7077dd0 - Show all commits

fix external cache race on concurrent builds
Some checks failed
CI / build-test-release (pull_request) Has been cancelled
Details

Prev Next 
Two crafter-build invocations resolving to the same external dep both
cloned into <cache>/<name>-<hash>.tmp, corrupting each other's pack
tempfiles. Use a random per-invocation tmp suffix and treat a failed
rename whose destination now exists as the loser-of-the-race case —
discard the local clone and reuse the winner's.
catbot 2026-05-27 01:55:29 +00:00
commit 5ac7077dd0

12
implementations/Crafter.Build-External.cpp
View file

@ -103,9 +103,13 @@ std::string FetchGit(const GitSource& source, const fs::path& cloneDir) {
} }
} }
// Random per-invocation tmp suffix so concurrent crafter-build processes
// resolving to the same cache key don't share an in-flight `.tmp`
// directory — git's pack tempfiles would get yanked when one process
// removes or renames the dir out from under the other.
std::random_device rd;
fs::path tmpDir = cloneDir; fs::path tmpDir = cloneDir;
tmpDir += ".tmp"; tmpDir += std::format(".tmp.{:08x}{:08x}", rd(), rd());
if (fs::exists(tmpDir)) fs::remove_all(tmpDir);
if (auto err = runGit(std::format("git clone --recursive {} {}", ShellQuote(source.url), ShellQuote(tmpDir.string())))) { if (auto err = runGit(std::format("git clone --recursive {} {}", ShellQuote(source.url), ShellQuote(tmpDir.string())))) {
if (fs::exists(tmpDir)) fs::remove_all(tmpDir); if (fs::exists(tmpDir)) fs::remove_all(tmpDir);
@ -128,6 +132,10 @@ std::string FetchGit(const GitSource& source, const fs::path& cloneDir) {
fs::rename(tmpDir, cloneDir, ec); fs::rename(tmpDir, cloneDir, ec);
if (ec) { if (ec) {
fs::remove_all(tmpDir); fs::remove_all(tmpDir);
// If a concurrent build won the race, cloneDir now exists with the
// same source revision (cache key includes url+branch+commit), so
// discard our clone and reuse theirs.
if (fs::exists(cloneDir)) return "";
return std::format("rename {} -> {} failed: {}", tmpDir.string(), cloneDir.string(), ec.message()); return std::format("rename {} -> {} failed: {}", tmpDir.string(), cloneDir.string(), ec.message());
} }
return ""; return "";