Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Don't kill child processes on normal exit on Windows #5887

Merged
merged 4 commits into from
Aug 14, 2018
Merged

Don't kill child processes on normal exit on Windows #5887

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5a13061
Fix typo
Seeker14491 Aug 13, 2018
5a4af4e
Don't kill child processes on normal exit on Windows
Seeker14491 Aug 13, 2018
54d8303
Merge remote-tracking branch 'origin/master' into job-fix
Seeker14491 Aug 13, 2018
4a7a946
Remove unused imports
Seeker14491 Aug 13, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
138 changes: 4 additions & 134 deletions src/cargo/util/job.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -93,7 +85,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();
Expand Down Expand Up @@ -121,24 +113,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(
Expand All @@ -154,114 +132,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 {
Expand Down