Auto merge of #97178 - sunfishcode:ownedfd-and-dup, r=joshtriplett

Add a `BorrowedFd::try_clone_to_owned` and accompanying documentation

Add a `BorrowedFd::try_clone_to_owned`, which returns a new `OwnedFd` sharing the underlying file description. And similar for `BorrowedHandle` and `BorrowedSocket` on WIndows.

This is similar to the existing `OwnedFd::try_clone`, but it's named differently to reflect that it doesn't return `Result<Self, ...>`. I'm open to suggestions for better names.

Also, extend the `unix::io` documentation to mention that `dup` is permitted on `BorrowedFd`.

This was originally requsted [here](#88564 (comment)). At the time I wasn't sure whether it was desirable, but it does have uses and it helps clarify the API. The documentation previously didn't rule out using `dup` on a `BorrowedFd`, but the API only offered convenient ways to do it from an `OwnedFd`. With this patch, the API allows one to do `try_clone` on any type where it's permitted.

library/std/src/os/fd/owned.rs

@@ -77,11 +77,20 @@ impl BorrowedFd<'_> {
 }
 
 impl OwnedFd {
- /// Creates a new `OwnedFd` instance that shares the same underlying file handle
- /// as the existing `OwnedFd` instance.
- #[cfg(not(target_arch = "wasm32"))]
+ /// Creates a new `OwnedFd` instance that shares the same underlying file
+ /// description as the existing `OwnedFd` instance.
  #[stable(feature = "io_safety", since = "1.63.0")]
  pub fn try_clone(&self) -> crate::io::Result<Self> {
+ self.as_fd().try_clone_to_owned()
+ }
+}
+
+impl BorrowedFd<'_> {
+ /// Creates a new `OwnedFd` instance that shares the same underlying file
+ /// description as the existing `BorrowedFd` instance.
+ #[cfg(not(target_arch = "wasm32"))]
+ #[stable(feature = "io_safety", since = "1.63.0")]
+ pub fn try_clone_to_owned(&self) -> crate::io::Result<OwnedFd> {
  // We want to atomically duplicate this file descriptor and set the
  // CLOEXEC flag, and currently that's done via F_DUPFD_CLOEXEC. This
  // is a POSIX flag that was added to Linux in 2.6.24.
@@ -96,12 +105,14 @@ impl OwnedFd {
  let cmd = libc::F_DUPFD;
 
  let fd = cvt(unsafe { libc::fcntl(self.as_raw_fd(), cmd, 0) })?;
- Ok(unsafe { Self::from_raw_fd(fd) })
+ Ok(unsafe { OwnedFd::from_raw_fd(fd) })
  }
 
+ /// Creates a new `OwnedFd` instance that shares the same underlying file
+ /// description as the existing `BorrowedFd` instance.
  #[cfg(target_arch = "wasm32")]
  #[stable(feature = "io_safety", since = "1.63.0")]
- pub fn try_clone(&self) -> crate::io::Result<Self> {
+ pub fn try_clone_to_owned(&self) -> crate::io::Result<OwnedFd> {
  Err(crate::io::const_io_error!(
  crate::io::ErrorKind::Unsupported,
  "operation not supported on WASI yet",

library/std/src/os/unix/io/mod.rs

@@ -26,20 +26,30 @@
 //! that they don't outlive the resource they point to. These are safe to
 //! use. `BorrowedFd` values may be used in APIs which provide safe access to
 //! any system call except for:
+//!
 //! - `close`, because that would end the dynamic lifetime of the resource
 //! without ending the lifetime of the file descriptor.
+//!
 //! - `dup2`/`dup3`, in the second argument, because this argument is
 //! closed and assigned a new resource, which may break the assumptions
 //! other code using that file descriptor.
-//! This list doesn't include `mmap`, since `mmap` does do a proper borrow of
-//! its file descriptor argument. That said, `mmap` is unsafe for other
-//! reasons: it operates on raw pointers, and it can have undefined behavior if
-//! the underlying storage is mutated. Mutations may come from other processes,
-//! or from the same process if the API provides `BorrowedFd` access, since as
-//! mentioned earlier, `BorrowedFd` values may be used in APIs which provide
-//! safe access to any system call. Consequently, code using `mmap` and
-//! presenting a safe API must take full responsibility for ensuring that safe
-//! Rust code cannot evoke undefined behavior through it.
+//!
+//! `BorrowedFd` values may be used in APIs which provide safe access to `dup`
+//! system calls, so types implementing `AsFd` or `From<OwnedFd>` should not
+//! assume they always have exclusive access to the underlying file
+//! description.
+//!
+//! `BorrowedFd` values may also be used with `mmap`, since `mmap` uses the
+//! provided file descriptor in a manner similar to `dup` and does not require
+//! the `BorrowedFd` passed to it to live for the lifetime of the resulting
+//! mapping. That said, `mmap` is unsafe for other reasons: it operates on raw
+//! pointers, and it can have undefined behavior if the underlying storage is
+//! mutated. Mutations may come from other processes, or from the same process
+//! if the API provides `BorrowedFd` access, since as mentioned earlier,
+//! `BorrowedFd` values may be used in APIs which provide safe access to any
+//! system call. Consequently, code using `mmap` and presenting a safe API must
+//! take full responsibility for ensuring that safe Rust code cannot evoke
+//! undefined behavior through it.
 //!
 //! Like boxes, `OwnedFd` values conceptually own the resource they point to,
 //! and free (close) it when they are dropped.

library/std/src/os/windows/io/handle.rs

@@ -177,10 +177,19 @@ impl TryFrom<HandleOrNull> for OwnedHandle {
 }
 
 impl OwnedHandle {
- /// Creates a new `OwnedHandle` instance that shares the same underlying file handle
- /// as the existing `OwnedHandle` instance.
+ /// Creates a new `OwnedHandle` instance that shares the same underlying
+ /// object as the existing `OwnedHandle` instance.
  #[stable(feature = "io_safety", since = "1.63.0")]
  pub fn try_clone(&self) -> crate::io::Result<Self> {
+ self.as_handle().try_clone_to_owned()
+ }
+}
+
+impl BorrowedHandle<'_> {
+ /// Creates a new `OwnedHandle` instance that shares the same underlying
+ /// object as the existing `BorrowedHandle` instance.
+ #[stable(feature = "io_safety", since = "1.63.0")]
+ pub fn try_clone_to_owned(&self) -> crate::io::Result<OwnedHandle> {
  self.duplicate(0, false, c::DUPLICATE_SAME_ACCESS)
  }
 
@@ -189,15 +198,15 @@ impl OwnedHandle {
  access: c::DWORD,
  inherit: bool,
  options: c::DWORD,
- ) -> io::Result<Self> {
+ ) -> io::Result<OwnedHandle> {
  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)) };
+ return unsafe { Ok(OwnedHandle::from_raw_handle(handle)) };
  }
 
  let mut ret = ptr::null_mut();
@@ -213,7 +222,7 @@ impl OwnedHandle {
  options,
  )
  })?;
