From 596664bf9d18136adb3d0bb5af948ddf5090b77b Mon Sep 17 00:00:00 2001 From: Miles Johnson Date: Fri, 10 Jan 2025 22:44:15 -0800 Subject: [PATCH] fix: Add fs locks for important operations. (#1788) * Add api. * Implement. * Update changelog. * Update changelog. * Fix windows. * Polish. * Fixes. --- CHANGELOG.md | 5 ++ Cargo.lock | 90 +++++++++---------- Cargo.toml | 4 +- crates/actions/src/actions/install_deps.rs | 14 ++- crates/actions/src/actions/setup_toolchain.rs | 16 ++-- crates/actions/src/actions/sync_project.rs | 4 + crates/actions/src/actions/sync_workspace.rs | 2 + crates/app/src/session.rs | 10 ++- crates/app/src/systems/analyze.rs | 4 + crates/cache/Cargo.toml | 2 +- crates/cache/src/cache_engine.rs | 15 +++- crates/cli/tests/run_node_test.rs | 2 +- crates/common/src/path.rs | 6 +- 13 files changed, 108 insertions(+), 66 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3495c541c33..b90885b1e0a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,11 @@ ## Unreleased +#### 🚀 Updates + +- Added file locks for certain operations to avoid race collisions when multiple `moon` commands are + ran in parallel. + #### ⚙️ Internal - Updated Rust to v1.84. diff --git a/Cargo.lock b/Cargo.lock index b9b3782c225..4e6a4ba875f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3202,7 +3202,7 @@ dependencies = [ "petgraph", "rustc-hash", "starbase_sandbox", - "thiserror 2.0.10", + "thiserror 2.0.11", "tokio", "tracing", ] @@ -3308,7 +3308,7 @@ dependencies = [ "semver", "serde", "starbase_utils", - "thiserror 2.0.10", + "thiserror 2.0.11", "tokio", "tokio-util", "tracing", @@ -3398,7 +3398,7 @@ dependencies = [ "starbase_utils", "system_env", "tera", - "thiserror 2.0.10", + "thiserror 2.0.11", "tiny_http", "tokio", "tracing", @@ -3420,7 +3420,7 @@ version = "0.0.1" dependencies = [ "miette 7.4.0", "shell-words", - "thiserror 2.0.10", + "thiserror 2.0.11", ] [[package]] @@ -3569,7 +3569,7 @@ dependencies = [ "starbase_sandbox", "starbase_utils", "tera", - "thiserror 2.0.10", + "thiserror 2.0.11", "tokio", "tracing", ] @@ -3600,7 +3600,7 @@ dependencies = [ "schematic", "serde", "starbase_styles", - "thiserror 2.0.10", + "thiserror 2.0.11", ] [[package]] @@ -3774,7 +3774,7 @@ dependencies = [ "serde", "starbase_sandbox", "starbase_utils", - "thiserror 2.0.10", + "thiserror 2.0.11", ] [[package]] @@ -3999,7 +3999,7 @@ dependencies = [ "starbase_sandbox", "starbase_styles", "starbase_utils", - "thiserror 2.0.10", + "thiserror 2.0.11", "tokio", "tracing", "warpgate", @@ -4018,7 +4018,7 @@ dependencies = [ "rustc-hash", "starbase_shell", "system_env", - "thiserror 2.0.10", + "thiserror 2.0.11", "tokio", "tracing", ] @@ -4033,7 +4033,7 @@ dependencies = [ "moon_file_group", "moon_task", "serde", - "thiserror 2.0.10", + "thiserror 2.0.11", ] [[package]] @@ -4062,7 +4062,7 @@ dependencies = [ "moon_common", "moon_config", "moon_project", - "thiserror 2.0.10", + "thiserror 2.0.11", ] [[package]] @@ -4076,7 +4076,7 @@ dependencies = [ "rustc-hash", "starbase_sandbox", "starbase_utils", - "thiserror 2.0.10", + "thiserror 2.0.11", "tracing", ] @@ -4101,7 +4101,7 @@ dependencies = [ "starbase_events", "starbase_sandbox", "starbase_utils", - "thiserror 2.0.10", + "thiserror 2.0.11", "tokio", "tracing", ] @@ -4179,7 +4179,7 @@ dependencies = [ "pest", "pest_derive", "starbase_utils", - "thiserror 2.0.10", + "thiserror 2.0.11", "tracing", ] @@ -4200,7 +4200,7 @@ dependencies = [ "serde", "sha2", "starbase_utils", - "thiserror 2.0.10", + "thiserror 2.0.11", "tokio", "tonic", "tracing", @@ -4303,7 +4303,7 @@ dependencies = [ "regex", "schematic", "serde", - "thiserror 2.0.10", + "thiserror 2.0.11", "tracing", ] @@ -4344,7 +4344,7 @@ dependencies = [ "moon_toolchain", "rustc-hash", "starbase_sandbox", - "thiserror 2.0.10", + "thiserror 2.0.11", "tokio", "tracing", ] @@ -4368,7 +4368,7 @@ dependencies = [ "rustc-hash", "starbase_sandbox", "starbase_utils", - "thiserror 2.0.10", + "thiserror 2.0.11", "tracing", ] @@ -4386,7 +4386,7 @@ dependencies = [ "moon_task_expander", "petgraph", "rustc-hash", - "thiserror 2.0.10", + "thiserror 2.0.11", "tracing", ] @@ -4437,7 +4437,7 @@ dependencies = [ "starbase_archive", "starbase_sandbox", "starbase_utils", - "thiserror 2.0.10", + "thiserror 2.0.11", "tokio", "tokio-util", "tracing", @@ -4506,7 +4506,7 @@ dependencies = [ "proto_core", "rustc-hash", "starbase_styles", - "thiserror 2.0.10", + "thiserror 2.0.11", "tokio", "warpgate", ] @@ -4613,7 +4613,7 @@ dependencies = [ "serde", "starbase_sandbox", "starbase_utils", - "thiserror 2.0.10", + "thiserror 2.0.11", "tokio", "tracing", ] @@ -4659,7 +4659,7 @@ dependencies = [ "serde", "starbase_events", "starbase_utils", - "thiserror 2.0.10", + "thiserror 2.0.11", "tracing", ] @@ -4731,7 +4731,7 @@ dependencies = [ "semver", "serde", "serde_json", - "thiserror 2.0.10", + "thiserror 2.0.11", ] [[package]] @@ -4994,7 +4994,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8b7cafe60d6cf8e62e1b9b2ea516a089c008945bb5a275416789e7db0bc199dc" dependencies = [ "memchr", - "thiserror 2.0.10", + "thiserror 2.0.11", "ucd-trie", ] @@ -5336,7 +5336,7 @@ dependencies = [ "starbase_archive", "starbase_styles", "starbase_utils", - "thiserror 2.0.10", + "thiserror 2.0.11", "tracing", "url", "uuid", @@ -5356,7 +5356,7 @@ dependencies = [ "starbase_styles", "starbase_utils", "system_env", - "thiserror 2.0.10", + "thiserror 2.0.11", "tracing", ] @@ -5372,7 +5372,7 @@ dependencies = [ "serde", "serde_json", "system_env", - "thiserror 2.0.10", + "thiserror 2.0.11", "version_spec", "warpgate_api", ] @@ -5956,7 +5956,7 @@ dependencies = [ "serde_path_to_error", "serde_yaml", "starbase_styles", - "thiserror 2.0.10", + "thiserror 2.0.11", "toml", "tracing", ] @@ -6399,7 +6399,7 @@ dependencies = [ "rustc-hash", "starbase_styles", "starbase_utils", - "thiserror 2.0.10", + "thiserror 2.0.11", "tracing", "xz2", "zip", @@ -6453,15 +6453,15 @@ checksum = "d4a1e16cb1a23e18cb58c0e4793a786a6b3a459bef6f689333ee7a65a379d6b4" dependencies = [ "regex", "sysinfo", - "thiserror 2.0.10", + "thiserror 2.0.11", "tracing", ] [[package]] name = "starbase_styles" -version = "0.4.10" +version = "0.4.11" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1a4df972f8b4010b3ca083555953f6b320de4962aa2b63823c273de9830a3e9f" +checksum = "fa762771c938c1d2435b6ee09791cf47492ba9f80ac2496ad93f722a33c15b7e" dependencies = [ "dirs 5.0.1", "miette 7.4.0", @@ -6472,9 +6472,9 @@ dependencies = [ [[package]] name = "starbase_utils" -version = "0.9.1" +version = "0.9.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a6e34eb4c6d9fc4f9d6159738a8844363d0dc23393718a53b4ea84e570ef9f58" +checksum = "8813b123653ee27546530a4f215c863bd067960c6c5f672b42ff8a7ec8cc9aff" dependencies = [ "async-trait", "dirs 5.0.1", @@ -6488,7 +6488,7 @@ dependencies = [ "serde_json", "serde_yaml", "starbase_styles", - "thiserror 2.0.10", + "thiserror 2.0.11", "tokio", "toml", "tracing", @@ -6779,11 +6779,11 @@ dependencies = [ [[package]] name = "thiserror" -version = "2.0.10" +version = "2.0.11" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a3ac7f54ca534db81081ef1c1e7f6ea8a3ef428d2fc069097c079443d24124d3" +checksum = "d452f284b73e6d76dd36758a0c8684b1d5be31f92b89d07fd5822175732206fc" dependencies = [ - "thiserror-impl 2.0.10", + "thiserror-impl 2.0.11", ] [[package]] @@ -6799,9 +6799,9 @@ dependencies = [ [[package]] name = "thiserror-impl" -version = "2.0.10" +version = "2.0.11" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9e9465d30713b56a37ede7185763c3492a91be2f5fa68d958c44e41ab9248beb" +checksum = "26afc1baea8a989337eeb52b6e72a039780ce45c3edfcc9c5b9d112feeb173c2" dependencies = [ "proc-macro2", "quote", @@ -7395,7 +7395,7 @@ dependencies = [ "schematic", "semver", "serde", - "thiserror 2.0.10", + "thiserror 2.0.11", ] [[package]] @@ -7452,7 +7452,7 @@ dependencies = [ "starbase_styles", "starbase_utils", "system_env", - "thiserror 2.0.10", + "thiserror 2.0.11", "tokio", "tracing", "ureq", @@ -8426,7 +8426,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1123fd5b3197a2de22c3f41904d82ee680577c634b8d2cbb56e61d0b5ed312d5" dependencies = [ "nom", - "thiserror 2.0.10", + "thiserror 2.0.11", ] [[package]] @@ -8536,7 +8536,7 @@ dependencies = [ "flate2", "indexmap 2.7.0", "memchr", - "thiserror 2.0.10", + "thiserror 2.0.11", "zopfli", ] diff --git a/Cargo.toml b/Cargo.toml index e00c736013e..478b0185155 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -73,12 +73,12 @@ starbase_events = "0.6.6" starbase_sandbox = "0.8.0" starbase_shell = "0.6.10" starbase_styles = { version = "0.4.4", features = ["relative-path"] } -starbase_utils = { version = "0.9.1", default-features = false, features = [ +starbase_utils = { version = "0.9.4", default-features = false, features = [ "editor-config", "miette", ] } tera = { version = "1.20.0", features = ["preserve_order"] } -thiserror = "2.0.10" +thiserror = "2.0.11" tokio = { version = "1.43.0", default-features = false, features = [ "macros", "process", diff --git a/crates/actions/src/actions/install_deps.rs b/crates/actions/src/actions/install_deps.rs index 289294f388f..07c76966e25 100644 --- a/crates/actions/src/actions/install_deps.rs +++ b/crates/actions/src/actions/install_deps.rs @@ -37,10 +37,13 @@ pub async fn install_deps( let pid = process::id().to_string(); let log_label = runtime.label(); + let action_key = get_skip_key(runtime, project); - if let Some(value) = - should_skip_action_matching("MOON_SKIP_INSTALL_DEPS", get_skip_key(runtime, project)) - { + let _lock = app_context + .cache_engine + .create_lock(format!("installDeps-{action_key}"))?; + + if let Some(value) = should_skip_action_matching("MOON_SKIP_INSTALL_DEPS", action_key) { debug!( env = value, "Skipping {} dependency install because {} is set", @@ -286,7 +289,10 @@ fn get_state_path( runtime: &Runtime, project: Option<&Project>, ) -> PathBuf { - let state_path = PathBuf::from(format!("deps-{}.json", encode_component(runtime.id()))); + let state_path = PathBuf::from(format!( + "installDeps-{}.json", + encode_component(runtime.id()) + )); if let Some(project) = project { return app_context diff --git a/crates/actions/src/actions/setup_toolchain.rs b/crates/actions/src/actions/setup_toolchain.rs index c10568b4219..d07ed9f5a9f 100644 --- a/crates/actions/src/actions/setup_toolchain.rs +++ b/crates/actions/src/actions/setup_toolchain.rs @@ -4,7 +4,7 @@ use moon_action_context::ActionContext; use moon_app_context::AppContext; use moon_cache_item::cache_item; use moon_common::color; -use moon_common::path::{encode_component, hash_component}; +use moon_common::path::encode_component; use moon_config::UnresolvedVersionSpec; use moon_platform::PlatformManager; use moon_time::now_millis; @@ -34,10 +34,13 @@ pub async fn setup_toolchain( ) -> miette::Result { let log_label = node.runtime.label(); let cache_engine = &app_context.cache_engine; + let action_key = node.runtime.target(); - if let Some(value) = - should_skip_action_matching("MOON_SKIP_SETUP_TOOLCHAIN", node.runtime.target()) - { + let _lock = app_context + .cache_engine + .create_lock(format!("setupToolchain-{action_key}"))?; + + if let Some(value) = should_skip_action_matching("MOON_SKIP_SETUP_TOOLCHAIN", &action_key) { debug!( env = value, "Skipping {} toolchain setup because {} is set", @@ -51,9 +54,8 @@ pub async fn setup_toolchain( debug!("Setting up {} toolchain", log_label); let mut state = cache_engine.state.load_state::(format!( - "toolchain-{}-{}.json", - encode_component(node.runtime.id()), - hash_component(node.runtime.requirement.to_string()), + "setupToolchain-{}.json", + encode_component(action_key), ))?; // Acquire a lock for the toolchain ID diff --git a/crates/actions/src/actions/sync_project.rs b/crates/actions/src/actions/sync_project.rs index bc8f3e12231..8d6b8c3c37c 100644 --- a/crates/actions/src/actions/sync_project.rs +++ b/crates/actions/src/actions/sync_project.rs @@ -20,6 +20,10 @@ pub async fn sync_project( // Include tasks for snapshot! let project = workspace_graph.get_project_with_tasks(&node.project)?; + let _lock = app_context + .cache_engine + .create_lock(format!("syncProject-{}", project.id))?; + if let Some(value) = should_skip_action_matching("MOON_SKIP_SYNC_PROJECT", &project.id) { debug!( env = value, diff --git a/crates/actions/src/actions/sync_workspace.rs b/crates/actions/src/actions/sync_workspace.rs index 37857a75ed0..90ebd10e055 100644 --- a/crates/actions/src/actions/sync_workspace.rs +++ b/crates/actions/src/actions/sync_workspace.rs @@ -22,6 +22,8 @@ pub async fn sync_workspace( workspace_graph: WorkspaceGraph, toolchain_registry: Arc, ) -> miette::Result { + let _lock = app_context.cache_engine.create_lock("syncWorkspace")?; + // Connect to the remote service in this action, // as it always runs before tasks, and we don't need it // for non-pipeline related features! diff --git a/crates/app/src/session.rs b/crates/app/src/session.rs index 8260f0f0881..bf9421b60c2 100644 --- a/crates/app/src/session.rs +++ b/crates/app/src/session.rs @@ -289,9 +289,15 @@ impl AppSession for CliSession { analyze::check_pkl_install()?; if self.requires_workspace_setup() { - self.get_cache_engine()?; + let cache_engine = self.get_cache_engine()?; - analyze::install_proto(&self.console, &self.proto_env, &self.toolchain_config).await?; + analyze::install_proto( + &self.console, + &self.proto_env, + &cache_engine, + &self.toolchain_config, + ) + .await?; analyze::register_platforms( &self.console, diff --git a/crates/app/src/systems/analyze.rs b/crates/app/src/systems/analyze.rs index 61d39a20957..63b2866bfaf 100644 --- a/crates/app/src/systems/analyze.rs +++ b/crates/app/src/systems/analyze.rs @@ -1,6 +1,7 @@ use crate::app_error::AppError; use moon_actions::utils::should_skip_action; use moon_bun_platform::BunPlatform; +use moon_cache::CacheEngine; use moon_common::{consts::PROTO_CLI_VERSION, is_test_env, path::exe_name, supports_pkl_configs}; use moon_config::{BunConfig, PlatformType, ToolchainConfig}; use moon_console::{Checkpoint, Console}; @@ -46,8 +47,11 @@ pub fn check_pkl_install() -> AppResult { pub async fn install_proto( console: &Console, proto_env: &Arc, + cache_engine: &CacheEngine, toolchain_config: &ToolchainConfig, ) -> AppResult { + let _lock = cache_engine.create_lock("proto-install")?; + let bin_name = exe_name("proto"); let install_dir = proto_env .store diff --git a/crates/cache/Cargo.toml b/crates/cache/Cargo.toml index 44e96ce348f..0274f666c06 100644 --- a/crates/cache/Cargo.toml +++ b/crates/cache/Cargo.toml @@ -16,7 +16,7 @@ moon_target = { path = "../target" } moon_time = { path = "../time" } miette = { workspace = true } serde = { workspace = true } -starbase_utils = { workspace = true, features = ["json"] } +starbase_utils = { workspace = true, features = ["fs-lock", "json"] } tracing = { workspace = true } [dev-dependencies] diff --git a/crates/cache/src/cache_engine.rs b/crates/cache/src/cache_engine.rs index 240da9f7604..119c6e906b1 100644 --- a/crates/cache/src/cache_engine.rs +++ b/crates/cache/src/cache_engine.rs @@ -1,10 +1,11 @@ use crate::{merge_clean_results, resolve_path, HashEngine, StateEngine}; use moon_cache_item::*; use moon_common::consts; +use moon_common::path::encode_component; use moon_time::parse_duration; use serde::de::DeserializeOwned; use serde::Serialize; -use starbase_utils::fs::RemoveDirContentsResult; +use starbase_utils::fs::{FileLock, RemoveDirContentsResult}; use starbase_utils::{fs, json}; use std::env; use std::ffi::OsStr; @@ -110,6 +111,18 @@ impl CacheEngine { Ok((result.files_deleted, result.bytes_saved)) } + pub fn create_lock>(&self, name: T) -> miette::Result { + let mut name = encode_component(name.as_ref()); + + if !name.ends_with(".lock") { + name.push_str(".lock"); + } + + let guard = fs::lock_file(self.cache_dir.join("locks").join(name))?; + + Ok(guard) + } + pub fn write(&self, path: K, data: &T) -> miette::Result<()> where K: AsRef, diff --git a/crates/cli/tests/run_node_test.rs b/crates/cli/tests/run_node_test.rs index ec44f4dd90d..be53e76b074 100644 --- a/crates/cli/tests/run_node_test.rs +++ b/crates/cli/tests/run_node_test.rs @@ -456,7 +456,7 @@ mod install_deps { assert!(sandbox .path() // 18.0.0 - .join(".moon/cache/states/toolchain-node-4912669032959148481.json") + .join(".moon/cache/states/setupToolchain-node-18.0.0.json") .exists()); } diff --git a/crates/common/src/path.rs b/crates/common/src/path.rs index ef46ac1b064..305926c0d85 100644 --- a/crates/common/src/path.rs +++ b/crates/common/src/path.rs @@ -104,10 +104,10 @@ pub fn encode_component(value: impl AsRef) -> String { // Handle supported characters from `Id` for ch in value.as_ref().chars() { match ch { - '@' => { + '@' | '*' => { // Skip these } - '.' | '/' => { + '/' | ':' => { output.push('-'); } _ => { @@ -116,7 +116,7 @@ pub fn encode_component(value: impl AsRef) -> String { } } - output + output.trim_matches(['-', '.']).to_owned() } /// Hash a value that may contain special characters into a valid file name.