Skip to content

Commit

Permalink
Unrolled build for rust-lang#119029
Browse files Browse the repository at this point in the history
Rollup merge of rust-lang#119029 - dylni:avoid-closing-invalid-handles, r=ChrisDenton

Avoid closing invalid handles

Documentation for [`HandleOrInvalid`] has this note:

> If holds a handle other than `INVALID_HANDLE_VALUE`, it will close the handle on drop.

Documentation for [`HandleOrNull`] has this note:

> If this holds a non-null handle, it will close the handle on drop.

Currently, both will call `CloseHandle` on their invalid handles as a result of using `OwnedHandle` internally, contradicting the above paragraphs. This PR adds destructors that match the documentation.

```@rustbot``` label A-io O-windows T-libs

[`HandleOrInvalid`]: https://doc.rust-lang.org/std/os/windows/io/struct.HandleOrInvalid.html
[`HandleOrNull`]: https://doc.rust-lang.org/std/os/windows/io/struct.HandleOrNull.html
  • Loading branch information
rust-timer authored Mar 14, 2024
2 parents fe61575 + a82587c commit 6bd6b6e
Showing 1 changed file with 47 additions and 21 deletions.
68 changes: 47 additions & 21 deletions library/std/src/os/windows/io/handle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use crate::fmt;
use crate::fs;
use crate::io;
use crate::marker::PhantomData;
use crate::mem::forget;
use crate::mem::{forget, ManuallyDrop};
use crate::ptr;
use crate::sys;
use crate::sys::cvt;
Expand Down Expand Up @@ -91,7 +91,7 @@ pub struct OwnedHandle {
#[repr(transparent)]
#[stable(feature = "io_safety", since = "1.63.0")]
#[derive(Debug)]
pub struct HandleOrNull(OwnedHandle);
pub struct HandleOrNull(RawHandle);

/// 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
Expand All @@ -110,7 +110,7 @@ pub struct HandleOrNull(OwnedHandle);
#[repr(transparent)]
#[stable(feature = "io_safety", since = "1.63.0")]
#[derive(Debug)]
pub struct HandleOrInvalid(OwnedHandle);
pub struct HandleOrInvalid(RawHandle);

// The Windows [`HANDLE`] type may be transferred across and shared between
// thread boundaries (despite containing a `*mut void`, which in general isn't
Expand Down Expand Up @@ -163,15 +163,24 @@ impl TryFrom<HandleOrNull> for OwnedHandle {

#[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);

Err(NullHandleError(()))
let handle_or_null = ManuallyDrop::new(handle_or_null);
if handle_or_null.is_valid() {
// SAFETY: The handle is not null.
Ok(unsafe { OwnedHandle::from_raw_handle(handle_or_null.0) })
} else {
Ok(owned_handle)
Err(NullHandleError(()))
}
}
}

#[stable(feature = "io_safety", since = "1.63.0")]
impl Drop for HandleOrNull {
#[inline]
fn drop(&mut self) {
if self.is_valid() {
unsafe {
let _ = sys::c::CloseHandle(self.0);
}
}
}
}
Expand Down Expand Up @@ -232,15 +241,24 @@ impl TryFrom<HandleOrInvalid> for OwnedHandle {

#[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(()))
let handle_or_invalid = ManuallyDrop::new(handle_or_invalid);
if handle_or_invalid.is_valid() {
// SAFETY: The handle is not invalid.
Ok(unsafe { OwnedHandle::from_raw_handle(handle_or_invalid.0) })
} else {
Ok(owned_handle)
Err(InvalidHandleError(()))
}
}
}

#[stable(feature = "io_safety", since = "1.63.0")]
impl Drop for HandleOrInvalid {
#[inline]
fn drop(&mut self) {
if self.is_valid() {
unsafe {
let _ = sys::c::CloseHandle(self.0);
}
}
}
}
Expand Down Expand Up @@ -333,7 +351,11 @@ 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(handle)
}

fn is_valid(&self) -> bool {
!self.0.is_null()
}
}

Expand All @@ -356,7 +378,11 @@ 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(handle)
}

fn is_valid(&self) -> bool {
self.0 != sys::c::INVALID_HANDLE_VALUE
}
}

Expand Down

0 comments on commit 6bd6b6e

Please sign in to comment.