Implement package cache lock

scarb/src/core/config.rs

@@ -1,7 +1,7 @@
-use std::env;
 use std::path::{Path, PathBuf};
 use std::sync::Arc;
 use std::time::{Duration, Instant};
+use std::{env, mem};
 
 use anyhow::{anyhow, Context, Result};
 use camino::{Utf8Path, Utf8PathBuf};
@@ -12,7 +12,7 @@ use which::which_in;
 #[cfg(doc)]
 use crate::core::Workspace;
 use crate::dirs::AppDirs;
-use crate::flock::RootFilesystem;
+use crate::flock::{AdvisoryLock, RootFilesystem};
 use crate::ui::Ui;
 use crate::DEFAULT_TARGET_DIR_NAME;
 use crate::SCARB_ENV;
@@ -25,6 +25,9 @@ pub struct Config {
     app_exe: OnceCell<PathBuf>,
     ui: Ui,
     creation_time: Instant,
+    // HACK: This should be the lifetime of Config itself, but we cannot express that, so we
+    //   put 'static here and transmute in getter function.
+    package_cache_lock: OnceCell<AdvisoryLock<'static>>,
 }
 
 impl Config {
@@ -46,14 +49,14 @@ impl Config {
 
         let dirs = Arc::new(dirs);
 
-
         Ok(Self {
             manifest_path,
             dirs,
             target_dir,
             app_exe: OnceCell::new(),
             ui,
             creation_time,
+            package_cache_lock: OnceCell::new(),
         })
     }
 
@@ -130,4 +133,17 @@ impl Config {
     pub fn elapsed_time(&self) -> Duration {
         self.creation_time.elapsed()
     }
+
+    pub fn package_cache_lock<'a>(&'a self) -> &AdvisoryLock<'a> {
+        // UNSAFE: These mem::transmute calls only change generic lifetime parameters.
+        let static_al: &AdvisoryLock<'static> = self.package_cache_lock.get_or_init(|| {
+            let not_static_al =
+                self.dirs()
+                    .cache_dir
+                    .advisory_lock(".package-cache.lock", "package cache", self);
+            unsafe { mem::transmute(not_static_al) }
+        });
+        let not_static_al: &AdvisoryLock<'a> = unsafe { mem::transmute(static_al) };
+        not_static_al
+    }
 }

scarb/src/core/registry/download.rs

@@ -10,7 +10,10 @@ pub fn download_package_to_cache(
     config: &Config,
     downloader: impl FnOnce(&Utf8Path) -> Result<()>,
 ) -> Result<Utf8PathBuf> {
+    // TODO(mkaput): Operate on temporary directory and do atomic swap while locked.
     // TODO(mkaput): Computing checksum.
+    let _lock = config.package_cache_lock().acquire()?;
+
     let registry_dir = config.dirs().registry_dir();
     let category_dir = registry_dir.child(category);
     let extracted_path = category_dir.child(package_key);

scarb/src/dirs.rs

@@ -9,8 +9,8 @@ use directories::ProjectDirs;
 use crate::flock::{Filesystem, RootFilesystem};
 use crate::internal::fsx::{PathBufUtf8Ext, PathUtf8Ext};
 
+// TODO(mkaput): Construction needs refinement here.
 #[derive(Debug)]
-#[non_exhaustive]
 pub struct AppDirs {
     pub cache_dir: RootFilesystem,
     pub config_dir: RootFilesystem,

scarb/src/flock.rs

@@ -1,5 +1,6 @@
 use std::fs::{File, OpenOptions};
 use std::ops::{Deref, DerefMut};
+use std::sync::{Arc, Mutex, Weak};
 use std::{fmt, io};
 
 use anyhow::{Context, Result};
@@ -55,6 +56,49 @@ impl Drop for FileLockGuard {
     }
 }
 
+/// An exclusive lock over a global entity identified by a path within a [`Filesystem`].
+#[derive(Debug)]
+pub struct AdvisoryLock<'f> {
+    path: Utf8PathBuf,
+    description: String,
+    file_lock: Mutex<
+        // This Arc is shared between all guards within the process.
+        // Here it is Weak, because AdvisoryLock itself does not keep the lock
+        // (only guards do).
+        Weak<FileLockGuard>,
+    >,
+    filesystem: &'f Filesystem<'f>,
+    config: &'f Config,
+}
+
+#[derive(Debug)]
+pub struct AdvisoryLockGuard(Arc<FileLockGuard>);
+
+impl<'f> AdvisoryLock<'f> {
+    /// Acquires this advisory lock.
+    ///
+    /// This lock is global per-process and can be acquired recursively.
+    /// An RAII structure is returned to release the lock, and if this process abnormally
+    /// terminates the lock is also released.
+    pub fn acquire(&self) -> Result<AdvisoryLockGuard> {
+        let mut slot = self.file_lock.lock().unwrap();
+
+        let file_lock_arc = match slot.upgrade() {
+            Some(arc) => arc,
+            None => {
+                let arc = Arc::new(self.filesystem.open_rw(
+                    &self.path,
+                    &self.description,
+                    self.config,
+                )?);
+                *slot = Arc::downgrade(&arc);
+                arc
+            }
+        };
+        Ok(AdvisoryLockGuard(file_lock_arc))
+    }
+}
+
 /// A [`Filesystem`] that does not have a parent.
 pub type RootFilesystem = Filesystem<'static>;
 
