use lock files to avoid concurrent cache collisions

Author: 2bndy5

clang-installer/src/downloader/pypi.rs

@@ -1,5 +1,5 @@
 use super::{DownloadError, caching::Cacher, download, hashing::HashAlgorithm};
-use crate::{ClangTool, progress_bar::ProgressBar};
+use crate::{ClangTool, progress_bar::ProgressBar, utils::lock_path};
 
 use semver::{Version, VersionReq};
 use serde::{Deserialize, de::Visitor};
@@ -452,7 +452,7 @@ impl PyPiDownloader {
         let cache_file = Self::get_cache_dir()
             .join("pypi")
             .join(format!("{clang_tool}_pypi.json"));
-
+        let file_lock = lock_path(&cache_file)?;
         // PyPI package info cache should not be refreshed unless it is more than 10 minutes old.
         // This is behavior recommended by PyPI response header `Cache-Control: max-age=600`.
         // Instead of caching the `Cache-Control` header, we'll just check the cached file's "last modified" time.
@@ -462,14 +462,15 @@ impl PyPiDownloader {
                 "Using cached PyPI info for {clang_tool} from {}",
                 cache_file.to_string_lossy()
             );
-            std::fs::read_to_string(cache_file)?
+            std::fs::read_to_string(&cache_file)?
         } else {
             let api_url = format!("{PYPI_JSON_API_URL}/pypi/{clang_tool}/");
             let endpoint = Url::parse(&api_url)?.join("json")?;
             log::info!("Fetching PyPI info for {clang_tool} from {endpoint}");
             download(&endpoint, &cache_file, 10).await?;
-            std::fs::read_to_string(cache_file)?
+            std::fs::read_to_string(&cache_file)?
         };
+        file_lock.unlock()?;
         Ok(serde_json::from_str(body.as_str())?)
     }
 
@@ -487,6 +488,7 @@ impl PyPiDownloader {
         let cached_filename = format!("{clang_tool}_{ver}.whl");
         let cached_dir = Self::get_cache_dir();
         let cached_wheel = cached_dir.join("pypi").join(&cached_filename);
+        let file_lock = lock_path(&cached_wheel)?;
         if Self::is_cache_valid(&cached_wheel, None) {
             log::info!(
                 "Using cached wheel for {clang_tool} version {ver} from {}",
@@ -510,6 +512,7 @@ impl PyPiDownloader {
             Some(dir) => dir.join(&bin_name),
         };
         Self::extract_bin(clang_tool, &cached_wheel, &extracted_bin)?;
+        file_lock.unlock()?;
         Ok(extracted_bin)
     }
 

clang-installer/src/downloader/static_dist.rs

@@ -13,6 +13,7 @@ use url::Url;
 use crate::{
     Cacher, ClangTool, DownloadError,
     downloader::{download, hashing::HashAlgorithm},
+    utils::lock_path,
 };
 
 /// An error that can occur while downloading a static binary.
@@ -155,6 +156,7 @@ impl StaticDistDownloader {
             None => cache_path.join("bin").join(&bin_name),
             Some(dir) => dir.join(&bin_name),
         };
+        let file_lock = lock_path(&download_path)?;
         if download_path.exists() {
             log::info!(
                 "Using cached static binary for {tool} version {ver_str} from {:?}",
@@ -182,6 +184,7 @@ impl StaticDistDownloader {
             download(&sha512_url, &sha512_cache_path, 10).await?;
         }
         Self::verify_sha512(&download_path, &sha512_cache_path)?;
+        file_lock.unlock()?;
         Ok(download_path)
     }
 }

clang-installer/src/utils.rs

@@ -1,5 +1,8 @@
 //! A utility module for path normalization.
-use std::path::{Component, Path, PathBuf};
+use std::{
+    fs::{self, File, OpenOptions},
+    path::{Component, Path, PathBuf},
+};
 
 /// This was copied from [cargo source code](https://github.com/rust-lang/cargo/blob/8cc0cb136772b8f54eafe0d163fcb7226a06af0c/crates/cargo-util/src/paths.rs#L84).
 ///
@@ -31,6 +34,32 @@ pub fn normalize_path(path: &Path) -> PathBuf {
     ret
 }
 
+/// Creates (and returns) a lock [`File`] for the given `path` and locks it.
+///
+/// This function will create a lock file with `.lock` appended to the
+/// given `path`'s extension. It will then acquire an exclusive lock on
+/// the file to prevent concurrent access.
+///
+/// Note, This will block until a lock is obtained.
+///
+/// The caller is responsible for cleanup, which includes
+///
+/// 1. unlocking the returned file and
+/// 2. deleting the file when done (if desired).
+pub fn lock_path(path: &Path) -> Result<File, std::io::Error> {
+    let lock_path = path.with_added_extension("lock");
+    if let Some(parent) = lock_path.parent() {
+        fs::create_dir_all(parent)?;
+    }
+    let file_lock = OpenOptions::new()
+        .write(true)
+        .create(true)
+        .truncate(true)
+        .open(&lock_path)?;
+    file_lock.lock()?;
+    Ok(file_lock)
+}
+
 #[cfg(test)]
 mod tests {
     use std::{env::current_dir, path::PathBuf};