Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix windows lock race: Lock exclusive after all try lock errors #2800

Merged
merged 1 commit into from
Apr 3, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 12 additions & 4 deletions crates/uv-fs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use std::path::{Path, PathBuf};
use fs2::FileExt;
use fs_err as fs;
use tempfile::NamedTempFile;
use tracing::{error, warn};
use tracing::{debug, error, warn};

use uv_warnings::warn_user;

Expand Down Expand Up @@ -282,18 +282,26 @@ pub struct LockedFile(fs_err::File);
impl LockedFile {
pub fn acquire(path: impl AsRef<Path>, resource: impl Display) -> Result<Self, std::io::Error> {
let file = fs_err::File::create(path.as_ref())?;
debug!("Trying to lock if free: {}", path.as_ref().user_display());
match file.file().try_lock_exclusive() {
Ok(()) => Ok(Self(file)),
Err(err) if err.kind() == std::io::ErrorKind::WouldBlock => {
Err(err) => {
// Log error code and enum kind to help debugging more exotic failures
debug!("Try lock error, waiting for exclusive lock: {:?}", err);
warn_user!(
"Waiting to acquire lock for {} (lockfile: {})",
resource,
path.user_display(),
);
file.file().lock_exclusive()?;
file.file().lock_exclusive().map_err(|err| {
// Not an fs_err method, we need to build our own path context
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@konstin can you open an upstream issue to add?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that's in scope for either project

std::io::Error::new(
std::io::ErrorKind::Other,
format!("Could not lock {}: {}", path.as_ref().user_display(), err),
)
})?;
Ok(Self(file))
}
Err(err) => Err(err),
}
}
}
Expand Down
Loading