Consider different way of inheriting handles in child processes on Windows #38227
Comments
|
This seems like a worthwhile implementation to me. We've got lots of fallbacks all over the place to support older OSes, but supporting XP isn't required so I'd also be fine with a patch to just switch to this. This would also bring Windows in line with Unix, which essentially is already using the equivalent of this. |
Mark-Simulacrum
added
the
C-feature-request
|
What is the API vision here? Does |
|
Ideally it would know which of the stdio handles it would have to pass down by itself, but would also provide an API for the programmer to ask for additional handles to be inherited. |
rustbot
added
the
T-libs-api
Summary: When test runner waits for `buck2` process it hangs because buckd process is still running. This happens because buckd inherits handles from parent process and wait on the parent process awaits for these handles to be closed. Example: https://stackoverflow.com/questions/13243807/popen-waiting-for-child-process-even-when-the-immediate-child-has-terminated `Popen` in Python has `close_fds` argument to solve this: > On Windows, if close_fds is true then no handles will be inherited by the child process unless explicitly passed in the handle_list element of STARTUPINFO.lpAttributeList, or by standard handle redirection. https://docs.python.org/3/library/subprocess.html Rust sets `bInheritHandles` to `TRUE` unconditionally: https://github.com/rust-lang/rust/blob/acb5c16fa8acf7fd3b48fc218881f006577bab1a/library/std/src/sys/windows/process.rs#L327 There are several open issues related to this: - rust-lang/rust#54760 - rust-lang/rust#38227 Because of that we have to call `CreateProcessW` ourselves. Implementation is inspired by: - std library: https://github.com/rust-lang/rust/blob/master/library/std/src/sys/windows/process.rs - subprocess library: https://github.com/hniksic/rust-subprocess/blob/master/src/win32.rs - docs for `CreateProcessW`: https://docs.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessw Reviewed By: stepancheg Differential Revision: D36663797 fbshipit-source-id: 14431002a763042f9c160b8a7ff7f8a62dd6befa
No, Unixoid semantics are worse: At least |
Currently to avoid handles being inherited incorrectly by child processes due to race conditions, Rust wraps the whole blob of code in a mutex to synchronize it. However, it will still accidentally inherit any handles created elsewhere that are inheritable, can cause race conditions with people creating processes using libraries other than std, and isn't very efficient.
By switching from
STARTUPINFOtoSTARTUPINFOEX, we can pass a value forlpAttributeListand that attribute list can specify an explicit list of handles to inherit. For more information on this, see https://blogs.msdn.microsoft.com/oldnewthing/20111216-00/?p=8873Unfortunately this does require vista, so XP users will lose out.
The text was updated successfully, but these errors were encountered: