Cygwin: pipe: improve writing when pipe buffer is almost full

So far fhandler_pipe_fifo::raw_write always returns -1/EINTR
if a signal arrived.  Linux does not do that if there's still
space left in the pipe buffer.

The Linux buffer handling can't be emulated by Cygwin, but we
can do something similar which makes it much more likely to still
write successfully even if the buffer is almost full.

Utilize pipe_data_available to return valid pipe buffer usage
to raw_write, allowing a more sophisticated way to fill the
buffer while maintaining comaptibility with non-Cygwin pipes.

Signed-off-by: Corinna Vinschen <corinna@vinschen.de>

3 files changed, 75 insertions, 18 deletions

diff --git a/winsup/cygwin/fhandler/pipe.cc b/winsup/cygwin/fhandler/pipe.cc
index ce6099d..33ab117 100644
--- a/winsup/cygwin/fhandler/pipe.cc
+++ b/winsup/cygwin/fhandler/pipe.cc
@@ -13,6 +13,7 @@ details. */
 #include "security.h"
 #include "path.h"
 #include "fhandler.h"
+#include "select.h"
 #include "dtable.h"
 #include "cygheap.h"
 #include "pinfo.h"
@@ -433,6 +434,7 @@ fhandler_pipe_fifo::raw_write (const void *ptr, size_t len)
   NTSTATUS status = STATUS_SUCCESS;
   IO_STATUS_BLOCK io;
   HANDLE evt;
+  bool short_write_once = false;
 
   if (!len)
     return 0;
@@ -471,6 +473,7 @@ fhandler_pipe_fifo::raw_write (const void *ptr, size_t len)
 	len1 = chunk;
       else
 	len1 = (ULONG) left;
+
       /* NtWriteFile returns success with # of bytes written == 0 if writing
          on a non-blocking pipe fails because the pipe buffer doesn't have
 	 sufficient space.
@@ -517,6 +520,13 @@ fhandler_pipe_fifo::raw_write (const void *ptr, size_t len)
 		    }
 		  else
 		    cygwait (select_sem, 10, cw_cancel);
+		  /* If we got a timeout in the blocking case, and we already
+		     did a short write, we got a signal in the previous loop. */
+		  if (waitret == WAIT_TIMEOUT && short_write_once)
+		    {
+		      waitret = WAIT_SIGNALED;
+		      break;
+		    }
 		}
 	      /* Loop in case of blocking write or SA_RESTART */
 	      while (waitret == WAIT_TIMEOUT || waitret == WAIT_SIGNALED);
@@ -533,13 +543,39 @@ fhandler_pipe_fifo::raw_write (const void *ptr, size_t len)
 		status = STATUS_THREAD_CANCELED;
 	      else if (waitret == WAIT_SIGNALED)
 		status = STATUS_THREAD_SIGNALED;
+	      else if (waitret == WAIT_TIMEOUT && io.Status == STATUS_CANCELLED)
+		status = STATUS_SUCCESS;
 	      else
 		status = io.Status;
 	    }
-	  if (!is_nonblocking () || !NT_SUCCESS (status) || io.Information > 0
-	      || len <= PIPE_BUF)
+	  if (status != STATUS_THREAD_SIGNALED && !NT_SUCCESS (status))
 	    break;
