From bf96c60e3eb805ba9d66630e87cd639c75f81f0a Mon Sep 17 00:00:00 2001 From: konsti Date: Fri, 6 Jun 2025 14:16:40 +0200 Subject: [PATCH] Lock during `uv sync`, `uv add` and `uv remove` to avoid race conditions (#13869) Surprisingly, we weren't locking during `uv sync` so far, so running `uv sync` in parallel could cause errors in filesystem races. I've also added locks to `uv add` and `uv remove` which concurrently modify `pyproject.toml`. These locks only apply after we determined the interpreter, so they don't actually prevent computing the same thing twice when running `uv add` in parallel. All other subcommands that I checked were already locking (with no claim to exhaustiveness) Fixes #12751 # Test Plan I don't have CI-sized reproducer for this. ```toml [project] name = "debug" version = "0.1.0" requires-python = ">=3.12" dependencies = [ "boto3>=1.38.30", "fastapi>=0.115.12", "numba>=0.61.2", "polars>=1.30.0", "protobuf>=6.31.1", "pyarrow>=20.0.0", "pydantic>=2.11.5", "requests>=2.32.3", "urllib3>=2.4.0", "scikit-learn>=1.6.1", "jupyter>=1.1.1", ] [build-system] requires = ["hatchling"] build-backend = "hatchling.build" ``` ``` rm -rf .venv && parallel -n0 "uv sync -q" ::: {1..10} ``` --- crates/uv-python/src/environment.rs | 20 +--------------- crates/uv-python/src/interpreter.rs | 29 ++++++++++++++++++++++-- crates/uv/src/commands/project/add.rs | 13 ++++++++++- crates/uv/src/commands/project/remove.rs | 2 ++ crates/uv/src/commands/project/sync.rs | 2 ++ 5 files changed, 44 insertions(+), 22 deletions(-) diff --git a/crates/uv-python/src/environment.rs b/crates/uv-python/src/environment.rs index de6cf59d8..34fa3eee9 100644 --- a/crates/uv-python/src/environment.rs +++ b/crates/uv-python/src/environment.rs @@ -1,5 +1,4 @@ use std::borrow::Cow; -use std::env; use std::fmt; use std::path::{Path, PathBuf}; use std::sync::Arc; @@ -8,7 +7,6 @@ use owo_colors::OwoColorize; use tracing::debug; use uv_cache::Cache; -use uv_cache_key::cache_digest; use uv_fs::{LockedFile, Simplified}; use uv_pep440::Version; @@ -316,23 +314,7 @@ impl PythonEnvironment { /// Grab a file lock for the environment to prevent concurrent writes across processes. pub async fn lock(&self) -> Result { - if let Some(target) = self.0.interpreter.target() { - // If we're installing into a `--target`, use a target-specific lockfile. - LockedFile::acquire(target.root().join(".lock"), target.root().user_display()).await - } else if let Some(prefix) = self.0.interpreter.prefix() { - // Likewise, if we're installing into a `--prefix`, use a prefix-specific lockfile. - LockedFile::acquire(prefix.root().join(".lock"), prefix.root().user_display()).await - } else if self.0.interpreter.is_virtualenv() { - // If the environment a virtualenv, use a virtualenv-specific lockfile. - LockedFile::acquire(self.0.root.join(".lock"), self.0.root.user_display()).await - } else { - // Otherwise, use a global lockfile. - LockedFile::acquire( - env::temp_dir().join(format!("uv-{}.lock", cache_digest(&self.0.root))), - self.0.root.user_display(), - ) - .await - } + self.0.interpreter.lock().await } /// Return the [`Interpreter`] for this environment. diff --git a/crates/uv-python/src/interpreter.rs b/crates/uv-python/src/interpreter.rs index 4d6f1f81d..26d810db5 100644 --- a/crates/uv-python/src/interpreter.rs +++ b/crates/uv-python/src/interpreter.rs @@ -1,10 +1,10 @@ use std::borrow::Cow; use std::env::consts::ARCH; use std::fmt::{Display, Formatter}; -use std::io; use std::path::{Path, PathBuf}; use std::process::{Command, ExitStatus}; use std::sync::OnceLock; +use std::{env, io}; use configparser::ini::Ini; use fs_err as fs; @@ -17,7 +17,7 @@ use tracing::{debug, trace, warn}; use uv_cache::{Cache, CacheBucket, CachedByTimestamp, Freshness}; use uv_cache_info::Timestamp; use uv_cache_key::cache_digest; -use uv_fs::{PythonExt, Simplified, write_atomic_sync}; +use uv_fs::{LockedFile, PythonExt, Simplified, write_atomic_sync}; use uv_install_wheel::Layout; use uv_pep440::Version; use uv_pep508::{MarkerEnvironment, StringVersion}; @@ -581,6 +581,31 @@ impl Interpreter { .into_iter() .any(|default_name| name == default_name.to_string()) } + + /// Grab a file lock for the environment to prevent concurrent writes across processes. + pub async fn lock(&self) -> Result { + if let Some(target) = self.target() { + // If we're installing into a `--target`, use a target-specific lockfile. + LockedFile::acquire(target.root().join(".lock"), target.root().user_display()).await + } else if let Some(prefix) = self.prefix() { + // Likewise, if we're installing into a `--prefix`, use a prefix-specific lockfile. + LockedFile::acquire(prefix.root().join(".lock"), prefix.root().user_display()).await + } else if self.is_virtualenv() { + // If the environment a virtualenv, use a virtualenv-specific lockfile. + LockedFile::acquire( + self.sys_prefix.join(".lock"), + self.sys_prefix.user_display(), + ) + .await + } else { + // Otherwise, use a global lockfile. + LockedFile::acquire( + env::temp_dir().join(format!("uv-{}.lock", cache_digest(&self.sys_executable))), + self.sys_prefix.user_display(), + ) + .await + } + } } /// The `EXTERNALLY-MANAGED` file in a Python installation. diff --git a/crates/uv/src/commands/project/add.rs b/crates/uv/src/commands/project/add.rs index 7e50ac76f..2c64d9dbb 100644 --- a/crates/uv/src/commands/project/add.rs +++ b/crates/uv/src/commands/project/add.rs @@ -26,7 +26,7 @@ use uv_distribution_types::{ Index, IndexName, IndexUrls, NameRequirementSpecification, Requirement, RequirementSource, UnresolvedRequirement, VersionId, }; -use uv_fs::Simplified; +use uv_fs::{LockedFile, Simplified}; use uv_git::GIT_STORE; use uv_git_types::GitReference; use uv_normalize::{DEV_DEPENDENCIES, DefaultExtras, PackageName}; @@ -277,6 +277,8 @@ pub(crate) async fn add( } }; + let _lock = target.acquire_lock().await?; + let client_builder = BaseClientBuilder::new() .connectivity(network_settings.connectivity) .native_tls(network_settings.native_tls) @@ -1152,6 +1154,15 @@ impl<'lock> From<&'lock AddTarget> for LockTarget<'lock> { } impl AddTarget { + /// Acquire a file lock mapped to the underlying interpreter to prevent concurrent + /// modifications. + pub(super) async fn acquire_lock(&self) -> Result { + match self { + Self::Script(_, interpreter) => interpreter.lock().await, + Self::Project(_, python_target) => python_target.interpreter().lock().await, + } + } + /// Returns the [`Interpreter`] for the target. pub(super) fn interpreter(&self) -> &Interpreter { match self { diff --git a/crates/uv/src/commands/project/remove.rs b/crates/uv/src/commands/project/remove.rs index 336641fce..6dab60012 100644 --- a/crates/uv/src/commands/project/remove.rs +++ b/crates/uv/src/commands/project/remove.rs @@ -268,6 +268,8 @@ pub(crate) async fn remove( } }; + let _lock = target.acquire_lock().await?; + // Determine the lock mode. let mode = if locked { LockMode::Locked(target.interpreter()) diff --git a/crates/uv/src/commands/project/sync.rs b/crates/uv/src/commands/project/sync.rs index 8e78836b4..ed96795e5 100644 --- a/crates/uv/src/commands/project/sync.rs +++ b/crates/uv/src/commands/project/sync.rs @@ -166,6 +166,8 @@ pub(crate) async fn sync( ), }; + let _lock = environment.lock().await?; + // Notify the user of any environment changes. match &environment { SyncEnvironment::Project(ProjectEnvironment::Existing(environment))