fix: atomic-rename host-cache PCMs to close concurrent-build race
All checks were successful
CI / build-test-release (pull_request) Successful in 11m50s
All checks were successful
CI / build-test-release (pull_request) Successful in 11m50s
Two crafter-build invocations sharing XDG_CACHE_HOME used to clobber each other's writes to <cache>/<target>-<march>/std.pcm and the Crafter.Build-*.pcm modules: each LoadProject path wrote directly to the final path, so a reader could see a half-written file and die with "malformed or corrupted precompiled file: 'can't skip to bit X from Y'" (issue #14). Every BuildStdPcm / EnsureCrafterBuildPcms write now goes via <final>.tmp.<pid>.<seq> and atomic-renames into place; concurrent writers always see either the old or the new file, never torn bytes. The mingw-on- Linux std.cppm copy is per-PID for the same reason. Adds a regression test (ConcurrentCacheRace) that races four LoadProject() calls against a cold scratch cache — reproduces the race 5/5 without the fix and passes 5/5 with it. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
parent
a930a4abbd
commit
96d1df9233
3 changed files with 237 additions and 12 deletions
|
|
@ -36,6 +36,42 @@ import :Progress;
|
|||
namespace fs = std::filesystem;
|
||||
using namespace Crafter;
|
||||
|
||||
namespace {
|
||||
// Used to make per-process temp filenames so concurrent crafter-build
|
||||
// invocations sharing the host cache dir don't collide on the same
|
||||
// partially-written .pcm. Cross-process uniqueness comes from the PID;
|
||||
// intra-process uniqueness from the counter (multiple host PCMs precompile
|
||||
// back-to-back, and tests can call BuildStdPcm from several threads).
|
||||
unsigned long CurrentProcessId() {
|
||||
#ifdef CRAFTER_BUILD_CONFIGURATION_TARGET_x86_64_pc_linux_gnu
|
||||
return static_cast<unsigned long>(getpid());
|
||||
#else
|
||||
return static_cast<unsigned long>(GetCurrentProcessId());
|
||||
#endif
|
||||
}
|
||||
|
||||
fs::path MakeTempPcmPath(const fs::path& finalPath) {
|
||||
static std::atomic<unsigned> seq{0};
|
||||
return fs::path(finalPath.string() + std::format(".tmp.{}.{}",
|
||||
CurrentProcessId(), seq.fetch_add(1, std::memory_order_relaxed)));
|
||||
}
|
||||
|
||||
// Atomically swap `tmpPath` into `finalPath`. If a parallel crafter-build
|
||||
// committed its own equivalent build first (identical .cppm + flags →
|
||||
// identical .pcm), `fs::rename` may fail on some filesystems; accept the
|
||||
// outcome as long as the destination now exists.
|
||||
std::string CommitPcm(const fs::path& tmpPath, const fs::path& finalPath) {
|
||||
std::error_code ec;
|
||||
fs::rename(tmpPath, finalPath, ec);
|
||||
if (!ec) return "";
|
||||
std::error_code rmEc;
|
||||
fs::remove(tmpPath, rmEc);
|
||||
if (fs::exists(finalPath)) return "";
|
||||
return std::format("rename {} -> {}: {}",
|
||||
tmpPath.string(), finalPath.string(), ec.message());
|
||||
}
|
||||
}
|
||||
|
||||
fs::path Crafter::GetCrafterBuildHome() {
|
||||
if (const char* envHome = std::getenv("CRAFTER_BUILD_HOME")) {
|
||||
return fs::path(envHome);
|
||||
|
|
@ -250,7 +286,14 @@ std::string Crafter::BuildStdPcm(const Configuration& config, fs::path stdPcm) {
|
|||
std::string stdcppm = std::format("{}\\modules\\c++\\v1\\std.cppm", libcxx);
|
||||
|
||||
if(!fs::exists(stdPcm) || fs::last_write_time(stdPcm) < fs::last_write_time(stdcppm)) {
|
||||
return RunCommand(std::format("clang++ --target={} -march={} -mtune={} -isystem %LIBCXX_DIR%\\include\\c++\\v1 -nostdinc++ -nostdlib++ -std=c++26 -Wno-reserved-identifier -Wno-reserved-module-identifier --precompile %LIBCXX_DIR%\\modules\\c++\\v1\\std.cppm -o {}", config.target, config.march, config.mtune, stdPcm.string()));
|
||||
fs::path tmpPcm = MakeTempPcmPath(stdPcm);
|
||||
std::string out = RunCommand(std::format("clang++ --target={} -march={} -mtune={} -isystem %LIBCXX_DIR%\\include\\c++\\v1 -nostdinc++ -nostdlib++ -std=c++26 -Wno-reserved-identifier -Wno-reserved-module-identifier --precompile %LIBCXX_DIR%\\modules\\c++\\v1\\std.cppm -o {}", config.target, config.march, config.mtune, tmpPcm.string()));
|
||||
if (!out.empty()) {
|
||||
std::error_code ec;
|
||||
fs::remove(tmpPcm, ec);
|
||||
return out;
|
||||
}
|
||||
return CommitPcm(tmpPcm, stdPcm);
|
||||
}
|
||||
return "";
|
||||
}
|
||||
|
|
@ -270,6 +313,7 @@ namespace {
|
|||
if (fs::exists(pcmPath) && fs::last_write_time(cppmPath) < fs::last_write_time(pcmPath)) {
|
||||
continue;
|
||||
}
|
||||
fs::path tmpPcm = MakeTempPcmPath(pcmPath);
|
||||
std::string cmd = std::format(
|
||||
"clang++ --target=x86_64-pc-windows-msvc -march=native -mtune=native "
|
||||
"-std=c++26 -O3 -D CRAFTER_BUILD_DLL_IMPORT "
|
||||
|
|
@ -277,11 +321,16 @@ namespace {
|
|||
"-Wno-reserved-identifier -Wno-reserved-module-identifier "
|
||||
"-fprebuilt-module-path={} "
|
||||
"--precompile {} -o {}",
|
||||
cacheDir.string(), cppmPath.string(), pcmPath.string());
|
||||
cacheDir.string(), cppmPath.string(), tmpPcm.string());
|
||||
CommandResult r = Crafter::RunCommandChecked(cmd);
|
||||
if (r.exitCode != 0) {
|
||||
std::error_code ec;
|
||||
fs::remove(tmpPcm, ec);
|
||||
throw std::runtime_error(std::format("Failed to precompile {} (exit {}): {}", name, r.exitCode, r.output));
|
||||
}
|
||||
if (std::string err = CommitPcm(tmpPcm, pcmPath); !err.empty()) {
|
||||
throw std::runtime_error(std::format("Failed to commit {} pcm: {}", name, err));
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -412,11 +461,18 @@ std::string Crafter::BuildStdPcm(const Configuration& config, fs::path stdPcm) {
|
|||
if (fs::exists(stdPcm) && fs::last_write_time(stdPcm) >= fs::last_write_time(stdcppm)) {
|
||||
return "";
|
||||
}
|
||||
return RunCommand(std::format(
|
||||
fs::path tmpPcm = MakeTempPcmPath(stdPcm);
|
||||
std::string out = RunCommand(std::format(
|
||||
"clang++ --target={} -march={} -mtune={} -isystem %LIBCXX_DIR%\\include\\c++\\v1 "
|
||||
"-nostdinc++ -nostdlib++ -std=c++26 -Wno-reserved-identifier -Wno-reserved-module-identifier "
|
||||
"--precompile %LIBCXX_DIR%\\modules\\c++\\v1\\std.cppm -o {}",
|
||||
config.target, config.march, config.mtune, stdPcm.string()));
|
||||
config.target, config.march, config.mtune, tmpPcm.string()));
|
||||
if (!out.empty()) {
|
||||
std::error_code ec;
|
||||
fs::remove(tmpPcm, ec);
|
||||
return out;
|
||||
}
|
||||
return CommitPcm(tmpPcm, stdPcm);
|
||||
}
|
||||
|
||||
// Default: mingw target. Look up mingw-w64 libstdc++ via the msys2 prefix.
|
||||
|
|
@ -430,14 +486,16 @@ std::string Crafter::BuildStdPcm(const Configuration& config, fs::path stdPcm) {
|
|||
}
|
||||
// Copy std.cc → std.cppm in C++ rather than via cmd's `copy /Y` because
|
||||
// `copy` always prints "1 file(s) copied." to stdout and RunCommand
|
||||
// treats any output as an error.
|
||||
fs::path stdCppm = stdPcm.parent_path() / "std.cppm";
|
||||
// treats any output as an error. Per-PID filename so concurrent
|
||||
// crafter-build invocations don't see each other's half-written copies.
|
||||
fs::path stdCppm = stdPcm.parent_path() / std::format("std.cppm.{}", CurrentProcessId());
|
||||
std::error_code ec;
|
||||
fs::copy_file(stdCc, stdCppm, fs::copy_options::overwrite_existing, ec);
|
||||
if (ec) {
|
||||
return std::format("copy {} -> {}: {}", stdCc.string(), stdCppm.string(), ec.message());
|
||||
}
|
||||
return RunCommand(std::format(
|
||||
fs::path tmpPcm = MakeTempPcmPath(stdPcm);
|
||||
std::string out = RunCommand(std::format(
|
||||
"clang++ --target={} -march={} -mtune={} "
|
||||
"--sysroot=\"{}\" -femulated-tls "
|
||||
"-O3 -std=c++26 -Wno-reserved-identifier -Wno-reserved-module-identifier "
|
||||
|
|
@ -445,7 +503,14 @@ std::string Crafter::BuildStdPcm(const Configuration& config, fs::path stdPcm) {
|
|||
config.target, config.march, config.mtune,
|
||||
prefix.string(),
|
||||
stdCppm.string(),
|
||||
stdPcm.string()));
|
||||
tmpPcm.string()));
|
||||
std::error_code rmEc;
|
||||
fs::remove(stdCppm, rmEc);
|
||||
if (!out.empty()) {
|
||||
fs::remove(tmpPcm, rmEc);
|
||||
return out;
|
||||
}
|
||||
return CommitPcm(tmpPcm, stdPcm);
|
||||
}
|
||||
|
||||
std::string Crafter::GetBaseCommand(const Configuration& config) {
|
||||
|
|
@ -469,6 +534,7 @@ namespace {
|
|||
if (fs::exists(pcmPath) && fs::last_write_time(cppmPath) < fs::last_write_time(pcmPath)) {
|
||||
continue;
|
||||
}
|
||||
fs::path tmpPcm = MakeTempPcmPath(pcmPath);
|
||||
std::string cmd = std::format(
|
||||
"clang++ --target=x86_64-w64-mingw32 -march=native -mtune=native "
|
||||
"--sysroot=\"{}\" -femulated-tls "
|
||||
|
|
@ -476,12 +542,18 @@ namespace {
|
|||
"-Wno-reserved-identifier -Wno-reserved-module-identifier "
|
||||
"-fprebuilt-module-path={} "
|
||||
"--precompile {} -o {}",
|
||||
prefix.string(), cacheDir.string(), cppmPath.string(), pcmPath.string());
|
||||
prefix.string(), cacheDir.string(), cppmPath.string(), tmpPcm.string());
|
||||
CommandResult r = Crafter::RunCommandChecked(cmd);
|
||||
if (r.exitCode != 0) {
|
||||
std::error_code ec;
|
||||
fs::remove(tmpPcm, ec);
|
||||
throw std::runtime_error(std::format(
|
||||
"Failed to precompile {} (exit {}): {}", name, r.exitCode, r.output));
|
||||
}
|
||||
if (std::string err = CommitPcm(tmpPcm, pcmPath); !err.empty()) {
|
||||
throw std::runtime_error(std::format(
|
||||
"Failed to commit {} pcm: {}", name, err));
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -680,7 +752,24 @@ std::string Crafter::BuildStdPcm(const Configuration& config, fs::path stdPcm) {
|
|||
if(!fs::exists(stdPcm) || fs::last_write_time(stdPcm) < fs::last_write_time(stdCc)) {
|
||||
// -femulated-tls keeps PCM TLS access matching libstdc++'s emutls
|
||||
// definitions; mismatch surfaces as undefined std::__once_callable.
|
||||
return RunCommand(std::format("cp {} {}/std.cppm\nclang++ --target={} -march={} -mtune={} -femulated-tls -O3 -std=c++26 -Wno-reserved-identifier -Wno-reserved-module-identifier --precompile {}/std.cppm -o {}", stdCc.string(), stdPcm.parent_path().string(), config.target, config.march, config.mtune, stdPcm.parent_path().string(), stdPcm.string()));
|
||||
// Per-PID std.cppm and a tmp .pcm + atomic rename so concurrent
|
||||
// crafter-build invocations sharing the cache dir don't read each
|
||||
// other's half-written files.
|
||||
fs::path stdCppm = stdPcm.parent_path() / std::format("std.cppm.{}", CurrentProcessId());
|
||||
std::error_code copyEc;
|
||||
fs::copy_file(stdCc, stdCppm, fs::copy_options::overwrite_existing, copyEc);
|
||||
if (copyEc) {
|
||||
return std::format("copy {} -> {}: {}", stdCc.string(), stdCppm.string(), copyEc.message());
|
||||
}
|
||||
fs::path tmpPcm = MakeTempPcmPath(stdPcm);
|
||||
std::string out = RunCommand(std::format("clang++ --target={} -march={} -mtune={} -femulated-tls -O3 -std=c++26 -Wno-reserved-identifier -Wno-reserved-module-identifier --precompile {} -o {}", config.target, config.march, config.mtune, stdCppm.string(), tmpPcm.string()));
|
||||
std::error_code rmEc;
|
||||
fs::remove(stdCppm, rmEc);
|
||||
if (!out.empty()) {
|
||||
fs::remove(tmpPcm, rmEc);
|
||||
return out;
|
||||
}
|
||||
return CommitPcm(tmpPcm, stdPcm);
|
||||
} else {
|
||||
return "";
|
||||
}
|
||||
|
|
@ -708,7 +797,14 @@ std::string Crafter::BuildStdPcm(const Configuration& config, fs::path stdPcm) {
|
|||
? std::string(" -fno-exceptions -msimd128 -fno-c++-static-destructors -mllvm -wasm-enable-sjlj -D_WASI_EMULATED_SIGNAL")
|
||||
: std::format(" -march={} -mtune={}", config.march, config.mtune);
|
||||
if(!fs::exists(stdPcm) || fs::last_write_time(stdPcm) < fs::last_write_time(stdCppm)) {
|
||||
return RunCommand(std::format("clang++ --target={} -std=c++26 -stdlib=libc++{}{} -O3 -Wno-reserved-identifier -Wno-reserved-module-identifier --precompile {} -o {}", config.target, sysrootFlag, archFlags, stdCppm, stdPcm.string()));
|
||||
fs::path tmpPcm = MakeTempPcmPath(stdPcm);
|
||||
std::string out = RunCommand(std::format("clang++ --target={} -std=c++26 -stdlib=libc++{}{} -O3 -Wno-reserved-identifier -Wno-reserved-module-identifier --precompile {} -o {}", config.target, sysrootFlag, archFlags, stdCppm, tmpPcm.string()));
|
||||
if (!out.empty()) {
|
||||
std::error_code ec;
|
||||
fs::remove(tmpPcm, ec);
|
||||
return out;
|
||||
}
|
||||
return CommitPcm(tmpPcm, stdPcm);
|
||||
} else {
|
||||
return "";
|
||||
}
|
||||
|
|
@ -761,17 +857,23 @@ namespace {
|
|||
if (fs::exists(pcmPath) && fs::last_write_time(cppmPath) < fs::last_write_time(pcmPath)) {
|
||||
continue;
|
||||
}
|
||||
fs::path tmpPcm = MakeTempPcmPath(pcmPath);
|
||||
std::string cmd = std::format(
|
||||
"clang++ --target=x86_64-pc-linux-gnu -march=native -mtune=native "
|
||||
"-std=c++26 -stdlib=libc++ -O3 "
|
||||
"-Wno-reserved-identifier -Wno-reserved-module-identifier "
|
||||
"-fprebuilt-module-path={} "
|
||||
"--precompile {} -o {}",
|
||||
cacheDir.string(), cppmPath.string(), pcmPath.string());
|
||||
cacheDir.string(), cppmPath.string(), tmpPcm.string());
|
||||
CommandResult r = Crafter::RunCommandChecked(cmd);
|
||||
if (r.exitCode != 0) {
|
||||
std::error_code ec;
|
||||
fs::remove(tmpPcm, ec);
|
||||
throw std::runtime_error(std::format("Failed to precompile {} (exit {}): {}", name, r.exitCode, r.output));
|
||||
}
|
||||
if (std::string err = CommitPcm(tmpPcm, pcmPath); !err.empty()) {
|
||||
throw std::runtime_error(std::format("Failed to commit {} pcm: {}", name, err));
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue