Merge pull request #103 from dscho/fix-ssh-hang-revolutions

Fix SSH hang (next attempt)

Author: dscho

winsup/cygwin/fhandler/pipe.cc

@@ -491,24 +491,29 @@ fhandler_pipe_fifo::raw_write (const void *ptr, size_t len)
 				       FilePipeLocalInformation);
       if (NT_SUCCESS (status))
 	{
-	  if (fpli.WriteQuotaAvailable != 0)
+	  if (fpli.WriteQuotaAvailable == fpli.InboundQuota)
 	    avail = fpli.WriteQuotaAvailable;
-	  else /* WriteQuotaAvailable == 0 */
+	  else /* WriteQuotaAvailable != InboundQuota */
 	    { /* Refer to the comment in select.cc: pipe_data_available(). */
 	      /* NtSetInformationFile() in set_pipe_non_blocking(true) seems
 		 to fail with STATUS_PIPE_BUSY if the pipe is not empty.
-		 In this case, the pipe is really full if WriteQuotaAvailable
-		 is zero. Otherwise, the pipe is empty. */
+		 In this case, WriteQuotaAvailable indicates real pipe space.
+		 Otherwise, the pipe is empty. */
 	      status = fh->set_pipe_non_blocking (true);
 	      if (NT_SUCCESS (status))
 		/* Pipe should be empty because reader is waiting for data. */
 		/* Restore the pipe mode to blocking. */
 		fh->set_pipe_non_blocking (false);
 	      else if (status == STATUS_PIPE_BUSY)
 		{
-		  /* Full */
-		  set_errno (EAGAIN);
-		  goto err;
+		  if (fpli.WriteQuotaAvailable == 0)
+		    {
+		      /* Full */
+		      set_errno (EAGAIN);
+		      goto err;
+		    }
+		  avail = fpli.WriteQuotaAvailable;
+		  status = STATUS_SUCCESS;
 		}
 	    }
 	}
@@ -561,7 +566,7 @@ fhandler_pipe_fifo::raw_write (const void *ptr, size_t len)
       ULONG len1;
       DWORD waitret = WAIT_OBJECT_0;
 
-      if (left > chunk && !real_non_blocking_mode)
+      if (left > chunk && !is_nonblocking ())
 	len1 = chunk;
       else
 	len1 = (ULONG) left;
@@ -650,22 +655,21 @@ fhandler_pipe_fifo::raw_write (const void *ptr, size_t len)
 	  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
+	     the pipe has less space than requested.  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. */
 	  avail = pipe_data_available (-1, this, get_handle (), PDA_WRITE);
-	  if (avail < 1)	/* error or pipe closed */
+	  if (avail == PDA_UNKNOWN && real_non_blocking_mode)
+	    avail = len1;
+	  else if (avail == 0 || !PDA_NOERROR (avail))
+	    /* 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

winsup/cygwin/local_includes/select.h

@@ -143,5 +143,8 @@ ssize_t pipe_data_available (int, fhandler_base *, HANDLE, int);
 
 #define PDA_READ	0x00
 #define PDA_WRITE	0x01
+#define PDA_ERROR	-1
+#define PDA_UNKNOWN	-2
+#define PDA_NOERROR(x)	(x >= 0)
 
 #endif /* _SELECT_H_ */

winsup/cygwin/select.cc

@@ -601,7 +601,7 @@ pipe_data_available (int fd, fhandler_base *fh, HANDLE h, int mode)
       if (mode == PDA_READ
 	  && PeekNamedPipe (h, NULL, 0, NULL, &nbytes_in_pipe, NULL))
 	return nbytes_in_pipe;
-      return -1;
+      return PDA_ERROR;
     }
 
   IO_STATUS_BLOCK iosb = {{0}, 0};
@@ -618,46 +618,49 @@ pipe_data_available (int fd, fhandler_base *fh, HANDLE h, int mode)
 	 access on the write end.  */
       select_printf ("fd %d, %s, NtQueryInformationFile failed, status %y",
 		     fd, fh->get_name (), status);
