Skip to content

Commit

Permalink
Extract protected_run_if_not_ok! macro (#787)
Browse files Browse the repository at this point in the history
**Stack**:
- #783
- #767
- #787⚠️ *Part of a stack created by [spr](https://github.com/ejoffe/spr). Do
not merge manually using the UI - doing so may have unexpected results.*
  • Loading branch information
mkaput authored Oct 11, 2023
1 parent b69de1b commit 977945c
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 27 deletions.
27 changes: 27 additions & 0 deletions scarb/src/flock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,33 @@ impl<'a> fmt::Debug for Filesystem<'a> {
}
}

/// The following sequence of if statements & advisory locks implements a file system-based
/// mutex, that synchronizes extraction logic. The first condition checks if extraction has
/// happened in the past. If not, then we acquire the advisory lock (which means waiting for
/// our time slice to do the job). Successful lock acquisition does not mean though that we
/// still have to perform the extraction! While we waited for our time slice, another process
/// could just do the extraction! The second condition prevents repeating the work.
///
/// This is actually very important for correctness. The another process that performed
/// the extraction, will highly probably soon try to read the extracted files. If we recreate
/// the filesystem now, we will cause that process to crash. That's what happened on Windows
/// in examples tests, when the second condition was missing.
macro_rules! protected_run_if_not_ok {
($fs:expr, $lock:expr, $body:block) => {{
let fs: &$crate::flock::Filesystem<'_> = $fs;
let lock: &$crate::flock::AdvisoryLock<'_> = $lock;
if !fs.is_ok() {
let _lock = lock.acquire_async().await?;
if !fs.is_ok() {
$body
fs.mark_ok()?;
}
}
}};
}

pub(crate) use protected_run_if_not_ok;

fn acquire(
file: &File,
path: &Utf8Path,
Expand Down
39 changes: 12 additions & 27 deletions scarb/src/sources/standard_lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use crate::core::manifest::{ManifestDependency, Summary, TomlManifest};
use crate::core::package::{Package, PackageId};
use crate::core::source::Source;
use crate::core::SourceId;
use crate::flock::protected_run_if_not_ok;
use crate::internal::fsx;
use crate::internal::fsx::PathUtf8Ext;
use crate::sources::PathSource;
Expand Down Expand Up @@ -50,35 +51,19 @@ impl<'c> StandardLibSource<'c> {

let tag_path = tag_fs.path_existent()?;

// The following sequence of if statements & advisory locks implements a file system-based
// mutex, that synchronizes extraction logic. The first condition checks if extraction has
// happened in the past. If not, then we acquire the advisory lock (which means waiting for
// our time slice to do the job). Successful lock acquisition does not mean though that we
// still have to perform the extraction! While we waited for our time slice, another process
// could just do the extraction! The second condition prevents repeating the work.
//
// This is actually very important for correctness. The another process that performed
// the extraction, will highly probably soon try to read the extracted files. If we recreate
// the filesystem now, we will cause that process to crash. That's what happened on Windows
// in examples tests, when the second condition was missing.
if !tag_fs.is_ok() {
let _lock = self.config.package_cache_lock().acquire_async().await?;
if !tag_fs.is_ok() {
trace!("extracting Cairo standard library: {tag}");
unsafe {
tag_fs.recreate()?;
}

let base_path = tag_fs.path_existent()?;
protected_run_if_not_ok!(&tag_fs, &self.config.package_cache_lock(), {
trace!("extracting Cairo standard library: {tag}");
unsafe {
tag_fs.recreate()?;
}

extract_with_templating(&CORELIB, &base_path.join("core"))
.context("failed to extract Cairo standard library (corelib)")?;
extract_with_templating(&SCARBLIB, base_path)
.context("failed to extract Cairo standard library (scarblib)")?;
let base_path = tag_fs.path_existent()?;

tag_fs.mark_ok()?;
}
}
extract_with_templating(&CORELIB, &base_path.join("core"))
.context("failed to extract Cairo standard library (corelib)")?;
extract_with_templating(&SCARBLIB, base_path)
.context("failed to extract Cairo standard library (scarblib)")?;
});

check_corelib_version(&tag_fs.path_existent()?.join("core").join("Scarb.toml"))?;

Expand Down

0 comments on commit 977945c

Please sign in to comment.