Skip to content

Commit 9f4664d

Browse files
committed
Add a 5 min default timeout for deadlocks
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. Handle not found errors better Windows fix Fix windows Update docs Review Simplify test case Clippy Use async and write docs Update snapshot
1 parent 97e8644 commit 9f4664d

File tree

45 files changed

+570
-274
lines changed

Some content is hidden

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

45 files changed

+570
-274
lines changed

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
@@ -179,7 +179,7 @@ tempfile = { version = "3.14.0" }
179179
textwrap = { version = "0.16.1" }
180180
thiserror = { version = "2.0.0" }
181181
tl = { git = "https://github.com/astral-sh/tl.git", rev = "6e25b2ee2513d75385101a8ff9f591ef51f314ec" }
182-
tokio = { version = "1.40.0", features = ["fs", "io-util", "macros", "process", "rt", "signal", "sync"] }
182+
tokio = { version = "1.40.0", features = ["fs", "io-util", "macros", "process", "rt", "signal", "sync", "time"] }
183183
tokio-stream = { version = "0.1.16" }
184184
tokio-util = { version = "0.7.12", features = ["compat", "io"] }
185185
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
@@ -22,7 +22,7 @@ use crate::{
2222
index::{AuthPolicy, Indexes},
2323
realm::Realm,
2424
};
25-
use crate::{Index, TextCredentialStore, TomlCredentialError};
25+
use crate::{Index, TextCredentialStore};
2626

