From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 2210) id 284EE396EC5C; Fri, 8 May 2020 11:29:22 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 284EE396EC5C 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: simplify the listen_client_thread code X-Act-Checkin: newlib-cygwin X-Git-Author: Ken Brown X-Git-Refname: refs/heads/master X-Git-Oldrev: 32dbc3d21538c814da27ed3e311336700a8387d4 X-Git-Newrev: 9b2afd78ce612f4d1fda6d17149bb0c78b4a0e1c Message-Id: <20200508112922.284EE396EC5C@sourceware.org> Date: Fri, 8 May 2020 11:29:22 +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:22 -0000 https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;h=9b2afd78ce612f4d1fda6d17149bb0c78b4a0e1c commit 9b2afd78ce612f4d1fda6d17149bb0c78b4a0e1c Author: Ken Brown Date: Wed Apr 29 18:53:05 2020 -0400 Cygwin: FIFO: simplify the listen_client_thread code 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. Diff: --- winsup/cygwin/fhandler.h | 4 +- winsup/cygwin/fhandler_fifo.cc | 117 ++++++++++++++++------------------------- 2 files changed, 47 insertions(+), 74 deletions(-) diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h index c1f47025a..c8f7a39a2 100644 --- a/winsup/cygwin/fhandler.h +++ b/winsup/cygwin/fhandler.h @@ -1319,7 +1319,7 @@ class fhandler_fifo: public fhandler_base int add_client_handler (); void delete_client_handler (int); bool listen_client (); - int stop_listen_client (); + void stop_listen_client (); int check_listen_client_thread (); void record_connection (fifo_client_handler&, fifo_client_connect_state = fc_connected); @@ -1345,7 +1345,7 @@ public: ssize_t __reg3 raw_write (const void *ptr, size_t ulen); bool arm (HANDLE h); bool need_fixup_before () const { return reader; } - int fixup_before_fork_exec (DWORD) { return stop_listen_client (); } + int fixup_before_fork_exec (DWORD) { stop_listen_client (); return 0; } void init_fixup_before (); void fixup_after_fork (HANDLE); void fixup_after_exec (); diff --git a/winsup/cygwin/fhandler_fifo.cc b/winsup/cygwin/fhandler_fifo.cc index ba3dbb124..fb20e5a7e 100644 --- a/winsup/cygwin/fhandler_fifo.cc +++ b/winsup/cygwin/fhandler_fifo.cc @@ -324,11 +324,10 @@ fhandler_fifo::record_connection (fifo_client_handler& fc, DWORD fhandler_fifo::listen_client_thread () { - DWORD ret = -1; - HANDLE evt; + HANDLE conn_evt; - if (!(evt = create_event ())) - goto out; + if (!(conn_evt = CreateEvent (NULL, false, false, NULL))) + api_fatal ("Can't create connection event, %E"); while (1) { @@ -346,7 +345,7 @@ fhandler_fifo::listen_client_thread () /* Create a new client handler. */ if (add_client_handler () < 0) - goto out; + api_fatal ("Can't add a client handler, %E"); /* Allow a writer to open. */ if (!arm (read_ready)) @@ -359,12 +358,13 @@ fhandler_fifo::listen_client_thread () fifo_client_handler& fc = fc_handler[nhandlers - 1]; NTSTATUS status; IO_STATUS_BLOCK io; + bool cancel = false; - status = NtFsControlFile (fc.h, evt, NULL, NULL, &io, + status = NtFsControlFile (fc.h, conn_evt, NULL, NULL, &io, FSCTL_PIPE_LISTEN, NULL, 0, NULL, 0); if (status == STATUS_PENDING) { - HANDLE w[2] = { evt, lct_termination_evt }; + HANDLE w[2] = { conn_evt, lct_termination_evt }; DWORD waitret = WaitForMultipleObjects (2, w, false, INFINITE); switch (waitret) { @@ -372,83 +372,65 @@ fhandler_fifo::listen_client_thread () status = io.Status; break; case WAIT_OBJECT_0 + 1: - ret = 0; status = STATUS_THREAD_IS_TERMINATING; + cancel = true; break; default: - __seterrno (); - debug_printf ("WaitForMultipleObjects failed, %E"); - status = STATUS_THREAD_IS_TERMINATING; - break; + api_fatal ("WFMO failed, %E"); } } HANDLE ph = NULL; - int ps = -1; + NTSTATUS status1; + fifo_client_lock (); switch (status) { case STATUS_SUCCESS: case STATUS_PIPE_CONNECTED: record_connection (fc); - ResetEvent (evt); break; case STATUS_PIPE_CLOSING: record_connection (fc, fc_closing); - ResetEvent (evt); break; case STATUS_THREAD_IS_TERMINATING: - /* Force NtFsControlFile to complete. Otherwise the next - writer to connect might not be recorded in the client - handler list. */ - status = open_pipe (ph); - if (NT_SUCCESS (status) - && (NT_SUCCESS (io.Status) || io.Status == STATUS_PIPE_CONNECTED)) - { - debug_printf ("successfully connected bogus client"); - delete_client_handler (nhandlers - 1); - } - else if ((ps = fc.pipe_state ()) == FILE_PIPE_CONNECTED_STATE - || ps == FILE_PIPE_INPUT_AVAILABLE_STATE) - { - /* A connection was made under our nose. */ - debug_printf ("recording connection before terminating"); - record_connection (fc); - } + /* Try to connect a bogus client. Otherwise fc is still + listening, and the next connection might not get recorded. */ + status1 = open_pipe (ph); + WaitForSingleObject (conn_evt, INFINITE); + if (NT_SUCCESS (status1)) + /* Bogus cilent connected. */ + delete_client_handler (nhandlers - 1); else - { - debug_printf ("failed to terminate NtFsControlFile cleanly"); - delete_client_handler (nhandlers - 1); - ret = -1; - } - if (ph) - NtClose (ph); - fifo_client_unlock (); - goto out; + /* Did a real client connect? */ + switch (io.Status) + { + case STATUS_SUCCESS: + case STATUS_PIPE_CONNECTED: + record_connection (fc); + break; + case STATUS_PIPE_CLOSING: + record_connection (fc, fc_closing); + break; + default: + debug_printf ("NtFsControlFile status %y after failing to connect bogus client or real client", io.Status); + fc.state = fc_unknown; + break; + } + break; default: - debug_printf ("NtFsControlFile status %y", status); - __seterrno_from_nt_status (status); - delete_client_handler (nhandlers - 1); - fifo_client_unlock (); - goto out; + break; } fifo_client_unlock (); - /* Check for thread termination in case WaitForMultipleObjects - didn't get called above. */ - if (IsEventSignalled (lct_termination_evt)) - { - ret = 0; - goto out; - } + if (ph) + NtClose (ph); + if (cancel) + goto out; } out: - if (evt) - NtClose (evt); + if (conn_evt) + NtClose (conn_evt); ResetEvent (read_ready); - if (ret < 0) - debug_printf ("exiting with error, %E"); - else - debug_printf ("exiting without error"); - return ret; + return 0; } int @@ -945,10 +927,9 @@ fifo_client_handler::pipe_state () return fpli.NamedPipeState; } -int +void fhandler_fifo::stop_listen_client () { - int ret = 0; HANDLE thr, evt; thr = InterlockedExchangePointer (&listen_client_thr, NULL); @@ -957,19 +938,11 @@ fhandler_fifo::stop_listen_client () if (lct_termination_evt) SetEvent (lct_termination_evt); WaitForSingleObject (thr, INFINITE); - DWORD err; - GetExitCodeThread (thr, &err); - if (err) - { - ret = -1; - debug_printf ("listen_client_thread exited with error"); - } NtClose (thr); } evt = InterlockedExchangePointer (&lct_termination_evt, NULL); if (evt) NtClose (evt); - return ret; } int @@ -978,7 +951,7 @@ fhandler_fifo::close () /* Avoid deadlock with lct in case this is called from a signal handler or another thread. */ fifo_client_unlock (); - int ret = stop_listen_client (); + stop_listen_client (); if (read_ready) NtClose (read_ready); if (write_ready) @@ -987,7 +960,7 @@ fhandler_fifo::close () for (int i = 0; i < nhandlers; i++) fc_handler[i].close (); fifo_client_unlock (); - return fhandler_base::close () || ret; + return fhandler_base::close (); } /* If we have a write handle (i.e., we're a duplexer or a writer),