-      return (mode == PDA_WRITE) ? 1 : -1;
+      return (mode == PDA_WRITE) ? PDA_UNKNOWN : PDA_ERROR;
     }
   if (mode == 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
-        be trying to write more to the pipe than is available in the
-        buffer but that is the hazard of select().
-
-        Note that WriteQuotaAvailable is unreliable.
-
-        Usually WriteQuotaAvailable on the write side reflects the space
-        available in the inbound buffer on the read side.  However, if a
-        pipe read is currently pending, WriteQuotaAvailable on the write side
-        is decremented by the number of bytes the read side is requesting.
-        So it's possible (even likely) that WriteQuotaAvailable is 0, even
-        if the inbound buffer on the read side is not full.  This can lead to
-        a deadlock situation: The reader is waiting for data, but select
-        on the writer side assumes that no space is available in the read
-        side inbound buffer.
-
-	Consequentially, there are two possibilities when WriteQuotaAvailable
-	is 0. One is that the buffer is really full. The other is that the
-	reader is currently trying to read the pipe and it is pending.
-	In the latter case, the fact that the reader cannot read the data
-	immediately means that the pipe is empty. In the former case,
-	NtSetInformationFile() in set_pipe_non_blocking(true) will fail
-	with STATUS_PIPE_BUSY, while it succeeds in the latter case.
-	Therefore, we can distinguish these cases by calling set_pipe_non_
-	blocking(true). If it returns success, the pipe is empty, so we
-	return the pipe buffer size. Otherwise, we return 0. */
-      if (fh->get_device () == FH_PIPEW && fpli.WriteQuotaAvailable == 0)
+	 that.  This means that a pipe could still block since you could
+	 be trying to write more to the pipe than is available in the
+	 buffer but that is the hazard of select().
+
+	 Note that WriteQuotaAvailable is unreliable.
+
+	 Usually WriteQuotaAvailable on the write side reflects the space
+	 available in the inbound buffer on the read side.  However, if a
+	 pipe read is currently pending, WriteQuotaAvailable on the write side
+	 is decremented by the number of bytes the read side is requesting.
+	 So it's possible (even likely) that WriteQuotaAvailable is less than
+	 actual space available in the pipe, even if the inbound buffer is
+	 empty. This can lead to a deadlock situation: The reader is waiting
+	 for data, but select on the writer side assumes that no space is
+	 available in the read side inbound buffer.
+
+	 Consequentially, there are two possibilities when WriteQuotaAvailable
+	 is less than pipe size. One is that the buffer is really not empty.
+	 The other is that the reader is currently trying to read the pipe
+	 and it is pending.
+	 In the latter case, the fact that the reader cannot read the data
+	 immediately means that the pipe is empty. In the former case,
+	 NtSetInformationFile() in set_pipe_non_blocking(true) will fail
+	 with STATUS_PIPE_BUSY, while it succeeds in the latter case.
+	 Therefore, we can distinguish these cases by calling set_pipe_non_
+	 blocking(true). If it returns success, the pipe is empty, so we
+	 return the pipe buffer size. Otherwise, we return the value of
+	 WriteQuotaAvailable as is. */
+      if (fh->get_device () == FH_PIPEW
+	  && fpli.WriteQuotaAvailable < fpli.InboundQuota)
 	{
 	  NTSTATUS status =
 	    ((fhandler_pipe *) fh)->set_pipe_non_blocking (true);
 	  if (status == STATUS_PIPE_BUSY)
-	    return 0; /* Full */
+	    return fpli.WriteQuotaAvailable; /* Not empty */
 	  else if (!NT_SUCCESS (status))
 	    /* We cannot know actual write pipe space. */
-	    return 1;
+	    return PDA_UNKNOWN;
 	  /* Restore pipe mode to blocking mode */
 	  ((fhandler_pipe *) fh)->set_pipe_non_blocking (false);
 	  /* Empty */
@@ -681,7 +684,7 @@ pipe_data_available (int fd, fhandler_base *fh, HANDLE h, int mode)
       return fpli.ReadDataAvailable;
     }
   if (fpli.NamedPipeState & FILE_PIPE_CLOSING_STATE)
-    return -1;
+    return PDA_ERROR;
   return 0;
 }
 
@@ -731,7 +734,7 @@ peek_pipe (select_record *s, bool from_select)
       if (n == 0 && fh->get_echo_handle ())
 	n = pipe_data_available (s->fd, fh, fh->get_echo_handle (), PDA_READ);
 
-      if (n < 0)
+      if (n == PDA_ERROR)
 	{
 	  select_printf ("read: %s, n %d", fh->get_name (), n);
 	  if (s->except_selected)
@@ -772,8 +775,8 @@ peek_pipe (select_record *s, bool from_select)
 	}
       ssize_t n = pipe_data_available (s->fd, fh, h, PDA_WRITE);
       select_printf ("write: %s, n %d", fh->get_name (), n);
-      gotone += s->write_ready = (n > 0);
-      if (n < 0 && s->except_selected)
+      gotone += s->write_ready = (n > 0 || n == PDA_UNKNOWN);
+      if (n == PDA_ERROR && s->except_selected)
 	gotone += s->except_ready = true;
     }
   return gotone;
@@ -986,7 +989,7 @@ peek_fifo (select_record *s, bool from_select)
       ssize_t n = pipe_data_available (s->fd, fh, fh->get_handle (), PDA_WRITE);
       select_printf ("write: %s, n %d", fh->get_name (), n);
       gotone += s->write_ready = (n > 0);
-      if (n < 0 && s->except_selected)
+      if (n == PDA_ERROR && s->except_selected)
 	gotone += s->except_ready = true;
     }
   return gotone;
@@ -1412,7 +1415,7 @@ peek_pty_slave (select_record *s, bool from_select)
       ssize_t n = pipe_data_available (s->fd, fh, h, PDA_WRITE);
       select_printf ("write: %s, n %d", fh->get_name (), n);
       gotone += s->write_ready = (n > 0);
-      if (n < 0 && s->except_selected)
+      if (n == PDA_ERROR && s->except_selected)
 	gotone += s->except_ready = true;
     }
   return gotone;