From 5a13061199f3c9aeea8cd4d2ded286cfcd536854 Mon Sep 17 00:00:00 2001 From: Brian Bowman Date: Mon, 13 Aug 2018 15:36:20 -0500 Subject: [PATCH 1/3] Fix typo --- src/cargo/util/job.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cargo/util/job.rs b/src/cargo/util/job.rs index fb99e7c0e4f..c8bfc780aa7 100644 --- a/src/cargo/util/job.rs +++ b/src/cargo/util/job.rs @@ -93,7 +93,7 @@ mod imp { // Indicate that when all handles to the job object are gone that all // process in the object should be killed. Note that this includes our - // entire process tree by default because we've added ourselves and and + // entire process tree by default because we've added ourselves and // our children will reside in the job once we spawn a process. let mut info: JOBOBJECT_EXTENDED_LIMIT_INFORMATION; info = mem::zeroed(); From 5a4af4ef198ba6b44d6fcd626750923f6f5cfd44 Mon Sep 17 00:00:00 2001 From: Brian Bowman Date: Mon, 13 Aug 2018 15:47:20 -0500 Subject: [PATCH 2/3] Don't kill child processes on normal exit on Windows Fixes #5598 --- src/cargo/util/job.rs | 128 +----------------------------------------- 1 file changed, 3 insertions(+), 125 deletions(-) diff --git a/src/cargo/util/job.rs b/src/cargo/util/job.rs index c8bfc780aa7..8462fa80ef9 100644 --- a/src/cargo/util/job.rs +++ b/src/cargo/util/job.rs @@ -121,24 +121,10 @@ mod imp { impl Drop for Setup { fn drop(&mut self) { - // This is a litte subtle. By default if we are terminated then all - // processes in our job object are terminated as well, but we - // intentionally want to whitelist some processes to outlive our job - // object (see below). - // - // To allow for this, we manually kill processes instead of letting - // the job object kill them for us. We do this in a loop to handle - // processes spawning other processes. - // - // Finally once this is all done we know that the only remaining - // ones are ourselves and the whitelisted processes. The destructor - // here then configures our job object to *not* kill everything on - // close, then closes the job object. + // On normal exits (not ctrl-c), we don't want to kill any child + // processes. The destructor here configures our job object to + // *not* kill everything on close, then closes the job object. unsafe { - while self.kill_remaining() { - info!("killed some, going for more"); - } - let mut info: JOBOBJECT_EXTENDED_LIMIT_INFORMATION; info = mem::zeroed(); let r = SetInformationJobObject( @@ -154,114 +140,6 @@ mod imp { } } - impl Setup { - unsafe fn kill_remaining(&mut self) -> bool { - #[repr(C)] - struct Jobs { - header: JOBOBJECT_BASIC_PROCESS_ID_LIST, - list: [ULONG_PTR; 1024], - } - - let mut jobs: Jobs = mem::zeroed(); - let r = QueryInformationJobObject( - self.job.inner, - JobObjectBasicProcessIdList, - &mut jobs as *mut _ as LPVOID, - mem::size_of_val(&jobs) as DWORD, - ptr::null_mut(), - ); - if r == 0 { - info!("failed to query job object: {}", last_err()); - return false; - } - - let mut killed = false; - let list = &jobs.list[..jobs.header.NumberOfProcessIdsInList as usize]; - assert!(!list.is_empty()); - info!("found {} remaining processes", list.len() - 1); - - let list = list.iter() - .filter(|&&id| { - // let's not kill ourselves - id as DWORD != GetCurrentProcessId() - }) - .filter_map(|&id| { - // Open the process with the necessary rights, and if this - // fails then we probably raced with the process exiting so we - // ignore the problem. - let flags = PROCESS_QUERY_INFORMATION | PROCESS_TERMINATE | SYNCHRONIZE; - let p = OpenProcess(flags, FALSE, id as DWORD); - if p.is_null() { - None - } else { - Some(Handle { inner: p }) - } - }) - .filter(|p| { - // Test if this process was actually in the job object or not. - // If it's not then we likely raced with something else - // recycling this PID, so we just skip this step. - let mut res = 0; - let r = IsProcessInJob(p.inner, self.job.inner, &mut res); - if r == 0 { - info!("failed to test is process in job: {}", last_err()); - return false; - } - res == TRUE - }); - - for p in list { - // Load the file which this process was spawned from. We then - // later use this for identification purposes. - let mut buf = [0; 1024]; - let r = GetProcessImageFileNameW(p.inner, buf.as_mut_ptr(), buf.len() as DWORD); - if r == 0 { - info!("failed to get image name: {}", last_err()); - continue; - } - let s = OsString::from_wide(&buf[..r as usize]); - info!("found remaining: {:?}", s); - - // And here's where we find the whole purpose for this - // function! Currently, our only whitelisted process is - // `mspdbsrv.exe`, and more details about that can be found - // here: - // - // https://github.com/rust-lang/rust/issues/33145 - // - // The gist of it is that all builds on one machine use the - // same `mspdbsrv.exe` instance. If we were to kill this - // instance then we could erroneously cause other builds to - // fail. - if let Some(s) = s.to_str() { - if s.contains("mspdbsrv") { - info!("\toops, this is mspdbsrv"); - continue; - } - } - - // Ok, this isn't mspdbsrv, let's kill the process. After we - // kill it we wait on it to ensure that the next time around in - // this function we're not going to see it again. - let r = TerminateProcess(p.inner, 1); - if r == 0 { - info!("\tfailed to kill subprocess: {}", last_err()); - info!("\tassuming subprocess is dead..."); - } else { - info!("\tterminated subprocess"); - } - let r = WaitForSingleObject(p.inner, INFINITE); - if r != 0 { - info!("failed to wait for process to die: {}", last_err()); - return false; - } - killed = true; - } - - killed - } - } - impl Drop for Handle { fn drop(&mut self) { unsafe { From 4a7a946cc7738f8e3353bc59fca459c72946c100 Mon Sep 17 00:00:00 2001 From: Brian Bowman Date: Mon, 13 Aug 2018 16:47:40 -0500 Subject: [PATCH 3/3] Remove unused imports --- src/cargo/util/job.rs | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/cargo/util/job.rs b/src/cargo/util/job.rs index 8462fa80ef9..44c61f0ca92 100644 --- a/src/cargo/util/job.rs +++ b/src/cargo/util/job.rs @@ -44,22 +44,14 @@ mod imp { mod imp { extern crate winapi; - use std::ffi::OsString; use std::io; use std::mem; - use std::os::windows::prelude::*; use std::ptr; - use self::winapi::shared::basetsd::*; use self::winapi::shared::minwindef::*; - use self::winapi::shared::minwindef::{FALSE, TRUE}; use self::winapi::um::handleapi::*; use self::winapi::um::jobapi2::*; - use self::winapi::um::jobapi::*; use self::winapi::um::processthreadsapi::*; - use self::winapi::um::psapi::*; - use self::winapi::um::synchapi::*; - use self::winapi::um::winbase::*; use self::winapi::um::winnt::*; use self::winapi::um::winnt::HANDLE;