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.
This commit is contained in:
Ken Brown 2020-08-03 09:17:06 -04:00
parent b4d6e6747b
commit 8e83c7b65a
3 changed files with 30 additions and 53 deletions

View File

@ -1493,7 +1493,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 (); }

View File

@ -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 ())
{

View File

@ -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++)