From ed08061ef4dd7e9e1cde374445f5d326aed571bf Mon Sep 17 00:00:00 2001 From: dylni <46035563+dylni@users.noreply.github.com> Date: Sun, 17 Dec 2023 19:46:42 -0500 Subject: [PATCH] Avoid closing invalid handles --- library/std/src/os/windows/io/handle.rs | 73 ++++++++++++------------- 1 file changed, 36 insertions(+), 37 deletions(-) diff --git a/library/std/src/os/windows/io/handle.rs b/library/std/src/os/windows/io/handle.rs index b0540872c0b62..4e0961cce3b09 100644 --- a/library/std/src/os/windows/io/handle.rs +++ b/library/std/src/os/windows/io/handle.rs @@ -8,6 +8,7 @@ use crate::fs; use crate::io; use crate::marker::PhantomData; use crate::mem::forget; +use crate::mem::ManuallyDrop; use crate::ptr; use crate::sys; use crate::sys::cvt; @@ -91,7 +92,7 @@ pub struct OwnedHandle { #[repr(transparent)] #[stable(feature = "io_safety", since = "1.63.0")] #[derive(Debug)] -pub struct HandleOrNull(OwnedHandle); +pub struct HandleOrNull(ManuallyDrop); /// FFI type for handles in return values or out parameters, where `INVALID_HANDLE_VALUE` is used /// as a sentry value to indicate errors, such as in the return value of `CreateFileW`. This uses @@ -110,7 +111,7 @@ pub struct HandleOrNull(OwnedHandle); #[repr(transparent)] #[stable(feature = "io_safety", since = "1.63.0")] #[derive(Debug)] -pub struct HandleOrInvalid(OwnedHandle); +pub struct HandleOrInvalid(ManuallyDrop); // The Windows [`HANDLE`] type may be transferred across and shared between // thread boundaries (despite containing a `*mut void`, which in general isn't @@ -157,24 +158,41 @@ impl BorrowedHandle<'_> { } } -#[stable(feature = "io_safety", since = "1.63.0")] -impl TryFrom for OwnedHandle { - type Error = NullHandleError; +macro_rules! impl_handle_or { + ( $name:ident, $invalid_handle:expr, $error:ident ) => { + impl $name { + unsafe fn take_owned_handle(&mut self) -> Option { + // Filter before taking the value, to avoid calling `Drop` on + // the invalid value. + Some(&mut self.0) + .filter(|handle| handle.handle != $invalid_handle) + .map(|handle| unsafe { ManuallyDrop::take(handle) }) + } + } - #[inline] - fn try_from(handle_or_null: HandleOrNull) -> Result { - let owned_handle = handle_or_null.0; - if owned_handle.handle.is_null() { - // Don't call `CloseHandle`; it'd be harmless, except that it could - // overwrite the `GetLastError` error. - forget(owned_handle); + #[stable(feature = "io_safety", since = "1.63.0")] + impl TryFrom<$name> for OwnedHandle { + type Error = $error; - Err(NullHandleError(())) - } else { - Ok(owned_handle) + #[inline] + fn try_from(handle_or: $name) -> Result { + // SAFETY: `ManuallyDrop` prevents calling `Drop`. + unsafe { ManuallyDrop::new(handle_or).take_owned_handle() }.ok_or($error(())) + } } - } + + #[stable(feature = "handle_or_drop", since = "1.75.0")] + impl Drop for $name { + #[inline] + fn drop(&mut self) { + // SAFETY: `Drop` has already been called (and is running). + let _ = unsafe { self.take_owned_handle() }; + } + } + }; } +impl_handle_or!(HandleOrNull, ptr::null_mut(), NullHandleError); +impl_handle_or!(HandleOrInvalid, sys::c::INVALID_HANDLE_VALUE, InvalidHandleError); impl OwnedHandle { /// Creates a new `OwnedHandle` instance that shares the same underlying @@ -226,25 +244,6 @@ impl BorrowedHandle<'_> { } } -#[stable(feature = "io_safety", since = "1.63.0")] -impl TryFrom for OwnedHandle { - type Error = InvalidHandleError; - - #[inline] - fn try_from(handle_or_invalid: HandleOrInvalid) -> Result { - let owned_handle = handle_or_invalid.0; - if owned_handle.handle == sys::c::INVALID_HANDLE_VALUE { - // Don't call `CloseHandle`; it'd be harmless, except that it could - // overwrite the `GetLastError` error. - forget(owned_handle); - - Err(InvalidHandleError(())) - } else { - Ok(owned_handle) - } - } -} - /// This is the error type used by [`HandleOrNull`] when attempting to convert /// into a handle, to indicate that the value is null. // The empty field prevents constructing this, and allows extending it in the future. @@ -333,7 +332,7 @@ impl HandleOrNull { #[stable(feature = "io_safety", since = "1.63.0")] #[inline] pub unsafe fn from_raw_handle(handle: RawHandle) -> Self { - Self(OwnedHandle::from_raw_handle(handle)) + Self(ManuallyDrop::new(OwnedHandle::from_raw_handle(handle))) } } @@ -356,7 +355,7 @@ impl HandleOrInvalid { #[stable(feature = "io_safety", since = "1.63.0")] #[inline] pub unsafe fn from_raw_handle(handle: RawHandle) -> Self { - Self(OwnedHandle::from_raw_handle(handle)) + Self(ManuallyDrop::new(OwnedHandle::from_raw_handle(handle))) } }