This reverts commit 39a9cd9465.
There is no need to explicitly take ownership in fixup_after_exec; if
ownership transfer is needed, it will be taken care of by
fhandler_fifo::close when the parent closes. Moreover, closing the
parent's fifo_reader_thread can cause problems, such as the one
reported here:
https://cygwin.com/pipermail/cygwin-patches/2020q2/010235.html
During a blocking read, we sleep for 1 ms after each iteration through
the connected writers. Currently we do this by calling Sleep (1).
Remove this call to Sleep and instead change the timeout in the
cygwait call from 0 to 1, so that raw_read can be interrupted while
sleeping.
There are currently three functions that call NtQueryInformationFile
to determine the state of a pipe instance. Do this only once, in a
new fifo_client_handler::set_state () function, and call that when
state information is needed.
Remove the fifo_client_handler methods pipe_state and get_state, which
are no longer needed.
Make fhandler_fifo::get_fc_handler return a reference, for use in
select.cc:peek_fifo.
Make other small changes to ensure that this commit doesn't change any
decisions based on the state of a fifo_client_handler.
The tricky part is interpreting FILE_PIPE_CLOSING_STATE, which we
translate to fc_closing. Our current interpretation, which is not
changing as a result of this commit, is that the writer at the other
end of the pipe instance is viewed as still connected from the point
of view of raw_read and determining EOF.
But it is not viewed as still connected if we are deciding whether to
unblock a new reader that is trying to open.
The beginning of fhandler_fifo.cc contains a long comment giving an
overview of the FIFO implementation. This is now updated to describe
the support for multiple readers.
Although we can have multiple readers open because of dup/fork/exec,
the current code does not support multiple readers opening a FIFO by
explicitly calling 'open'.
The main complication in supporting this is that when a blocking
reader tries to open and there's already one open, it has to check
whether there any writers open. It can't rely on the write_ready
event, whose state hasn't changed since the first writer opened.
To fix this, add two new named events, check_write_ready_evt and
write_ready_ok_evt, and a new method, check_write_ready().
The first event signals the owner's reader thread to call
check_write_ready(), which polls the fc_handler list to check for
connected writers. If it finds none, it checks to see if there's a
writer in the process and then sets/resets write_ready appropriately.
When check_write_ready() finishes it sets write_ready_ok_evt to signal
the reader that write_ready has been updated.
The polling is done via fifo_client_handler::pipe_state(). As long as
it's calling that function anyway, check_write_ready() updates the
state of each handler.
Also add a new lock to prevent a race if two readers are trying to
open simultaneously.
Add a take_ownership method, used by raw_read and select.cc:peek_fifo.
It wakes up all fifo_reader_threads and allows the caller to become
owner. The work is done by the fifo_reader_threads.
For synchronization we introduce several new fhandler_fifo data
members and methods:
- update_needed_evt signals the current owner to stop listening for
writer connections and update its fc_handler list.
- shared_fc_handler() gets and sets the status of the fc_handler
update process.
- get_pending_owner() and set_pending_owner() get and set the reader
that is requesting ownership.
Finally, a new 'reading_lock' prevents two readers from trying to take
ownership simultaneously.
If the owning reader is closing, wait for another reader (if there is
one) to take ownership before closing the owner's pipe handles.
To synchronize the ownership transfer, add events owner_needed_evt and
owner_found_evt, and add methods owner_needed and owner_found to
set/reset them.
Modify the fifo_reader_thread function to wake up all non-owners when
a new owner is needed.
Make a cosmetic change in close so that fhandler_base::close is called
only if we have a write handle. This prevents strace output from
being littered with statements that the null handle is being closed.
If fixup_after_exec is called on a non-close-on-exec reader whose
parent is the owner, transfer ownership to the child. Otherwise the
parent's pipe handles will be closed before any other reader can
duplicate them.
To help with this, make the cancel_evt and thr_sync_evt handles
inheritable, so that the child can terminate the parent's
fifo_reader_thread (and the parent will update the shared fc_handler
list).
Add an optional argument 'from_exec' to update_my_handlers to simplify
its use in this case; no handle duplication is required.
This is in a new shared memory section. We will use it for temporary
storage of the owner's fc_handler list when we need to change owner.
The new owner can then duplicate the pipe handles from that list
before taking ownership.
Add several shared data members and methods that are needed for the
duplication process
Add methods update_my_handlers and update_shared_handlers that carry
out the duplication.
Allow the shared list to grow dynamically, up to a point. Do this by
initially reserving a block of memory (currently 100 pages) and only
committing pages as needed.
Add methods create_shared_fc_handler, reopen_shared_fc_handler, and
remap_shared_fc_handler to create the new shared memory section,
reopen it, and commit new pages. The first is called in open, the
second is called in dup/fork/exec, and the third is called in
update_shared_handlers if more shared memory is needed.
Modify the fifo_reader_thread function to call update_my_handlers when
it finds that there is no owner. Also make it call
update_shared_handlers when the owner's thread terminates, so that the
new owner will have an accurate shared fc_handler list from which to
duplicate.
For convenience, add new methods cleanup_handlers and
close_all_handlers. And add an optional arg to add_client_handler
that allows it to create a new fifo_client_handler without creating a
new pipe instance.
Make fc_handler a pointer to malloc'd memory instead of a fixed-size
array. The size is now a new data member 'shandlers'. Call realloc
in add_client_handler if we need to grow the array.
free fc_handler in close. As long as we're touching that code, also
remove an unneeded lock.
Among all the open readers of a FIFO, one is declared to be the owner.
This is the only reader that listens for client connections, and it is
the only one that has an accurate fc_handler list.
Add shared data and methods for getting and setting the owner, as well
as a lock to prevent more than one reader from accessing these data
simultaneously.
Modify the fifo_reader_thread so that it checks the owner at the
beginning of its loop. If there is no owner, it takes ownership. If
there is an owner but it is a different reader, the thread just waits
to be canceled. Otherwise, it listens for client connections as
before.
Remove the 'first' argument from create_pipe_instance. It is not
needed, and it may be confusing in the future since only the owner
knows whether a pipe instance is the first.
When opening a reader, don't return until the fifo_reader_thread has
time to set an owner.
If the owner closes, indicate that there is no longer an owner.
Clear the child's fc_handler list in dup, and don't bother duplicating
the handles. The child never starts out as owner, so it can't use
those handles.
Do the same thing in fixup_after_fork in the close-on-exec case. In
the non-close-on-exec case, the child inherits an fc_handler list that
it can't use, but we can just leave it alone; the handles will be
closed when the child is closed.
This uniquely identifies an fhandler_fifo open for reading in any
process.
Add a new data member 'me' of this type, which is set in open, dup,
fork, and exec.
Add data and methods to the shared memory that keep track of the
number of open readers.
Increment this number in open, dup, fork, and exec. Decrement it in
close. Reset read_ready if there are no readers left.
Even though we currently allow a FIFO to be opened for reading only
once, we can still have more than one reader open because of dup and
fork. Add a named shared memory section accessible to all readers of
a given FIFO. In future commits we will add information needed by all
readers to this section
Add a class fifo_shmem_t that lets us access this information.
Add a method create_shmem that is called when a reader opens, and add
a method reopen_shmem that is called by dup, fork, and exec. (Each
new reader needs its own view of the shared memory.)
This will simplify future work.
Rename the thread from "listen_client_thread" to "fifo_reader_thread"
because it will be used for more than just listening.
Remove the fixup_before stuff, which won't be needed after future
changes to fixup_after_fork and fixup_after_exec.
- Make read_ready a manual-reset event.
- Signal read_ready in open instead of in the listen_client_thread.
- Don't reset read_ready when the listen_client thread terminates;
instead do it in close().
- Rearrange open and change its error handling.
- Add a wait_open_pipe method that waits for a pipe instance to be
available and then calls open_pipe. Use it when opening a writer if
we can't connect immediately. This can happen if the system is
heavily loaded and/or if many writers are trying to open
simultaneously.
According to Posix, a FIFO open for reading is at EOF if it is empty
and there are no writers open.
The only way to test this is to poll the fifo_client_handlers as in
raw_read and select.cc:peek_fifo. The current hit_eof instead relies
on the value of nconnected, which can be out of date. On the one
hand, it doesn't take into account writers that were connected but
have since closed. On the other hand, it doesn't take into account
writers that are in the process of opening but haven't yet connected.
Fix this by introducing a maybe_eof method that tentatively assumes
EOF if there are no connected writers after polling. Then check for
writers currently opening (via a new 'writer_opening' event), and wait
for the fifo_reader_thread to record any new connection that was made
while we were polling.
To handle the needs of peek_fifo, replace the get_fc_handle method
by a get_fc_handler method, and add a fifo_client_handler::get_state
method.
Remove the is_connected method, which was used only in peek_fifo and
is no longer needed.
Remove the nconnected data member, which was used only for the flawed
hit_eof.
Add some comments about events to fhandler.h.
Always return 0; no one is doing anything with the return value
anyway.
Remove the return value from stop_listen_client.
Make the connection event auto-reset, so that we don't have to reset
it later.
Simplify the process of connecting a bogus client when thread
termination is signaled.
Make some failures fatal.
Remove the unnecessary extra check for thread termination near the end
of listen_client_thread.
Make the values correspond to the possible return values of
fifo_client_handler::pipe_state().
When cleaning up the fc_handler list in listen_client_thread(), don't
delete handlers in the fc_closing state. I think the pipe might still
have input to be read in that case.
Set the state to fc_closing later in the same function if a connection
is made and the status returned by NtFsControlFile is
STATUS_PIPE_CLOSING.
In raw_read, don't error out if NtReadFile returns an unexpected
status; just set the state of that handler to fc_error. One writer in
a bad state doesn't justify giving up on reading.
Replace the 'fhandler_base *' member by a HANDLE to the server side of
the Windows named pipe instance. Make the corresponding
simplifications throughout.
If O_PATH is set, then the fhandler_fifo object has a handle that can
be used for getting the statvfs information. Use it by calling
fhandler_base::fstatvfs_by_handle. Before this change,
fhandler_disk_file::fstatfvs was called on a new fhandler_disk_file
object, which would then have to be opened.
If the O_PATH flag is set, fhandler_fifo::open now simply calls
fhandler_base::open_fs.
The previous attempt to handle O_PATH in commit aa55d22c, "Cygwin:
honor the O_PATH flag when opening a FIFO", fixed a hang but otherwise
didn't do anything useful.
Add that flag to the pipe type argument when creating the Windows
named pipe. And add a definition of that flag to ntdll.h (copied from
/usr/include/w32api/ddk/ntifs.h).
Make it a manual reset event. It's only used once to allow a reader
to open, and there's no reason to ever reset it. Defensively set it
when a client connection is recorded, even though it should be set by
the writer that connected.
Call NtReadFile directly instead of calling fhandler_base::raw_read.
In addition to being simpler, this gives us access to the return value
from NtReadFile.
Add a method fifo_client_handler::pipe_state that queries Windows for
the state of a pipe instance. Use this to help terminate the
listen_client thread cleanly.
If the last client handler is useless, delete it instead of declaring
it invalid.
fhandler_fifo::close could be called from a signal handler or another
thread at a time when another function is holding the fifo_client
lock. This could prevent the listen_client thread from acting on the
thread termination event. Avoid a deadlock by calling
fifo_client_unlock at the beginning of fhandler_fifo::close.
There's no longer a need to consider the connect_evt after fork/exec.
After stopping the listen_client thread, all client handlers should be
in the fc_connected or fc_invalid states, so their connect_evt members
won't be used again.
Also remove code in fhandler_fifo::dup that just repeats things
already done in stop_listen_client.
Add a method fhandler_fifo::check_listen_client_thread that checks
whether the thread is running. Use it in raw_read instead of just
testing the handle listen_client_thr.
On exit from the listen_client thread, make sure there's no pending
FSCTL_PIPE_LISTEN request. Otherwise we might get a client connection
after restarting the thread, and we won't have a handle for
communicating with that client.
Remove the retry loop in the case of STATUS_PIPE_LISTENING; that case
shouldn't occur.
Remove the now-unused fc_connecting value from
fifo_client_connect_state.
It's now up to the caller to pass a handle to open_pipe and, if
desired, to call set_handle on return.
This will be useful for a future commit, in which we will open a
client connection without setting an io_handle.
When opening a duplexer, open a client connection to the first client
handler. Previously we gave the duplexer a bogus write handle, which
was just a duplicate of the first client handler's handle. This meant
that we had a pipe server with no clients connected, and all I/O
attempts failed with STATUS_PIPE_LISTENING.
Extend the last fcntl change to duplexers.
Remove a now unused fifo_client_handler constructor, as well as the
long unusued method fifo_client_handler::connect.
Don't create the pipe in duplex mode; the server handle will only be
used for reading.
Ken Brown