Skip to content

Commit 01fbc5a

Browse files
committed
Auto merge of #103459 - ChrisDenton:propagate-nulls, r=thomcc
Pass on null handle values to child process Fixes #101645 In Windows, stdio handles are (semantically speaking) `Option<Handle>` where `Handle` is a non-zero value. When spawning a process with `Stdio::Inherit`, Rust currently turns zero values into `-1` values. This has the unfortunate effect of breaking console subprocesses (which typically need stdio) that are spawned from gui applications (that lack stdio by default) because the console process won't be assigned handles from the newly created console (as they usually would in that situation). Worse, `-1` is actually [a valid handle](https://doc.rust-lang.org/std/os/windows/io/struct.OwnedHandle.html) which means "the current process". So if a console process, for example, waits on stdin and it has a `-1` value then the process will end up waiting on itself. This PR fixes it by propagating the nulls instead of converting them to `-1`. While I think the current behaviour is a mistake, changing it (however justified) is an API change so I think this PR should at least have some input from t-libs-api. So choosing at random... r? `@joshtriplett`
2 parents 91b8f34 + 93b774a commit 01fbc5a

File tree

1 file changed

+20
-14
lines changed

1 file changed

+20
-14
lines changed

library/std/src/sys/windows/process.rs

+20-14
Original file line numberDiff line numberDiff line change
@@ -252,10 +252,6 @@ impl Command {
252252
) -> io::Result<(Process, StdioPipes)> {
253253
let maybe_env = self.env.capture_if_changed();
254254

255-
let mut si = zeroed_startupinfo();
256-
si.cb = mem::size_of::<c::STARTUPINFO>() as c::DWORD;
257-
si.dwFlags = c::STARTF_USESTDHANDLES;
258-
259255
let child_paths = if let Some(env) = maybe_env.as_ref() {
260256
env.get(&EnvKey::new("PATH")).map(|s| s.as_os_str())
261257
} else {
@@ -314,9 +310,21 @@ impl Command {
314310
let stdin = stdin.to_handle(c::STD_INPUT_HANDLE, &mut pipes.stdin)?;
315311
let stdout = stdout.to_handle(c::STD_OUTPUT_HANDLE, &mut pipes.stdout)?;
316312
let stderr = stderr.to_handle(c::STD_ERROR_HANDLE, &mut pipes.stderr)?;
317-
si.hStdInput = stdin.as_raw_handle();
318-
si.hStdOutput = stdout.as_raw_handle();
319-
si.hStdError = stderr.as_raw_handle();
313+
314+
let mut si = zeroed_startupinfo();
315+
si.cb = mem::size_of::<c::STARTUPINFO>() as c::DWORD;
316+
317+
// If at least one of stdin, stdout or stderr are set (i.e. are non null)
318+
// then set the `hStd` fields in `STARTUPINFO`.
319+
// Otherwise skip this and allow the OS to apply its default behaviour.
320+
// This provides more consistent behaviour between Win7 and Win8+.
321+
let is_set = |stdio: &Handle| !stdio.as_raw_handle().is_null();
322+
if is_set(&stderr) || is_set(&stdout) || is_set(&stdin) {
323+
si.dwFlags |= c::STARTF_USESTDHANDLES;
324+
si.hStdInput = stdin.as_raw_handle();
325+
si.hStdOutput = stdout.as_raw_handle();
326+
si.hStdError = stderr.as_raw_handle();
327+
}
320328

321329
unsafe {
322330
cvt(c::CreateProcessW(
@@ -513,17 +521,15 @@ fn program_exists(path: &Path) -> Option<Vec<u16>> {
513521
impl Stdio {
514522
fn to_handle(&self, stdio_id: c::DWORD, pipe: &mut Option<AnonPipe>) -> io::Result<Handle> {
515523
match *self {
516-
// If no stdio handle is available, then inherit means that it
517-
// should still be unavailable so propagate the
518-
// INVALID_HANDLE_VALUE.
519524
Stdio::Inherit => match stdio::get_handle(stdio_id) {
520525
Ok(io) => unsafe {
521526
let io = Handle::from_raw_handle(io);
522527
let ret = io.duplicate(0, true, c::DUPLICATE_SAME_ACCESS);
523528
io.into_raw_handle();
524529
ret
525530
},
526-
Err(..) => unsafe { Ok(Handle::from_raw_handle(c::INVALID_HANDLE_VALUE)) },
531+
// If no stdio handle is available, then propagate the null value.
532+
Err(..) => unsafe { Ok(Handle::from_raw_handle(ptr::null_mut())) },
527533
},
528534

529535
Stdio::MakePipe => {
@@ -730,9 +736,9 @@ fn zeroed_startupinfo() -> c::STARTUPINFO {
730736
wShowWindow: 0,
731737
cbReserved2: 0,
732738
lpReserved2: ptr::null_mut(),
733-
hStdInput: c::INVALID_HANDLE_VALUE,
734-
hStdOutput: c::INVALID_HANDLE_VALUE,
735-
hStdError: c::INVALID_HANDLE_VALUE,
739+
hStdInput: ptr::null_mut(),
740+
hStdOutput: ptr::null_mut(),
741+
hStdError: ptr::null_mut(),
736742
}
737743
}
738744

0 commit comments

Comments
 (0)