From d1c37ab128e07b6bf1331f1ea9b37fc3bcee5769 Mon Sep 17 00:00:00 2001 From: MrAzteca Date: Fri, 7 Feb 2025 15:59:51 +0100 Subject: [PATCH] Fix theoretical lockfile data race. (#1087) --- src/executor/contract.rs | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/executor/contract.rs b/src/executor/contract.rs index 9a90ce34a..445465366 100644 --- a/src/executor/contract.rs +++ b/src/executor/contract.rs @@ -256,9 +256,13 @@ impl AotContractExecutor { }) .collect::>>()?; - // Build the shared library. + // Build the shared library into a temporary path. + let temp_path = NamedTempFile::new()? + .into_temp_path() + .keep() + .to_native_assert_error("can only fail on windows")?; let object_data = crate::module_to_object(&module, opt_level)?; - crate::object_to_shared_lib(&object_data, &output_path)?; + crate::object_to_shared_lib(&object_data, &temp_path)?; // Write the contract info. fs::write( @@ -269,6 +273,10 @@ impl AotContractExecutor { })?, )?; + // Atomically move the built shared library to the correct path. This will avoid data races + // when loading contracts. + fs::rename(temp_path, &output_path)?; + drop(lock_file); Self::from_path(output_path) } @@ -280,10 +288,9 @@ impl AotContractExecutor { /// again. pub fn from_path(path: impl Into) -> Result> { let path = path.into(); - if LockFile::exists(&path)? { - return Ok(None); - } + // Note: Library should load first, otherwise there could theoretically be a race condition. + // See the `new_into` function's code for details. let library = Arc::new(unsafe { Library::new(&path)? }); let contract_info = serde_json::from_str(&fs::read_to_string(path.with_extension("json"))?)?; @@ -630,13 +637,6 @@ impl LockFile { Err(e) => Err(e), } } - - pub fn exists(path: impl Into) -> io::Result { - let path: PathBuf = path.into(); - let path = path.with_extension("lock"); - - fs::exists(path) - } } impl Drop for LockFile {