Skip to content

Commit b7fa5b5

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 3d1a4b7 commit b7fa5b5

45 files changed

Lines changed: 570 additions & 274 deletions

File tree

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.10" }
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
}
@@ -729,7 +735,7 @@ impl AuthMiddleware {
729735
Some(credentials)
730736

731737
// Text credential store support.
732-
} else if let Some(credentials) = self.text_store.get().and_then(|text_store| {
738+
} else if let Some(credentials) = self.text_store.get().await.and_then(|text_store| {
733739
debug!("Checking text store for credentials for {url}");
734740
text_store.get_credentials(url, credentials.as_ref().and_then(|credentials| credentials.username())).cloned()
735741
}) {

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;
@@ -490,12 +490,16 @@ impl SourceBuild {
490490
"uv-setuptools-{}.lock",
491491
cache_digest(&canonical_source_path)
492492
));
493-
source_tree_lock = LockedFile::acquire(lock_path, self.source_tree.to_string_lossy())
494-
.await
495-
.inspect_err(|err| {
496-
warn!("Failed to acquire build lock: {err}");
497-
})
498-
.ok();
493+
source_tree_lock = LockedFile::acquire(
494+
lock_path,
495+
LockedFileMode::Exclusive,
496+
self.source_tree.to_string_lossy(),
497+
)
498+
.await
499+
.inspect_err(|err| {
500+
warn!("Failed to acquire build lock: {err}");
501+
})
502+
.ok();
499503
}
500504
Ok(source_tree_lock)
501505
}

0 commit comments

Comments
 (0)