- unsafe { Ok(Self::from_raw_handle(ret)) }
+ unsafe { Ok(OwnedHandle::from_raw_handle(ret)) }
  }
 }
 

library/std/src/os/windows/io/mod.rs

@@ -36,6 +36,12 @@
 //! dynamic lifetime of the resource without ending the lifetime of the
 //! handle or socket.
 //!
+//! `BorrowedHandle` and `BorrowedSocket` values may be used in APIs which
+//! provide safe access to `DuplicateHandle` and `WSADuplicateSocketW` and
+//! related functions, so types implementing `AsHandle`, `AsSocket`,
+//! `From<OwnedHandle>`, or `From<OwnedSocket>` should not assume they always
+//! have exclusive access to the underlying object.
+//!
 //! Like boxes, `OwnedHandle` and `OwnedSocket` values conceptually own the
 //! resource they point to, and free (close) it when they are dropped.
 //!

library/std/src/os/windows/io/socket.rs

@@ -82,10 +82,33 @@ impl BorrowedSocket<'_> {
 }
 
 impl OwnedSocket {
- /// Creates a new `OwnedSocket` instance that shares the same underlying socket
- /// as the existing `OwnedSocket` instance.
+ /// Creates a new `OwnedSocket` instance that shares the same underlying
+ /// object as the existing `OwnedSocket` instance.
  #[stable(feature = "io_safety", since = "1.63.0")]
  pub fn try_clone(&self) -> io::Result<Self> {
+ self.as_socket().try_clone_to_owned()
+ }
+
+ // FIXME(strict_provenance_magic): we defined RawSocket to be a u64 ;-;
+ #[cfg(not(target_vendor = "uwp"))]
+ pub(crate) fn set_no_inherit(&self) -> io::Result<()> {
+ cvt(unsafe {
+ c::SetHandleInformation(self.as_raw_socket() as c::HANDLE, c::HANDLE_FLAG_INHERIT, 0)
+ })
+ .map(drop)
+ }
+
+ #[cfg(target_vendor = "uwp")]
+ pub(crate) fn set_no_inherit(&self) -> io::Result<()> {
+ Err(io::const_io_error!(io::ErrorKind::Unsupported, "Unavailable on UWP"))
+ }
+}
+
+impl BorrowedSocket<'_> {
+ /// Creates a new `OwnedSocket` instance that shares the same underlying
+ /// object as the existing `BorrowedSocket` instance.
+ #[stable(feature = "io_safety", since = "1.63.0")]
+ pub fn try_clone_to_owned(&self) -> io::Result<OwnedSocket> {
  let mut info = unsafe { mem::zeroed::<c::WSAPROTOCOL_INFO>() };
  let result = unsafe {
  c::WSADuplicateSocketW(self.as_raw_socket(), c::GetCurrentProcessId(), &mut info)
@@ -133,20 +156,6 @@ impl OwnedSocket {
  }
  }
  }
-
- // FIXME(strict_provenance_magic): we defined RawSocket to be a u64 ;-;
- #[cfg(not(target_vendor = "uwp"))]
- pub(crate) fn set_no_inherit(&self) -> io::Result<()> {
- cvt(unsafe {
- c::SetHandleInformation(self.as_raw_socket() as c::HANDLE, c::HANDLE_FLAG_INHERIT, 0)
- })
- .map(drop)
- }
-
- #[cfg(target_vendor = "uwp")]
- pub(crate) fn set_no_inherit(&self) -> io::Result<()> {
- Err(io::const_io_error!(io::ErrorKind::Unsupported, "Unavailable on UWP"))
- }
 }
 
 /// Returns the last error from the Windows socket interface.

library/std/src/sys/windows/handle.rs

@@ -218,7 +218,7 @@ impl Handle {
  inherit: bool,
  options: c::DWORD,
  ) -> io::Result<Self> {
- Ok(Self(self.0.duplicate(access, inherit, options)?))
+ Ok(Self(self.0.as_handle().duplicate(access, inherit, options)?))
  }
 
  /// Performs a synchronous read.