https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;h=170e6badb621fc07d236e540c693ea5deb53fc27
commit 170e6badb621fc07d236e540c693ea5deb53fc27 Author: Corinna Vinschen <[email protected]> AuthorDate: Sun Aug 18 22:07:16 2024 +0200 Commit: Corinna Vinschen <[email protected]> CommitDate: Sun Aug 18 22:07:16 2024 +0200 Cygwin: pipe: improve writing when pipe buffer is almost full So far fhandler_pipe_fifo::raw_write always returns -1/EINTR if a signal arrived. Linux does not do that if there's still space left in the pipe buffer. The Linux buffer handling can't be emulated by Cygwin, but we can do something similar which makes it much more likely to still write successfully even if the buffer is almost full. Utilize pipe_data_available to return valid pipe buffer usage to raw_write, allowing a more sophisticated way to fill the buffer while maintaining comaptibility with non-Cygwin pipes. Signed-off-by: Corinna Vinschen <[email protected]> Diff: --- winsup/cygwin/fhandler/pipe.cc | 44 +++++++++++++++++++++++++++++++---- winsup/cygwin/local_includes/select.h | 5 ++++ winsup/cygwin/select.cc | 44 ++++++++++++++++++++++++----------- 3 files changed, 75 insertions(+), 18 deletions(-) diff --git a/winsup/cygwin/fhandler/pipe.cc b/winsup/cygwin/fhandler/pipe.cc index ce6099d8bc52..33ab11744e91 100644 --- a/winsup/cygwin/fhandler/pipe.cc +++ b/winsup/cygwin/fhandler/pipe.cc @@ -13,6 +13,7 @@ details. */ #include "security.h" #include "path.h" #include "fhandler.h" +#include "select.h" #include "dtable.h" #include "cygheap.h" #include "pinfo.h" @@ -433,6 +434,7 @@ fhandler_pipe_fifo::raw_write (const void *ptr, size_t len) NTSTATUS status = STATUS_SUCCESS; IO_STATUS_BLOCK io; HANDLE evt; + bool short_write_once = false; if (!len) return 0; @@ -471,6 +473,7 @@ fhandler_pipe_fifo::raw_write (const void *ptr, size_t len) len1 = chunk; else len1 = (ULONG) left; + /* NtWriteFile returns success with # of bytes written == 0 if writing on a non-blocking pipe fails because the pipe buffer doesn't have sufficient space. @@ -517,6 +520,13 @@ fhandler_pipe_fifo::raw_write (const void *ptr, size_t len) } else cygwait (select_sem, 10, cw_cancel); + /* If we got a timeout in the blocking case, and we already + did a short write, we got a signal in the previous loop. */ + if (waitret == WAIT_TIMEOUT && short_write_once) + { + waitret = WAIT_SIGNALED; + break; + } } /* Loop in case of blocking write or SA_RESTART */ while (waitret == WAIT_TIMEOUT || waitret == WAIT_SIGNALED); @@ -533,13 +543,39 @@ fhandler_pipe_fifo::raw_write (const void *ptr, size_t len) status = STATUS_THREAD_CANCELED; else if (waitret == WAIT_SIGNALED) status = STATUS_THREAD_SIGNALED; + else if (waitret == WAIT_TIMEOUT && io.Status == STATUS_CANCELLED) + status = STATUS_SUCCESS; else status = io.Status; } - if (!is_nonblocking () || !NT_SUCCESS (status) || io.Information > 0 - || len <= PIPE_BUF) + if (status != STATUS_THREAD_SIGNALED && !NT_SUCCESS (status)) break; - len1 >>= 1; + 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 + 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 + strategy because Linux is filling the pages of a pipe buffer + in a very implementation-defined way we can't emulate, but it + resembles it closely enough to get useful results. */ + ssize_t avail = pipe_data_available (-1, this, get_handle (), + PDA_WRITE); + if (avail < 1) /* error or pipe closed */ + break; + if (avail >= len1) /* somebody read from the pipe */ + avail = len1; + if (avail == 1) /* 1 byte left or non-Cygwin pipe */ + len1 >>= 1; + else if (avail >= PIPE_BUF) + len1 = avail & ~(PIPE_BUF - 1); + else + len1 = 1 << (31 - __builtin_clzl (avail)); + if (!is_nonblocking ()) + short_write_once = true; } if (isclosed ()) /* A signal handler might have closed the fd. */ { @@ -569,7 +605,7 @@ fhandler_pipe_fifo::raw_write (const void *ptr, size_t len) else __seterrno_from_nt_status (status); - if (nbytes_now == 0) + if (nbytes_now == 0 || short_write_once) break; } out: diff --git a/winsup/cygwin/local_includes/select.h b/winsup/cygwin/local_includes/select.h index 4e202128fec8..dea023343f7c 100644 --- a/winsup/cygwin/local_includes/select.h +++ b/winsup/cygwin/local_includes/select.h @@ -139,4 +139,9 @@ public: extern "C" int cygwin_select (int , fd_set *, fd_set *, fd_set *, struct timeval *to); +ssize_t pipe_data_available (int, fhandler_base *, HANDLE, int); + +#define PDA_WRITE 0x01 +#define PDA_SELECT 0x02 + #endif /* _SELECT_H_ */ diff --git a/winsup/cygwin/select.cc b/winsup/cygwin/select.cc index db52ffcccf8e..bc02c3f9d466 100644 --- a/winsup/cygwin/select.cc +++ b/winsup/cygwin/select.cc @@ -584,13 +584,14 @@ no_verify (select_record *, fd_set *, fd_set *, fd_set *) return 0; } -static int -pipe_data_available (int fd, fhandler_base *fh, HANDLE h, bool writing) +ssize_t +pipe_data_available (int fd, fhandler_base *fh, HANDLE h, int flags) { if (fh->get_device () == FH_PIPER) { DWORD nbytes_in_pipe; - if (!writing && PeekNamedPipe (h, NULL, 0, NULL, &nbytes_in_pipe, NULL)) + if (!(flags & PDA_WRITE) + && PeekNamedPipe (h, NULL, 0, NULL, &nbytes_in_pipe, NULL)) return nbytes_in_pipe; return -1; } @@ -609,9 +610,17 @@ pipe_data_available (int fd, fhandler_base *fh, HANDLE h, bool writing) access on the write end. */ select_printf ("fd %d, %s, NtQueryInformationFile failed, status %y", fd, fh->get_name (), status); - return writing ? PIPE_BUF : -1; + switch (flags) + { + case PDA_WRITE: + return 1; + case PDA_SELECT | PDA_WRITE: + return PIPE_BUF; + default: + return -1; + } } - if (writing) + if (flags & 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 @@ -638,20 +647,26 @@ pipe_data_available (int fd, fhandler_base *fh, HANDLE h, bool writing) /* Note: Do not use NtQueryInformationFile() for query_hdl because NtQueryInformationFile() seems to interfere with reading pipes in non-cygwin apps. Instead, use PeekNamedPipe() here. */ + /* Note 2: we return the number of available bytes. Select for writing + returns writable *only* if at least PIPE_BUF bytes are left in the + buffer. If we can't fetch the real number of available bytes, the + number of bytes returned depends on the caller. For select we return + PIPE_BUF to fake writability, for writing we return 1 to allow + handling this fact. */ if (fh->get_device () == FH_PIPEW && fpli.WriteQuotaAvailable == 0) { HANDLE query_hdl = ((fhandler_pipe *) fh)->get_query_handle (); if (!query_hdl) query_hdl = ((fhandler_pipe *) fh)->temporary_query_hdl (); - if (!query_hdl) - return PIPE_BUF; /* We cannot know actual write pipe space. */ + if (!query_hdl) /* We cannot know actual write pipe space. */ + return (flags & PDA_SELECT) ? PIPE_BUF : 1; DWORD nbytes_in_pipe; BOOL res = PeekNamedPipe (query_hdl, NULL, 0, NULL, &nbytes_in_pipe, NULL); if (!((fhandler_pipe *) fh)->get_query_handle ()) CloseHandle (query_hdl); /* Close temporary query_hdl */ - if (!res) - return PIPE_BUF; /* We cannot know actual write pipe space. */ + if (!res) /* We cannot know actual write pipe space. */ + return (flags & PDA_SELECT) ? PIPE_BUF : 1; fpli.WriteQuotaAvailable = fpli.InboundQuota - nbytes_in_pipe; } if (fpli.WriteQuotaAvailable > 0) @@ -715,10 +730,10 @@ peek_pipe (select_record *s, bool from_select) gotone = s->read_ready = true; goto out; } - int n = pipe_data_available (s->fd, fh, h, false); + ssize_t n = pipe_data_available (s->fd, fh, h, PDA_SELECT); /* On PTY masters, check if input from the echo pipe is available. */ if (n == 0 && fh->get_echo_handle ()) - n = pipe_data_available (s->fd, fh, fh->get_echo_handle (), false); + n = pipe_data_available (s->fd, fh, fh->get_echo_handle (), PDA_SELECT); if (n < 0) { @@ -759,7 +774,7 @@ out: gotone += s->except_ready = true; return gotone; } - int n = pipe_data_available (s->fd, fh, h, true); + ssize_t n = pipe_data_available (s->fd, fh, h, PDA_SELECT | PDA_WRITE); select_printf ("write: %s, n %d", fh->get_name (), n); gotone += s->write_ready = (n >= PIPE_BUF); if (n < 0 && s->except_selected) @@ -972,7 +987,8 @@ peek_fifo (select_record *s, bool from_select) out: if (s->write_selected) { - int n = pipe_data_available (s->fd, fh, fh->get_handle (), true); + ssize_t n = pipe_data_available (s->fd, fh, fh->get_handle (), + PDA_SELECT | PDA_WRITE); select_printf ("write: %s, n %d", fh->get_name (), n); gotone += s->write_ready = (n >= PIPE_BUF); if (n < 0 && s->except_selected) @@ -1398,7 +1414,7 @@ out: HANDLE h = ptys->get_output_handle (); if (s->write_selected) { - int n = pipe_data_available (s->fd, fh, h, true); + ssize_t n = pipe_data_available (s->fd, fh, h, PDA_SELECT | PDA_WRITE); select_printf ("write: %s, n %d", fh->get_name (), n); gotone += s->write_ready = (n >= PIPE_BUF); if (n < 0 && s->except_selected)
