Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Consistently present absent stdio handles on Windows as NULL handles. #93263

Merged
merged 3 commits into from
Mar 19, 2022
Merged

Consistently present absent stdio handles on Windows as NULL handles. #93263

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ee02f01
Consistently present absent stdio handles on Windows as NULL handles.
sunfishcode Jan 21, 2022
7dd3246
Fix a compilation error.
sunfishcode Jan 25, 2022
7ddf41c
Remove redundant code for handling NULL handles on Windows.
sunfishcode Mar 2, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 70 additions & 9 deletions library/std/src/io/stdio.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,12 +202,18 @@ fn handle_ebadf<T>(r: io::Result<T>, default: T) -> io::Result<T> {
///
/// [`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
Expand All @@ -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
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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")]
Expand All @@ -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> {
Expand All @@ -528,11 +561,18 @@ static STDOUT: SyncOnceCell<ReentrantMutex<RefCell<LineWriter<StdoutRaw>>>> = 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:
Expand Down Expand Up @@ -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<RefCell<StderrRaw>>>,
Expand All @@ -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> {
Expand All @@ -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:
Expand Down
12 changes: 11 additions & 1 deletion library/std/src/os/windows/io/handle.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,12 +164,22 @@ impl OwnedHandle {
inherit: bool,
options: c::DWORD,
) -> io::Result<Self> {
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(Self::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,
Expand Down
25 changes: 19 additions & 6 deletions library/std/src/os/windows/io/raw.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

INVALID_HANDLE_VALUE aliases the current process handle

Well, they are totally different types. No one would try to pass io::RawHandle where io::RawSocket is expected or where a process handle is expected.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In Windows, the current process handle, as returned by GetCurrentProcess has type HANDLE, aka io::RawHandle here, which is the same as the return type of eg. CreateFileW.

SOCKET, aka io::RawSocket here, on the other hand, is a different type.

Copy link
Contributor

@pravic pravic Mar 10, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And so does a dozens of other types that are also an alias (or a newtype if compiled with -DSTRICT) of HANDLE: https://docs.microsoft.com/en-us/windows/win32/winprog/windows-data-types

So, in fact it should be a newtype in Rust as well.

Okay, to be more formal: while both GetCurrentProcess and CreateFile (and CreateEvent, etc) have the same types (HANDLE), in fact they are different handle types and no one familiar with winapi would try to use them interchangeably.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some of the types linked there using HANDLE are so different that they don't even use CloseHandle. We already do document that it's not valid to use OwnedHandle/BorrowedHandle to hold those kinds of handles.

Beyond that though, OwnedHandle, BorrowedHandle, and AsHandle are intended to be used with both file handles and process handles, for three reasons:

  • Rust already uses the same RawHandle/AsRawHandle/IntoRawHandle types and traits for both processes and files, so it's consistent with existing APIs in Rust
  • Both process handles and file handles can be passed to CloseHandle, GetHandleInformation, SetHandleInformation, DuplicateHandle, and others, so they may be different, but they're not completely different.
  • Rust also does have distinct higher-level types for different kinds of handles, such as std::fs::File for file(-like) handles, and std::process::Child for process handles, and these are what most Rust code uses most of the time, so it isn't as important to make a static type distinction at the OwnedHandle/etc. level.

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]
Expand Down