From ee02f01ea6caa0dd56393012aa84ca4d09549702 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Fri, 21 Jan 2022 12:43:07 -0800 Subject: [PATCH 1/3] Consistently present absent stdio handles on Windows as NULL handles. This addresses #90964 by making the std API consistent about presenting absent stdio handles on Windows as NULL handles. Stdio handles may be absent due to `#![windows_subsystem = "windows"]`, due to the console being detached, or due to a child process having been launched from a parent where stdio handles are absent. Specifically, this fixes the case of child processes of parents with absent stdio, which previously ended up with `stdin().as_raw_handle()` returning `INVALID_HANDLE_VALUE`, which was surprising, and which overlapped with an unrelated valid handle value. With this patch, `stdin().as_raw_handle()` now returns null in these situation, which is consistent with what it does in the parent process. And, document this in the "Windows Portability Considerations" sections of the relevant documentation. --- library/std/src/io/stdio.rs | 79 ++++++++++++++++++++++--- library/std/src/os/windows/io/handle.rs | 12 +++- library/std/src/os/windows/io/raw.rs | 25 ++++++-- library/std/src/sys/windows/process.rs | 13 ++++ 4 files changed, 113 insertions(+), 16 deletions(-) diff --git a/library/std/src/io/stdio.rs b/library/std/src/io/stdio.rs index 5414ff648d4d5..5bb70ae472b33 100644 --- a/library/std/src/io/stdio.rs +++ b/library/std/src/io/stdio.rs @@ -202,12 +202,18 @@ fn handle_ebadf(r: io::Result, default: T) -> io::Result { /// /// [`io::stdin`]: stdin /// -/// ### Note: Windows Portability Consideration +/// ### Note: Windows Portability Considerations /// /// When operating in a console, the Windows implementation of this stream does not support /// non-UTF-8 byte sequences. Attempting to read bytes that are not valid UTF-8 will return /// an error. /// +/// In a process with a detached console, such as one using +/// `#![windows_subsystem = "windows"]`, or in a child process spawned from such a process, +/// the contained handle will be null. In such cases, the standard library's `Read` and +/// `Write` will do nothing and silently succeed. All other I/O operations, via the +/// standard library or via raw Windows API calls, will fail. +/// /// # Examples /// /// ```no_run @@ -230,12 +236,18 @@ pub struct Stdin { /// This handle implements both the [`Read`] and [`BufRead`] traits, and /// is constructed via the [`Stdin::lock`] method. /// -/// ### Note: Windows Portability Consideration +/// ### Note: Windows Portability Considerations /// /// When operating in a console, the Windows implementation of this stream does not support /// non-UTF-8 byte sequences. Attempting to read bytes that are not valid UTF-8 will return /// an error. /// +/// In a process with a detached console, such as one using +/// `#![windows_subsystem = "windows"]`, or in a child process spawned from such a process, +/// the contained handle will be null. In such cases, the standard library's `Read` and +/// `Write` will do nothing and silently succeed. All other I/O operations, via the +/// standard library or via raw Windows API calls, will fail. +/// /// # Examples /// /// ```no_run @@ -263,11 +275,18 @@ pub struct StdinLock<'a> { /// is synchronized via a mutex. If you need more explicit control over /// locking, see the [`Stdin::lock`] method. /// -/// ### Note: Windows Portability Consideration +/// ### Note: Windows Portability Considerations +/// /// When operating in a console, the Windows implementation of this stream does not support /// non-UTF-8 byte sequences. Attempting to read bytes that are not valid UTF-8 will return /// an error. /// +/// In a process with a detached console, such as one using +/// `#![windows_subsystem = "windows"]`, or in a child process spawned from such a process, +/// the contained handle will be null. In such cases, the standard library's `Read` and +/// `Write` will do nothing and silently succeed. All other I/O operations, via the +/// standard library or via raw Windows API calls, will fail. +/// /// # Examples /// /// Using implicit synchronization: @@ -490,11 +509,18 @@ impl fmt::Debug for StdinLock<'_> { /// /// Created by the [`io::stdout`] method. /// -/// ### Note: Windows Portability Consideration +/// ### Note: Windows Portability Considerations +/// /// When operating in a console, the Windows implementation of this stream does not support /// non-UTF-8 byte sequences. Attempting to write bytes that are not valid UTF-8 will return /// an error. /// +/// In a process with a detached console, such as one using +/// `#![windows_subsystem = "windows"]`, or in a child process spawned from such a process, +/// the contained handle will be null. In such cases, the standard library's `Read` and +/// `Write` will do nothing and silently succeed. All other I/O operations, via the +/// standard library or via raw Windows API calls, will fail. +/// /// [`lock`]: Stdout::lock /// [`io::stdout`]: stdout #[stable(feature = "rust1", since = "1.0.0")] @@ -510,10 +536,17 @@ pub struct Stdout { /// This handle implements the [`Write`] trait, and is constructed via /// the [`Stdout::lock`] method. See its documentation for more. /// -/// ### Note: Windows Portability Consideration +/// ### Note: Windows Portability Considerations +/// /// When operating in a console, the Windows implementation of this stream does not support /// non-UTF-8 byte sequences. Attempting to write bytes that are not valid UTF-8 will return /// an error. +/// +/// In a process with a detached console, such as one using +/// `#![windows_subsystem = "windows"]`, or in a child process spawned from such a process, +/// the contained handle will be null. In such cases, the standard library's `Read` and +/// `Write` will do nothing and silently succeed. All other I/O operations, via the +/// standard library or via raw Windows API calls, will fail. #[must_use = "if unused stdout will immediately unlock"] #[stable(feature = "rust1", since = "1.0.0")] pub struct StdoutLock<'a> { @@ -528,11 +561,18 @@ static STDOUT: SyncOnceCell>>> = Sy /// is synchronized via a mutex. If you need more explicit control over /// locking, see the [`Stdout::lock`] method. /// -/// ### Note: Windows Portability Consideration +/// ### Note: Windows Portability Considerations +/// /// When operating in a console, the Windows implementation of this stream does not support /// non-UTF-8 byte sequences. Attempting to write bytes that are not valid UTF-8 will return /// an error. /// +/// In a process with a detached console, such as one using +/// `#![windows_subsystem = "windows"]`, or in a child process spawned from such a process, +/// the contained handle will be null. In such cases, the standard library's `Read` and +/// `Write` will do nothing and silently succeed. All other I/O operations, via the +/// standard library or via raw Windows API calls, will fail. +/// /// # Examples /// /// Using implicit synchronization: @@ -710,10 +750,17 @@ impl fmt::Debug for StdoutLock<'_> { /// /// [`io::stderr`]: stderr /// -/// ### Note: Windows Portability Consideration +/// ### Note: Windows Portability Considerations +/// /// When operating in a console, the Windows implementation of this stream does not support /// non-UTF-8 byte sequences. Attempting to write bytes that are not valid UTF-8 will return /// an error. +/// +/// In a process with a detached console, such as one using +/// `#![windows_subsystem = "windows"]`, or in a child process spawned from such a process, +/// the contained handle will be null. In such cases, the standard library's `Read` and +/// `Write` will do nothing and silently succeed. All other I/O operations, via the +/// standard library or via raw Windows API calls, will fail. #[stable(feature = "rust1", since = "1.0.0")] pub struct Stderr { inner: Pin<&'static ReentrantMutex>>, @@ -724,10 +771,17 @@ pub struct Stderr { /// This handle implements the [`Write`] trait and is constructed via /// the [`Stderr::lock`] method. See its documentation for more. /// -/// ### Note: Windows Portability Consideration +/// ### Note: Windows Portability Considerations +/// /// When operating in a console, the Windows implementation of this stream does not support /// non-UTF-8 byte sequences. Attempting to write bytes that are not valid UTF-8 will return /// an error. +/// +/// In a process with a detached console, such as one using +/// `#![windows_subsystem = "windows"]`, or in a child process spawned from such a process, +/// the contained handle will be null. In such cases, the standard library's `Read` and +/// `Write` will do nothing and silently succeed. All other I/O operations, via the +/// standard library or via raw Windows API calls, will fail. #[must_use = "if unused stderr will immediately unlock"] #[stable(feature = "rust1", since = "1.0.0")] pub struct StderrLock<'a> { @@ -738,11 +792,18 @@ pub struct StderrLock<'a> { /// /// This handle is not buffered. /// -/// ### Note: Windows Portability Consideration +/// ### Note: Windows Portability Considerations +/// /// When operating in a console, the Windows implementation of this stream does not support /// non-UTF-8 byte sequences. Attempting to write bytes that are not valid UTF-8 will return /// an error. /// +/// In a process with a detached console, such as one using +/// `#![windows_subsystem = "windows"]`, or in a child process spawned from such a process, +/// the contained handle will be null. In such cases, the standard library's `Read` and +/// `Write` will do nothing and silently succeed. All other I/O operations, via the +/// standard library or via raw Windows API calls, will fail. +/// /// # Examples /// /// Using implicit synchronization: diff --git a/library/std/src/os/windows/io/handle.rs b/library/std/src/os/windows/io/handle.rs index 14b94d8dcdf92..bd8cb8f20d179 100644 --- a/library/std/src/os/windows/io/handle.rs +++ b/library/std/src/os/windows/io/handle.rs @@ -164,12 +164,22 @@ impl OwnedHandle { inherit: bool, options: c::DWORD, ) -> io::Result { + let handle = self.as_raw_handle(); + + // `Stdin`, `Stdout`, and `Stderr` can all hold null handles, such as + // in a process with a detached console. `DuplicateHandle` would fail + // if we passed it a null handle, but we can treat null as a valid + // handle which doesn't do any I/O, and allow it to be duplicated. + if handle.is_null() { + return unsafe { Ok(Handle::from_raw_handle(handle)) }; + } + let mut ret = 0 as c::HANDLE; cvt(unsafe { let cur_proc = c::GetCurrentProcess(); c::DuplicateHandle( cur_proc, - self.as_raw_handle(), + handle, cur_proc, &mut ret, access, diff --git a/library/std/src/os/windows/io/raw.rs b/library/std/src/os/windows/io/raw.rs index 48c5fd358d9db..68fa8918a56a0 100644 --- a/library/std/src/os/windows/io/raw.rs +++ b/library/std/src/os/windows/io/raw.rs @@ -9,6 +9,7 @@ use crate::net; use crate::os::windows::io::{AsHandle, AsSocket}; use crate::os::windows::io::{OwnedHandle, OwnedSocket}; use crate::os::windows::raw; +use crate::ptr; use crate::sys; use crate::sys::c; use crate::sys_common::{self, AsInner, FromInner, IntoInner}; @@ -96,45 +97,57 @@ impl AsRawHandle for fs::File { #[stable(feature = "asraw_stdio", since = "1.21.0")] impl AsRawHandle for io::Stdin { fn as_raw_handle(&self) -> RawHandle { - unsafe { c::GetStdHandle(c::STD_INPUT_HANDLE) as RawHandle } + stdio_handle(unsafe { c::GetStdHandle(c::STD_INPUT_HANDLE) as RawHandle }) } } #[stable(feature = "asraw_stdio", since = "1.21.0")] impl AsRawHandle for io::Stdout { fn as_raw_handle(&self) -> RawHandle { - unsafe { c::GetStdHandle(c::STD_OUTPUT_HANDLE) as RawHandle } + stdio_handle(unsafe { c::GetStdHandle(c::STD_OUTPUT_HANDLE) as RawHandle }) } } #[stable(feature = "asraw_stdio", since = "1.21.0")] impl AsRawHandle for io::Stderr { fn as_raw_handle(&self) -> RawHandle { - unsafe { c::GetStdHandle(c::STD_ERROR_HANDLE) as RawHandle } + stdio_handle(unsafe { c::GetStdHandle(c::STD_ERROR_HANDLE) as RawHandle }) } } #[stable(feature = "asraw_stdio_locks", since = "1.35.0")] impl<'a> AsRawHandle for io::StdinLock<'a> { fn as_raw_handle(&self) -> RawHandle { - unsafe { c::GetStdHandle(c::STD_INPUT_HANDLE) as RawHandle } + stdio_handle(unsafe { c::GetStdHandle(c::STD_INPUT_HANDLE) as RawHandle }) } } #[stable(feature = "asraw_stdio_locks", since = "1.35.0")] impl<'a> AsRawHandle for io::StdoutLock<'a> { fn as_raw_handle(&self) -> RawHandle { - unsafe { c::GetStdHandle(c::STD_OUTPUT_HANDLE) as RawHandle } + stdio_handle(unsafe { c::GetStdHandle(c::STD_OUTPUT_HANDLE) as RawHandle }) } } #[stable(feature = "asraw_stdio_locks", since = "1.35.0")] impl<'a> AsRawHandle for io::StderrLock<'a> { fn as_raw_handle(&self) -> RawHandle { - unsafe { c::GetStdHandle(c::STD_ERROR_HANDLE) as RawHandle } + stdio_handle(unsafe { c::GetStdHandle(c::STD_ERROR_HANDLE) as RawHandle }) } } +// Translate a handle returned from `GetStdHandle` into a handle to return to +// the user. +fn stdio_handle(raw: RawHandle) -> RawHandle { + // `GetStdHandle` isn't expected to actually fail, so when it returns + // `INVALID_HANDLE_VALUE`, it means we were launched from a parent which + // didn't provide us with stdio handles, such as a parent with a detached + // console. In that case, return null to the user, which is consistent + // with what they'd get in the parent, and which avoids the problem that + // `INVALID_HANDLE_VALUE` aliases the current process handle. + if raw == c::INVALID_HANDLE_VALUE { ptr::null_mut() } else { raw } +} + #[stable(feature = "from_raw_os", since = "1.1.0")] impl FromRawHandle for fs::File { #[inline] diff --git a/library/std/src/sys/windows/process.rs b/library/std/src/sys/windows/process.rs index fafd1412d4cb3..ec53c46cd7b4e 100644 --- a/library/std/src/sys/windows/process.rs +++ b/library/std/src/sys/windows/process.rs @@ -309,6 +309,19 @@ impl Command { si.hStdOutput = stdout.as_raw_handle(); si.hStdError = stderr.as_raw_handle(); + // `CreateProcessW` fails with `ERROR_FILE_NOT_FOUND` if any of the + // stdio handles are null, so if we have null handles, set them to + // `INVALID_HANDLE_VALUE`. + if si.hStdInput.is_null() { + si.hStdInput = c::INVALID_HANDLE_VALUE; + } + if si.hStdOutput.is_null() { + si.hStdOutput = c::INVALID_HANDLE_VALUE; + } + if si.hStdError.is_null() { + si.hStdError = c::INVALID_HANDLE_VALUE; + } + let program = to_u16s(&program)?; unsafe { cvt(c::CreateProcessW( From 7dd32469e5f994cec98969b420e8846d5f4c6c34 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Tue, 25 Jan 2022 11:40:11 -0800 Subject: [PATCH 2/3] Fix a compilation error. --- library/std/src/os/windows/io/handle.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/std/src/os/windows/io/handle.rs b/library/std/src/os/windows/io/handle.rs index bd8cb8f20d179..be2ccbd98e9c2 100644 --- a/library/std/src/os/windows/io/handle.rs +++ b/library/std/src/os/windows/io/handle.rs @@ -171,7 +171,7 @@ impl OwnedHandle { // if we passed it a null handle, but we can treat null as a valid // handle which doesn't do any I/O, and allow it to be duplicated. if handle.is_null() { - return unsafe { Ok(Handle::from_raw_handle(handle)) }; + return unsafe { Ok(Self::from_raw_handle(handle)) }; } let mut ret = 0 as c::HANDLE; From 7ddf41c7b10e46c231180cecbbb5e78e0dd12ff8 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Wed, 2 Mar 2022 11:48:50 -0800 Subject: [PATCH 3/3] Remove redundant code for handling NULL handles on Windows. Before calling `CreateProcessW`, stdio handles are passed through `stdio::get_handle`, which already converts NULL to `INVALID_HANDLE_VALUE`, so we don't need extra checks for NULL after that point. --- library/std/src/sys/windows/process.rs | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/library/std/src/sys/windows/process.rs b/library/std/src/sys/windows/process.rs index ec53c46cd7b4e..fafd1412d4cb3 100644 --- a/library/std/src/sys/windows/process.rs +++ b/library/std/src/sys/windows/process.rs @@ -309,19 +309,6 @@ impl Command { si.hStdOutput = stdout.as_raw_handle(); si.hStdError = stderr.as_raw_handle(); - // `CreateProcessW` fails with `ERROR_FILE_NOT_FOUND` if any of the - // stdio handles are null, so if we have null handles, set them to - // `INVALID_HANDLE_VALUE`. - if si.hStdInput.is_null() { - si.hStdInput = c::INVALID_HANDLE_VALUE; - } - if si.hStdOutput.is_null() { - si.hStdOutput = c::INVALID_HANDLE_VALUE; - } - if si.hStdError.is_null() { - si.hStdError = c::INVALID_HANDLE_VALUE; - } - let program = to_u16s(&program)?; unsafe { cvt(c::CreateProcessW(