Skip to content

Commit 62bf921

Browse files
authored
Add a 5 min default timeout for deadlocks (#16342)
When a process is running and another calls `uv cache clean` or `uv cache prune` we currently deadlock - sometimes until the CI timeout (astral-sh/setup-uv#588). To avoid this, we add a default 5 min timeout waiting for a lock. 5 min balances allowing in-progress builds to finish, especially with larger native dependencies, while also giving timely errors for deadlocks on (remote) systems. Commit 1 is a refactoring. This branch also fixes a problem with the logging where acquired and released resources currently mismatch: ``` DEBUG Acquired lock for `https://github.com/tqdm/tqdm` DEBUG Using existing Git source `https://github.com/tqdm/tqdm` DEBUG Released lock at `C:\Users\Konsti\AppData\Local\uv\cache\git-v0\locks\16bb813afef8edd2` ```
1 parent 2748dce commit 62bf921

47 files changed

Lines changed: 707 additions & 397 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

Cargo.lock

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,7 @@ tempfile = { version = "3.14.0" }
178178
textwrap = { version = "0.16.1" }
179179
thiserror = { version = "2.0.0" }
180180
astral-tl = { version = "0.7.9" }
181-
tokio = { version = "1.40.0", features = ["fs", "io-util", "macros", "process", "rt", "signal", "sync"] }
181+
tokio = { version = "1.40.0", features = ["fs", "io-util", "macros", "process", "rt", "signal", "sync", "time"] }
182182
tokio-stream = { version = "0.1.16" }
183183
tokio-util = { version = "0.7.12", features = ["compat", "io"] }
184184
toml = { version = "0.9.2", features = ["fast_hash"] }

crates/uv-auth/src/middleware.rs

Lines changed: 39 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ use crate::{
2323
index::{AuthPolicy, Indexes},
2424
realm::Realm,
2525
};
26-
use crate::{Index, TextCredentialStore, TomlCredentialError};
26+
use crate::{Index, TextCredentialStore};
2727

2828
/// Cached check for whether we're running in Dependabot.
2929
static IS_DEPENDABOT: LazyLock<bool> =
@@ -65,49 +65,55 @@ impl NetrcMode {
6565

6666
/// Strategy for loading text-based credential files.
6767
enum TextStoreMode {
68-
Automatic(LazyLock<Option<TextCredentialStore>>),
68+
Automatic(tokio::sync::OnceCell<Option<TextCredentialStore>>),
6969
Enabled(TextCredentialStore),
7070
Disabled,
7171
}
7272

7373
impl Default for TextStoreMode {
7474
fn default() -> Self {
75-
// TODO(zanieb): Reconsider this pattern. We're just mirroring the [`NetrcMode`]
76-
// implementation for now.
77-
Self::Automatic(LazyLock::new(|| {
78-
let path = TextCredentialStore::default_file()
79-
.inspect_err(|err| {
80-
warn!("Failed to determine credentials file path: {}", err);
81-
})
82-
.ok()?;
83-
84-
match TextCredentialStore::read(&path) {
85-
Ok((store, _lock)) => {
86-
debug!("Loaded credential file {}", path.display());
87-
Some(store)
88-
}
89-
Err(TomlCredentialError::Io(err)) if err.kind() == std::io::ErrorKind::NotFound => {
90-
debug!("No credentials file found at {}", path.display());
91-
None
92-
}
93-
Err(err) => {
94-
warn!(
95-
"Failed to load credentials from {}: {}",
96-
path.display(),
97-
err
98-
);
99-
None
100-
}
101-
}
102-
}))
75+
Self::Automatic(tokio::sync::OnceCell::new())
10376
}
10477
}
10578

10679
impl TextStoreMode {
80+
async fn load_default_store() -> Option<TextCredentialStore> {
81+
let path = TextCredentialStore::default_file()
82+
.inspect_err(|err| {
83+
warn!("Failed to determine credentials file path: {}", err);
84+
})
85+
.ok()?;
86+
87+
match TextCredentialStore::read(&path).await {
88+
Ok((store, _lock)) => {
89+
debug!("Loaded credential file {}", path.display());
90+
Some(store)
91+
}
92+
Err(err)
93+
if err
94+
.as_io_error()
95+
.is_some_and(|err| err.kind() == std::io::ErrorKind::NotFound) =>
96+
{
97+
debug!("No credentials file found at {}", path.display());
98+
None
99+
}
100+
Err(err) => {
101+
warn!(
102+
"Failed to load credentials from {}: {}",
103+
path.display(),
104+
err
105+
);
106+
None
107+
}
108+
}
109+
}
110+
107111
/// Get the parsed credential store, if enabled.
108-
fn get(&self) -> Option<&TextCredentialStore> {
112+
async fn get(&self) -> Option<&TextCredentialStore> {
109113
match self {
110-
Self::Automatic(lock) => lock.as_ref(),
114+
// TODO(zanieb): Reconsider this pattern. We're just mirroring the [`NetrcMode`]
115+
// implementation for now.
116+
Self::Automatic(lock) => lock.get_or_init(Self::load_default_store).await.as_ref(),
111117
Self::Enabled(store) => Some(store),
112118
Self::Disabled => None,
113119
}
@@ -736,7 +742,7 @@ impl AuthMiddleware {
736742
Some(credentials)
737743

738744
// Text credential store support.
739-
} else if let Some(credentials) = self.text_store.get().and_then(|text_store| {
745+
} else if let Some(credentials) = self.text_store.get().await.and_then(|text_store| {
740746
debug!("Checking text store for credentials for {url}");
741747
text_store.get_credentials(url, credentials.as_ref().and_then(|credentials| credentials.username())).cloned()
742748
}) {

crates/uv-auth/src/store.rs

Lines changed: 33 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use fs_err as fs;
55
use rustc_hash::FxHashMap;
66
use serde::{Deserialize, Serialize};
77
use thiserror::Error;
8-
use uv_fs::{LockedFile, with_added_extension};
8+
use uv_fs::{LockedFile, LockedFileError, LockedFileMode, with_added_extension};
99
use uv_preview::{Preview, PreviewFeatures};
1010
use uv_redacted::DisplaySafeUrl;
1111

@@ -28,20 +28,24 @@ pub enum AuthBackend {
2828
}
2929

3030
impl AuthBackend {
31-
pub fn from_settings(preview: Preview) -> Result<Self, TomlCredentialError> {
31+
pub async fn from_settings(preview: Preview) -> Result<Self, TomlCredentialError> {
3232
// If preview is enabled, we'll use the system-native store
3333
if preview.is_enabled(PreviewFeatures::NATIVE_AUTH) {
3434
return Ok(Self::System(KeyringProvider::native()));
3535
}
3636

3737
// Otherwise, we'll use the plaintext credential store
3838
let path = TextCredentialStore::default_file()?;
39-
match TextCredentialStore::read(&path) {
39+
match TextCredentialStore::read(&path).await {
4040
Ok((store, lock)) => Ok(Self::TextStore(store, lock)),
41-
Err(TomlCredentialError::Io(err)) if err.kind() == std::io::ErrorKind::NotFound => {
41+
Err(err)
42+
if err
43+
.as_io_error()
44+
.is_some_and(|err| err.kind() == std::io::ErrorKind::NotFound) =>
45+
{
4246
Ok(Self::TextStore(
4347
TextCredentialStore::default(),
44-
TextCredentialStore::lock(&path)?,
48+
TextCredentialStore::lock(&path).await?,
4549
))
4650
}
4751
Err(err) => Err(err),
@@ -69,6 +73,8 @@ pub enum AuthScheme {
6973
pub enum TomlCredentialError {
7074
#[error(transparent)]
7175
Io(#[from] std::io::Error),
76+
#[error(transparent)]
77+
LockedFile(#[from] LockedFileError),
7278
#[error("Failed to parse TOML credential file: {0}")]
7379
ParseError(#[from] toml::de::Error),
7480
#[error("Failed to serialize credentials to TOML")]
@@ -83,6 +89,21 @@ pub enum TomlCredentialError {
8389
TokenNotUnicode(#[from] std::string::FromUtf8Error),
8490
}
8591

92+
impl TomlCredentialError {
93+
pub fn as_io_error(&self) -> Option<&std::io::Error> {
94+
match self {
95+
Self::Io(err) => Some(err),
96+
Self::LockedFile(err) => err.as_io_error(),
97+
Self::ParseError(_)
98+
| Self::SerializeError(_)
99+
| Self::BasicAuthError(_)
100+
| Self::BearerAuthError(_)
101+
| Self::CredentialsDirError
102+
| Self::TokenNotUnicode(_) => None,
103+
}
104+
}
105+
}
106+
86107
#[derive(Debug, Error)]
87108
pub enum BasicAuthError {
88109
#[error("`username` is required with `scheme = basic`")]
@@ -233,12 +254,12 @@ impl TextCredentialStore {
233254
}
234255

235256
/// Acquire a lock on the credentials file at the given path.
236-
pub fn lock(path: &Path) -> Result<LockedFile, TomlCredentialError> {
257+
pub async fn lock(path: &Path) -> Result<LockedFile, TomlCredentialError> {
237258
if let Some(parent) = path.parent() {
238259
fs::create_dir_all(parent)?;
239260
}
240261
let lock = with_added_extension(path, ".lock");
241-
Ok(LockedFile::acquire_blocking(lock, "credentials store")?)
262+
Ok(LockedFile::acquire(lock, LockedFileMode::Exclusive, "credentials store").await?)
242263
}
243264

244265
/// Read credentials from a file.
@@ -269,8 +290,8 @@ impl TextCredentialStore {
269290
/// Returns [`TextCredentialStore`] and a [`LockedFile`] to hold if mutating the store.
270291
///
271292
/// If the store will not be written to following the read, the lock can be dropped.
272-
pub fn read<P: AsRef<Path>>(path: P) -> Result<(Self, LockedFile), TomlCredentialError> {
273-
let lock = Self::lock(path.as_ref())?;
293+
pub async fn read<P: AsRef<Path>>(path: P) -> Result<(Self, LockedFile), TomlCredentialError> {
294+
let lock = Self::lock(path.as_ref()).await?;
274295
let store = Self::from_file(path)?;
275296
Ok((store, lock))
276297
}
@@ -450,8 +471,8 @@ mod tests {
450471
assert!(store.get_credentials(&url, None).is_none());
451472
}
452473

453-
#[test]
454-
fn test_file_operations() {
474+
#[tokio::test]
475+
async fn test_file_operations() {
455476
let mut temp_file = NamedTempFile::new().unwrap();
456477
writeln!(
457478
temp_file,
@@ -487,7 +508,7 @@ password = "pass2"
487508
store
488509
.write(
489510
temp_output.path(),
490-
TextCredentialStore::lock(temp_file.path()).unwrap(),
511+
TextCredentialStore::lock(temp_file.path()).await.unwrap(),
491512
)
492513
.unwrap();
493514

crates/uv-bench/benches/uv.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,10 @@ fn setup(manifest: Manifest) -> impl Fn(bool) {
5959
.build()
6060
.unwrap();
6161

62-
let cache = Cache::from_path("../../.cache").init().unwrap();
62+
let cache = Cache::from_path("../../.cache")
63+
.init_no_wait()
64+
.expect("No cache contention when running benchmarks")
65+
.unwrap();
6366
let interpreter = PythonEnvironment::from_root("../../.venv", &cache)
6467
.unwrap()
6568
.into_interpreter();

crates/uv-bin-install/Cargo.toml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,11 @@ uv-cache = { workspace = true }
2020
uv-client = { workspace = true }
2121
uv-distribution-filename = { workspace = true }
2222
uv-extract = { workspace = true }
23+
uv-fs = { workspace = true }
2324
uv-pep440 = { workspace = true }
2425
uv-platform = { workspace = true }
2526
uv-redacted = { workspace = true }
27+
2628
fs-err = { workspace = true, features = ["tokio"] }
2729
futures = { workspace = true }
2830
reqwest = { workspace = true }

crates/uv-bin-install/src/lib.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ use uv_distribution_filename::SourceDistExtension;
2222
use uv_cache::{Cache, CacheBucket, CacheEntry};
2323
use uv_client::{BaseClient, is_transient_network_error};
2424
use uv_extract::{Error as ExtractError, stream};
25+
use uv_fs::LockedFileError;
2526
use uv_pep440::Version;
2627
use uv_platform::Platform;
2728
use uv_redacted::DisplaySafeUrl;
@@ -135,6 +136,9 @@ pub enum Error {
135136
#[error(transparent)]
136137
Io(#[from] std::io::Error),
137138

139+
#[error(transparent)]
140+
LockedFile(#[from] LockedFileError),
141+
138142
#[error("Failed to detect platform")]
139143
Platform(#[from] uv_platform::Error),
140144

crates/uv-build-frontend/src/lib.rs

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ use uv_distribution_types::{
3636
ConfigSettings, ExtraBuildRequirement, ExtraBuildRequires, IndexLocations, Requirement,
3737
Resolution,
3838
};
39-
use uv_fs::LockedFile;
39+
use uv_fs::{LockedFile, LockedFileMode};
4040
use uv_fs::{PythonExt, Simplified};
4141
use uv_normalize::PackageName;
4242
use uv_pep440::Version;
@@ -493,12 +493,16 @@ impl SourceBuild {
493493
"uv-setuptools-{}.lock",
494494
cache_digest(&canonical_source_path)
495495
));
496-
source_tree_lock = LockedFile::acquire(lock_path, self.source_tree.to_string_lossy())
497-
.await
498-
.inspect_err(|err| {
499-
warn!("Failed to acquire build lock: {err}");
500-
})
501-
.ok();
496+
source_tree_lock = LockedFile::acquire(
497+
lock_path,
498+
LockedFileMode::Exclusive,
499+
self.source_tree.to_string_lossy(),
500+
)
501+
.await
502+
.inspect_err(|err| {
503+
warn!("Failed to acquire build lock: {err}");
504+
})
505+
.ok();
502506
}
503507
Ok(source_tree_lock)
504508
}

0 commit comments

Comments
 (0)