JehanneOS
/
newlib
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Cygwin: FIFO: fix problems finding new owner 

When the owning reader closes and there are still readers open, the
owner needs to wait for a new owner to be found before closing its
fifo_client handlers.  This involves a loop in which dec_nreaders is
called at the beginning and inc_nreaders is called at the end.  Any
other reader that tries to access shmem->_nreaders during this loop
will therefore get an inaccurate answer.

Fix this by adding an nreaders method and using it instead of
dec_nreaders and inc_nreaders.  Also add nreaders_lock to control
access to the shmem->_nreaders.

Make various other changes to improve the reliability of finding a new
owner.
This commit is contained in:
Ken Brown 2020-07-15 09:46:42 -04:00
parent 0ee972d1b0
commit da9fea0759
2 changed files with 63 additions and 35 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
winsup/cygwin/fhandler.h
View File

@ -1328,7 +1328,7 @@ class fifo_shmem_t
{
LONG _nreaders;
fifo_reader_id_t _owner, _prev_owner, _pending_owner;
af_unix_spinlock_t _owner_lock, _reading_lock, _reader_opening_lock;
af_unix_spinlock_t _owner_lock, _reading_lock, _reader_opening_lock, _nreaders_lock;
/* Info about shared memory block used for temporary storage of the
owner's fc_handler list. */
@ -1336,6 +1336,7 @@ class fifo_shmem_t
_sh_fc_handler_updated;
public:
int nreaders () const { return (int) _nreaders; }
int inc_nreaders () { return (int) InterlockedIncrement (&_nreaders); }
int dec_nreaders () { return (int) InterlockedDecrement (&_nreaders); }
@ -1352,6 +1353,8 @@ public:
void reading_unlock () { _reading_lock.unlock (); }
void reader_opening_lock () { _reader_opening_lock.lock (); }
void reader_opening_unlock () { _reader_opening_lock.unlock (); }
void nreaders_lock () { _nreaders_lock.lock (); }
void nreaders_unlock () { _nreaders_lock.unlock (); }
int get_shared_nhandlers () const { return (int) _sh_nhandlers; }
void set_shared_nhandlers (int n) { InterlockedExchange (&_sh_nhandlers, n); }
@ -1420,8 +1423,11 @@ class fhandler_fifo: public fhandler_base
int reopen_shared_fc_handler ();
int remap_shared_fc_handler (size_t);
int nreaders () const { return shmem->nreaders (); }
int inc_nreaders () { return shmem->inc_nreaders (); }
int dec_nreaders () { return shmem->dec_nreaders (); }
void nreaders_lock () { shmem->nreaders_lock (); }
void nreaders_unlock () { shmem->nreaders_unlock (); }
fifo_reader_id_t get_prev_owner () const { return shmem->get_prev_owner (); }
void set_prev_owner (fifo_reader_id_t fr_id)

90
winsup/cygwin/fhandler_fifo.cc
View File

@ -371,6 +371,8 @@ fhandler_fifo::record_connection (fifo_client_handler& fc,
int
fhandler_fifo::update_my_handlers ()
{
int ret = 0;
close_all_handlers ();
fifo_reader_id_t prev = get_prev_owner ();
if (!prev)
@ -387,7 +389,7 @@ fhandler_fifo::update_my_handlers ()
{
debug_printf ("Can't open process of previous owner, %E");
__seterrno ();
return -1;
goto out;
}
for (int i = 0; i < get_shared_nhandlers (); i++)
@ -402,11 +404,13 @@ fhandler_fifo::update_my_handlers ()
debug_printf ("Can't duplicate handle of previous owner, %E");
--nhandlers;
__seterrno ();
return -1;
goto out;
}
fc.state = shared_fc_handler[i].state;
}
return 0;
out:
set_prev_owner (null_fr_id);
return ret;
}
int
@ -1414,41 +1418,59 @@ fhandler_fifo::close ()
{
if (reader)
{
/* If we're the owner, try to find a new owner. */
bool find_new_owner = false;
/* If we're the owner, we can't close our fc_handlers if a new
owner might need to duplicate them. */
bool close_fc_ok = false;
cancel_reader_thread ();
owner_lock ();
if (get_owner () == me)
{
find_new_owner = true;
set_owner (null_fr_id);
set_prev_owner (me);
owner_needed ();
}
owner_unlock ();
nreaders_lock ();
if (dec_nreaders () == 0)
ResetEvent (read_ready);
else if (find_new_owner && !IsEventSignalled (owner_found_evt))
{
bool found = false;
do
if (dec_nreaders () >= 0)
{
/* There's still another reader open. */
if (WaitForSingleObject (owner_found_evt, 1) == WAIT_OBJECT_0)
found = true;
else
{
owner_lock ();
if (get_owner ()) /* We missed owner_found_evt? */
found = true;
else
owner_needed ();
owner_unlock ();
}
}
while (inc_nreaders () > 0 && !found);
close_fc_ok = true;
ResetEvent (read_ready);
ResetEvent (owner_needed_evt);
ResetEvent (owner_found_evt);
set_owner (null_fr_id);
set_prev_owner (null_fr_id);
set_pending_owner (null_fr_id);
set_shared_nhandlers (0);
}
else
{
owner_lock ();
if (get_owner () != me)
close_fc_ok = true;
else
{
set_owner (null_fr_id);
set_prev_owner (me);
if (!get_pending_owner ())
owner_needed ();
}
owner_unlock ();
}
nreaders_unlock ();
while (!close_fc_ok)
{
if (WaitForSingleObject (owner_found_evt, 1) == WAIT_OBJECT_0)
close_fc_ok = true;
else
{
nreaders_lock ();
if (!nreaders ())
{
close_fc_ok = true;
nreaders_unlock ();
}
else
{
nreaders_unlock ();
owner_lock ();
if (get_owner () || get_prev_owner () != me)
close_fc_ok = true;
owner_unlock ();
}
}
}
close_all_handlers ();
if (fc_handler)