[newlib-cygwin] Cygwin: pipe; Update source comment align with previous commit
Takashi Yano via Cygwin-cvs <[email protected]>
| Newsgroups | gmane.os.cygwin.cvs |
|---|---|
| Message-ID | <[email protected]> |
https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;h=82d4ad1dfb28c72d5ce5fd3135ff4dde99e85fca commit 82d4ad1dfb28c72d5ce5fd3135ff4dde99e85fca Author: Takashi Yano <[email protected]> Date: Fri Jun 27 20:02:31 2025 +0900 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: Johannes Schindelin <[email protected]> Signed-off-by: Takashi Yano <[email protected]> Diff: --- winsup/cygwin/fhandler/pipe.cc | 8 +++---- winsup/cygwin/select.cc | 54 ++++++++++++++++++++++-------------------- 2 files changed, 31 insertions(+), 31 deletions(-) diff --git a/winsup/cygwin/fhandler/pipe.cc b/winsup/cygwin/fhandler/pipe.cc index bda0a9b25..22addbb18 100644 --- a/winsup/cygwin/fhandler/pipe.cc +++ b/winsup/cygwin/fhandler/pipe.cc @@ -497,8 +497,8 @@ fhandler_pipe_fifo::raw_write (const void *ptr, size_t len) { /* 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. */ @@ -655,9 +655,7 @@ 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 diff --git a/winsup/cygwin/select.cc b/winsup/cygwin/select.cc index 0b9afb359..701f4d9a6 100644 --- a/winsup/cygwin/select.cc +++ b/winsup/cygwin/select.cc @@ -623,32 +623,34 @@ pipe_data_available (int fd, fhandler_base *fh, HANDLE h, int mode) 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. */ + 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) {