Cygwin: FIFO: stop using overlapped I/O

Make fhandler_fifo a derived class of fhandler_base instead of
fhandler_base_overlapped.

Replace the create_pipe macro, which is based on
fhandler_pipe::create, by new create_pipe and open_pipe methods.
These use NT functions instead of Win32 functions.  Replace fifo_name
by get_pipe_name, which returns a pointer to a UNICODE_STRING.

Remove the fnevent macro, which would now be needed only once.

Add a raw_write method, adapted from fhandler_base::raw_write.

Adapt all functions to the changes above.

winsup/cygwin/fhandler.h

@@ -1234,14 +1234,21 @@ public:
   }
 };
 
-class fhandler_fifo: public fhandler_base_overlapped
+#define CYGWIN_FIFO_PIPE_NAME_LEN     47
+
+class fhandler_fifo: public fhandler_base
 {
   HANDLE read_ready;
   HANDLE write_ready;
+  UNICODE_STRING pipe_name;
+  WCHAR pipe_name_buf[CYGWIN_FIFO_PIPE_NAME_LEN + 1];
   bool __reg2 wait (HANDLE);
-  char __reg2 *fifo_name (char *, const char *);
+  NTSTATUS npfs_handle (HANDLE &);
+  HANDLE create_pipe ();
+  NTSTATUS open_pipe ();
 public:
   fhandler_fifo ();
+  PUNICODE_STRING get_pipe_name ();
   int open (int, mode_t);
   off_t lseek (off_t offset, int whence);
   int close ();
@@ -1249,6 +1256,7 @@ public:
   bool isfifo () const { return true; }
   void set_close_on_exec (bool val);
   void __reg3 raw_read (void *ptr, size_t& ulen);
+  ssize_t __reg3 raw_write (const void *ptr, size_t ulen);
   bool arm (HANDLE h);
   void fixup_after_fork (HANDLE);
   int __reg2 fstatvfs (struct statvfs *buf);
@@ -1262,7 +1270,6 @@ public:
   {
     x->pc.free_strings ();
     *reinterpret_cast<fhandler_fifo *> (x) = *this;
-    reinterpret_cast<fhandler_fifo *> (x)->atomic_write_buf = NULL;
     x->reset (this);
   }
 

winsup/cygwin/fhandler_fifo.cc

@@ -7,6 +7,7 @@
    details. */
 
 #include "winsup.h"
+#include <w32api/winioctl.h>
 #include "miscfuncs.h"
 
 #include "cygerrno.h"
@@ -21,26 +22,32 @@
 #include "ntdll.h"
 #include "cygwait.h"
 
+/* This is only to be used for writers.  When reading,
+STATUS_PIPE_EMPTY simply means there's no data to be read. */
+#define STATUS_PIPE_IS_CLOSED(status)	\
+		({ NTSTATUS _s = (status); \
+		   _s == STATUS_PIPE_CLOSING \
+		   || _s == STATUS_PIPE_BROKEN \
+		   || _s == STATUS_PIPE_EMPTY; })
+
 fhandler_fifo::fhandler_fifo ():
-  fhandler_base_overlapped (),
+  fhandler_base (),
   read_ready (NULL), write_ready (NULL)
 {
-  max_atomic_write = DEFAULT_PIPEBUFSIZE;
+  pipe_name_buf[0] = L'\0';
   need_fork_fixup (true);
 }
 
-#define fnevent(w) fifo_name (npbuf, w "-event")
+PUNICODE_STRING
-#define fnpipe() fifo_name (npbuf, "fifo")
+fhandler_fifo::get_pipe_name ()
-#define create_pipe(r, w) \
-  fhandler_pipe::create (sa_buf, (r), (w), 0, fnpipe (), open_mode)
-
-char *
-fhandler_fifo::fifo_name (char *buf, const char *what)
 {
-  /* Generate a semi-unique name to associate with this fifo. */
+  if (!pipe_name_buf[0])
-  __small_sprintf (buf, "%s.%08x.%016X", what, get_dev (),
+    {
-		   get_ino ());
+      __small_swprintf (pipe_name_buf, L"%S-fifo.%08x.%016X",
-  return buf;
+			&cygheap->installation_key, get_dev (), get_ino ());
+      RtlInitUnicodeString (&pipe_name, pipe_name_buf);
+    }
+  return &pipe_name;
 }
 
 inline PSECURITY_ATTRIBUTES
@@ -56,10 +63,8 @@ fhandler_fifo::arm (HANDLE h)
   const char *what;
   if (h == read_ready)
     what = "reader";
-  else if (h == write_ready)
-    what = "writer";
   else
-    what = "overlapped event";
+    what = "writer";
   debug_only_printf ("arming %s", what);
 #endif
 
@@ -73,6 +78,102 @@ fhandler_fifo::arm (HANDLE h)
   return res;
 }
 
