- The commit f79a4611 introduced query_hdl, which is the read pipe
handle kept in the write pipe instance in order to determine if
the pipe is ready to write in select(). This implementation has
a potential risk that the write side fails to detect the closure
of the read side if more than one writer exists and one of them
is a non-cygwin process.
With this patch, the strategy of commit f79a4611 is used only if
the process is running as a service. For a normal process,
instead of keeping query_hdl in the write pipe instance, it is
retrieved temporarily when select() is called. Actually, we
want to use tenporary query_hdl for all processes, however, it
does not work for service processes due to OpenProcess()
failure.
open_setup is called by dtable::init_std_file_from_handle and
fhandler_base::open_with_arch. In both cases, failure of open_setup
is now a fatal error.
Currently this can only happen in the following situation: A Cygwin
process is started by a non-Cygwin process, one of the standard IO
handles is a pipe handle, and Cygwin is unable to create a required
mutex (read_mtx or hdl_cnt_mtx).
- query_hdl and hdl_cnt_mtx are moved from fhandler_pipe_fifo to
fhandler_pipe. Then reader_closed() is changed to virtual and
overridden in fhandler_pipe.
- This patch fixes the race issue in the handle counting to detect
closure of read pipe, which is introduced by commit f79a4611.
A mutex hdl_cnt_mtx is introduced for this issue.
- Currently, the stderr handle is duplicated in close_all_files().
This interferes the handle counting for detecting closure of read
pipe, which is introduced by commit f79a4611. This patch stops
duplicating stderr handle if it is write pipe.
NtQueryInformationFile hangs if it's called on the read side handle of
a pipe while another thread or process is performing a blocking read.
Avoid select potentially hanging by calling NtQueryInformationFile
only on the write side of the pipe and using PeekNamedPipe otherwise.
Signed-off-by: Corinna Vinschen <corinna@vinschen.de>
- In pipe_data_available() in select.cc, PeekNamedPipe() call is
not needed if WriteQuotaAvailable is non-zero because we already
know the write pipe has a space. Therefore, with this patch,
PeekNamedPipe() is called only when WriteQuotaAvailable is zero.
This makes select() on pipe faster a bit.
- Currently, raw_read(), raw_write() and close() release select_sem
unconditionally even if no waiter for select_sem exists. With this
patch, only the minimum number of semaphores required is released.
- Usually WriteQuotaAvailable retrieved by NtQueryInformationFile()
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.
Currently, to avoid this stuation, read() does not request larger
block than pipe size - 1. However, this mechanism does not take
effect if the reader side is non-cygwin app.
The only reliable information is available on the read side, so
fetch info from the read side via the pipe-specific query handle
(query_hdl) introduced.
If the query_hdl (read handle) is kept in write side, writer can
not detect closure of read pipe. Therefore, raw_write() counts
write handle and query_hdl. If they are equal, only the pairs of
write handle and query_hdl are alive. In this case, raw_write()
returns EPIPE and raises SIGPIPE.
- Nonblocking pipes (PIPE_NOWAIT) are not well handled by non-Cygwin
tools, so convert pipe handles to PIPE_WAIT handles when spawning
a non-Cygwin process.
We already fetched the correct SECURITY_ATTRIBUTES at the start of
fhandler_pipe::create, so using another SECURITY_ATTRIBUTES object for
the mutex and semaphore objects doesn't make much sense.
Signed-off-by: Corinna Vinschen <corinna@vinschen.de>
select_sem gets created on the read side with inheritence settings
depending on the O_CLOEXEC flag. Then it gets duplicated to the write
side with unconditional inheritence. Fix that.
Signed-off-by: Corinna Vinschen <corinna@vinschen.de>
Fold all code branches potentially having read or written data into
a single if branch, so signalling select_sem catches all cases.
Signed-off-by: Corinna Vinschen <corinna@vinschen.de>
Rename fhandler_pipe_and_fifo::max_atomic_write to pipe_buf_size.
This reflect its actual meaning better. The fhandler_pipe_and_fifo
constructor initializes it to DEFAULT_PIPEBUFSIZE (== 64K), which is
the buffer size for the windows pipes created by fhandler_pipe and
fhandler_fifo. But if we inherit a stdio pipe handle from a
non-Cygwin process, the buffer size could be different.
To remedy this, add a method fhandler_pipe::set_pipe_buf_size that
queries the OS for the pipe buffer size, and use it in
dtable::init_std_file_from_handle.
Given we return 1 already if WriteQuotaAvailable is > 0, the condition
for tiny pipes is never true. Fix the comments.
Signed-off-by: Corinna Vinschen <corinna@vinschen.de>
In blocking mode, the underlying IO must always be terminated,
one way or the other, to make sure the application knows the exact
state after returning from the IO function. Therefore, always call
CancelIo in blocking mode.
Signed-off-by: Corinna Vinschen <corinna@vinschen.de>
Just cancelling a thread doesn't cancel async IO started by this thread.
Fix this by returning from cygwait and calling CancelIo before canceling
self.
Signed-off-by: Corinna Vinschen <corinna@vinschen.de>
- By guarding read with read_mtx, no more than one ReadFile can
be called simultaneously. So couting read handles is no longer
necessary.
- Make raw_read code as similar as possible to raw_write code.
This is a parent of fhandler_pipe and fhandler_fifo for code that is
common between the two classes. Currently it just contains
max_atomic_write and raw_write(). The latter is identical to what
used to be fhandler_pipe::raw_write().
FILE_PIPE_LOCAL_INFORMATION::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.
This patch implements a workaround by never trying to read more than
half the buffer size blocking if the read buffer is empty. This first
cut tries to take the number of open readers into account by reducing
the amount of requested bytes accordingly.
Signed-off-by: Corinna Vinschen <corinna@vinschen.de>
POSIX requires atomicity for non-blocking writes <= PIPE_BUF bytes
and writing of at least 1 byte if any buffer space is left.
Windows NtWriteFile returns STATUS_SUCCESS and "0 bytes written"
if the write doesn't match buffer space. Fix this discrepancy.
Signed-off-by: Corinna Vinschen <corinna@vinschen.de>
The buffer pointer is incremented by "chunk", which is what we
typically try to write, but this isn't what actually got written.
Increment the buffer pointer by what we actually wrote, as returned
by NtWriteFile.
Signed-off-by: Corinna Vinschen <corinna@vinschen.de>
get_obj_handle_count() is used in flock only so far, but pipe
handling might have a usage, too, soon. Given that this function
might be generally useful and isn't restricted to flock usage,
move it to miscfuncs.cc and make it non-static. Add a prototype
in miscfuncs.h.
Signed-off-by: Corinna Vinschen <corinna@vinschen.de>
I wasted valuable minutes of my life just to find out why we export
this weird version of pipe. In the pre-2000 era the idea was Cygwin
could be used as drop-in replacement for msvcrt.dll, apparently.
Signed-off-by: Corinna Vinschen <corinna@vinschen.de>
Takashi Yano
Ken Brown
Corinna Vinschen