[PATCH 10/21] Cygwin: FIFO: use a cygthread instead of a homemade thread
Ken Brown
kbrown@cornell.edu
Thu May 7 20:21:13 GMT 2020
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.
---
winsup/cygwin/fhandler.h | 17 ++--
winsup/cygwin/fhandler_fifo.cc | 173 +++++++++++----------------------
2 files changed, 65 insertions(+), 125 deletions(-)
diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
index 2516c93b4..5e6a1d1db 100644
--- a/winsup/cygwin/fhandler.h
+++ b/winsup/cygwin/fhandler.h
@@ -1307,9 +1307,9 @@ class fhandler_fifo: public fhandler_base
HANDLE write_ready; /* A writer is open; OK for a reader to open. */
HANDLE writer_opening; /* A writer is opening; no EOF. */
- /* Non-shared handles needed for the listen_client_thread. */
- HANDLE listen_client_thr;
- HANDLE lct_termination_evt;
+ /* Handles to non-shared events needed for fifo_reader_threads. */
+ HANDLE cancel_evt; /* Signal thread to terminate. */
+ HANDLE thr_sync_evt; /* The thread has terminated. */
UNICODE_STRING pipe_name;
WCHAR pipe_name_buf[CYGWIN_FIFO_PIPE_NAME_LEN + 1];
@@ -1326,11 +1326,10 @@ class fhandler_fifo: public fhandler_base
NTSTATUS wait_open_pipe (HANDLE&);
int add_client_handler ();
void delete_client_handler (int);
- bool listen_client ();
- void stop_listen_client ();
- int check_listen_client_thread ();
+ void cancel_reader_thread ();
void record_connection (fifo_client_handler&,
fifo_client_connect_state = fc_connected);
+
public:
fhandler_fifo ();
bool hit_eof ();
@@ -1339,7 +1338,7 @@ public:
int get_nhandlers () const { return nhandlers; }
fifo_client_handler get_fc_handler (int i) const { return fc_handler[i]; }
PUNICODE_STRING get_pipe_name ();
- DWORD listen_client_thread ();
+ DWORD fifo_reader_thread_func ();
void fifo_client_lock () { _fifo_client_lock.lock (); }
void fifo_client_unlock () { _fifo_client_lock.unlock (); }
int open (int, mode_t);
@@ -1351,9 +1350,6 @@ public:
void set_close_on_exec (bool val);
void __reg3 raw_read (void *ptr, size_t& ulen);
ssize_t __reg3 raw_write (const void *ptr, size_t ulen);
- bool need_fixup_before () const { return reader; }
- int fixup_before_fork_exec (DWORD) { stop_listen_client (); return 0; }
- void init_fixup_before ();
void fixup_after_fork (HANDLE);
void fixup_after_exec ();
int __reg2 fstatvfs (struct statvfs *buf);
@@ -1375,7 +1371,6 @@ public:
void *ptr = (void *) ccalloc (malloc_type, 1, sizeof (fhandler_fifo));
fhandler_fifo *fhf = new (ptr) fhandler_fifo (ptr);
/* We don't want our client list to change any more. */
- stop_listen_client ();
copyto (fhf);
/* fhf->pipe_name_buf is a *copy* of this->pipe_name_buf, but
fhf->pipe_name.Buffer == this->pipe_name_buf. */
diff --git a/winsup/cygwin/fhandler_fifo.cc b/winsup/cygwin/fhandler_fifo.cc
index 5c3df5497..09a7eb321 100644
--- a/winsup/cygwin/fhandler_fifo.cc
+++ b/winsup/cygwin/fhandler_fifo.cc
@@ -32,11 +32,11 @@
When a FIFO is opened for reading,
fhandler_fifo::create_pipe_instance is called to create the first
instance of a Windows named pipe server (Windows terminology). A
- "listen_client" thread is also started; it waits for pipe clients
+ "fifo_reader" thread is also started; it waits for pipe clients
(Windows terminology again) to connect. This happens every time
a process opens the FIFO for writing.
- The listen_client thread creates new instances of the pipe server
+ The fifo_reader thread creates new instances of the pipe server
as needed, so that there is always an instance available for a
writer to connect to.
@@ -68,7 +68,7 @@ STATUS_PIPE_EMPTY simply means there's no data to be read. */
fhandler_fifo::fhandler_fifo ():
fhandler_base (),
read_ready (NULL), write_ready (NULL), writer_opening (NULL),
- listen_client_thr (NULL), lct_termination_evt (NULL), _maybe_eof (false), nhandlers (0),
+ cancel_evt (NULL), thr_sync_evt (NULL), _maybe_eof (false), nhandlers (0),
reader (false), writer (false), duplexer (false),
max_atomic_write (DEFAULT_PIPEBUFSIZE)
{
@@ -319,34 +319,6 @@ fhandler_fifo::delete_client_handler (int i)
(nhandlers - i) * sizeof (fc_handler[i]));
}
-/* Just hop to the listen_client_thread method. */
-DWORD WINAPI
-listen_client_func (LPVOID param)
-{
- fhandler_fifo *fh = (fhandler_fifo *) param;
- return fh->listen_client_thread ();
-}
-
-/* Start a thread that listens for client connections. */
-bool
-fhandler_fifo::listen_client ()
-{
- if (!(lct_termination_evt = create_event ()))
- return false;
-
- listen_client_thr = CreateThread (NULL, PREFERRED_IO_BLKSIZE,
- listen_client_func, (PVOID) this, 0, NULL);
- if (!listen_client_thr)
- {
- __seterrno ();
- HANDLE evt = InterlockedExchangePointer (&lct_termination_evt, NULL);
- if (evt)
- NtClose (evt);
- return false;
- }
- return true;
-}
-
void
fhandler_fifo::record_connection (fifo_client_handler& fc,
fifo_client_connect_state s)
@@ -357,8 +329,15 @@ fhandler_fifo::record_connection (fifo_client_handler& fc,
set_pipe_non_blocking (fc.h, true);
}
+static DWORD WINAPI
+fifo_reader_thread (LPVOID param)
+{
+ fhandler_fifo *fh = (fhandler_fifo *) param;
+ return fh->fifo_reader_thread_func ();
+}
+
DWORD
-fhandler_fifo::listen_client_thread ()
+fhandler_fifo::fifo_reader_thread_func ()
{
HANDLE conn_evt;
@@ -377,7 +356,6 @@ fhandler_fifo::listen_client_thread ()
else
i++;
}
- fifo_client_unlock ();
/* Create a new client handler. */
if (add_client_handler () < 0)
@@ -385,6 +363,7 @@ fhandler_fifo::listen_client_thread ()
/* Listen for a writer to connect to the new client handler. */
fifo_client_handler& fc = fc_handler[nhandlers - 1];
+ fifo_client_unlock ();
NTSTATUS status;
IO_STATUS_BLOCK io;
bool cancel = false;
@@ -393,9 +372,8 @@ fhandler_fifo::listen_client_thread ()
FSCTL_PIPE_LISTEN, NULL, 0, NULL, 0);
if (status == STATUS_PENDING)
{
- HANDLE w[2] = { conn_evt, lct_termination_evt };
- DWORD waitret = WaitForMultipleObjects (2, w, false, INFINITE);
- switch (waitret)
+ HANDLE w[2] = { conn_evt, cancel_evt };
+ switch (WaitForMultipleObjects (2, w, false, INFINITE))
{
case WAIT_OBJECT_0:
status = io.Status;
@@ -453,11 +431,13 @@ fhandler_fifo::listen_client_thread ()
if (ph)
NtClose (ph);
if (cancel)
- goto out;
+ goto canceled;
}
-out:
+canceled:
if (conn_evt)
NtClose (conn_evt);
+ /* automatically return the cygthread to the cygthread pool */
+ _my_tls._ctinfo->auto_release ();
return 0;
}
@@ -521,16 +501,15 @@ fhandler_fifo::open (int flags, mode_t)
goto err_close_write_ready;
}
- /* If we're reading, signal read_ready and start the listen_client
- thread. */
+ /* If we're reading, signal read_ready and start the fifo_reader_thread. */
if (reader)
{
- if (!listen_client ())
- {
- debug_printf ("create of listen_client thread failed");
- goto err_close_writer_opening;
- }
SetEvent (read_ready);
+ if (!(cancel_evt = create_event ()))
+ goto err_close_writer_opening;
+ if (!(thr_sync_evt = create_event ()))
+ goto err_close_cancel_evt;
+ new cygthread (fifo_reader_thread, this, "fifo_reader", thr_sync_evt);
/* If we're a duplexer, we need a handle for writing. */
if (duplexer)
@@ -563,7 +542,6 @@ fhandler_fifo::open (int flags, mode_t)
/* Not a duplexer; wait for a writer to connect. */
else if (!wait (write_ready))
goto err_close_reader;
- init_fixup_before ();
goto success;
}
@@ -635,6 +613,8 @@ err_close_reader:
close ();
set_errno (saved_errno);
return 0;
+err_close_cancel_evt:
+ NtClose (cancel_evt);
err_close_writer_opening:
NtClose (writer_opening);
err_close_write_ready:
@@ -815,43 +795,9 @@ fhandler_fifo::hit_eof ()
return ret;
}
-/* Is the lct running? */
-int
-fhandler_fifo::check_listen_client_thread ()
-{
- int ret = 0;
-
- if (listen_client_thr)
- {
- DWORD waitret = WaitForSingleObject (listen_client_thr, 0);
- switch (waitret)
- {
- case WAIT_OBJECT_0:
- NtClose (listen_client_thr);
- break;
- case WAIT_TIMEOUT:
- ret = 1;
- break;
- default:
- debug_printf ("WaitForSingleObject failed, %E");
- ret = -1;
- __seterrno ();
- NtClose (listen_client_thr);
- break;
- }
- }
- return ret;
-}
-
void __reg3
fhandler_fifo::raw_read (void *in_ptr, size_t& len)
{
- /* Make sure the lct is running. */
- int res = check_listen_client_thread ();
- debug_printf ("lct status %d", res);
- if (res < 0 || (res == 0 && !listen_client ()))
- goto errout;
-
if (!len)
return;
@@ -976,35 +922,29 @@ fifo_client_handler::pipe_state ()
}
void
-fhandler_fifo::stop_listen_client ()
+fhandler_fifo::cancel_reader_thread ()
{
- HANDLE thr, evt;
-
- thr = InterlockedExchangePointer (&listen_client_thr, NULL);
- if (thr)
- {
- if (lct_termination_evt)
- SetEvent (lct_termination_evt);
- WaitForSingleObject (thr, INFINITE);
- NtClose (thr);
- }
- evt = InterlockedExchangePointer (&lct_termination_evt, NULL);
- if (evt)
- NtClose (evt);
+ if (cancel_evt)
+ SetEvent (cancel_evt);
+ if (thr_sync_evt)
+ WaitForSingleObject (thr_sync_evt, INFINITE);
}
int
fhandler_fifo::close ()
{
- /* Avoid deadlock with lct in case this is called from a signal
- handler or another thread. */
- fifo_client_unlock ();
- stop_listen_client ();
if (reader)
- /* FIXME: There could be several readers open because of
- dup/fork/exec; we should only reset read_ready when the last
- one closes. */
- ResetEvent (read_ready);
+ {
+ cancel_reader_thread ();
+ if (cancel_evt)
+ NtClose (cancel_evt);
+ if (thr_sync_evt)
+ NtClose (thr_sync_evt);
+ /* FIXME: There could be several readers open because of
+ dup/fork/exec; we should only reset read_ready when the last
+ one closes. */
+ ResetEvent (read_ready);
+ }
if (read_ready)
NtClose (read_ready);
if (write_ready)
@@ -1091,11 +1031,16 @@ fhandler_fifo::dup (fhandler_base *child, int flags)
goto err_close_handlers;
}
fifo_client_unlock ();
- if (!fhf->listen_client ())
+ if (!(fhf->cancel_evt = create_event ()))
goto err_close_handlers;
- fhf->init_fixup_before ();
+ if (!(fhf->thr_sync_evt = create_event ()))
+ goto err_close_cancel_evt;
+ new cygthread (fifo_reader_thread, fhf, "fifo_reader",
+ fhf->thr_sync_evt);
}
return 0;
+err_close_cancel_evt:
+ NtClose (fhf->cancel_evt);
err_close_handlers:
for (int j = 0; j < i; j++)
fhf->fc_handler[j].close ();
@@ -1109,12 +1054,6 @@ err:
return -1;
}
-void
-fhandler_fifo::init_fixup_before ()
-{
- cygheap->fdtab.inc_need_fixup_before ();
-}
-
void
fhandler_fifo::fixup_after_fork (HANDLE parent)
{
@@ -1131,8 +1070,11 @@ fhandler_fifo::fixup_after_fork (HANDLE parent)
for (int i = 0; i < nhandlers; i++)
fork_fixup (parent, fc_handler[i].h, "fc_handler[].h");
fifo_client_unlock ();
- if (!listen_client ())
- debug_printf ("failed to start lct, %E");
+ if (!(cancel_evt = create_event ()))
+ api_fatal ("Can't create reader thread cancel event during fork, %E");
+ if (!(thr_sync_evt = create_event ()))
+ api_fatal ("Can't create reader thread sync event during fork, %E");
+ new cygthread (fifo_reader_thread, this, "fifo_reader", thr_sync_evt);
}
}
@@ -1145,8 +1087,11 @@ fhandler_fifo::fixup_after_exec ()
/* Make sure the child starts unlocked. */
fifo_client_unlock ();
- if (!listen_client ())
- debug_printf ("failed to start lct, %E");
+ if (!(cancel_evt = create_event ()))
+ api_fatal ("Can't create reader thread cancel event during exec, %E");
+ if (!(thr_sync_evt = create_event ()))
+ api_fatal ("Can't create reader thread sync event during exec, %E");
+ new cygthread (fifo_reader_thread, this, "fifo_reader", thr_sync_evt);
}
}
--
2.21.0
More information about the Cygwin-patches
mailing list