Skip to content

Commit

Permalink
Rollup merge of #96932 - sunfishcode:sunfishcode/document-borrowed-ha…
Browse files Browse the repository at this point in the history
…ndle, r=joshtriplett

Clarify what values `BorrowedHandle`, `OwnedHandle` etc. can hold.

Reword the documentation to clarify that when `BorrowedHandle`, `OwnedHandle`, or `HandleOrNull` hold the value `-1`, it always means the current process handle, and not `INVALID_HANDLE_VALUE`.

`-1` should only mean `INVALID_HANDLE_VALUE` after a call to a function documented to return that to report errors, which should lead I/O functions to produce errors rather than succeeding and producing `OwnedHandle` or `BorrowedHandle` values. So if a consumer of an `OwnedHandle` or `BorrowedHandle` ever sees them holding a `-1`, it should always mean the current process handle.
  • Loading branch information
matthiaskrgr authored May 13, 2022
2 parents ebb80ec + 275812a commit f38b7ff
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 12 deletions.
31 changes: 19 additions & 12 deletions library/std/src/os/windows/io/handle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,10 @@ use crate::sys_common::{AsInner, FromInner, IntoInner};
/// so it can be used in FFI in places where a handle is passed as an argument,
/// it is not captured or consumed.
///
/// Note that it *may* have the value `INVALID_HANDLE_VALUE` (-1), which is
/// sometimes a valid handle value. See [here] for the full story.
/// Note that it *may* have the value `-1`, which in `BorrowedHandle` always
/// represents a valid handle value, such as [the current process handle], and
/// not `INVALID_HANDLE_VALUE`, despite the two having the same value. See
/// [here] for the full story.
///
/// And, it *may* have the value `NULL` (0), which can occur when consoles are
/// detached from processes, or when `windows_subsystem` is used.
Expand All @@ -33,6 +35,7 @@ use crate::sys_common::{AsInner, FromInner, IntoInner};
/// handle, which is then borrowed under the same lifetime.
///
/// [here]: https://devblogs.microsoft.com/oldnewthing/20040302-00/?p=40443
/// [the current process handle]: https://docs.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-getcurrentprocess#remarks
#[derive(Copy, Clone)]
#[repr(transparent)]
#[unstable(feature = "io_safety", issue = "87074")]
Expand All @@ -45,8 +48,10 @@ pub struct BorrowedHandle<'handle> {
///
/// This closes the handle on drop.
///
/// Note that it *may* have the value `INVALID_HANDLE_VALUE` (-1), which is
/// sometimes a valid handle value. See [here] for the full story.
/// Note that it *may* have the value `-1`, which in `OwnedHandle` always
/// represents a valid handle value, such as [the current process handle], and
/// not `INVALID_HANDLE_VALUE`, despite the two having the same value. See
/// [here] for the full story.
///
/// And, it *may* have the value `NULL` (0), which can occur when consoles are
/// detached from processes, or when `windows_subsystem` is used.
Expand All @@ -59,6 +64,7 @@ pub struct BorrowedHandle<'handle> {
/// [`RegCloseKey`]: https://docs.microsoft.com/en-us/windows/win32/api/winreg/nf-winreg-regclosekey
///
/// [here]: https://devblogs.microsoft.com/oldnewthing/20040302-00/?p=40443
/// [the current process handle]: https://docs.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-getcurrentprocess#remarks
#[repr(transparent)]
#[unstable(feature = "io_safety", issue = "87074")]
pub struct OwnedHandle {
Expand All @@ -75,11 +81,13 @@ pub struct OwnedHandle {
/// `NULL`. This ensures that such FFI calls cannot start using the handle without
/// checking for `NULL` first.
///
/// This type considers any value other than `NULL` to be valid, including `INVALID_HANDLE_VALUE`.
/// This is because APIs that use `NULL` as their sentry value don't treat `INVALID_HANDLE_VALUE`
/// as special.
/// This type may hold any handle value that [`OwnedHandle`] may hold. As with `OwnedHandle`, when
/// it holds `-1`, that value is interpreted as a valid handle value, such as
/// [the current process handle], and not `INVALID_HANDLE_VALUE`.
///
/// If this holds a valid handle, it will close the handle on drop.
/// If this holds a non-null handle, it will close the handle on drop.
///
/// [the current process handle]: https://docs.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-getcurrentprocess#remarks
#[repr(transparent)]
#[unstable(feature = "io_safety", issue = "87074")]
#[derive(Debug)]
Expand All @@ -95,11 +103,10 @@ pub struct HandleOrNull(OwnedHandle);
/// `INVALID_HANDLE_VALUE`. This ensures that such FFI calls cannot start using the handle without
/// checking for `INVALID_HANDLE_VALUE` first.
///
/// This type considers any value other than `INVALID_HANDLE_VALUE` to be valid, including `NULL`.
/// This is because APIs that use `INVALID_HANDLE_VALUE` as their sentry value may return `NULL`
/// under `windows_subsystem = "windows"` or other situations where I/O devices are detached.
/// This type may hold any handle value that [`OwnedHandle`] may hold, except that when it holds
/// `-1`, that value is interpreted to mean `INVALID_HANDLE_VALUE`.
///
/// If this holds a valid handle, it will close the handle on drop.
/// If holds a handle other than `INVALID_HANDLE_VALUE`, it will close the handle on drop.
#[repr(transparent)]
#[unstable(feature = "io_safety", issue = "87074")]
#[derive(Debug)]
Expand Down
7 changes: 7 additions & 0 deletions library/std/src/os/windows/io/raw.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,15 @@ pub trait AsRawHandle {
/// raw handle to the caller, and the handle is only guaranteed
/// to be valid while the original object has not yet been destroyed.
///
/// This function may return null, such as when called on [`Stdin`],
/// [`Stdout`], or [`Stderr`] when the console is detached.
///
/// However, borrowing is not strictly required. See [`AsHandle::as_handle`]
/// for an API which strictly borrows a handle.
///
/// [`Stdin`]: io::Stdin
/// [`Stdout`]: io::Stdout
/// [`Stderr`]: io::Stderr
#[stable(feature = "rust1", since = "1.0.0")]
fn as_raw_handle(&self) -> RawHandle;
}
Expand Down

0 comments on commit f38b7ff

Please sign in to comment.