Skip to content

Commit 4876f07

Browse files
blockifier_test_utils: fix ETXTBSY race in cairo package download (#13850)
`verify_cairo1_package` double-checks `cairo1_package_exists` — once without the lock (fast path) and once after acquiring it. The predicate only tested `.exists()` on the two binaries, so a concurrent process could observe the package as "ready" while `tar` was still mid-write: 1. Process A holds the download lock, `tar` is extracting the archive. 2. `tar` extracts sequentially: first binary is closed, second binary is created (so `.exists()` is true) and being written to. 3. Process B calls `cairo1_package_exists`, sees both files, skips the lock. 4. Process B forks + execs the second binary → Linux returns `ETXTBSY` because the file is still open for writing by `tar`. Fix with the same commit-marker pattern used in `compile_cache.rs`: write `.download_complete` after `tar` finishes, and require it in `cairo1_package_exists`. Until the marker exists, concurrent callers fall through to the lock and wait for the writer. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 4b4a329 commit 4876f07

1 file changed

Lines changed: 14 additions & 1 deletion

File tree

crates/blockifier_test_utils/src/cairo_compile.rs

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,13 @@ fn starknet_sierra_compile_binary_path(version: &String) -> PathBuf {
3434
cairo1_package_dir(version).join("cairo/bin/starknet-sierra-compile")
3535
}
3636

37+
/// Commit marker written after `tar` extraction fully completes. Required because
38+
/// a file `.exists()` while `tar` is still writing to it, and exec'ing such a file
39+
/// returns `ETXTBSY`.
40+
fn cairo1_package_complete_marker(version: &String) -> PathBuf {
41+
cairo1_package_dir(version).join(".download_complete")
42+
}
43+
3744
/// Returns the path to the allowed_libfuncs.json file.
3845
pub fn allowed_libfuncs_json_path() -> String {
3946
resolve_project_relative_path("crates/apollo_compile_to_casm/src/allowed_libfuncs.json")
@@ -93,13 +100,19 @@ fn download_cairo_package(version: &String) {
93100
let stderr_output = String::from_utf8(output.stderr).unwrap();
94101
panic!("{stderr_output}");
95102
}
103+
// Written last: acts as a commit marker so concurrent callers checking
104+
// `cairo1_package_exists` never observe a half-extracted package.
105+
let marker = cairo1_package_complete_marker(version);
106+
fs::write(&marker, b"")
107+
.unwrap_or_else(|e| panic!("Failed to write download marker {marker:?}: {e}"));
96108
info!("Done.");
97109
}
98110

99111
fn cairo1_package_exists(version: &String) -> bool {
100112
let cairo_compiler_path = starknet_compile_binary_path(version);
101113
let sierra_compiler_path = starknet_sierra_compile_binary_path(version);
102-
cairo_compiler_path.exists() && sierra_compiler_path.exists()
114+
let marker_path = cairo1_package_complete_marker(version);
115+
cairo_compiler_path.exists() && sierra_compiler_path.exists() && marker_path.exists()
103116
}
104117

105118
/// Appends `.lock` to a path, used by `with_file_lock` callers to derive a lock file path.

0 commit comments

Comments
 (0)