-	  len1 >>= 1;
+	  if (io.Information > 0 || len <= PIPE_BUF || short_write_once)
+	    break;
+	  /* Independent of being blocking or non-blocking, if we're here,
+	     the pipe has less space than requested.  If the pipe is a
+	     non-Cygwin pipe, just try the old strategy of trying a half
+	     write.  If the pipe has at
+	     least PIPE_BUF bytes available, try to write all matching
+	     PIPE_BUF sized blocks.  If it's less than PIPE_BUF,  try
+	     the next less power of 2 bytes.  This is not really the Linux
+	     strategy because Linux is filling the pages of a pipe buffer
+	     in a very implementation-defined way we can't emulate, but it
+	     resembles it closely enough to get useful results. */
+	  ssize_t avail = pipe_data_available (-1, this, get_handle (),
+					       PDA_WRITE);
+	  if (avail < 1)	/* error or pipe closed */
+	    break;
+	  if (avail >= len1)	/* somebody read from the pipe */
+	    avail = len1;
+	  if (avail == 1)	/* 1 byte left or non-Cygwin pipe */
+	    len1 >>= 1;
+	  else if (avail >= PIPE_BUF)
+	    len1 = avail & ~(PIPE_BUF - 1);
+	  else
+	    len1 = 1 << (31 - __builtin_clzl (avail));
+	  if (!is_nonblocking ())
+	    short_write_once = true;
 	}
       if (isclosed ())  /* A signal handler might have closed the fd. */
 	{
@@ -569,7 +605,7 @@ fhandler_pipe_fifo::raw_write (const void *ptr, size_t len)
       else
 	__seterrno_from_nt_status (status);
 
-      if (nbytes_now == 0)
+      if (nbytes_now == 0 || short_write_once)
 	break;
     }
 out:
diff --git a/winsup/cygwin/local_includes/select.h b/winsup/cygwin/local_includes/select.h
index 4e20212..dea0233 100644
--- a/winsup/cygwin/local_includes/select.h
+++ b/winsup/cygwin/local_includes/select.h
@@ -139,4 +139,9 @@ public:
 extern "C" int cygwin_select (int , fd_set *, fd_set *, fd_set *,
 			      struct timeval *to);
 
+ssize_t pipe_data_available (int, fhandler_base *, HANDLE, int);
+
+#define PDA_WRITE	0x01
+#define PDA_SELECT	0x02
+
 #endif /* _SELECT_H_ */
diff --git a/winsup/cygwin/select.cc b/winsup/cygwin/select.cc
index db52ffc..bc02c3f 100644
--- a/winsup/cygwin/select.cc
+++ b/winsup/cygwin/select.cc
@@ -584,13 +584,14 @@ no_verify (select_record *, fd_set *, fd_set *, fd_set *)
   return 0;
 }
 
-static int
-pipe_data_available (int fd, fhandler_base *fh, HANDLE h, bool writing)
+ssize_t
+pipe_data_available (int fd, fhandler_base *fh, HANDLE h, int flags)
 {
   if (fh->get_device () == FH_PIPER)
     {
       DWORD nbytes_in_pipe;
-      if (!writing && PeekNamedPipe (h, NULL, 0, NULL, &nbytes_in_pipe, NULL))
+      if (!(flags & PDA_WRITE)
+	  && PeekNamedPipe (h, NULL, 0, NULL, &nbytes_in_pipe, NULL))
 	return nbytes_in_pipe;
       return -1;
     }
@@ -609,9 +610,17 @@ pipe_data_available (int fd, fhandler_base *fh, HANDLE h, bool writing)
 	 access on the write end.  */
       select_printf ("fd %d, %s, NtQueryInformationFile failed, status %y",
 		     fd, fh->get_name (), status);
-      return writing ? PIPE_BUF : -1;
+      switch (flags)
+	{
+	case PDA_WRITE:
+	  return 1;
+	case PDA_SELECT | PDA_WRITE:
+	  return PIPE_BUF;
+	default:
+	  return -1;
+	}
     }
