Avoid closing invalid handles

Author: dylni

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<OwnedHandle>);
 
 /// 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<OwnedHandle>);
 
 // 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<HandleOrNull> 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<OwnedHandle> {
+                // 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<Self, NullHandleError> {
-        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<Self, $error> {
+                // 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<HandleOrInvalid> for OwnedHandle {
-    type Error = InvalidHandleError;
-
-    #[inline]
-    fn try_from(handle_or_invalid: HandleOrInvalid) -> Result<Self, InvalidHandleError> {
-        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)))
     }
 }