filedescriptor: probe and remember filetype at construction

This avoids a deadlock waiting for the named pipe information to be
returned if we're probing during drop().  This does make the
OwnedHandle struct slightly larger on windows.

filedescriptor/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "filedescriptor"
-version = "0.4.0"
+version = "0.5.0"
 authors = ["Wez Furlong"]
 edition = "2018"
 repository = "https://github.com/wez/wzsh"

filedescriptor/src/lib.rs

@@ -97,6 +97,7 @@
 //! # Ok::<(), Error>(())
 //! ```
 use failure::Fallible;
+
 #[cfg(unix)]
 mod unix;
 #[cfg(unix)]
@@ -147,6 +148,7 @@ pub trait FromRawSocketDescriptor {
 #[derive(Debug)]
 pub struct OwnedHandle {
     handle: RawFileDescriptor,
+    handle_type: HandleType,
 }
 
 impl OwnedHandle {
@@ -154,8 +156,10 @@ impl OwnedHandle {
     /// the system `RawFileDescriptor` type.  This consumes the parameter
     /// and replaces it with an `OwnedHandle` instance.
     pub fn new<F: IntoRawFileDescriptor>(f: F) -> Self {
+        let handle = f.into_raw_file_descriptor();
         Self {
-            handle: f.into_raw_file_descriptor(),
+            handle,
+            handle_type: Self::probe_handle_type(handle),
         }
     }
 
@@ -166,7 +170,7 @@ impl OwnedHandle {
     /// The returned handle has a separate lifetime from the source, but
     /// references the same object at the kernel level.
     pub fn try_clone(&self) -> Fallible<Self> {
-        Self::dup(self)
+        Self::dup_impl(self, self.handle_type)
     }
 
     /// Attempt to duplicate the underlying handle from an object that is
@@ -177,7 +181,7 @@ impl OwnedHandle {
     /// The returned handle has a separate lifetime from the source, but
     /// references the same object at the kernel level.
     pub fn dup<F: AsRawFileDescriptor>(f: &F) -> Fallible<Self> {
-        Self::dup_impl(f)
+        Self::dup_impl(f, Default::default())
     }
 }
 

filedescriptor/src/unix.rs

@@ -5,6 +5,8 @@ use crate::{
 use failure::{bail, Fallible};
 use std::os::unix::prelude::*;
 
+pub(crate) type HandleType = ();
+
 /// `RawFileDescriptor` is a platform independent type alias for the
 /// underlying platform file descriptor type.  It is primarily useful
 /// for avoiding using `cfg` blocks in platform independent code.
@@ -75,7 +77,10 @@ impl IntoRawFd for OwnedHandle {
 
 impl FromRawFd for OwnedHandle {
     unsafe fn from_raw_fd(fd: RawFd) -> Self {
-        Self { handle: fd }
+        Self {
+            handle: fd,
+            handle_type: (),
+        }
     }
 }
 
@@ -108,14 +113,20 @@ impl OwnedHandle {
                 std::io::Error::last_os_error()
             )
         } else {
-            let mut owned = OwnedHandle { handle: duped };
+            let mut owned = OwnedHandle {
+                handle: duped,
+                handle_type: (),
+            };
             owned.cloexec()?;
             Ok(owned)
         }
     }
 
     #[inline]
-    pub(crate) fn dup_impl<F: AsRawFileDescriptor>(fd: &F) -> Fallible<Self> {
+    pub(crate) fn dup_impl<F: AsRawFileDescriptor>(
+        fd: &F,
+        handle_type: HandleType,
+    ) -> Fallible<Self> {
         let fd = fd.as_raw_file_descriptor();
         let duped = unsafe { libc::fcntl(fd, libc::F_DUPFD_CLOEXEC, 0) };
         if duped == -1 {
@@ -128,9 +139,16 @@ impl OwnedHandle {
                 bail!("dup of fd {} failed: {:?}", fd, err)
             }
         } else {
-            Ok(OwnedHandle { handle: duped })
+            Ok(OwnedHandle {
+                handle: duped,
+                handle_type,
+            })
         }
     }
+
+    pub(crate) fn probe_handle_type(_handle: RawFileDescriptor) -> HandleType {
+        ()
+    }
 }
 
 impl std::io::Read for FileDescriptor {
@@ -200,10 +218,16 @@ impl Pipe {
             )
         } else {
             let read = FileDescriptor {
-                handle: OwnedHandle { handle: fds[0] },
+                handle: OwnedHandle {
+                    handle: fds[0],
+                    handle_type: (),
+                },
             };
             let write = FileDescriptor {
-                handle: OwnedHandle { handle: fds[1] },
+                handle: OwnedHandle {
+                    handle: fds[1],
+                    handle_type: (),
+                },
             };
             Ok(Pipe { read, write })
         }
@@ -251,10 +275,16 @@ pub fn socketpair_impl() -> Fallible<(FileDescriptor, FileDescriptor)> {
         )
     } else {
         let read = FileDescriptor {
-            handle: OwnedHandle { handle: fds[0] },
+            handle: OwnedHandle {
+                handle: fds[0],
+                handle_type: (),
+            },
         };
         let write = FileDescriptor {
-            handle: OwnedHandle { handle: fds[1] },
+            handle: OwnedHandle {
+                handle: fds[1],
+                handle_type: (),
+            },
         };
         Ok((read, write))
     }

filedescriptor/src/windows.rs

@@ -34,6 +34,21 @@ pub type RawFileDescriptor = RawHandle;
 /// for avoiding using `cfg` blocks in platform independent code.
 pub type SocketDescriptor = SOCKET;
 
+#[derive(Clone, Copy, Debug, PartialEq, Eq)]
+pub(crate) enum HandleType {
+    Char,
+    Disk,
+    Pipe,
+    Socket,
+    Unknown,
+}
+
+impl Default for HandleType {
+    fn default() -> Self {
+        HandleType::Unknown
+    }
+}
+
 impl<T: AsRawHandle> AsRawFileDescriptor for T {
     fn as_raw_file_descriptor(&self) -> RawFileDescriptor {
         self.as_raw_handle()
@@ -72,34 +87,43 @@ impl<T: FromRawSocket> FromRawSocketDescriptor for T {
 
 unsafe impl Send for OwnedHandle {}
 
-#[derive(Clone, Copy, Debug, PartialEq, Eq)]
-enum HandleType {
-    Char,
-    Disk,
-    Pipe,
-    Socket,
-    Unknown,
-}
-
-fn handle_type(handle: HANDLE) -> HandleType {
-    match unsafe { GetFileType(handle) } {
-        FILE_TYPE_CHAR => HandleType::Char,
-        FILE_TYPE_DISK => HandleType::Disk,
-        FILE_TYPE_PIPE => {
-            // Could be a pipe or a socket.  Test if for pipeness
-            let mut flags = 0;
-            let mut out_buf = 0;
-            let mut in_buf = 0;
-            let mut inst = 0;
-            if unsafe { GetNamedPipeInfo(handle, &mut flags, &mut out_buf, &mut in_buf, &mut inst) }
-                != 0
-            {
-                HandleType::Pipe
-            } else {
-                HandleType::Socket
-            }
+impl OwnedHandle {
+    fn probe_handle_type_if_unknown(handle: HANDLE, handle_type: HandleType) -> HandleType {
+        match handle_type {
+            HandleType::Unknown => Self::probe_handle_type(handle),
+            t => t,
+        }
+    }
+
+    pub(crate) fn probe_handle_type(handle: HANDLE) -> HandleType {
+        match unsafe { GetFileType(handle) } {
+            FILE_TYPE_CHAR => HandleType::Char,
+            FILE_TYPE_DISK => HandleType::Disk,
+            FILE_TYPE_PIPE => {
+                // Could be a pipe or a socket.  Test if for pipeness
+                let mut flags = 0;
+                let mut out_buf = 0;
+                let mut in_buf = 0;
+                let mut inst = 0;
+                if unsafe {
+                    GetNamedPipeInfo(handle, &mut flags, &mut out_buf, &mut in_buf, &mut inst)
+                } != 0
+                {
+                    HandleType::Pipe
+                } else {
+                    HandleType::Socket
+                }
+            }
+            _ => HandleType::Unknown,
+        }
+    }
+
+    fn is_socket_handle(&self) -> bool {
+        match self.handle_type {
+            HandleType::Socket => true,
+            HandleType::Unknown => Self::probe_handle_type(self.handle) == HandleType::Socket,
+            _ => false,
         }
-        _ => HandleType::Unknown,
     }
 }
 
@@ -107,7 +131,7 @@ impl Drop for OwnedHandle {
     fn drop(&mut self) {
         if self.handle != INVALID_HANDLE_VALUE as _ && !self.handle.is_null() {
             unsafe {
-                if handle_type(self.handle as _) == HandleType::Socket {
+                if self.is_socket_handle() {
                     closesocket(self.handle as _);
                 } else {
                     CloseHandle(self.handle as _);
@@ -119,18 +143,29 @@ impl Drop for OwnedHandle {
 
 impl FromRawHandle for OwnedHandle {
     unsafe fn from_raw_handle(handle: RawHandle) -> Self {
-        OwnedHandle { handle }
+        OwnedHandle {
+            handle,
+            handle_type: Self::probe_handle_type(handle),
+        }
     }
 }
 
 impl OwnedHandle {
     #[inline]
-    pub(crate) fn dup_impl<F: AsRawFileDescriptor>(f: &F) -> Fallible<Self> {
+    pub(crate) fn dup_impl<F: AsRawFileDescriptor>(
+        f: &F,
+        handle_type: HandleType,
+    ) -> Fallible<Self> {
         let handle = f.as_raw_file_descriptor();
         if handle == INVALID_HANDLE_VALUE as _ || handle.is_null() {
-            return Ok(OwnedHandle { handle });
+            return Ok(OwnedHandle {
+                handle,
+                handle_type,
+            });
         }
 
+        let handle_type = Self::probe_handle_type_if_unknown(handle, handle_type);
+
         let proc = unsafe { GetCurrentProcess() };
         let mut duped = INVALID_HANDLE_VALUE;
         let ok = unsafe {
@@ -149,6 +184,7 @@ impl OwnedHandle {
         } else {
             Ok(OwnedHandle {
                 handle: duped as *mut _,
+                handle_type,
             })
         }
     }
@@ -201,10 +237,7 @@ impl FromRawHandle for FileDescriptor {
 impl IntoRawSocket for FileDescriptor {
     fn into_raw_socket(self) -> RawSocket {
         // FIXME: this isn't a guaranteed conversion!
-        debug_assert_eq!(
-            handle_type(self.handle.as_raw_handle() as _),
-            HandleType::Socket
-        );
+        debug_assert!(self.handle.is_socket_handle());
         self.handle.into_raw_handle() as RawSocket
     }
 }
@@ -212,10 +245,7 @@ impl IntoRawSocket for FileDescriptor {
 impl AsRawSocket for FileDescriptor {
     fn as_raw_socket(&self) -> RawSocket {
         // FIXME: this isn't a guaranteed conversion!
-        debug_assert_eq!(
-            handle_type(self.handle.as_raw_handle() as _),
-            HandleType::Socket
-        );
+        debug_assert!(self.handle.is_socket_handle());
         self.handle.as_raw_handle() as RawSocket
     }
 }
@@ -290,10 +320,16 @@ impl Pipe {
         }
         Ok(Pipe {
             read: FileDescriptor {
-                handle: OwnedHandle { handle: read as _ },
+                handle: OwnedHandle {
+                    handle: read as _,
+                    handle_type: HandleType::Pipe,
+                },
             },
             write: FileDescriptor {
-                handle: OwnedHandle { handle: write as _ },
+                handle: OwnedHandle {
+                    handle: write as _,
+                    handle_type: HandleType::Pipe,
+                },
             },
         })
     }
@@ -326,7 +362,10 @@ fn socket(af: i32, sock_type: i32, proto: i32) -> Fallible<FileDescriptor> {
         bail!("socket failed: {}", IoError::last_os_error());
     }
     Ok(FileDescriptor {
-        handle: OwnedHandle { handle: s as _ },
+        handle: OwnedHandle {
+            handle: s as _,
+            handle_type: HandleType::Socket,
+        },
     })
 }
 
@@ -392,6 +431,7 @@ pub fn socketpair_impl() -> Fallible<(FileDescriptor, FileDescriptor)> {
     let server = FileDescriptor {
         handle: OwnedHandle {
             handle: server as _,
+            handle_type: HandleType::Socket,
         },
     };