Skip to content

Commit 034c762

Browse files
tyan0dscho
authored andcommitted
Cygwin: pipe; Update source comment align with previous commit
The commit "Cygwin: pipe: Fix SSH hang with non-cygwin pipe reader" modifies how the amount of writable data is evaluated. This patch updates the source comments to align with that change. Reviewed-by: Signed-off-by: Takashi Yano <[email protected]> Signed-off-by: Johannes Schindelin <[email protected]>
1 parent 0ae6a6f commit 034c762

File tree

2 files changed

+31
-31
lines changed

2 files changed

+31
-31
lines changed

winsup/cygwin/fhandler/pipe.cc

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -497,8 +497,8 @@ fhandler_pipe_fifo::raw_write (const void *ptr, size_t len)
497497
{ /* Refer to the comment in select.cc: pipe_data_available(). */
498498
/* NtSetInformationFile() in set_pipe_non_blocking(true) seems
499499
to fail with STATUS_PIPE_BUSY if the pipe is not empty.
500-
In this case, the pipe is really full if WriteQuotaAvailable
501-
is zero. Otherwise, the pipe is empty. */
500+
In this case, WriteQuotaAvailable indicates real pipe space.
501+
Otherwise, the pipe is empty. */
502502
status = fh->set_pipe_non_blocking (true);
503503
if (NT_SUCCESS (status))
504504
/* Pipe should be empty because reader is waiting for data. */
@@ -655,9 +655,7 @@ fhandler_pipe_fifo::raw_write (const void *ptr, size_t len)
655655
if (io.Information > 0 || len <= PIPE_BUF || short_write_once)
656656
break;
657657
/* Independent of being blocking or non-blocking, if we're here,
658-
the pipe has less space than requested. If the pipe is a
659-
non-Cygwin pipe, just try the old strategy of trying a half
660-
write. If the pipe has at
658+
the pipe has less space than requested. If the pipe has at
661659
least PIPE_BUF bytes available, try to write all matching
662660
PIPE_BUF sized blocks. If it's less than PIPE_BUF, try
663661
the next less power of 2 bytes. This is not really the Linux

winsup/cygwin/select.cc

Lines changed: 28 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -623,32 +623,34 @@ pipe_data_available (int fd, fhandler_base *fh, HANDLE h, int mode)
623623
if (mode == PDA_WRITE)
624624
{
625625
/* If there is anything available in the pipe buffer then signal
626-
that. This means that a pipe could still block since you could
627-
be trying to write more to the pipe than is available in the
628-
buffer but that is the hazard of select().
629-
630-
Note that WriteQuotaAvailable is unreliable.
631-
632-
Usually WriteQuotaAvailable on the write side reflects the space
633-
available in the inbound buffer on the read side. However, if a
634-
pipe read is currently pending, WriteQuotaAvailable on the write side
635-
is decremented by the number of bytes the read side is requesting.
636-
So it's possible (even likely) that WriteQuotaAvailable is 0, even
637-
if the inbound buffer on the read side is not full. This can lead to
638-
a deadlock situation: The reader is waiting for data, but select
639-
on the writer side assumes that no space is available in the read
640-
side inbound buffer.
641-
642-
Consequentially, there are two possibilities when WriteQuotaAvailable
643-
is 0. One is that the buffer is really full. The other is that the
644-
reader is currently trying to read the pipe and it is pending.
645-
In the latter case, the fact that the reader cannot read the data
646-
immediately means that the pipe is empty. In the former case,
647-
NtSetInformationFile() in set_pipe_non_blocking(true) will fail
648-
with STATUS_PIPE_BUSY, while it succeeds in the latter case.
649-
Therefore, we can distinguish these cases by calling set_pipe_non_
650-
blocking(true). If it returns success, the pipe is empty, so we
651-
return the pipe buffer size. Otherwise, we return 0. */
626+
that. This means that a pipe could still block since you could
627+
be trying to write more to the pipe than is available in the
628+
buffer but that is the hazard of select().
629+
630+
Note that WriteQuotaAvailable is unreliable.
631+
632+
Usually WriteQuotaAvailable on the write side reflects the space
633+
available in the inbound buffer on the read side. However, if a
634+
pipe read is currently pending, WriteQuotaAvailable on the write side
635+
is decremented by the number of bytes the read side is requesting.
636+
So it's possible (even likely) that WriteQuotaAvailable is less than
637+
actual space available in the pipe, even if the inbound buffer is
638+
empty. This can lead to a deadlock situation: The reader is waiting
639+
for data, but select on the writer side assumes that no space is
640+
available in the read side inbound buffer.
641+
642+
Consequentially, there are two possibilities when WriteQuotaAvailable
643+
is less than pipe size. One is that the buffer is really not empty.
644+
The other is that the reader is currently trying to read the pipe
645+
and it is pending.
646+
In the latter case, the fact that the reader cannot read the data
647+
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.
650+
Therefore, we can distinguish these cases by calling set_pipe_non_
651+
blocking(true). If it returns success, the pipe is empty, so we
652+
return the pipe buffer size. Otherwise, we return the value of
653+
WriteQuotaAvailable as is. */
652654
if (fh->get_device () == FH_PIPEW
653655
&& fpli.WriteQuotaAvailable < fpli.InboundQuota)
654656
{

0 commit comments

Comments
 (0)