From e19437be1b3de088d125c1f7ae9beb81aa1db3f8 Mon Sep 17 00:00:00 2001 From: Cloud0310 <60375730+Cloud0310@users.noreply.github.com> Date: Wed, 16 Sep 2026 23:55:49 +0800 Subject: [PATCH 1/7] refactor(windows): manage the GC handle with standard file APIs Use OpenOptions and inherited stdin to manage the GC handle with standard file APIs. Keep Command alive through the existing sleep to retain the handle. --- src/cli/self_update/windows.rs | 73 +++++++++++----------------------- 1 file changed, 23 insertions(+), 50 deletions(-) diff --git a/src/cli/self_update/windows.rs b/src/cli/self_update/windows.rs index 28fcdcfdac..29e7d9e3cb 100644 --- a/src/cli/self_update/windows.rs +++ b/src/cli/self_update/windows.rs @@ -4,7 +4,6 @@ use std::{ ffi::{OsStr, OsString}, fmt, io::Write, - os::windows::ffi::OsStrExt, path::Path, process::Command, }; @@ -370,10 +369,9 @@ pub fn complete_windows_uninstall(process: &Process) -> anyhow::Result anyhow::Result // - Open the gc exe with the FILE_FLAG_DELETE_ON_CLOSE and // FILE_SHARE_DELETE flags. This is going to be the last // file to remove, and the OS is going to do it for us. -// This file is opened as inheritable so that subsequent -// processes created with the option to inherit handles -// will also keep them open. +// Pass this handle as stdin so the standard library manages inheritance. +// GC does not read stdin; it uses it only to carry the deletion handle. // - Run the gc exe, which waits for the original rustup.exe // process to close, then deletes CARGO_HOME. This process // has inherited a FILE_FLAG_DELETE_ON_CLOSE handle to itself. -// - Finally, spawn yet another system binary with the inherit handles -// flag, so *it* inherits the FILE_FLAG_DELETE_ON_CLOSE handle to +// - Finally, spawn yet another system binary inheriting stdin, +// so *it* inherits the FILE_FLAG_DELETE_ON_CLOSE handle to // the gc exe. If the gc exe exits before the system exe then at // last it will be deleted when the handle closes. // @@ -711,15 +708,10 @@ pub(crate) fn self_replace(process: &Process) -> anyhow::Result // .. augmented with this SO answer // https://stackoverflow.com/questions/10319526/understanding-a-self-deleting-program-in-c pub(crate) fn spawn_uninstall_gc(no_modify_path: bool, process: &Process) -> anyhow::Result<()> { - use std::{io, ptr, thread, time::Duration}; + use std::{fs::OpenOptions, os::windows::fs::OpenOptionsExt, thread, time::Duration}; - use windows_sys::Win32::{ - Foundation::{CloseHandle, GENERIC_READ, INVALID_HANDLE_VALUE}, - Security::SECURITY_ATTRIBUTES, - Storage::FileSystem::{ - CreateFileW, FILE_FLAG_DELETE_ON_CLOSE, FILE_SHARE_DELETE, FILE_SHARE_READ, - OPEN_EXISTING, - }, + use windows_sys::Win32::Storage::FileSystem::{ + FILE_FLAG_DELETE_ON_CLOSE, FILE_SHARE_DELETE, FILE_SHARE_READ, }; // CARGO_HOME, hopefully empty except for bin/rustup.exe @@ -738,39 +730,20 @@ pub(crate) fn spawn_uninstall_gc(no_modify_path: bool, process: &Process) -> any let gc_exe = work_path.join(format!("rustup-gc-{numbah:x}.exe")); // Copy rustup (probably this process's exe) to the gc exe utils::copy_file_symlink_to_source(&rustup_path, &gc_exe)?; - let gc_exe_win: Vec<_> = gc_exe.as_os_str().encode_wide().chain(Some(0)).collect(); - - // Make the sub-process opened by gc exe inherit its attribute. - let sa = SECURITY_ATTRIBUTES { - nLength: size_of::() as u32, - lpSecurityDescriptor: ptr::null_mut(), - bInheritHandle: 1, - }; - - let _g = unsafe { - // Open an inheritable handle to the gc exe marked - // FILE_FLAG_DELETE_ON_CLOSE. - let gc_handle = CreateFileW( - gc_exe_win.as_ptr(), - GENERIC_READ, - FILE_SHARE_READ | FILE_SHARE_DELETE, - &sa, - OPEN_EXISTING, - FILE_FLAG_DELETE_ON_CLOSE, - ptr::null_mut(), - ); - - if gc_handle == INVALID_HANDLE_VALUE { - let err = io::Error::last_os_error(); - return Err(err).context(CliError::WindowsUninstallMadness); - } - - scopeguard::guard(gc_handle, |h| { - let _ = CloseHandle(h); - }) - }; + // OpenOptions preserves the read, sharing and delete-on-close flags while + // letting File own the handle until it is passed to Command below. + let gc_handle = OpenOptions::new() + .read(true) + .share_mode(FILE_SHARE_READ | FILE_SHARE_DELETE) + .custom_flags(FILE_FLAG_DELETE_ON_CLOSE) + .open(&gc_exe) + .context(CliError::WindowsUninstallMadness)?; - Command::new(gc_exe) + // Pass the file as GC stdin so the standard library manages inheritance. + // Command retains the parent handle after spawn; keep it alive through the sleep. + let mut command = Command::new(gc_exe); + command + .stdin(gc_handle) .env(GC_MODIFY_PATH, if no_modify_path { "0" } else { "1" }) .spawn() .context(CliError::WindowsUninstallMadness)?; From 45817dc8f9b1bca1c6e8caaf2f36d358a6511032 Mon Sep 17 00:00:00 2001 From: Cloud0310 <60375730+Cloud0310@users.noreply.github.com> Date: Wed, 16 Sep 2026 22:57:35 +0800 Subject: [PATCH 2/7] refactor(test): inline the uninstall GC cleanup check Inline the single-use ensure_empty helper and replace GcErr with an inline error. Preserve the existing directory and GC filename filter. --- tests/suite/cli_self_upd.rs | 46 +++++++++++++++---------------------- 1 file changed, 19 insertions(+), 27 deletions(-) diff --git a/tests/suite/cli_self_upd.rs b/tests/suite/cli_self_upd.rs index 83ca66b671..827fa19ff8 100644 --- a/tests/suite/cli_self_upd.rs +++ b/tests/suite/cli_self_upd.rs @@ -395,39 +395,31 @@ async fn uninstall_doesnt_leave_gc_file() { // 100ms, but during the contention of test suites can be substantially // longer while still succeeding. - let check = || ensure_empty(parent); + let check = || { + let garbage = fs::read_dir(parent) + .unwrap() + .filter_map(|entry| { + let path = entry.unwrap().path(); + let name = path.file_name()?.to_str()?; + // On Windows, this binary is cleaned up on exit + if !(name.starts_with("rustup-gc-") && name.ends_with(EXE_SUFFIX)) { + return None; + } + Some(path.to_string_lossy().to_string()) + }) + .collect::>(); + if garbage.is_empty() { + Ok(()) + } else { + Err(format!("garbage remaining: {garbage:?}")) + } + }; match retry(Fibonacci::from_millis(1).map(jitter).take(23), check) { Ok(_) => (), Err(e) => panic!("{e}"), } } -#[cfg(windows)] -fn ensure_empty(dir: &Path) -> Result<(), GcErr> { - let garbage = fs::read_dir(dir) - .unwrap() - .filter_map(|entry| { - let path = entry.unwrap().path(); - let name = path.file_name()?.to_str()?; - // On Windows, this binary is cleaned up on exit - if !(name.starts_with("rustup-gc-") && name.ends_with(EXE_SUFFIX)) { - return None; - } - Some(path.to_string_lossy().to_string()) - }) - .collect::>(); - if garbage.is_empty() { - Ok(()) - } else { - Err(GcErr(garbage)) - } -} - -#[derive(thiserror::Error, Debug)] -#[error("garbage remaining: {:?}", .0)] -#[cfg(windows)] -struct GcErr(Vec); - #[tokio::test] async fn update_exact() { let cx = SelfUpdateTestContext::new(TEST_VERSION).await; From 17c981f7e2b32d233eb4ea27540bbdeb662aac7f Mon Sep 17 00:00:00 2001 From: Cloud0310 <60375730+Cloud0310@users.noreply.github.com> Date: Thu, 17 Sep 2026 00:49:12 +0800 Subject: [PATCH 3/7] fix(windows): attempt GC cleanup after uninstall errors Attempt the existing GC self-cleanup even if waiting for the parent or removing cargo-home state fails. Preserve the original uninstall error when starting cleanup also fails; report the cleanup error when uninstalling succeeded. --- src/cli/self_update/windows.rs | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/cli/self_update/windows.rs b/src/cli/self_update/windows.rs index 29e7d9e3cb..a37b49a8cb 100644 --- a/src/cli/self_update/windows.rs +++ b/src/cli/self_update/windows.rs @@ -359,24 +359,27 @@ fn has_windows_sdk_libs(process: &Process) -> bool { pub fn complete_windows_uninstall(process: &Process) -> anyhow::Result { use std::process::Stdio; - wait_for_parent()?; - - let no_modify_path = process.var_os(GC_MODIFY_PATH).as_deref() != Some(OsStr::new("1")); + let uninstall = wait_for_parent().and_then(|()| { + let no_modify_path = process.var_os(GC_MODIFY_PATH).as_deref() != Some(OsStr::new("1")); - // Now that the parent has exited there are hopefully no more files open in CARGO_HOME. - super::clean_cargo_home(no_modify_path, process)?; + // Now that the parent has exited there are hopefully no more files open in CARGO_HOME. + super::clean_cargo_home(no_modify_path, process) + }); // Now, run a *system* binary to inherit the DELETE_ON_CLOSE // handle to *this* process, then exit. The OS will delete the gc - // exe when it exits. + // exe when it exits. Do this even if uninstalling failed. // Leave stdin inherited so the standard library passes GC's delete-on-close // handle to the cleanup child without raw handle APIs. - Command::new("net") + let cleanup = Command::new("net") .stdout(Stdio::null()) .stderr(Stdio::null()) .spawn() - .context(CliError::WindowsUninstallMadness)?; + .context(CliError::WindowsUninstallMadness); + // Preserve the original uninstall error if starting cleanup also failed. + uninstall?; + cleanup?; Ok(utils::ExitCode(0)) } From 27b619880f99b5aaeb73d4670437a7a45d31e30f Mon Sep 17 00:00:00 2001 From: Cloud0310 <60375730+Cloud0310@users.noreply.github.com> Date: Wed, 16 Sep 2026 23:02:07 +0800 Subject: [PATCH 4/7] refactor(windows): manage the uninstall GC path with tempfile Use tempfile to manage the GC path until the delete-on-close handle takes over cleanup. --- src/cli/self_update/windows.rs | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/src/cli/self_update/windows.rs b/src/cli/self_update/windows.rs index a37b49a8cb..930ef338da 100644 --- a/src/cli/self_update/windows.rs +++ b/src/cli/self_update/windows.rs @@ -711,7 +711,7 @@ pub(crate) fn self_replace(process: &Process) -> anyhow::Result // .. augmented with this SO answer // https://stackoverflow.com/questions/10319526/understanding-a-self-deleting-program-in-c pub(crate) fn spawn_uninstall_gc(no_modify_path: bool, process: &Process) -> anyhow::Result<()> { - use std::{fs::OpenOptions, os::windows::fs::OpenOptionsExt, thread, time::Duration}; + use std::{fs::OpenOptions, io, os::windows::fs::OpenOptionsExt, thread, time::Duration}; use windows_sys::Win32::Storage::FileSystem::{ FILE_FLAG_DELETE_ON_CLOSE, FILE_SHARE_DELETE, FILE_SHARE_READ, @@ -727,12 +727,14 @@ pub(crate) fn spawn_uninstall_gc(no_modify_path: bool, process: &Process) -> any .parent() .expect("CARGO_HOME doesn't have a parent?"); - // Generate a unique name for the files we're about to move out - // of CARGO_HOME. - let numbah: u32 = rand::random(); - let gc_exe = work_path.join(format!("rustup-gc-{numbah:x}.exe")); - // Copy rustup (probably this process's exe) to the gc exe - utils::copy_file_symlink_to_source(&rustup_path, &gc_exe)?; + let gc_exe = tempfile::Builder::new() + .prefix("rustup-gc-") + .suffix(".exe") + .make_in(work_path, |path| { + utils::copy_file_symlink_to_source(&rustup_path, path).map_err(io::Error::other) + }) + .context("error creating temporary GC executable")? + .into_temp_path(); // OpenOptions preserves the read, sharing and delete-on-close flags while // letting File own the handle until it is passed to Command below. let gc_handle = OpenOptions::new() @@ -742,6 +744,10 @@ pub(crate) fn spawn_uninstall_gc(no_modify_path: bool, process: &Process) -> any .open(&gc_exe) .context(CliError::WindowsUninstallMadness)?; + // Transfer cleanup to Windows only after the DELETE_ON_CLOSE handle is + // open. Until then, TempPath attempts cleanup if preparation fails. + let gc_exe = gc_exe.keep()?; + // Pass the file as GC stdin so the standard library manages inheritance. // Command retains the parent handle after spawn; keep it alive through the sleep. let mut command = Command::new(gc_exe); From 5a3bbe021ff3a0956ac5a23de02a2a9c9304ad63 Mon Sep 17 00:00:00 2001 From: Cloud0310 <60375730+Cloud0310@users.noreply.github.com> Date: Wed, 16 Sep 2026 23:02:30 +0800 Subject: [PATCH 5/7] fix(windows): copy executable contents for uninstall GC Copy the executable contents into a regular temporary file so delete-on-close removes the GC copy instead of a symlink target. Close the write handle before opening the executable for reading. --- src/cli/self_update/windows.rs | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/src/cli/self_update/windows.rs b/src/cli/self_update/windows.rs index 930ef338da..e578bbb043 100644 --- a/src/cli/self_update/windows.rs +++ b/src/cli/self_update/windows.rs @@ -711,7 +711,13 @@ pub(crate) fn self_replace(process: &Process) -> anyhow::Result // .. augmented with this SO answer // https://stackoverflow.com/questions/10319526/understanding-a-self-deleting-program-in-c pub(crate) fn spawn_uninstall_gc(no_modify_path: bool, process: &Process) -> anyhow::Result<()> { - use std::{fs::OpenOptions, io, os::windows::fs::OpenOptionsExt, thread, time::Duration}; + use std::{ + fs::{File, OpenOptions}, + io, + os::windows::fs::OpenOptionsExt, + thread, + time::Duration, + }; use windows_sys::Win32::Storage::FileSystem::{ FILE_FLAG_DELETE_ON_CLOSE, FILE_SHARE_DELETE, FILE_SHARE_READ, @@ -727,14 +733,20 @@ pub(crate) fn spawn_uninstall_gc(no_modify_path: bool, process: &Process) -> any .parent() .expect("CARGO_HOME doesn't have a parent?"); - let gc_exe = tempfile::Builder::new() + let mut source = File::open(&rustup_path) + .with_context(|| format!("could not open rustup '{}'", rustup_path.display()))?; + let mut gc_file = tempfile::Builder::new() .prefix("rustup-gc-") .suffix(".exe") - .make_in(work_path, |path| { - utils::copy_file_symlink_to_source(&rustup_path, path).map_err(io::Error::other) - }) - .context("error creating temporary GC executable")? - .into_temp_path(); + .tempfile_in(work_path) + .context("error creating temporary GC executable")?; + // copy_file_symlink_to_source would create a link when the source is a + // symlink. io::copy writes its contents into this independent regular file, + // so DELETE_ON_CLOSE applies to the GC copy rather than the source target. + io::copy(&mut source, gc_file.as_file_mut()) + .with_context(|| format!("could not copy rustup from '{}'", rustup_path.display()))?; + // Close the write handle before opening the executable for reading. + let gc_exe = gc_file.into_temp_path(); // OpenOptions preserves the read, sharing and delete-on-close flags while // letting File own the handle until it is passed to Command below. let gc_handle = OpenOptions::new() From ad25426fd89dfecd442d8476a054d4696a36a965 Mon Sep 17 00:00:00 2001 From: Cloud0310 <60375730+Cloud0310@users.noreply.github.com> Date: Thu, 17 Sep 2026 01:15:31 +0800 Subject: [PATCH 6/7] fix(windows): use the running executable for uninstall GC Copy the running executable so GC does not depend on the installed rustup copy. --- src/cli/self_update/windows.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/cli/self_update/windows.rs b/src/cli/self_update/windows.rs index e578bbb043..3c4e60539e 100644 --- a/src/cli/self_update/windows.rs +++ b/src/cli/self_update/windows.rs @@ -1,6 +1,6 @@ use std::{ borrow::Cow, - env::{consts::EXE_SUFFIX, split_paths}, + env::split_paths, ffi::{OsStr, OsString}, fmt, io::Write, @@ -687,7 +687,7 @@ pub(crate) fn self_replace(process: &Process) -> anyhow::Result // while they are open, like when they are running. // // Here's what we're going to do: -// - Copy rustup.exe to a temporary file in +// - Copy the running rustup.exe to a temporary file in // CARGO_HOME/../rustup-gc-$random.exe. // - Open the gc exe with the FILE_FLAG_DELETE_ON_CLOSE and // FILE_SHARE_DELETE flags. This is going to be the last @@ -723,10 +723,9 @@ pub(crate) fn spawn_uninstall_gc(no_modify_path: bool, process: &Process) -> any FILE_FLAG_DELETE_ON_CLOSE, FILE_SHARE_DELETE, FILE_SHARE_READ, }; - // CARGO_HOME, hopefully empty except for bin/rustup.exe + // Copy the running executable so GC does not depend on the installed copy. + let rustup_path = utils::current_exe()?; let cargo_home = process.cargo_home()?; - // The rustup.exe bin - let rustup_path = cargo_home.join(format!("bin/rustup{EXE_SUFFIX}")); // The directory containing CARGO_HOME let work_path = cargo_home From 7194785a0ba3820ba0de5d7e6cc7f71f94af82d4 Mon Sep 17 00:00:00 2001 From: Cloud0310 <60375730+Cloud0310@users.noreply.github.com> Date: Thu, 17 Sep 2026 01:15:32 +0800 Subject: [PATCH 7/7] fix(windows): run uninstall GC from the system temporary directory Create GC in the system temporary directory to avoid requiring write access to the parent of CARGO_HOME. Update the cleanup test to use an isolated temporary directory. --- src/cli/self_update.rs | 2 +- src/cli/self_update/windows.rs | 15 +++++---------- tests/suite/cli_self_upd.rs | 22 +++++++++------------- 3 files changed, 15 insertions(+), 24 deletions(-) diff --git a/src/cli/self_update.rs b/src/cli/self_update.rs index 621dd80afe..1a4d61b859 100644 --- a/src/cli/self_update.rs +++ b/src/cli/self_update.rs @@ -992,7 +992,7 @@ pub(crate) fn uninstall( // the process exits. // see: windows::{complete_windows_uninstall,spawn_uninstall_gc} #[cfg(windows)] - windows::spawn_uninstall_gc(no_modify_path, process)?; + windows::spawn_uninstall_gc(no_modify_path)?; info!("rustup is uninstalled"); diff --git a/src/cli/self_update/windows.rs b/src/cli/self_update/windows.rs index 3c4e60539e..d2c1471606 100644 --- a/src/cli/self_update/windows.rs +++ b/src/cli/self_update/windows.rs @@ -688,7 +688,7 @@ pub(crate) fn self_replace(process: &Process) -> anyhow::Result // // Here's what we're going to do: // - Copy the running rustup.exe to a temporary file in -// CARGO_HOME/../rustup-gc-$random.exe. +// the system temporary directory as rustup-gc-$random.exe. // - Open the gc exe with the FILE_FLAG_DELETE_ON_CLOSE and // FILE_SHARE_DELETE flags. This is going to be the last // file to remove, and the OS is going to do it for us. @@ -710,7 +710,7 @@ pub(crate) fn self_replace(process: &Process) -> anyhow::Result // // .. augmented with this SO answer // https://stackoverflow.com/questions/10319526/understanding-a-self-deleting-program-in-c -pub(crate) fn spawn_uninstall_gc(no_modify_path: bool, process: &Process) -> anyhow::Result<()> { +pub(crate) fn spawn_uninstall_gc(no_modify_path: bool) -> anyhow::Result<()> { use std::{ fs::{File, OpenOptions}, io, @@ -725,19 +725,14 @@ pub(crate) fn spawn_uninstall_gc(no_modify_path: bool, process: &Process) -> any // Copy the running executable so GC does not depend on the installed copy. let rustup_path = utils::current_exe()?; - let cargo_home = process.cargo_home()?; - - // The directory containing CARGO_HOME - let work_path = cargo_home - .parent() - .expect("CARGO_HOME doesn't have a parent?"); - let mut source = File::open(&rustup_path) .with_context(|| format!("could not open rustup '{}'", rustup_path.display()))?; + // Use the system temporary directory so GC creation does not require + // write access to CARGO_HOME's parent. let mut gc_file = tempfile::Builder::new() .prefix("rustup-gc-") .suffix(".exe") - .tempfile_in(work_path) + .tempfile() .context("error creating temporary GC executable")?; // copy_file_symlink_to_source would create a link when the source is a // symlink. io::copy writes its contents into this independent regular file, diff --git a/tests/suite/cli_self_upd.rs b/tests/suite/cli_self_upd.rs index 827fa19ff8..cdd0d05ac7 100644 --- a/tests/suite/cli_self_upd.rs +++ b/tests/suite/cli_self_upd.rs @@ -380,33 +380,29 @@ async fn uninstall_self_delete_works() { } // On windows rustup self uninstall temporarily puts a rustup-gc-$randomnumber.exe -// file in CONFIG.CARGODIR/.. ; check that it doesn't exist. +// file in the system temporary directory; check that it is cleaned up. #[tokio::test] #[cfg(windows)] async fn uninstall_doesnt_leave_gc_file() { let cx = setup_empty_installed().await; + let gc_dir = tempfile::tempdir().unwrap(); + let gc_path = gc_dir.path().to_str().unwrap(); cx.config - .expect(["rustup", "self", "uninstall", "-y"]) + .expect_with_env( + ["rustup", "self", "uninstall", "-y"], + [("TMP", gc_path), ("TEMP", gc_path), ("SystemTemp", gc_path)], + ) .await .is_ok(); - let parent = cx.config.cargodir.parent().unwrap(); // The gc removal happens after rustup terminates. Typically under // 100ms, but during the contention of test suites can be substantially // longer while still succeeding. let check = || { - let garbage = fs::read_dir(parent) + let garbage = fs::read_dir(gc_dir.path()) .unwrap() - .filter_map(|entry| { - let path = entry.unwrap().path(); - let name = path.file_name()?.to_str()?; - // On Windows, this binary is cleaned up on exit - if !(name.starts_with("rustup-gc-") && name.ends_with(EXE_SUFFIX)) { - return None; - } - Some(path.to_string_lossy().to_string()) - }) + .map(|entry| entry.unwrap().path()) .collect::>(); if garbage.is_empty() { Ok(())