From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 2210) id AD134385041D; Tue, 4 Aug 2020 15:29:26 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org AD134385041D 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: add a timeout to take_ownership X-Act-Checkin: newlib-cygwin X-Git-Author: Ken Brown X-Git-Refname: refs/heads/master X-Git-Oldrev: 6acce025d07aa9328968351e1b718c0974e8780d X-Git-Newrev: 6ed067a0ae5c39b43c8a77433e4c8ca30f87020a Message-Id: <20200804152926.AD134385041D@sourceware.org> Date: Tue, 4 Aug 2020 15:29:26 +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: Tue, 04 Aug 2020 15:29:26 -0000 https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;h=6ed067a0ae5c39b43c8a77433e4c8ca30f87020a commit 6ed067a0ae5c39b43c8a77433e4c8ca30f87020a Author: Ken Brown Date: Mon Aug 3 09:17:06 2020 -0400 Cygwin: FIFO: add a timeout to take_ownership fhandler_fifo::take_ownership() is called from select.cc::peek_fifo and fhandler_fifo::raw_read and could potentially block indefinitely if something goes wrong. This is always undesirable in peek_fifo, and it is undesirable in a nonblocking read. Fix this by adding a timeout parameter to take_ownership. Arbitrarily use a 1 ms timeout in peek_fifo and a 10 ms timeout in raw_read. These numbers may have to be tweaked based on experience. Replace the call to cygwait in take_ownership by a call to WFSO. There's no need to allow interruption now that we have a timeout. Diff: --- winsup/cygwin/fhandler.h | 2 +- winsup/cygwin/fhandler_fifo.cc | 74 ++++++++++++++++-------------------------- winsup/cygwin/select.cc | 7 +--- 3 files changed, 30 insertions(+), 53 deletions(-) diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h index 60bd27e00..5488327a2 100644 --- a/winsup/cygwin/fhandler.h +++ b/winsup/cygwin/fhandler.h @@ -1487,7 +1487,7 @@ public: void fifo_client_lock () { _fifo_client_lock.lock (); } void fifo_client_unlock () { _fifo_client_lock.unlock (); } - DWORD take_ownership (); + int take_ownership (DWORD timeout = INFINITE); void reading_lock () { shmem->reading_lock (); } void reading_unlock () { shmem->reading_unlock (); } diff --git a/winsup/cygwin/fhandler_fifo.cc b/winsup/cygwin/fhandler_fifo.cc index 7b87aab00..b8a47f27f 100644 --- a/winsup/cygwin/fhandler_fifo.cc +++ b/winsup/cygwin/fhandler_fifo.cc @@ -1214,16 +1214,17 @@ fhandler_fifo::raw_write (const void *ptr, size_t len) return ret; } -/* Called from raw_read and select.cc:peek_fifo. Return WAIT_OBJECT_0 - on success. */ -DWORD -fhandler_fifo::take_ownership () +/* Called from raw_read and select.cc:peek_fifo. */ +int +fhandler_fifo::take_ownership (DWORD timeout) { + int ret = 0; + owner_lock (); if (get_owner () == me) { owner_unlock (); - return WAIT_OBJECT_0; + return 0; } set_pending_owner (me); /* Wake up my fifo_reader_thread. */ @@ -1233,18 +1234,25 @@ fhandler_fifo::take_ownership () SetEvent (update_needed_evt); owner_unlock (); /* The reader threads should now do the transfer. */ - DWORD waitret = cygwait (owner_found_evt, cw_cancel | cw_sig_eintr); - owner_lock (); - if (waitret == WAIT_OBJECT_0 - && (get_owner () != me || get_pending_owner ())) + switch (WaitForSingleObject (owner_found_evt, timeout)) { - /* Something went wrong. Return WAIT_TIMEOUT, which can't be - returned by the above cygwait call. */ - set_pending_owner (null_fr_id); - waitret = WAIT_TIMEOUT; + case WAIT_OBJECT_0: + owner_lock (); + if (get_owner () != me) + { + debug_printf ("owner_found_evt signaled, but I'm not the owner"); + ret = -1; + } + owner_unlock (); + break; + case WAIT_TIMEOUT: + debug_printf ("timed out"); + ret = -1; + default: + debug_printf ("WFSO failed, %E"); + ret = -1; } - owner_unlock (); - return waitret; + return ret; } void __reg3 @@ -1255,38 +1263,12 @@ fhandler_fifo::raw_read (void *in_ptr, size_t& len) while (1) { + int nconnected = 0; + /* No one else can take ownership while we hold the reading_lock. */ reading_lock (); - switch (take_ownership ()) - { - case WAIT_OBJECT_0: - break; - case WAIT_SIGNALED: - if (_my_tls.call_signal_handler ()) - { - reading_unlock (); - continue; - } - else - { - set_errno (EINTR); - reading_unlock (); - goto errout; - } - break; - case WAIT_CANCELED: - reading_unlock (); - pthread::static_cancel_self (); - break; - case WAIT_TIMEOUT: - reading_unlock (); - debug_printf ("take_ownership returned an unexpected result; retry"); - continue; - default: - reading_unlock (); - debug_printf ("unknown error while trying to take ownership, %E"); - goto errout; - } + if (take_ownership (10) < 0) + goto maybe_retry; /* Poll the connected clients for input. Make two passes. On the first pass, just try to read from the client from which @@ -1332,7 +1314,6 @@ fhandler_fifo::raw_read (void *in_ptr, size_t& len) } /* Second pass. */ - int nconnected = 0; for (int i = 0; i < nhandlers; i++) if (fc_handler[i].state >= fc_closing) { @@ -1375,6 +1356,7 @@ fhandler_fifo::raw_read (void *in_ptr, size_t& len) len = 0; return; } +maybe_retry: reading_unlock (); if (is_nonblocking ()) { diff --git a/winsup/cygwin/select.cc b/winsup/cygwin/select.cc index 3f3f33fb5..1ba76c817 100644 --- a/winsup/cygwin/select.cc +++ b/winsup/cygwin/select.cc @@ -867,16 +867,11 @@ peek_fifo (select_record *s, bool from_select) } fh->reading_lock (); - if (fh->take_ownership () != WAIT_OBJECT_0) + if (fh->take_ownership (1) < 0) { - select_printf ("%s, unable to take ownership", fh->get_name ()); fh->reading_unlock (); - gotone += s->read_ready = true; - if (s->except_selected) - gotone += s->except_ready = true; goto out; } - fh->fifo_client_lock (); int nconnected = 0; for (int i = 0; i < fh->get_nhandlers (); i++)