diff --git a/src/dist/download.rs b/src/dist/download.rs index 6342d4cdd3..70050cbc1d 100644 --- a/src/dist/download.rs +++ b/src/dist/download.rs @@ -15,7 +15,7 @@ use url::Url; use crate::config::Cfg; use crate::dist::manifest::Manifest; use crate::dist::{Channel, DEFAULT_DIST_SERVER, ToolchainDesc, temp}; -use crate::download::{download_file, download_file_with_resume}; +use crate::download::{LockedFile, download_file, download_file_with_resume}; use crate::errors::RustupError; use crate::process::Process; use crate::utils; @@ -55,6 +55,18 @@ impl<'a> DownloadCfg<'a> { utils::ensure_dir_exists("Download Directory", self.download_dir)?; let target_file = self.download_dir.join(Path::new(hash)); + let file_name = target_file + .file_name() + .map(|s| s.to_str().unwrap_or("_")) + .unwrap_or("_") + .to_owned(); + let locked_file = target_file.with_file_name(file_name.clone() + ".lock"); + + let _locked_file = LockedFile::create(&locked_file) + .with_context(|| RustupError::CreateLockedFile(locked_file.clone()))? + .lock() + .with_context(|| RustupError::LockedFile(locked_file.clone()))?; + if target_file.exists() { let cached_result = file_hash(&target_file)?; if hash == cached_result { @@ -67,14 +79,7 @@ impl<'a> DownloadCfg<'a> { } } - let partial_file_path = target_file.with_file_name( - target_file - .file_name() - .map(|s| s.to_str().unwrap_or("_")) - .unwrap_or("_") - .to_owned() - + ".partial", - ); + let partial_file_path = target_file.with_file_name(file_name + ".partial"); let partial_file_existed = partial_file_path.exists(); diff --git a/src/download/mod.rs b/src/download/mod.rs index b32524a596..602b01ae4f 100644 --- a/src/download/mod.rs +++ b/src/download/mod.rs @@ -1,8 +1,9 @@ //! Easy file downloading -use std::fs::remove_file; +use std::fs::{File, OpenOptions, remove_file}; +use std::io; use std::num::NonZero; -use std::path::Path; +use std::path::{Path, PathBuf}; use std::str::FromStr; use std::time::Duration; @@ -26,6 +27,96 @@ use crate::{dist::download::DownloadStatus, errors::RustupError, process::Proces #[cfg(test)] mod tests; +/// An OS lock on a file that unlocks when dropping the file. +pub(crate) struct LockedFile { + path: PathBuf, + file: File, +} + +impl LockedFile { + /// Creates the file if it does not exit, does not lock. + #[cfg(unix)] + pub(crate) fn create(path: impl Into) -> Result { + use std::os::unix::fs::PermissionsExt; + use tempfile::NamedTempFile; + + let path = path.into(); + + // If path already exists, return it. + if let Ok(file) = OpenOptions::new().read(true).write(true).open(&path) { + return Ok(Self { path, file }); + } + + // Otherwise, create a temporary file with 777 permissions, to handle races between + // processes running under different UIDs (e.g., in Docker containers). We must set + // permissions _after_ creating the file, to override the `umask`. + let file = if let Some(parent) = path.parent() { + NamedTempFile::new_in(parent)? + } else { + NamedTempFile::new()? + }; + if let Err(err) = file + .as_file() + .set_permissions(std::fs::Permissions::from_mode(0o777)) + { + warn!("failed to set permissions on temporary file: {err}"); + } + + // Try to move the file to path, but if path exists now, just open path + match file.persist_noclobber(&path) { + Ok(file) => Ok(Self { path, file }), + Err(err) => { + if err.error.kind() == io::ErrorKind::AlreadyExists { + let file = OpenOptions::new().read(true).write(true).open(&path)?; + Ok(Self { path, file }) + } else { + Err(err.error) + } + } + } + } + + /// Creates the file if it does not exit, does not lock. + #[cfg(not(unix))] + pub(crate) fn create(path: impl Into) -> Result { + let path = path.into(); + let file = OpenOptions::new() + .read(true) + .write(true) + .create(true) + .truncate(true) + .open(&path)?; + Ok(Self { path, file }) + } + + /// Acquire an exclusive lock on a file, blocking until the file is ready. + pub(crate) fn lock(self) -> Result { + match self.file.lock() { + Err(err) if err.kind() == io::ErrorKind::Unsupported => { + warn!("locking files is not supported, running rustup in parallel may error"); + Ok(self) + } + Err(err) => Err(err), + Ok(()) => Ok(self), + } + } +} + +impl Drop for LockedFile { + /// Unlock the file. + fn drop(&mut self) { + if let Err(err) = self.file.unlock() { + warn!( + "failed to unlock {}; program may be stuck: {}", + self.path.display(), + err + ); + } else { + debug!("released lock at `{}`", self.path.display()); + } + } +} + pub(crate) async fn download_file( url: &Url, path: &Path, @@ -48,7 +139,7 @@ pub(crate) async fn download_file_with_resume( match download_file_(url, path, hasher, resume_from_partial, status, process).await { Ok(_) => Ok(()), Err(e) => { - if e.downcast_ref::().is_some() { + if e.downcast_ref::().is_some() { return Err(e); } let is_client_error = match e.downcast_ref::() { @@ -722,7 +813,7 @@ enum DownloadError { #[error("{0}")] Message(String), #[error(transparent)] - IoError(#[from] std::io::Error), + IoError(#[from] io::Error), #[cfg(any(feature = "reqwest-rustls-tls", feature = "reqwest-native-tls"))] #[error(transparent)] Reqwest(#[from] ::reqwest::Error), diff --git a/src/errors.rs b/src/errors.rs index 8fb12b6913..61bfccd850 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -27,6 +27,10 @@ pub struct OperationError(pub anyhow::Error); pub enum RustupError { #[error("partially downloaded file may have been damaged and was removed, please try again")] BrokenPartialFile, + #[error("failed to create '{0}' as lockfile")] + CreateLockedFile(PathBuf), + #[error("failed to lock '{0}'")] + LockedFile(PathBuf), #[error("component download failed for {0}")] ComponentDownloadFailed(String), #[error("failure removing component '{name}', directory does not exist: '{}'", .path.display())]