From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 2210) id 63C19396ECFE; Fri, 8 May 2020 11:29:52 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 63C19396ECFE Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: Ken Brown To: cygwin-cvs@sourceware.org Subject: [newlib-cygwin] Cygwin: FIFO: use a cygthread instead of a homemade thread X-Act-Checkin: newlib-cygwin X-Git-Author: Ken Brown X-Git-Refname: refs/heads/master X-Git-Oldrev: 9ee8fdf2b3a371cb14fb902a8c9aab76dd60f4e2 X-Git-Newrev: 71726ba70bccfed084590760f8b57f050e1bb017 Message-Id: <20200508112952.63C19396ECFE@sourceware.org> Date: Fri, 8 May 2020 11:29:52 +0000 (GMT) X-BeenThere: cygwin-cvs@cygwin.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Cygwin core component git logs List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 08 May 2020 11:29:52 -0000 https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;h=71726ba70bccfed084590760f8b57f050e1bb017 commit 71726ba70bccfed084590760f8b57f050e1bb017 Author: Ken Brown Date: Thu Mar 26 14:29:50 2020 -0400 Cygwin: FIFO: use a cygthread instead of a homemade thread 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. Diff: --- 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); } }