Cygwin: pipe: Fix SSH hang with non-cygwin pipe reader

If ssh is used with non-cygwin pipe reader, ssh some times hangs.
This happens when non-cygwin git (Git for Windows) starts cygwin
ssh. The background of the bug is as follows.

Before attempting to NtWriteFile() in raw_write() in non-blocking
mode, the amount of writable space in the pipe is checked by calling
NtQueryInformationFile with FilePipeLocalInformation parameter.
The same is also done by pipe_data_available() in select.cc.

However, if the read side of the pipe is simultaneously consuming
data, NtQueryInformationFile() returns less value than the amount
of writable space, i.e. the amount of writable space minus the size
of buffer to be read. This does not happen when the reader is a
cygwin app because cygwin read() for the pipe attempts to read
the amount of the data in the pipe at most. This means NtReadFile()
never enters a pending state. However, if the reader is non-cygwin
app, this cannot be expected. As a workaround for this problem,
the code checking the pipe space temporarily attempts to toggle
the pipe-mode. If the pipe contains data, this operation fails
with STATUS_PIPE_BUSY indicating that the pipe is not empty. If
it succeeds, the pipe is considered empty. The current code uses
this technic only when NtQueryInformationFile() retuns zero.

Therefore, if NtQueryInformationFile() returns 1, the amount of
writable space is assumed to be 1 even in the case that e.g. the
pipe size is 8192 bytes and reader is pending to read 8191 bytes.
Even worse, the current code fails to write more than 1 byte
to 1 byte pipe space due to the remnant of the past design.
Then the reader waits for data with 8191 bytes buffer while the
writer continues to fail to write to 1 byte space of the pipe.
This is the cause of the deadlock.

In practice, when using Git for Windows in combination with Cygwin
SSH, it has been observed that a read of 8191 bytes is occasionally
issued against a pipe with 8192 bytes of available space.

With this patch, the blocking-mode-toggling-check is performed
even if NtQueryInformationFile() returns non-zero value so that
the amount of the writable space in the pipe is always estimated
correctly.

Addresses: git-for-windows/git#5682
Fixes: 7ed9adb ("Cygwin: pipe: Switch pipe mode to blocking mode by default")
Reported-by: Vincent-Liem (@github), Johannes Schindelin <[email protected]>
Reviewed-by:
Signed-off-by: Takashi Yano <[email protected]>
Signed-off-by: Johannes Schindelin <[email protected]>

Author: tyan0

winsup/cygwin/fhandler/pipe.cc

@@ -491,9 +491,9 @@ 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.
@@ -506,9 +506,14 @@ fhandler_pipe_fifo::raw_write (const void *ptr, size_t len)
 		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;
 		}
 	    }
 	}

winsup/cygwin/select.cc

@@ -649,12 +649,13 @@ pipe_data_available (int fd, fhandler_base *fh, HANDLE h, int mode)
 	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)
+      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;