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/3] 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/3] 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/3] 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)) }