2727
/// Strategy for loading netrc files.
2828
enum NetrcMode {
@@ -60,49 +60,55 @@ impl NetrcMode {
6060

6161
/// Strategy for loading text-based credential files.
6262
enum TextStoreMode {
63-
Automatic(LazyLock<Option<TextCredentialStore>>),
63+
Automatic(tokio::sync::OnceCell<Option<TextCredentialStore>>),
6464
Enabled(TextCredentialStore),
6565
Disabled,
6666
}
6767

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

10174
impl TextStoreMode {
75+
async fn load_default_store() -> Option<TextCredentialStore> {
76+
let path = TextCredentialStore::default_file()
77+
.inspect_err(|err| {
78+
warn!("Failed to determine credentials file path: {}", err);
79+
})
80+
.ok()?;
81+
82+
match TextCredentialStore::read(&path).await {
83+
Ok((store, _lock)) => {
84+
debug!("Loaded credential file {}", path.display());
85+
Some(store)
86+
}
87+
Err(err)
88+
if err
89+
.as_io_error()
90+
.is_some_and(|err| err.kind() == std::io::ErrorKind::NotFound) =>
91+
{
92+
debug!("No credentials file found at {}", path.display());
93+
None
94+
}
95+
Err(err) => {
96+
warn!(
97+
"Failed to load credentials from {}: {}",
98+
path.display(),
99+
err
100+
);
101+
None
102+
}
103+
}
104+
}
105+
102106
/// Get the parsed credential store, if enabled.
103-
fn get(&self) -> Option<&TextCredentialStore> {
107+
async fn get(&self) -> Option<&TextCredentialStore> {
104108
match self {
105-
Self::Automatic(lock) => lock.as_ref(),
109+
// TODO(zanieb): Reconsider this pattern. We're just mirroring the [`NetrcMode`]
110+
// implementation for now.
111+
Self::Automatic(lock) => lock.get_or_init(Self::load_default_store).await.as_ref(),
106112
Self::Enabled(store) => Some(store),
107113
Self::Disabled => None,
108114
}
@@ -720,7 +726,7 @@ impl AuthMiddleware {
720726
Some(credentials)
721727

722728
// Text credential store support.
723-
} else if let Some(credentials) = self.text_store.get().and_then(|text_store| {
729+
} else if let Some(credentials) = self.text_store.get().await.and_then(|text_store| {
724730
debug!("Checking text store for credentials for {url}");
725731
text_store.get_credentials(url, credentials.as_ref().and_then(|credentials| credentials.username())).cloned()
726732
}) {

crates/uv-auth/src/store.rs

Lines changed: 33 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use rustc_hash::FxHashMap;
66
use serde::{Deserialize, Serialize};
77
use thiserror::Error;
88
use url::Url;
9-
use uv_fs::{LockedFile, with_added_extension};
9+
use uv_fs::{LockedFile, LockedFileError, LockedFileMode, with_added_extension};
1010
use uv_preview::{Preview, PreviewFeatures};
1111
use uv_redacted::DisplaySafeUrl;
1212

@@ -29,20 +29,24 @@ pub enum AuthBackend {
2929
}
3030

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

3838
// Otherwise, we'll use the plaintext credential store
3939
let path = TextCredentialStore::default_file()?;
40-
match TextCredentialStore::read(&path) {
40+
match TextCredentialStore::read(&path).await {
4141
Ok((store, lock)) => Ok(Self::TextStore(store, lock)),
42-
Err(TomlCredentialError::Io(err)) if err.kind() == std::io::ErrorKind::NotFound => {
42+
Err(err)
43+
if err
44+
.as_io_error()
45+
.is_some_and(|err| err.kind() == std::io::ErrorKind::NotFound) =>
46+
{
4347
Ok(Self::TextStore(
4448
TextCredentialStore::default(),
45-
TextCredentialStore::lock(&path)?,
49+
TextCredentialStore::lock(&path).await?,
4650
))
4751
}
4852
Err(err) => Err(err),
@@ -70,6 +74,8 @@ pub enum AuthScheme {
7074
pub enum TomlCredentialError {
7175
#[error(transparent)]
7276
Io(#[from] std::io::Error),
77+
#[error(transparent)]
78+
LockedFile(#[from] LockedFileError),
7379
#[error("Failed to parse TOML credential file: {0}")]
7480
ParseError(#[from] toml::de::Error),
7581
#[error("Failed to serialize credentials to TOML")]
@@ -84,6 +90,21 @@ pub enum TomlCredentialError {
8490
TokenNotUnicode(#[from] std::string::FromUtf8Error),
8591
}
8692

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

236257
/// Acquire a lock on the credentials file at the given path.
237-
pub fn lock(path: &Path) -> Result<LockedFile, TomlCredentialError> {
258+
pub async fn lock(path: &Path) -> Result<LockedFile, TomlCredentialError> {
238259
if let Some(parent) = path.parent() {
239260
fs::create_dir_all(parent)?;
240261
}
241262
let lock = with_added_extension(path, ".lock");
242-
Ok(LockedFile::acquire_blocking(lock, "credentials store")?)
263+
Ok(LockedFile::acquire(lock, LockedFileMode::Exclusive, "credentials store").await?)
243264
}
244265

245266
/// Read credentials from a file.
@@ -270,8 +291,8 @@ impl TextCredentialStore {
270291
/// Returns [`TextCredentialStore`] and a [`LockedFile`] to hold if mutating the store.
271292
///
272293
/// If the store will not be written to following the read, the lock can be dropped.
273-
pub fn read<P: AsRef<Path>>(path: P) -> Result<(Self, LockedFile), TomlCredentialError> {
274-
let lock = Self::lock(path.as_ref())?;
294+
pub async fn read<P: AsRef<Path>>(path: P) -> Result<(Self, LockedFile), TomlCredentialError> {
295+
let lock = Self::lock(path.as_ref()).await?;
275296
let store = Self::from_file(path)?;
276297
Ok((store, lock))
277298
}
@@ -447,8 +468,8 @@ mod tests {
447468
assert!(store.get_credentials(&url, None).is_none());
448469
}
449470

450-
#[test]
451-
fn test_file_operations() {
471+
#[tokio::test]
472+
async fn test_file_operations() {
452473
let mut temp_file = NamedTempFile::new().unwrap();
453474
writeln!(
454475
temp_file,
@@ -484,7 +505,7 @@ password = "pass2"
484505
store
485506
.write(
486507
temp_output.path(),
487-
TextCredentialStore::lock(temp_file.path()).unwrap(),
508+
TextCredentialStore::lock(temp_file.path()).await.unwrap(),
488509
)
489510
.unwrap();
490511

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
@@ -21,8 +21,10 @@ uv-cache = { workspace = true }
2121
uv-client = { workspace = true }
2222
uv-distribution-filename = { workspace = true }
2323
uv-extract = { workspace = true }
24+
uv-fs = { workspace = true }
2425
uv-pep440 = { workspace = true }
2526
uv-platform = { 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

@@ -134,6 +135,9 @@ pub enum Error {
134135
#[error(transparent)]
135136
Io(#[from] std::io::Error),
136137

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

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)