+NTSTATUS
+fhandler_fifo::npfs_handle (HANDLE &nph)
+{
+  static NO_COPY SRWLOCK npfs_lock;
+  static NO_COPY HANDLE npfs_dirh;
+
+  NTSTATUS status = STATUS_SUCCESS;
+  OBJECT_ATTRIBUTES attr;
+  IO_STATUS_BLOCK io;
+
+  /* Lockless after first call. */
+  if (npfs_dirh)
+    {
+      nph = npfs_dirh;
+      return STATUS_SUCCESS;
+    }
+  AcquireSRWLockExclusive (&npfs_lock);
+  if (!npfs_dirh)
+    {
+      InitializeObjectAttributes (&attr, &ro_u_npfs, 0, NULL, NULL);
+      status = NtOpenFile (&npfs_dirh, FILE_READ_ATTRIBUTES | SYNCHRONIZE,
+			   &attr, &io, FILE_SHARE_READ | FILE_SHARE_WRITE,
+			   0);
+    }
+  ReleaseSRWLockExclusive (&npfs_lock);
+  if (NT_SUCCESS (status))
+    nph = npfs_dirh;
+  return status;
+}
+
+/* Called when pipe is opened for reading. */
+HANDLE
+fhandler_fifo::create_pipe ()
+{
+  NTSTATUS status;
+  HANDLE npfsh;
+  HANDLE ph = NULL;
+  ACCESS_MASK access;
+  OBJECT_ATTRIBUTES attr;
+  IO_STATUS_BLOCK io;
+  ULONG hattr;
+  ULONG sharing;
+  ULONG nonblocking = FILE_PIPE_QUEUE_OPERATION;
+  ULONG max_instances = 1;
+  LARGE_INTEGER timeout;
+
+  status = npfs_handle (npfsh);
+  if (!NT_SUCCESS (status))
+    {
+      __seterrno_from_nt_status (status);
+      return NULL;
+    }
+  access = GENERIC_READ | FILE_READ_ATTRIBUTES | FILE_WRITE_ATTRIBUTES
+    | SYNCHRONIZE;
+  sharing = FILE_SHARE_READ | FILE_SHARE_WRITE;
+  hattr = OBJ_INHERIT | OBJ_CASE_INSENSITIVE;
+  InitializeObjectAttributes (&attr, get_pipe_name (),
+			      hattr, npfsh, NULL);
+  timeout.QuadPart = -500000;
+  status = NtCreateNamedPipeFile (&ph, access, &attr, &io, sharing,
+				  FILE_CREATE, 0,
+				  FILE_PIPE_MESSAGE_TYPE,
+				  FILE_PIPE_MESSAGE_MODE,
+				  nonblocking, max_instances,
+				  DEFAULT_PIPEBUFSIZE, DEFAULT_PIPEBUFSIZE,
+				  &timeout);
+  if (!NT_SUCCESS (status))
+    __seterrno_from_nt_status (status);
+  return ph;
+}
+
+/* Called when file is opened for writing. */
+NTSTATUS
+fhandler_fifo::open_pipe ()
+{
+  NTSTATUS status;
+  HANDLE npfsh;
+  ACCESS_MASK access;
+  OBJECT_ATTRIBUTES attr;
+  IO_STATUS_BLOCK io;
+  ULONG sharing;
+  HANDLE ph = NULL;
+
+  status = npfs_handle (npfsh);
+  if (!NT_SUCCESS (status))
+    return status;
+  access = GENERIC_WRITE | SYNCHRONIZE;
+  InitializeObjectAttributes (&attr, get_pipe_name (), OBJ_INHERIT,
+			      npfsh, NULL);
+  sharing = FILE_SHARE_READ | FILE_SHARE_WRITE;
+  status = NtOpenFile (&ph, access, &attr, &io, sharing, 0);
+  if (NT_SUCCESS (status))
+    set_io_handle (ph);
+  return status;
+}
+
 int
 fhandler_fifo::open (int flags, mode_t)
 {
@@ -83,7 +184,7 @@ fhandler_fifo::open (int flags, mode_t)
    error_set_errno
   } res;
   bool reader, writer, duplexer;
-  DWORD open_mode = FILE_FLAG_OVERLAPPED;
+  HANDLE ph = NULL;
 
   /* Determine what we're doing with this fhandler: reading, writing, both */
   switch (flags & O_ACCMODE)
