Remove fifo_client_handler::connect and move its code into
listen_client_thread. That way we can check the return status when a
client handler's connect_evt is signaled. Previously we incorrectly
assumed there was a successful connection.
Also simplify listen_client_thread in the following ways:
- Replace fhandler_fifo::disconnect_and_reconnect by a new
delete_client_handler method. Now we just delete invalid client
handlers rather than trying to re-use them.
- Try to maintain a client handler list that consists of connected
client handlers and exactly one that is listening for a connection.
This allows us to call WaitForMultipleObjects with only two wait
objects.
- Remove 'dummy_evt' from the fifo_client_handler struct; it is no
longer needed.
- On exit from listen_client_thread, delete the "extra" (listening)
client handler. Otherwise there could be a connection that doesn't
get recorded in the client handler list. This could happen when a
file descriptor is being duplicated.
---
winsup/cygwin/fhandler.h | 11 +-
winsup/cygwin/fhandler_fifo.cc | 251 ++++++++++++++-------------------
2 files changed, 109 insertions(+), 153 deletions(-)
diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
index fd205a6db..8fb176b24 100644
--- a/winsup/cygwin/fhandler.h
+++ b/winsup/cygwin/fhandler.h
@@ -1250,13 +1250,10 @@ struct fifo_client_handler
fhandler_base *fh;
fifo_client_connect_state state;
HANDLE connect_evt;
- HANDLE dummy_evt; /* Never signaled. */
- fifo_client_handler () : fh (NULL), state (fc_unknown), connect_evt (NULL),
- dummy_evt (NULL) {}
+ fifo_client_handler () : fh (NULL), state (fc_unknown), connect_evt (NULL) {}
fifo_client_handler (fhandler_base *_fh, fifo_client_connect_state _state,
- HANDLE _connect_evt, HANDLE _dummy_evt)
- : fh (_fh), state (_state), connect_evt (_connect_evt),
- dummy_evt (_dummy_evt) {}
+ HANDLE _connect_evt)
+ : fh (_fh), state (_state), connect_evt (_connect_evt) {}
int connect ();
int close ();
};
@@ -1278,8 +1275,8 @@ class fhandler_fifo: public fhandler_base
NTSTATUS npfs_handle (HANDLE &);
HANDLE create_pipe_instance (bool);
NTSTATUS open_pipe ();
- int disconnect_and_reconnect (int);
int add_client_handler ();
+ void delete_client_handler (int);
bool listen_client ();
int stop_listen_client ();
public:
diff --git a/winsup/cygwin/fhandler_fifo.cc b/winsup/cygwin/fhandler_fifo.cc
index ce078d74d..abbb29a61 100644
--- a/winsup/cygwin/fhandler_fifo.cc
+++ b/winsup/cygwin/fhandler_fifo.cc
@@ -121,62 +121,6 @@ set_pipe_non_blocking (HANDLE ph, bool nonblocking)
debug_printf ("NtSetInformationFile(FilePipeInformation): %y", status);
}
-/* The pipe instance is always in blocking mode when this is called. */
-int
-fifo_client_handler::connect ()
-{
- NTSTATUS status;
- IO_STATUS_BLOCK io;
-
- if (connect_evt)
- ResetEvent (connect_evt);
- else if (!(connect_evt = create_event ()))
- return -1;
- status = NtFsControlFile (fh->get_handle (), connect_evt, NULL, NULL, &io,
- FSCTL_PIPE_LISTEN, NULL, 0, NULL, 0);
- switch (status)
- {
- case STATUS_PENDING:
- case STATUS_PIPE_LISTENING:
- state = fc_connecting;
- break;
- case STATUS_PIPE_CONNECTED:
- state = fc_connected;
- set_pipe_non_blocking (fh->get_handle (), true);
- break;
- default:
- __seterrno_from_nt_status (status);
- return -1;
- }
- return 0;
-}
-
-int
-fhandler_fifo::disconnect_and_reconnect (int i)
-{
- NTSTATUS status;
- IO_STATUS_BLOCK io;
- HANDLE ph = fc_handler[i].fh->get_handle ();
-
- status = NtFsControlFile (ph, NULL, NULL, NULL, &io, FSCTL_PIPE_DISCONNECT,
- NULL, 0, NULL, 0);
- /* Short-lived. Don't use cygwait. We don't want to be interrupted. */
- if (status == STATUS_PENDING
- && NtWaitForSingleObject (ph, FALSE, NULL) == WAIT_OBJECT_0)
- status = io.Status;
- if (!NT_SUCCESS (status))
- {
- __seterrno_from_nt_status (status);
- return -1;
- }
- set_pipe_non_blocking (fc_handler[i].fh->get_handle (), false);
- if (fc_handler[i].connect () < 0)
- return -1;
- if (fc_handler[i].state == fc_connected)
- nconnected++;
- return 0;
-}
-
NTSTATUS
fhandler_fifo::npfs_handle (HANDLE &nph)
{
@@ -283,41 +227,49 @@ fhandler_fifo::open_pipe ()
int
fhandler_fifo::add_client_handler ()
{
+ int ret = -1;
fifo_client_handler fc;
fhandler_base *fh;
+ HANDLE ph = NULL;
bool first = (nhandlers == 0);
if (nhandlers == MAX_CLIENTS)
{
set_errno (EMFILE);
- return -1;
+ goto out;
}
- if (!(fc.dummy_evt = create_event ()))
- return -1;
+ if (!(fc.connect_evt = create_event ()))
+ goto out;
if (!(fh = build_fh_dev (dev ())))
{
set_errno (EMFILE);
- return -1;
+ goto out;
}
- fc.fh = fh;
- HANDLE ph = create_pipe_instance (first);
+ ph = create_pipe_instance (first);
if (!ph)
- goto errout;
- fh->set_handle (ph);
- fh->set_flags (get_flags ());
- if (fc.connect () < 0)
{
- fc.close ();
- goto errout;
+ delete fh;
+ goto out;
}
- if (fc.state == fc_connected)
- nconnected++;
- fc_handler[nhandlers++] = fc;
- return 0;
-errout:
- delete fh;
- return -1;
+ else
+ {
+ fh->set_handle (ph);
+ fh->set_flags (get_flags ());
+ ret = 0;
+ fc.fh = fh;
+ fc_handler[nhandlers++] = fc;
+ }
+out:
+ return ret;
+}
+void
+fhandler_fifo::delete_client_handler (int i)
+{
+ fc_handler[i].close ();
+ if (i < --nhandlers)
+ memmove (fc_handler + i, fc_handler + i + 1,
+ (nhandlers - i) * sizeof (fc_handler[i]));
}
/* Just hop to the listen_client_thread method. */
@@ -358,46 +310,23 @@ fhandler_fifo::listen_client_thread ()
while (1)
{
- bool found;
- HANDLE w[MAX_CLIENTS + 1];
- int i;
- DWORD wait_ret;
+ /* At the beginning of the loop, all client handlers are
+ in the fc_connected or fc_invalid state. */
+ /* Delete any invalid clients. */
fifo_client_lock ();
- found = false;
- for (i = 0; i < nhandlers; i++)
- switch (fc_handler[i].state)
- {
- case fc_invalid:
- if (disconnect_and_reconnect (i) < 0)
- {
- fifo_client_unlock ();
- goto out;
- }
- else
- /* Recheck fc_handler[i].state. */
- i--;
- break;
- case fc_connected:
- w[i] = fc_handler[i].dummy_evt;
- break;
- case fc_connecting:
- found = true;
- w[i] = fc_handler[i].connect_evt;
- break;
- case fc_unknown: /* Shouldn't happen. */
- default:
- break;
- }
- w[nhandlers] = lct_termination_evt;
- int res = 0;
- if (!found)
- res = add_client_handler ();
- fifo_client_unlock ();
- if (res < 0)
+ int i = 0;
+ while (i < nhandlers)
+ {
+ if (fc_handler[i].state == fc_invalid)
+ delete_client_handler (i);
+ else
+ i++;
+ }
+
+ /* Create a new client handler. */
+ if (add_client_handler () < 0)
goto out;
- else if (!found)
- continue;
/* Allow a writer to open. */
if (!arm (read_ready))
@@ -405,26 +334,73 @@ fhandler_fifo::listen_client_thread ()
__seterrno ();
goto out;
}
+ fifo_client_unlock ();
- /* Wait for a client to connect. */
- wait_ret = WaitForMultipleObjects (nhandlers + 1, w, false, INFINITE);
- i = wait_ret - WAIT_OBJECT_0;
- if (i < 0 || i > nhandlers)
- goto out;
- else if (i == nhandlers) /* Thread termination requested. */
+ /* Listen for a writer to connect to the new client handler. */
+ fifo_client_handler& fc = fc_handler[nhandlers - 1];
+ do
+ {
+ NTSTATUS status;
+ IO_STATUS_BLOCK io;
+
+ status = NtFsControlFile (fc.fh->get_handle (), fc.connect_evt,
+ NULL, NULL, &io, FSCTL_PIPE_LISTEN,
+ NULL, 0, NULL, 0);
+ if (status == STATUS_PENDING)
+ {
+ HANDLE w[2] = { fc.connect_evt, lct_termination_evt };
+ DWORD waitret = WaitForMultipleObjects (2, w, false, INFINITE);
+ switch (waitret)
+ {
+ case WAIT_OBJECT_0:
+ status = io.Status;
+ break;
+ case WAIT_OBJECT_0 + 1:
+ ret = 0;
+ status = STATUS_THREAD_IS_TERMINATING;
+ break;
+ default:
+ __seterrno ();
+ debug_printf ("WaitForMultipleObjects failed, %E");
+ status = STATUS_THREAD_IS_TERMINATING;
+ break;
+ }
+ }
+ switch (status)
+ {
+ case STATUS_SUCCESS:
+ case STATUS_PIPE_CONNECTED:
+ fifo_client_lock ();
+ fc.state = fc_connected;
+ nconnected++;
+ set_pipe_non_blocking (fc.fh->get_handle (), true);
+ fifo_client_unlock ();
+ break;
+ case STATUS_PIPE_LISTENING:
+ /* Retry. */
+ fc.state = fc_connecting;
+ ResetEvent (fc.connect_evt);
+ break;
+ case STATUS_THREAD_IS_TERMINATING:
+ fifo_client_lock ();
+ delete_client_handler (nhandlers - 1);
+ fifo_client_unlock ();
+ goto out;
+ default:
+ __seterrno_from_nt_status (status);
+ fifo_client_lock ();
+ delete_client_handler (nhandlers - 1);
+ fifo_client_unlock ();
+ goto out;
+ }
+ } while (fc.state == fc_connecting);
+ /* Check for thread termination in case WaitForMultipleObjects
+ didn't get called above. */
+ if (IsEventSignalled (lct_termination_evt))
{
ret = 0;
goto out;
}
- else
- {
- fifo_client_lock ();
- fc_handler[i].state = fc_connected;
- nconnected++;
- set_pipe_non_blocking (fc_handler[i].fh->get_handle (), true);
- fifo_client_unlock ();
- yield ();
- }
}
out:
ResetEvent (read_ready);
@@ -490,7 +466,7 @@ fhandler_fifo::open (int flags, mode_t)
/* If we're a duplexer, create the pipe and the first client handler. */
if (duplexer)
{
- HANDLE ph, connect_evt, dummy_evt;
+ HANDLE ph, connect_evt;
fhandler_base *fh;
ph = create_pipe_instance (true);
@@ -516,16 +492,7 @@ fhandler_fifo::open (int flags, mode_t)
delete fh;
goto out;
}
- if (!(dummy_evt = create_event ()))
- {
- res = error_errno_set;
- delete fh;
- fh->close ();
- CloseHandle (connect_evt);
- goto out;
- }
- fc_handler[0] = fifo_client_handler (fh, fc_connected, connect_evt,
- dummy_evt);
+ fc_handler[0] = fifo_client_handler (fh, fc_connected, connect_evt);
nconnected = nhandlers = 1;
}
@@ -866,8 +833,6 @@ fifo_client_handler::close ()
}
if (connect_evt)
CloseHandle (connect_evt);
- if (dummy_evt)
- CloseHandle (dummy_evt);
return res;
}
@@ -946,10 +911,6 @@ fhandler_fifo::dup (fhandler_base *child, int flags)
|| !DuplicateHandle (GetCurrentProcess (), fc_handler[i].connect_evt,
GetCurrentProcess (),
&fhf->fc_handler[i].connect_evt,
- 0, true, DUPLICATE_SAME_ACCESS)
- || !DuplicateHandle (GetCurrentProcess (), fc_handler[i].dummy_evt,
- GetCurrentProcess (),
- &fhf->fc_handler[i].dummy_evt,
0, true, DUPLICATE_SAME_ACCESS))
{
CloseHandle (fhf->read_ready);
@@ -978,7 +939,6 @@ fhandler_fifo::fixup_after_fork (HANDLE parent)
{
fc_handler[i].fh->fhandler_base::fixup_after_fork (parent);
fork_fixup (parent, fc_handler[i].connect_evt, "connect_evt");
- fork_fixup (parent, fc_handler[i].dummy_evt, "dummy_evt");
}
listen_client_thr = NULL;
lct_termination_evt = NULL;
@@ -995,6 +955,5 @@ fhandler_fifo::set_close_on_exec (bool val)
{
fc_handler[i].fh->fhandler_base::set_close_on_exec (val);
set_no_inheritance (fc_handler[i].connect_evt, val);
- set_no_inheritance (fc_handler[i].dummy_evt, val);
}
}
--
2.17.0
