diff --git a/scarb/src/core/config.rs b/scarb/src/core/config.rs index 06801f723..f08832b18 100644 --- a/scarb/src/core/config.rs +++ b/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, 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>, } impl Config { @@ -46,7 +49,6 @@ impl Config { let dirs = Arc::new(dirs); - Ok(Self { manifest_path, dirs, @@ -54,6 +56,7 @@ impl Config { 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 + } } diff --git a/scarb/src/core/registry/download.rs b/scarb/src/core/registry/download.rs index 4fcf45087..d5fd637ae 100644 --- a/scarb/src/core/registry/download.rs +++ b/scarb/src/core/registry/download.rs @@ -10,7 +10,10 @@ pub fn download_package_to_cache( config: &Config, downloader: impl FnOnce(&Utf8Path) -> Result<()>, ) -> Result { + // 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); diff --git a/scarb/src/dirs.rs b/scarb/src/dirs.rs index e90828a1b..ddbbf5361 100644 --- a/scarb/src/dirs.rs +++ b/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, diff --git a/scarb/src/flock.rs b/scarb/src/flock.rs index 0e9c4a025..9a6c9fddf 100644 --- a/scarb/src/flock.rs +++ b/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, + >, + filesystem: &'f Filesystem<'f>, + config: &'f Config, +} + +#[derive(Debug)] +pub struct AdvisoryLockGuard(Arc); + +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 { + 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, + 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> { diff --git a/scarb/tests/e2e/flock.rs b/scarb/tests/e2e/flock.rs index f41cc59ee..e51a06021 100644 --- a/scarb/tests/e2e/flock.rs +++ b/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 [..] + "#}); + }); +}