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: https://github.com/git-for-windows/git/issues/5682 Fixes: 7ed9adb356df ("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]> --- winsup/cygwin/fhandler/pipe.cc | 15 ++++++++++----- winsup/cygwin/select.cc | 5 +++-- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/winsup/cygwin/fhandler/pipe.cc b/winsup/cygwin/fhandler/pipe.cc index e35d523bb..bda0a9b25 100644 --- a/winsup/cygwin/fhandler/pipe.cc +++ b/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; } } } diff --git a/winsup/cygwin/select.cc b/winsup/cygwin/select.cc index bb141b065..0b9afb359 100644 --- a/winsup/cygwin/select.cc +++ b/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; -- 2.45.1