-  if (writing)
+  if (flags & PDA_WRITE)
     {
       /* If there is anything available in the pipe buffer then signal
         that.  This means that a pipe could still block since you could
@@ -638,20 +647,26 @@ pipe_data_available (int fd, fhandler_base *fh, HANDLE h, bool writing)
       /* Note: Do not use NtQueryInformationFile() for query_hdl because
 	 NtQueryInformationFile() seems to interfere with reading pipes
 	 in non-cygwin apps. Instead, use PeekNamedPipe() here. */
+      /* Note 2: we return the number of available bytes.  Select for writing
+         returns writable *only* if at least PIPE_BUF bytes are left in the
+	 buffer.  If we can't fetch the real number of available bytes, the
+	 number of bytes returned depends on the caller.  For select we return
+	 PIPE_BUF to fake writability, for writing we return 1 to allow
+	 handling this fact. */
       if (fh->get_device () == FH_PIPEW && fpli.WriteQuotaAvailable == 0)
 	{
 	  HANDLE query_hdl = ((fhandler_pipe *) fh)->get_query_handle ();
 	  if (!query_hdl)
 	    query_hdl = ((fhandler_pipe *) fh)->temporary_query_hdl ();
-	  if (!query_hdl)
-	    return PIPE_BUF; /* We cannot know actual write pipe space. */
+	  if (!query_hdl) /* We cannot know actual write pipe space. */
+	    return (flags & PDA_SELECT) ? PIPE_BUF : 1;
 	  DWORD nbytes_in_pipe;
 	  BOOL res =
 	    PeekNamedPipe (query_hdl, NULL, 0, NULL, &nbytes_in_pipe, NULL);
 	  if (!((fhandler_pipe *) fh)->get_query_handle ())
 	    CloseHandle (query_hdl); /* Close temporary query_hdl */
-	  if (!res)
-	    return PIPE_BUF; /* We cannot know actual write pipe space. */
+	  if (!res) /* We cannot know actual write pipe space. */
+	    return (flags & PDA_SELECT) ? PIPE_BUF : 1;
 	  fpli.WriteQuotaAvailable = fpli.InboundQuota - nbytes_in_pipe;
 	}
       if (fpli.WriteQuotaAvailable > 0)
@@ -715,10 +730,10 @@ peek_pipe (select_record *s, bool from_select)
 	  gotone = s->read_ready = true;
 	  goto out;
 	}
-      int n = pipe_data_available (s->fd, fh, h, false);
+      ssize_t n = pipe_data_available (s->fd, fh, h, PDA_SELECT);
       /* On PTY masters, check if input from the echo pipe is available. */
       if (n == 0 && fh->get_echo_handle ())
-	n = pipe_data_available (s->fd, fh, fh->get_echo_handle (), false);
+	n = pipe_data_available (s->fd, fh, fh->get_echo_handle (), PDA_SELECT);
 
       if (n < 0)
 	{
@@ -759,7 +774,7 @@ out:
 	    gotone += s->except_ready = true;
 	  return gotone;
 	}
-      int n = pipe_data_available (s->fd, fh, h, true);
+      ssize_t n = pipe_data_available (s->fd, fh, h, PDA_SELECT | PDA_WRITE);
       select_printf ("write: %s, n %d", fh->get_name (), n);
       gotone += s->write_ready = (n >= PIPE_BUF);
       if (n < 0 && s->except_selected)
@@ -972,7 +987,8 @@ peek_fifo (select_record *s, bool from_select)
 out:
   if (s->write_selected)
     {
-      int n = pipe_data_available (s->fd, fh, fh->get_handle (), true);
+      ssize_t n = pipe_data_available (s->fd, fh, fh->get_handle (),
+				       PDA_SELECT | PDA_WRITE);
       select_printf ("write: %s, n %d", fh->get_name (), n);
       gotone += s->write_ready = (n >= PIPE_BUF);
       if (n < 0 && s->except_selected)
@@ -1398,7 +1414,7 @@ out:
   HANDLE h = ptys->get_output_handle ();
   if (s->write_selected)
     {
-      int n = pipe_data_available (s->fd, fh, h, true);
+      ssize_t n = pipe_data_available (s->fd, fh, h, PDA_SELECT | PDA_WRITE);
       select_printf ("write: %s, n %d", fh->get_name (), n);
       gotone += s->write_ready = (n >= PIPE_BUF);
       if (n < 0 && s->except_selected)