bjoernager/rust
bjoernager/rust
1
Fork 0
Code Activity

Rollup merge of #136029 - ChrisDenton:py-job, r=jieyouxu

Browse source 
Bootstrap: Don't move ownership of job object

I've been thinking about this since the last time I looked at bootstrap's use of job objects. We currently pass ownership of the job object to Python. I feel this is unneeded complexity.

The rationale given (in a comment) is that it helps with `ctrl-c` on `x.py`. But using `ctrl-c` when running `x.py` will also cause `bootstrap.exe` to immediately exit so I don't find that convincing.
This commit is contained in:
Matthias Krüger 2025-01-25 08:03:38 +01:00 • committed by GitHub
commit 7b16b4e4cb
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 4 additions and 50 deletions
Download patch file Download diff file Expand all files Collapse all files

3
src/bootstrap/bootstrap.py
View file

@ -1310,9 +1310,6 @@ def bootstrap(args):
args = [build.bootstrap_binary()]
args.extend(sys.argv[1:])
env = os.environ.copy()
# The Python process ID is used when creating a Windows job object
# (see src\bootstrap\src\utils\job.rs)
env["BOOTSTRAP_PARENT_ID"] = str(os.getpid())
env["BOOTSTRAP_PYTHON"] = sys.executable
run(args, env=env, verbose=build.verbose, is_bootstrap=True)

51
src/bootstrap/src/utils/job.rs
View file

@ -42,9 +42,9 @@ pub unsafe fn setup(build: &mut crate::Build) {
#[cfg(windows)]
mod for_windows {
use std::ffi::c_void;
use std::{env, io, mem};
use std::{io, mem};
use windows::Win32::Foundation::{CloseHandle, DUPLICATE_SAME_ACCESS, DuplicateHandle, HANDLE};
use windows::Win32::Foundation::CloseHandle;
use windows::Win32::System::Diagnostics::Debug::{
SEM_NOGPFAULTERRORBOX, SetErrorMode, THREAD_ERROR_MODE,
};
@ -53,9 +53,7 @@ mod for_windows {
JOB_OBJECT_LIMIT_PRIORITY_CLASS, JOBOBJECT_EXTENDED_LIMIT_INFORMATION,
JobObjectExtendedLimitInformation, SetInformationJobObject,
};
use windows::Win32::System::Threading::{
BELOW_NORMAL_PRIORITY_CLASS, GetCurrentProcess, OpenProcess, PROCESS_DUP_HANDLE,
};
use windows::Win32::System::Threading::{BELOW_NORMAL_PRIORITY_CLASS, GetCurrentProcess};
use windows::core::PCWSTR;
use crate::Build;
@ -95,49 +93,8 @@ mod for_windows {
return;
}
// If we've got a parent process (e.g., the python script that called us)
// then move ownership of this job object up to them. That way if the python
// script is killed (e.g., via ctrl-c) then we'll all be torn down.
//
// If we don't have a parent (e.g., this was run directly) then we
// intentionally leak the job object handle. When our process exits
// Note: we intentionally leak the job object handle. When our process exits
// (normally or abnormally) it will close the handle implicitly, causing all
// processes in the job to be cleaned up.
let pid = match env::var("BOOTSTRAP_PARENT_ID") {
Ok(s) => s,
Err(..) => return,
};
let parent = match OpenProcess(PROCESS_DUP_HANDLE, false, pid.parse().unwrap()).ok() {
Some(parent) => parent,
_ => {
// If we get a null parent pointer here, it is possible that either
// we have an invalid pid or the parent process has been closed.
// Since the first case rarely happens
// (only when wrongly setting the environmental variable),
// it might be better to improve the experience of the second case
// when users have interrupted the parent process and we haven't finish
// duplicating the handle yet.
return;
}
};
let mut parent_handle = HANDLE::default();
// If this fails, well at least we tried! An example of DuplicateHandle
// failing in the past has been when the wrong python2 package spawned this
// build system (e.g., the `python2` package in MSYS instead of
// `mingw-w64-x86_64-python2`). Not sure why it failed, but the "failure
// mode" here is that we only clean everything up when the build system
// dies, not when the python parent does, so not too bad.
let _ = DuplicateHandle(
GetCurrentProcess(),
job,
parent,
&mut parent_handle,
0,
false,
DUPLICATE_SAME_ACCESS,
);
CloseHandle(parent).ok();
}
}