Skip to content

Commit 5a4af4e

Browse files
committed
Don't kill child processes on normal exit on Windows
Fixes #5598
1 parent 5a13061 commit 5a4af4e

File tree

1 file changed

+3
-125
lines changed

1 file changed

+3
-125
lines changed

src/cargo/util/job.rs

Lines changed: 3 additions & 125 deletions
Original file line numberDiff line numberDiff line change
@@ -121,24 +121,10 @@ mod imp {
121121

122122
impl Drop for Setup {
123123
fn drop(&mut self) {
124-
// This is a litte subtle. By default if we are terminated then all
125-
// processes in our job object are terminated as well, but we
126-
// intentionally want to whitelist some processes to outlive our job
127-
// object (see below).
128-
//
129-
// To allow for this, we manually kill processes instead of letting
130-
// the job object kill them for us. We do this in a loop to handle
131-
// processes spawning other processes.
132-
//
133-
// Finally once this is all done we know that the only remaining
134-
// ones are ourselves and the whitelisted processes. The destructor
135-
// here then configures our job object to *not* kill everything on
136-
// close, then closes the job object.
124+
// On normal exits (not ctrl-c), we don't want to kill any child
125+
// processes. The destructor here configures our job object to
126+
// *not* kill everything on close, then closes the job object.
137127
unsafe {
138-
while self.kill_remaining() {
139-
info!("killed some, going for more");
140-
}
141-
142128
let mut info: JOBOBJECT_EXTENDED_LIMIT_INFORMATION;
143129
info = mem::zeroed();
144130
let r = SetInformationJobObject(
@@ -154,114 +140,6 @@ mod imp {
154140
}
155141
}
156142

157-
impl Setup {
158-
unsafe fn kill_remaining(&mut self) -> bool {
159-
#[repr(C)]
160-
struct Jobs {
161-
header: JOBOBJECT_BASIC_PROCESS_ID_LIST,
162-
list: [ULONG_PTR; 1024],
163-
}
164-
165-
let mut jobs: Jobs = mem::zeroed();
166-
let r = QueryInformationJobObject(
167-
self.job.inner,
168-
JobObjectBasicProcessIdList,
169-
&mut jobs as *mut _ as LPVOID,
170-
mem::size_of_val(&jobs) as DWORD,
171-
ptr::null_mut(),
172-
);
173-
if r == 0 {
174-
info!("failed to query job object: {}", last_err());
175-
return false;
176-
}
177-
178-
let mut killed = false;
179-
let list = &jobs.list[..jobs.header.NumberOfProcessIdsInList as usize];
180-
assert!(!list.is_empty());
181-
info!("found {} remaining processes", list.len() - 1);
182-
183-
let list = list.iter()
184-
.filter(|&&id| {
185-
// let's not kill ourselves
186-
id as DWORD != GetCurrentProcessId()
187-
})
188-
.filter_map(|&id| {
189-
// Open the process with the necessary rights, and if this
190-
// fails then we probably raced with the process exiting so we
191-
// ignore the problem.
192-
let flags = PROCESS_QUERY_INFORMATION | PROCESS_TERMINATE | SYNCHRONIZE;
193-
let p = OpenProcess(flags, FALSE, id as DWORD);
194-
if p.is_null() {
195-
None
196-
} else {
197-
Some(Handle { inner: p })
198-
}
199-
})
200-
.filter(|p| {
201-
// Test if this process was actually in the job object or not.
202-
// If it's not then we likely raced with something else
203-
// recycling this PID, so we just skip this step.
204-
let mut res = 0;
205-
let r = IsProcessInJob(p.inner, self.job.inner, &mut res);
206-
if r == 0 {
207-
info!("failed to test is process in job: {}", last_err());
208-
return false;
209-
}
210-
res == TRUE
211-
});
212-
213-
for p in list {
214-
// Load the file which this process was spawned from. We then
215-
// later use this for identification purposes.
216-
let mut buf = [0; 1024];
217-
let r = GetProcessImageFileNameW(p.inner, buf.as_mut_ptr(), buf.len() as DWORD);
218-
if r == 0 {
219-
info!("failed to get image name: {}", last_err());
220-
continue;
221-
}
222-
let s = OsString::from_wide(&buf[..r as usize]);
223-
info!("found remaining: {:?}", s);
224-
225-
// And here's where we find the whole purpose for this
226-
// function! Currently, our only whitelisted process is
227-
// `mspdbsrv.exe`, and more details about that can be found
228-
// here:
229-
//
230-
// https://github.com/rust-lang/rust/issues/33145
231-
//
232-
// The gist of it is that all builds on one machine use the
233-
// same `mspdbsrv.exe` instance. If we were to kill this
234-
// instance then we could erroneously cause other builds to
235-
// fail.
236-
if let Some(s) = s.to_str() {
237-
if s.contains("mspdbsrv") {
238-
info!("\toops, this is mspdbsrv");
239-
continue;
240-
}
241-
}
242-
243-
// Ok, this isn't mspdbsrv, let's kill the process. After we
244-
// kill it we wait on it to ensure that the next time around in
245-
// this function we're not going to see it again.
246-
let r = TerminateProcess(p.inner, 1);
247-
if r == 0 {
248-
info!("\tfailed to kill subprocess: {}", last_err());
249-
info!("\tassuming subprocess is dead...");
250-
} else {
251-
info!("\tterminated subprocess");
252-
}
253-
let r = WaitForSingleObject(p.inner, INFINITE);
254-
if r != 0 {
255-
info!("failed to wait for process to die: {}", last_err());
256-
return false;
257-
}
258-
killed = true;
259-
}
260-
261-
killed
262-
}
263-
}
264-
265143
impl Drop for Handle {
266144
fn drop(&mut self) {
267145
unsafe {

0 commit comments

Comments
 (0)