From 5b664bd7b50a0f08e50d9fbad24c6bb890c49618 Mon Sep 17 00:00:00 2001 From: akrm al-hakimi Date: Fri, 7 Aug 2026 13:48:53 -0400 Subject: [PATCH 1/2] feat(dist): stage fresh toolchain installs before atomic publication A fresh distribution install now builds under a private staging directory at `toolchains/+rustup-staging-` and is published to `toolchains/` with a same-filesystem rename, with the alias-scoped update hash written only after publication. An interrupted install therefore never leaves a partial toolchain at a selectable path, and a retry always converges. The stage path is deterministic per toolchain, and the owning process holds an exclusive advisory file lock on the stage for its lifetime. The OS releases the lock if the process dies, so the next install can distinguish an abandoned stage from a live one and reclaim it, while a concurrent install of the same toolchain fails fast with a clear error. Blocking instead of failing, and serializing whole operations, is left to the locking work tracked in #988. Existing updates still modify toolchains in place. --- src/dist/mod.rs | 13 +-- src/install.rs | 193 +++++++++++++++++++++++++++++++++++++++--- src/lib.rs | 2 +- tests/suite/cli_v2.rs | 17 ++-- 4 files changed, 194 insertions(+), 31 deletions(-) diff --git a/src/dist/mod.rs b/src/dist/mod.rs index f348188943..d8f49d62dc 100644 --- a/src/dist/mod.rs +++ b/src/dist/mod.rs @@ -17,7 +17,7 @@ use itertools::Itertools; use regex::{Match, Regex}; use serde::{Deserialize, Serialize}; use thiserror::Error as ThisError; -use tracing::{debug, info, warn}; +use tracing::{debug, info}; use crate::{ config::Cfg, @@ -944,17 +944,10 @@ impl<'cfg, 'a> DistOptions<'cfg, 'a> { pub(crate) async fn install_into( &self, prefix: &InstallPrefix, + update_hash: &Path, manifest: Option, ) -> Result> { let fresh_install = !prefix.path().exists(); - // fresh_install means the toolchain isn't present, but hash_exists means there is a stray hash file - if fresh_install && self.update_hash.exists() { - warn!( - "removing stray hash file in order to continue: {}", - self.update_hash.display() - ); - std::fs::remove_file(&self.update_hash)?; - } let mut fetched = String::new(); let mut first_err = None; @@ -1007,7 +1000,7 @@ impl<'cfg, 'a> DistOptions<'cfg, 'a> { let res = loop { let result = try_update_from_dist_( &self.dl_cfg, - &self.update_hash, + update_hash, &toolchain, match self.exists { false => Some(self.profile), diff --git a/src/install.rs b/src/install.rs index bb5e275c8b..ddd14fffc5 100644 --- a/src/install.rs +++ b/src/install.rs @@ -1,9 +1,13 @@ //! Installation and upgrade of both distribution-managed and local //! toolchains -use std::path::{Path, PathBuf}; +use std::{ + ffi::OsString, + fs, + path::{Path, PathBuf}, +}; -use anyhow::Result; -use tracing::debug; +use anyhow::{Context, Result, bail}; +use tracing::{debug, warn}; use crate::{ config::Cfg, @@ -13,6 +17,114 @@ use crate::{ utils, }; +/// A fresh toolchain being constructed in a private staging directory, +/// invisible to toolchain enumeration until it is published. +struct StagedToolchain { + root: PathBuf, + prefix: PathBuf, + /// Exclusive lock marking the stage as owned by a live process; `None` + /// only while dropping. + lock: Option, +} + +impl StagedToolchain { + fn new(destination: &Path) -> Result { + let parent = destination + .parent() + .expect("toolchain destination must have a parent"); + let name = destination + .file_name() + .expect("toolchain destination must have a base name"); + utils::ensure_dir_exists("toolchains", parent)?; + + // The stage lives at a deterministic path so that interrupted + // installs of the same toolchain reuse a single staging directory + // instead of accumulating abandoned ones. + let mut dir_name = OsString::from(STAGING_DIR_PREFIX); + dir_name.push(name); + let root = parent.join(dir_name); + utils::ensure_dir_exists("staging toolchain", &root)?; + + // The lock is held for the lifetime of the stage and released by the + // OS if this process dies, so an abandoned stage can be reclaimed + // safely. Blocking on (rather than failing under) contention and + // serializing whole operations is left to the locking work tracked + // in rust-lang/rustup#988. + let lock = fs::File::create(root.join(STAGING_LOCK_FILE)) + .with_context(|| format!("failed to create lock file in {}", root.display()))?; + match lock.try_lock() { + Ok(()) => {} + Err(fs::TryLockError::WouldBlock) => bail!( + "toolchain '{}' is already being installed by another rustup process; \ + wait for it to finish and retry", + name.display() + ), + Err(fs::TryLockError::Error(error)) => { + return Err(error).with_context(|| { + format!("failed to lock staging directory {}", root.display()) + }); + } + } + + // This process owns the stage: clear anything left over from an + // interrupted attempt so installation starts from a clean prefix. + let prefix = root.join("toolchain"); + if utils::path_exists(&prefix) { + utils::remove_dir("staging toolchain", &prefix)?; + } + utils::ensure_file_removed("staging update hash", &root.join(STAGING_HASH_FILE))?; + + Ok(Self { + root, + prefix, + lock: Some(lock), + }) + } + + fn prefix(&self) -> &Path { + &self.prefix + } + + /// Path of the update hash tracked next to the stage. The hash under + /// `$RUSTUP_HOME/update-hashes` is alias-scoped metadata and must not be + /// touched before the staged toolchain is published. + fn update_hash(&self) -> PathBuf { + self.root.join(STAGING_HASH_FILE) + } + + fn publish(self, destination: &Path) -> Result<()> { + // Staging is a sibling of the destination inside `toolchains/`, so + // publication must be a same-filesystem rename. Never permit the + // copy-and-delete fallback. + match utils::rename("toolchain", &self.prefix, destination, false) { + Err(error) if utils::path_exists(destination) => Err(error).with_context(|| { + format!( + "toolchain directory {} was created by another process \ + while this installation was in progress", + destination.display() + ) + }), + result => result, + } + } +} + +impl Drop for StagedToolchain { + fn drop(&mut self) { + // Release the lock before removing the stage: an open handle inside + // the directory can prevent its removal on Windows. + drop(self.lock.take()); + if utils::path_exists(&self.root) + && let Err(error) = utils::remove_dir("staging toolchain", &self.root) + { + warn!( + path = %self.root.display(), + "could not remove staging toolchain: {error}" + ); + } + } +} + #[derive(Clone, Debug)] pub(crate) enum UpdateStatus { Installed, @@ -53,8 +165,9 @@ impl InstallMethod<'_, '_> { _ => debug!("updating existing install for '{}'", self.dest_basename()), } - debug!("toolchain directory: {}", self.dest_path().display()); - let updated = self.run(&self.dest_path(), manifest).await?; + let destination = self.dest_path(); + debug!("toolchain directory: {}", destination.display()); + let updated = self.run(&destination, manifest).await?; let status = match updated { false => { @@ -102,16 +215,49 @@ impl InstallMethod<'_, '_> { utils::symlink_dir(src, path)?; Ok(true) } - InstallMethod::Dist(opts) => { + // Updates modify the published toolchain in place. + InstallMethod::Dist(opts) if opts.exists => { let prefix = &InstallPrefix::from(path.to_owned()); - let maybe_new_hash = opts.install_into(prefix, manifest).await?; + let Some(hash) = opts + .install_into(prefix, &opts.update_hash, manifest) + .await? + else { + return Ok(false); + }; - if let Some(hash) = maybe_new_hash { - utils::write_file("update hash", &opts.update_hash, &hash)?; - Ok(true) - } else { - Ok(false) - } + utils::write_file("update hash", &opts.update_hash, &hash)?; + Ok(true) + } + // A fresh toolchain is constructed under a private stage and only + // published once complete, so that an interruption can never + // leave a partial toolchain behind at a selectable path. + InstallMethod::Dist(opts) => { + let staging = StagedToolchain::new(path)?; + let prefix = InstallPrefix::from(staging.prefix().to_owned()); + // A stray alias-scoped update hash (e.g. left behind by an + // interrupted uninstall) is deliberately ignored rather than + // removed: the staged install only ever reads the + // staging-local hash, and the alias-scoped one is rewritten + // after publication. + let staging_hash = staging.update_hash(); + let Some(hash) = opts.install_into(&prefix, &staging_hash, manifest).await? else { + return Ok(false); + }; + + #[cfg(feature = "test")] + opts.cfg + .process + .checkpoint(CHECKPOINT_INSTALL_BEFORE_PUBLISH); + + staging.publish(path)?; + + #[cfg(feature = "test")] + opts.cfg + .process + .checkpoint(CHECKPOINT_INSTALL_AFTER_PUBLISH); + + utils::write_file("update hash", &opts.update_hash, &hash)?; + Ok(true) } } } @@ -156,3 +302,24 @@ impl InstallMethod<'_, '_> { pub(crate) fn uninstall(path: &Path) -> Result<()> { utils::remove_dir("install", path) } + +/// Prefix of staging directory names inside `toolchains/`. `+` is not valid +/// at the start of a toolchain name, so stages are identifiable and excluded +/// by the existing toolchain enumeration. +pub const STAGING_DIR_PREFIX: &str = "+rustup-staging-"; + +/// Lock file inside a stage, held exclusively by the owning process. +const STAGING_LOCK_FILE: &str = "lock"; + +/// Update hash file inside a stage (see [`StagedToolchain::update_hash`]). +const STAGING_HASH_FILE: &str = "update-hash"; + +/// Checkpoint reached once a staged toolchain is complete, right before its +/// publication. +#[cfg(feature = "test")] +pub const CHECKPOINT_INSTALL_BEFORE_PUBLISH: &str = "install-before-publish"; + +/// Checkpoint reached right after publication, before the alias-scoped +/// update hash is written. +#[cfg(feature = "test")] +pub const CHECKPOINT_INSTALL_AFTER_PUBLISH: &str = "install-after-publish"; diff --git a/src/lib.rs b/src/lib.rs index 0daf2f6ede..7204044d69 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -75,7 +75,7 @@ mod download; pub mod env_var; pub mod errors; mod fallback_settings; -mod install; +pub mod install; pub mod process; mod settings; #[cfg(feature = "test")] diff --git a/tests/suite/cli_v2.rs b/tests/suite/cli_v2.rs index ebbb0b89ca..d0c3b5df76 100644 --- a/tests/suite/cli_v2.rs +++ b/tests/suite/cli_v2.rs @@ -1949,9 +1949,11 @@ async fn remove_target_missing_update_hash() { .is_ok(); } -// Issue #1777 +// Issue #1777: a stray hash file must not prevent installation. Staged +// installs never consult the alias-scoped hash, so it is silently refreshed +// after publication rather than removed up front. #[tokio::test] -async fn warn_about_and_remove_stray_hash() { +async fn stray_hash_is_ignored_and_refreshed() { let mut cx = CliTestContext::new(Scenario::None).await; let mut hash_path = cx.config.rustupdir.join("update-hashes"); fs::create_dir_all(&hash_path).expect("Unable to make the update-hashes directory"); @@ -1965,12 +1967,13 @@ async fn warn_about_and_remove_stray_hash() { cx.config .expect(["rustup", "toolchain", "install", "nightly"]) .await - .with_stderr(snapbox::str![[r#" -... -warn: removing stray hash file in order to continue: [..]/update-hashes/nightly-[HOST_TUPLE] -... -"#]]) .is_ok(); + + let refreshed = fs::read_to_string(&hash_path).expect("Unable to read update-hash file"); + assert_ne!( + refreshed, "LEGITHASH", + "stray hash file was not refreshed after installation" + ); } fn make_component_unavailable(config: &Config, name: &str, target: String) { From 53cc3e4061c298e9354b6d91dba3bd195935749d Mon Sep 17 00:00:00 2001 From: akrm al-hakimi Date: Fri, 7 Aug 2026 13:48:53 -0400 Subject: [PATCH 2/2] test(cli): cover interrupted staged toolchain installs Covers kills before and after publication, stage reuse across repeated interruptions, reclamation of a stale stage without leaking its contents into the published toolchain, and a stray update hash being ignored during a staged install and refreshed after publication. --- tests/suite/cli_crash.rs | 210 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 206 insertions(+), 4 deletions(-) diff --git a/tests/suite/cli_crash.rs b/tests/suite/cli_crash.rs index 70c5c95498..a3d10f808f 100644 --- a/tests/suite/cli_crash.rs +++ b/tests/suite/cli_crash.rs @@ -1,17 +1,204 @@ -use rustup::{ - dist::manifestation::CHECKPOINT_UPDATE_BEFORE_METADATA, - test::{CliTestContext, Scenario}, +use std::{fs, path::PathBuf, process::Command, time::Duration}; + +use rustup::dist::manifestation::CHECKPOINT_UPDATE_BEFORE_METADATA; +use rustup::install::{ + CHECKPOINT_INSTALL_AFTER_PUBLISH, CHECKPOINT_INSTALL_BEFORE_PUBLISH, STAGING_DIR_PREFIX, }; +use rustup::test::{CliTestContext, Scenario, this_host_tuple}; +use wait_timeout::ChildExt; + +fn assert_completes_successfully(mut command: Command) { + let mut child = command.spawn().expect("failed to start command"); + let Some(status) = child + .wait_timeout(Duration::from_secs(10)) + .expect("failed to wait for command") + else { + let _ = child.kill(); + let _ = child.wait(); + panic!("command did not complete within 10 seconds"); + }; + assert!(status.success(), "command failed with status {status}"); +} + +fn nightly_path(cx: &CliTestContext) -> PathBuf { + cx.config + .rustupdir + .join("toolchains") + .join(format!("nightly-{}", this_host_tuple())) +} + +fn nightly_update_hash_path(cx: &CliTestContext) -> PathBuf { + cx.config + .rustupdir + .join("update-hashes") + .join(format!("nightly-{}", this_host_tuple())) +} + +fn nightly_staging_root(cx: &CliTestContext) -> PathBuf { + cx.config + .rustupdir + .join("toolchains") + .join(format!("{STAGING_DIR_PREFIX}nightly-{}", this_host_tuple())) +} + +fn staging_paths(cx: &CliTestContext) -> Vec { + fs::read_dir(cx.config.rustupdir.join("toolchains")) + .expect("failed to read toolchains directory") + .map(|entry| entry.expect("failed to read toolchains entry").path()) + .filter(|path| { + path.file_name() + .and_then(|name| name.to_str()) + .is_some_and(|name| name.starts_with(STAGING_DIR_PREFIX)) + }) + .collect() +} + +async fn assert_nightly_is_complete(cx: &CliTestContext) { + cx.config + .expect(["rustup", "+nightly", "component", "list", "--installed"]) + .await + .with_stdout(snapbox::str![[r#" +cargo-[HOST_TUPLE] +rust-docs-[HOST_TUPLE] +rust-std-[HOST_TUPLE] +rustc-[HOST_TUPLE] + +"#]]) + .is_ok(); + cx.config + .expect(["rustc", "+nightly", "--version"]) + .await + .with_stdout(snapbox::str![[r#" +1.3.0 (hash-nightly-2) + +"#]]) + .is_ok(); +} + +async fn assert_unpublished_install_can_be_retried(checkpoint: &str) { + let cx = CliTestContext::new(Scenario::SimpleV2).await; + let status = cx.kill_at(checkpoint, ["rustup", "toolchain", "install", "nightly"]); + assert!(!status.success()); + assert!( + !nightly_path(&cx).exists(), + "an interrupted staging operation published the toolchain" + ); + assert_eq!(staging_paths(&cx), vec![nightly_staging_root(&cx)]); + cx.config + .expect(["rustup", "toolchain", "list"]) + .await + .without_stdout("rustup-staging") + .without_stdout("nightly") + .is_ok(); + + assert_completes_successfully(cx.config.cmd("rustup", ["toolchain", "install", "nightly"])); + assert!(nightly_path(&cx).is_dir()); + assert_nightly_is_complete(&cx).await; +} #[tokio::test] async fn interrupted_install_can_be_retried() { + assert_unpublished_install_can_be_retried(CHECKPOINT_UPDATE_BEFORE_METADATA).await; +} + +#[tokio::test] +async fn interrupted_install_before_publication_can_be_retried() { + assert_unpublished_install_can_be_retried(CHECKPOINT_INSTALL_BEFORE_PUBLISH).await; +} + +#[tokio::test] +async fn interrupted_installs_do_not_accumulate_stages() { let cx = CliTestContext::new(Scenario::SimpleV2).await; + + for _ in 0..2 { + let status = cx.kill_at( + CHECKPOINT_INSTALL_BEFORE_PUBLISH, + ["rustup", "toolchain", "install", "nightly"], + ); + assert!(!status.success()); + assert_eq!(staging_paths(&cx), vec![nightly_staging_root(&cx)]); + } + + assert_completes_successfully(cx.config.cmd("rustup", ["toolchain", "install", "nightly"])); + assert!(staging_paths(&cx).is_empty()); + assert_nightly_is_complete(&cx).await; +} + +#[tokio::test] +async fn stale_stage_is_reclaimed_by_the_next_install() { + let cx = CliTestContext::new(Scenario::SimpleV2).await; + let junk = nightly_staging_root(&cx) + .join("toolchain") + .join("bin") + .join("stale-junk"); + fs::create_dir_all(junk.parent().unwrap()).unwrap(); + fs::write(&junk, "junk").unwrap(); + fs::write(nightly_staging_root(&cx).join("update-hash"), "stale-hash").unwrap(); + + assert_completes_successfully(cx.config.cmd("rustup", ["toolchain", "install", "nightly"])); + assert!(staging_paths(&cx).is_empty()); + assert!( + !nightly_path(&cx).join("bin").join("stale-junk").exists(), + "contents of a stale stage leaked into the published toolchain" + ); + assert_nightly_is_complete(&cx).await; +} + +#[tokio::test] +async fn unpublished_install_does_not_change_update_hash() { + let cx = CliTestContext::new(Scenario::SimpleV2).await; + let update_hash = nightly_update_hash_path(&cx); + fs::create_dir_all(update_hash.parent().unwrap()).unwrap(); + fs::write(&update_hash, "stale-hash").unwrap(); + let status = cx.kill_at( - CHECKPOINT_UPDATE_BEFORE_METADATA, + CHECKPOINT_INSTALL_BEFORE_PUBLISH, + ["rustup", "toolchain", "install", "nightly"], + ); + + assert!(!status.success()); + assert_eq!(fs::read_to_string(update_hash).unwrap(), "stale-hash"); + assert!(!nightly_path(&cx).exists()); +} + +#[tokio::test] +async fn interrupted_install_after_publication_is_complete() { + let cx = CliTestContext::new(Scenario::SimpleV2).await; + let status = cx.kill_at( + CHECKPOINT_INSTALL_AFTER_PUBLISH, ["rustup", "toolchain", "install", "nightly"], ); assert!(!status.success()); + assert!(nightly_path(&cx).is_dir()); + assert!(staging_paths(&cx).is_empty()); + assert_nightly_is_complete(&cx).await; + + assert_completes_successfully(cx.config.cmd("rustup", ["toolchain", "install", "nightly"])); + assert_nightly_is_complete(&cx).await; +} + +#[tokio::test] +async fn failed_install_removes_staging_directory() { + let cx = CliTestContext::new(Scenario::UnavailableRls).await; + cx.config.set_current_dist_date("2015-01-01"); + cx.config + .expect(["rustup", "set", "profile", "complete"]) + .await + .is_ok(); + cx.config + .expect(["rustup", "toolchain", "install", "nightly"]) + .await + .is_err(); + + assert!(!nightly_path(&cx).exists()); + assert!(staging_paths(&cx).is_empty()); +} + +#[tokio::test] +async fn interrupted_update_can_be_retried() { + let cx = CliTestContext::new(Scenario::ArchivesV2).await; + cx.config.set_current_dist_date("2015-01-01"); cx.config .expect(["rustup", "toolchain", "install", "nightly"]) .await @@ -19,5 +206,20 @@ async fn interrupted_install_can_be_retried() { cx.config .expect(["rustc", "+nightly", "--version"]) .await + .with_stdout(snapbox::str![[r#" +1.2.0 (hash-nightly-1) + +"#]]) .is_ok(); + + cx.config.set_current_dist_date("2015-01-02"); + let status = cx.kill_at( + CHECKPOINT_UPDATE_BEFORE_METADATA, + ["rustup", "update", "nightly"], + ); + assert!(!status.success()); + + assert_completes_successfully(cx.config.cmd("rustup", ["update", "nightly"])); + + assert_nightly_is_complete(&cx).await; }