@@ -99,7 +200,6 @@ fhandler_fifo::open (int flags, mode_t)
       duplexer = false;
       break;
     case O_RDWR:
-      open_mode |= PIPE_ACCESS_DUPLEX;
       reader = true;
       writer = false;
       duplexer = true;
@@ -112,22 +212,24 @@ fhandler_fifo::open (int flags, mode_t)
 
   debug_only_printf ("reader %d, writer %d, duplexer %d", reader, writer, duplexer);
   set_flags (flags);
+  /* Create control events for this named pipe */
   char char_sa_buf[1024];
   LPSECURITY_ATTRIBUTES sa_buf;
   sa_buf = sec_user_cloexec (flags & O_CLOEXEC, (PSECURITY_ATTRIBUTES) char_sa_buf,
 		      cygheap->user.sid());
-  char npbuf[MAX_PATH];
 
-  /* Create control events for this named pipe */
+  char npbuf[MAX_PATH];
-  if (!(read_ready = CreateEvent (sa_buf, duplexer, false, fnevent ("r"))))
+  __small_sprintf (npbuf, "r-event.%08x.%016X", get_dev (), get_ino ());
+  if (!(read_ready = CreateEvent (sa_buf, duplexer, false, npbuf)))
     {
-      debug_printf ("CreatEvent for %s failed, %E", npbuf);
+      debug_printf ("CreateEvent for %s failed, %E", npbuf);
       res = error_set_errno;
       goto out;
     }
-  if (!(write_ready = CreateEvent (sa_buf, false, false, fnevent ("w"))))
+  npbuf[0] = 'w';
+  if (!(write_ready = CreateEvent (sa_buf, false, false, npbuf)))
     {
-      debug_printf ("CreatEvent for %s failed, %E", npbuf);
+      debug_printf ("CreateEvent for %s failed, %E", npbuf);
       res = error_set_errno;
       goto out;
     }
@@ -135,12 +237,13 @@ fhandler_fifo::open (int flags, mode_t)
   /* If we're reading, create the pipe, signal that we're ready and wait for
      a writer.
      FIXME: Probably need to special case O_RDWR case.  */
-  if (!reader)
+  if (reader)
-    /* We are not a reader */;
+    {
-  else if (create_pipe (&get_io_handle (), NULL))
+      ph = create_pipe ();
+      if (!ph)
 	{
 	  debug_printf ("create of reader failed");
-      res = error_set_errno;
+	  res = error_errno_set;
 	  goto out;
 	}
       else if (!arm (read_ready))
@@ -153,52 +256,35 @@ fhandler_fifo::open (int flags, mode_t)
 	  res = error_errno_set;
 	  goto out;
 	}
+      else
+	res = success;
+    }
 
-  /* If we're writing, it's a little tricky since it is possible that
+  /* If we're writing, wait for read_ready and then connect to the
-     we're attempting to open the other end of a pipe which is already
+     pipe.  Then signal write_ready.  */
-     connected.  In that case, we detect ERROR_PIPE_BUSY, reset the
-     read_ready event and wait for the reader to allow us to connect
-     by signalling read_ready.
-
-     Once the pipe has been set up, we signal write_ready.  */
   if (writer)
     {
-      int err;
-      while (1)
       if (!wait (read_ready))
 	{
 	  res = error_errno_set;
 	  goto out;
 	}
-	else if ((err = create_pipe (NULL, &get_io_handle ())) == 0)
+      NTSTATUS status = open_pipe ();
-	  break;
+      if (!NT_SUCCESS (status))
-	else if (err == ERROR_PIPE_BUSY)
-	  {
-	    debug_only_printf ("pipe busy");
-	    ResetEvent (read_ready);
-	  }
-	else
 	{
 	  debug_printf ("create of writer failed");
-	    res = error_set_errno;
+	  __seterrno_from_nt_status (status);
+	  res = error_errno_set;
 	  goto out;
 	}
-      if (!arm (write_ready))
+      else if (!arm (write_ready))
 	{
 	  res = error_set_errno;
 	  goto out;
 	}
-    }
-
-  /* If setup_overlapped() succeeds (and why wouldn't it?) we are all set. */
-  if (setup_overlapped () == 0)
-    res = success;
       else
-    {
+	res = success;
-      debug_printf ("setup_overlapped failed, %E");
-      res = error_set_errno;
     }
-
 out:
   if (res == error_set_errno)
     __seterrno ();
@@ -236,10 +322,8 @@ fhandler_fifo::wait (HANDLE h)
   const char *what;
   if (h == read_ready)
     what = "reader";