@@ -197,6 +241,22 @@ impl<'a> Filesystem<'a> {
             lock_kind,
         })
     }
+
+    /// Construct an [`AdvisoryLock`] within this file system.
+    pub fn advisory_lock(
+        &'a self,
+        path: impl AsRef<Utf8Path>,
+        description: impl ToString,
+        config: &'a Config,
+    ) -> AdvisoryLock<'a> {
+        AdvisoryLock {
+            path: path.as_ref().to_path_buf(),
+            description: description.to_string(),
+            file_lock: Mutex::new(Weak::new()),
+            filesystem: self,
+            config,
+        }
+    }
 }
 
 impl<'a> fmt::Display for Filesystem<'a> {

scarb/tests/e2e/flock.rs

@@ -6,10 +6,11 @@ use indoc::indoc;
 
 use scarb::core::Config;
 use scarb::dirs::AppDirs;
+use scarb::flock::RootFilesystem;
 use scarb::ui::{OutputFormat, Ui};
 
 use crate::support::command::scarb_command;
-use crate::support::fsx::AssertFsUtf8Ext;
+use crate::support::fsx::{AssertFsUtf8Ext, PathUtf8Ext};
 
 #[test]
 fn locking_build_artifacts() {
@@ -59,3 +60,58 @@ fn locking_build_artifacts() {
             "#});
     });
 }
+
+#[test]
+fn locking_package_cache() {
+    let cache_dir = assert_fs::TempDir::new().unwrap();
+    let config_dir = assert_fs::TempDir::new().unwrap();
+
+    let t = assert_fs::TempDir::new().unwrap();
+    let manifest = t.child("Scarb.toml");
+    manifest
+        .write_str(
+            r#"
+            [package]
+            name = "hello"
+            version = "0.1.0"
+            "#,
+        )
+        .unwrap();
+    t.child("src/lib.cairo")
+        .write_str(r#"fn f() -> felt { 42 }"#)
+        .unwrap();
+
+    let config = Config::init(
+        manifest.utf8_path().to_path_buf(),
+        AppDirs {
+            cache_dir: RootFilesystem::new(cache_dir.try_as_utf8().unwrap().to_path_buf()),
+            config_dir: RootFilesystem::new(config_dir.try_as_utf8().unwrap().to_path_buf()),
+            path_dirs: Vec::new(),
+        },
+        Ui::new(OutputFormat::Text),
+    )
+    .unwrap();
+
+    let lock = config.package_cache_lock().acquire();
+
+    thread::scope(|s| {
+        s.spawn(|| {
+            thread::sleep(Duration::from_secs(2));
+            drop(lock);
+        });
+
+        scarb_command()
+            .env("SCARB_CACHE", cache_dir.path())
+            .env("SCARB_CONFIG", config_dir.path())
+            .arg("build")
+            .current_dir(&t)
+            .timeout(Duration::from_secs(5))
+            .assert()
+            .success()
+            .stdout_matches(indoc! {r#"
+                [..]  Blocking waiting for file lock on package cache
+                [..] Compiling hello v0.1.0 ([..])
+                [..]  Finished release target(s) in [..]
+            "#});
+    });
+}