diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h index 132e6002133b..1f0f28077a7c 100644 --- a/winsup/cygwin/fhandler.h +++ b/winsup/cygwin/fhandler.h @@ -1171,6 +1171,7 @@ class fhandler_socket_unix : public fhandler_socket class fhandler_pipe: public fhandler_base { private: + HANDLE query_hdl; pid_t popen_pid; size_t max_atomic_write; void set_pipe_non_blocking (bool nonblocking); @@ -1179,6 +1180,8 @@ public: bool ispipe() const { return true; } + HANDLE get_query_handle () const { return query_hdl; } + void set_popen_pid (pid_t pid) {popen_pid = pid;} pid_t get_popen_pid () const {return popen_pid;} off_t lseek (off_t offset, int whence); @@ -1187,7 +1190,9 @@ public: select_record *select_except (select_stuff *); char *get_proc_fd_name (char *buf); int open (int flags, mode_t mode = 0); + void fixup_after_fork (HANDLE); int dup (fhandler_base *child, int); + int close (); void __reg3 raw_read (void *ptr, size_t& len); ssize_t __reg3 raw_write (const void *ptr, size_t len); int ioctl (unsigned int cmd, void *); diff --git a/winsup/cygwin/fhandler_pipe.cc b/winsup/cygwin/fhandler_pipe.cc index 2dec0a84817c..479b62bbd4aa 100644 --- a/winsup/cygwin/fhandler_pipe.cc +++ b/winsup/cygwin/fhandler_pipe.cc @@ -405,22 +405,45 @@ fhandler_pipe::raw_write (const void *ptr, size_t len) return ret; } +void +fhandler_pipe::fixup_after_fork (HANDLE parent) +{ + if (query_hdl) + fork_fixup (parent, query_hdl, "query_hdl"); + fhandler_base::fixup_after_fork (parent); +} + int fhandler_pipe::dup (fhandler_base *child, int flags) { fhandler_pipe *ftp = (fhandler_pipe *) child; ftp->set_popen_pid (0); - int res; - if (get_handle () && fhandler_base::dup (child, flags)) + int res = 0; + if (fhandler_base::dup (child, flags)) res = -1; - else - res = 0; + else if (query_hdl && + !DuplicateHandle (GetCurrentProcess (), query_hdl, + GetCurrentProcess (), &ftp->query_hdl, + 0, !(flags & O_CLOEXEC), DUPLICATE_SAME_ACCESS)) + { + __seterrno (); + ftp->close (); + res = -1; + } debug_printf ("res %d", res); return res; } +int +fhandler_pipe::close () +{ + if (query_hdl) + NtClose (query_hdl); + return fhandler_base::close (); +} + #define PIPE_INTRO "\\\\.\\pipe\\cygwin-" /* Create a pipe, and return handles to the read and write ends, @@ -608,6 +631,7 @@ fhandler_pipe::create (fhandler_pipe *fhs[2], unsigned psize, int mode) else if ((fhs[1] = (fhandler_pipe *) build_fh_dev (*pipew_dev)) == NULL) { delete fhs[0]; + CloseHandle (r); CloseHandle (w); } else @@ -617,10 +641,23 @@ fhandler_pipe::create (fhandler_pipe *fhs[2], unsigned psize, int mode) unique_id); fhs[1]->init (w, FILE_CREATE_PIPE_INSTANCE | GENERIC_WRITE, mode, unique_id); - res = 0; + /* For the write side of the pipe, duplicate the handle to the read side + into query_hdl just for calling NtQueryInformationFile. See longish + comment in select.cc, pipe_data_available() for the reasoning. */ + if (!DuplicateHandle (GetCurrentProcess (), r, GetCurrentProcess (), + &fhs[1]->query_hdl, GENERIC_READ, + !(mode & O_CLOEXEC), 0)) + { + delete fhs[0]; + CloseHandle (r); + delete fhs[1]; + CloseHandle (w); + } + else + res = 0; } - debug_printf ("%R = pipe([%p, %p], %d, %y)", res, fhs[0], fhs[1], psize, mode); + debug_printf ("%R = pipe(%d, %y)", res, psize, mode); return res; } @@ -661,7 +698,7 @@ nt_create (LPSECURITY_ATTRIBUTES sa_ptr, PHANDLE r, PHANDLE w, access = GENERIC_READ | FILE_WRITE_ATTRIBUTES; ULONG pipe_type = pipe_byte ? FILE_PIPE_BYTE_STREAM_TYPE - : FILE_PIPE_MESSAGE_TYPE; + : FILE_PIPE_MESSAGE_TYPE; /* Retry NtCreateNamedPipeFile as long as the pipe name is in use. Retrying will probably never be necessary, but we want diff --git a/winsup/cygwin/select.cc b/winsup/cygwin/select.cc index 83e1c00e0ac7..7e69ad834d9a 100644 --- a/winsup/cygwin/select.cc +++ b/winsup/cygwin/select.cc @@ -608,15 +608,33 @@ pipe_data_available (int fd, fhandler_base *fh, HANDLE h, bool writing) } if (writing) { - /* 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(). */ - fpli.WriteQuotaAvailable = fpli.OutboundQuota - fpli.ReadDataAvailable; + /* 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, the only reliable information is available on the + read side, so fetch info from the read side via the pipe-specific + query handle. Use fpli.WriteQuotaAvailable as storage for the actual + interesting value, which is the InboundQuote on the read side, + decremented by the number of bytes of data in that buffer. */ + fpli.WriteQuotaAvailable = fpli.InboundQuota - fpli.ReadDataAvailable; if (fpli.WriteQuotaAvailable > 0) { paranoid_printf ("fd %d, %s, write: size %u, avail %u", fd, - fh->get_name (), fpli.OutboundQuota, + fh->get_name (), fpli.InboundQuota, fpli.WriteQuotaAvailable); return 1; } @@ -718,9 +736,14 @@ out: fhandler_pty_master *fhm = (fhandler_pty_master *) fh; fhm->set_mask_flusho (s->read_ready); } - h = fh->get_output_handle (); if (s->write_selected && dev != FH_PIPER) { + /* For the write side of a pipe, fetch the handle to the read side. + See the longish comment in pipe_data_available for the reasoning. */ + if (dev == FH_PIPEW) + h = ((fhandler_pipe *) fh)->get_query_handle (); + else + h = fh->get_output_handle (); gotone += s->write_ready = pipe_data_available (s->fd, fh, h, true); select_printf ("write: %s, gotone %d", fh->get_name (), gotone); }