Skip to content

Commit dd4ce2e

Browse files
committed
Cygwin: pipe: Fix unexpected blocking mode change by pipe_data_available()
ipipe_data_available() is called from raw_write(). If the pipe is in real_non_blocking_mode at that time, calling pipe_data_available() can, in some cases, inadvertently revert the pipe to blocking mode. Here is the background: pipe_data_available() checks the amount of writable space in the pipe by calling NtQueryInformationFile() with the FilePipeLocalInformation parameter. However, if the read side of the pipe is simultaneously consuming data with a large buffer, NtQueryInformationFile() may return 0 for WriteQuotaAvailable. As a workaround for this behavior, pipe_data_available() temporarily attempts to change the pipe-mode to blocking. If the pipe contains data, this operation fails-indicating that the pipe is full. If it succeeds, the pipe is considered empty. The problem arises from the assumption that the pipe is always in real blocking mode before attempting to flip the mode. However, if raw_write() has already set the pipe to non-blocking mode due to its failure to determine available space, two issues occur: 1) Changing to non-blocking mode in pipe_data_available() always succeeds, since the pipe is already in non-blocking mode. 2) After this, pipe_data_available() sets the pipe back to blocking mode, unintentionally overriding the non-blocking state required by raw_write(). This patch addresses the issue by having pipe_data_available() check the current real blocking mode, temporarily flip the pipe-mode and then restore the pipe-mode to its original state. Addresses: git-for-windows/git#5682 (comment) Fixes: 7ed9adb ("Cygwin: pipe: Switch pipe mode to blocking mode by default") Reported-by: Andrew Ng <[email protected]> Reviewed-by: Johannes Schindelin <[email protected]> Signed-off-by: Takashi Yano <[email protected]> (cherry picked from commit 940dbeffa7134dcd0795bd146f60edac3548e32e)
1 parent 1722f12 commit dd4ce2e

File tree

3 files changed

+9
-7
lines changed

3 files changed

+9
-7
lines changed

winsup/cygwin/fhandler/pipe.cc

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -326,7 +326,6 @@ fhandler_pipe::raw_read (void *ptr, size_t& len)
326326
ULONG_PTR nbytes_now = 0;
327327
ULONG len1 = (ULONG) (len - nbytes);
328328
DWORD select_sem_timeout = 0;
329-
bool real_non_blocking_mode = false;
330329

331330
FILE_PIPE_LOCAL_INFORMATION fpli;
332331
status = NtQueryInformationFile (get_handle (), &io,
@@ -453,7 +452,6 @@ fhandler_pipe_fifo::raw_write (const void *ptr, size_t len)
453452
return 0;
454453

455454
ssize_t avail = pipe_buf_size;
456-
bool real_non_blocking_mode = false;
457455

458456
/* Workaround for native ninja. Native ninja creates pipe with size == 0,
459457
and starts cygwin process with that pipe. */

winsup/cygwin/local_includes/fhandler.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1203,6 +1203,7 @@ class fhandler_pipe_fifo: public fhandler_base
12031203
protected:
12041204
size_t pipe_buf_size;
12051205
HANDLE pipe_mtx; /* Used only in the pipe case */
1206+
bool real_non_blocking_mode; /* Used only in the pipe case */
12061207
virtual void release_select_sem (const char *) {};
12071208

12081209
IMPLEMENT_STATUS_FLAG (bool, isclosed)
@@ -1212,6 +1213,8 @@ class fhandler_pipe_fifo: public fhandler_base
12121213

12131214
virtual bool reader_closed () { return false; };
12141215
ssize_t raw_write (const void *ptr, size_t len);
1216+
1217+
friend ssize_t pipe_data_available (int, fhandler_base *, HANDLE, int);
12151218
};
12161219

12171220
class fhandler_pipe: public fhandler_pipe_fifo

winsup/cygwin/select.cc

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -645,24 +645,25 @@ pipe_data_available (int fd, fhandler_base *fh, HANDLE h, int mode)
645645
and it is pending.
646646
In the latter case, the fact that the reader cannot read the data
647647
immediately means that the pipe is empty. In the former case,
648-
NtSetInformationFile() in set_pipe_non_blocking(true) will fail
649-
with STATUS_PIPE_BUSY, while it succeeds in the latter case.
648+
NtSetInformationFile() in set_pipe_non_blocking(!orig_mode) will
649+
fail with STATUS_PIPE_BUSY, while it succeeds in the latter case.
650650
Therefore, we can distinguish these cases by calling set_pipe_non_
651651
blocking(true). If it returns success, the pipe is empty, so we
652652
return the pipe buffer size. Otherwise, we return the value of
653653
WriteQuotaAvailable as is. */
654654
if (fh->get_device () == FH_PIPEW
655655
&& fpli.WriteQuotaAvailable < fpli.InboundQuota)
656656
{
657+
bool orig_mode = ((fhandler_pipe *) fh)->real_non_blocking_mode;
657658
NTSTATUS status =
658-
((fhandler_pipe *) fh)->set_pipe_non_blocking (true);
659+
((fhandler_pipe *) fh)->set_pipe_non_blocking (!orig_mode);
659660
if (status == STATUS_PIPE_BUSY)
660661
return fpli.WriteQuotaAvailable; /* Not empty */
661662
else if (!NT_SUCCESS (status))
662663
/* We cannot know actual write pipe space. */
663664
return PDA_UNKNOWN;
664-
/* Restore pipe mode to blocking mode */
665-
((fhandler_pipe *) fh)->set_pipe_non_blocking (false);
665+
/* Restore pipe mode to original blocking mode */
666+
((fhandler_pipe *) fh)->set_pipe_non_blocking (orig_mode);
666667
/* Empty */
667668
fpli.WriteQuotaAvailable = fpli.InboundQuota;
668669
}

0 commit comments

Comments
 (0)