Auto merge of rust-lang#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](rust-lang#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.

Author: bors

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.