-  else if (h == write_ready)
-    what = "writer";
   else
-    what = "overlapped event";
+    what = "writer";
 #endif
   /* Set the wait to zero for non-blocking I/O-related events. */
   DWORD wait = ((h == read_ready || h == write_ready)
@@ -279,41 +363,72 @@ fhandler_fifo::wait (HANDLE h)
    }
 }
 
+ssize_t __reg3
+fhandler_fifo::raw_write (const void *ptr, size_t len)
+{
+  ssize_t ret = -1;
+  NTSTATUS status;
+  IO_STATUS_BLOCK io;
+
+  status = NtWriteFile (get_handle (), NULL, NULL, NULL, &io,
+			(PVOID) ptr, len, NULL, NULL);
+  if (NT_SUCCESS (status))
+    {
+      /* NtWriteFile returns success with # of bytes written == 0 in
+	 case writing on a non-blocking pipe fails if the pipe buffer
+	 is full. */
+      if (io.Information == 0)
+	set_errno (EAGAIN);
+      else
+	ret = io.Information;
+    }
+  else if (STATUS_PIPE_IS_CLOSED (status))
+    {
+      set_errno (EPIPE);
+      raise (SIGPIPE);
+    }
+  else
+    __seterrno_from_nt_status (status);
+  return ret;
+}
+
 void __reg3
 fhandler_fifo::raw_read (void *in_ptr, size_t& len)
 {
   size_t orig_len = len;
-  for (int i = 0; i < 2; i++)
+  while (1)
     {
-      fhandler_base_overlapped::raw_read (in_ptr, len);
+      len = orig_len;
-      if (len || i || WaitForSingleObject (read_ready, 0) != WAIT_OBJECT_0)
+      fhandler_base::raw_read (in_ptr, len);
-	break;
+      ssize_t nread = (ssize_t) len;
-      /* If we got here, then fhandler_base_overlapped::raw_read returned 0,
+      if (nread > 0)
-	 indicating "EOF" and something has set read_ready to zero.  That means
-	 we should have a client waiting to connect.
-	 FIXME: If the client CTRL-C's the open during this time then this
-	 could hang indefinitely.  Maybe implement a timeout?  */
-      if (!DisconnectNamedPipe (get_io_handle ()))
-	{
-	  debug_printf ("DisconnectNamedPipe failed, %E");
-	  goto errno_out;
-	}
-      else if (!ConnectNamedPipe (get_io_handle (), get_overlapped ())
-	       && GetLastError () != ERROR_IO_PENDING)
-	{
-	  debug_printf ("ConnectNamedPipe failed, %E");
-	  goto errno_out;
-	}
-      else if (!arm (read_ready))
-	goto errno_out;
-      else if (!wait (get_overlapped_buffer ()->hEvent))
-	goto errout;	/* If wait() fails, errno is set so no need to set it */
-      len = orig_len;	/* Reset since raw_read above set it to zero. */
-    }
 	return;
-
+      else if (nread < 0 && GetLastError () != ERROR_NO_DATA)
-errno_out:
+	goto errout;
-  __seterrno ();
+      else if (nread == 0) /* Writer has disconnected. */
+	{
+	  /* Not implemented yet. */
+	}
+      if (is_nonblocking ())
+	{
+	  set_errno (EAGAIN);
+	  goto errout;
+	}
+      else
+	{
+	  /* Allow interruption.  Copied from
+	     fhandler_socket_unix::open_reparse_point. */
+	  pthread_testcancel ();
+	  if (cygwait (NULL, cw_nowait, cw_sig_eintr) == WAIT_SIGNALED
+	      && !_my_tls.call_signal_handler ())
+	    {
+	      set_errno (EINTR);
+	      goto errout;
+	    }
+	  /* Don't hog the CPU. */
+	  Sleep (1);
+	}
+    }
 errout:
   len = -1;
 }
@@ -337,7 +452,7 @@ fhandler_fifo::close ()
 int
 fhandler_fifo::dup (fhandler_base *child, int flags)
 {
-  if (fhandler_base_overlapped::dup (child, flags))
+  if (fhandler_base::dup (child, flags))
     {
       __seterrno ();
       return -1;
@@ -366,7 +481,7 @@ fhandler_fifo::dup (fhandler_base *child, int flags)
 void
 fhandler_fifo::fixup_after_fork (HANDLE parent)
 {
-  fhandler_base_overlapped::fixup_after_fork (parent);
+  fhandler_base::fixup_after_fork (parent);
   fork_fixup (parent, read_ready, "read_ready");
   fork_fixup (parent, write_ready, "write_ready");
 }