Cygwin: FIFO: clean up locks

Make sure to use the fifo_client lock when (and only when) it is
needed.
This commit is contained in:
Ken Brown 2019-06-22 10:07:48 -04:00
parent d46b3f83c8
commit a92e5ecd0d
1 changed files with 17 additions and 8 deletions

View File

@ -251,7 +251,9 @@ fhandler_fifo::add_client_handler ()
fh->set_nonblocking (false); fh->set_nonblocking (false);
ret = 0; ret = 0;
fc.fh = fh; fc.fh = fh;
fifo_client_lock ();
fc_handler[nhandlers++] = fc; fc_handler[nhandlers++] = fc;
fifo_client_unlock ();
} }
out: out:
return ret; return ret;
@ -298,12 +300,10 @@ fhandler_fifo::listen_client ()
void void
fhandler_fifo::record_connection (fifo_client_handler& fc) fhandler_fifo::record_connection (fifo_client_handler& fc)
{ {
fifo_client_lock ();
fc.state = fc_connected; fc.state = fc_connected;
nconnected++; nconnected++;
fc.fh->set_nonblocking (true); fc.fh->set_nonblocking (true);
set_pipe_non_blocking (fc.fh->get_handle (), true); set_pipe_non_blocking (fc.fh->get_handle (), true);
fifo_client_unlock ();
HANDLE evt = InterlockedExchangePointer (&fc.connect_evt, NULL); HANDLE evt = InterlockedExchangePointer (&fc.connect_evt, NULL);
if (evt) if (evt)
CloseHandle (evt); CloseHandle (evt);
@ -335,18 +335,15 @@ fhandler_fifo::listen_client_thread ()
else else
i++; i++;
} }
fifo_client_unlock ();
/* Create a new client handler. */ /* Create a new client handler. */
if (add_client_handler () < 0) if (add_client_handler () < 0)
{ goto out;
fifo_client_unlock ();
goto out;
}
/* Allow a writer to open. */ /* Allow a writer to open. */
if (!arm (read_ready)) if (!arm (read_ready))
{ {
fifo_client_unlock ();
__seterrno (); __seterrno ();
goto out; goto out;
} }
@ -359,7 +356,6 @@ fhandler_fifo::listen_client_thread ()
status = NtFsControlFile (fc.fh->get_handle (), fc.connect_evt, status = NtFsControlFile (fc.fh->get_handle (), fc.connect_evt,
NULL, NULL, &io, FSCTL_PIPE_LISTEN, NULL, NULL, &io, FSCTL_PIPE_LISTEN,
NULL, 0, NULL, 0); NULL, 0, NULL, 0);
fifo_client_unlock ();
if (status == STATUS_PENDING) if (status == STATUS_PENDING)
{ {
HANDLE w[2] = { fc.connect_evt, lct_termination_evt }; HANDLE w[2] = { fc.connect_evt, lct_termination_evt };
@ -381,6 +377,7 @@ fhandler_fifo::listen_client_thread ()
} }
} }
HANDLE ph = NULL; HANDLE ph = NULL;
fifo_client_lock ();
switch (status) switch (status)
{ {
case STATUS_SUCCESS: case STATUS_SUCCESS:
@ -406,12 +403,15 @@ fhandler_fifo::listen_client_thread ()
will be declared invalid on the next read attempt. */ will be declared invalid on the next read attempt. */
if (ph) if (ph)
CloseHandle (ph); CloseHandle (ph);
fifo_client_unlock ();
goto out; goto out;
default: default:
__seterrno_from_nt_status (status); __seterrno_from_nt_status (status);
fc.state = fc_invalid; fc.state = fc_invalid;
fifo_client_unlock ();
goto out; goto out;
} }
fifo_client_unlock ();
/* Check for thread termination in case WaitForMultipleObjects /* Check for thread termination in case WaitForMultipleObjects
didn't get called above. */ didn't get called above. */
if (IsEventSignalled (lct_termination_evt)) if (IsEventSignalled (lct_termination_evt))
@ -933,9 +933,11 @@ fhandler_fifo::close ()
CloseHandle (read_ready); CloseHandle (read_ready);
if (write_ready) if (write_ready)
CloseHandle (write_ready); CloseHandle (write_ready);
fifo_client_lock ();
for (int i = 0; i < nhandlers; i++) for (int i = 0; i < nhandlers; i++)
if (fc_handler[i].close () < 0) if (fc_handler[i].close () < 0)
ret = -1; ret = -1;
fifo_client_unlock ();
return fhandler_base::close () || ret; return fhandler_base::close () || ret;
} }
@ -983,6 +985,7 @@ fhandler_fifo::dup (fhandler_base *child, int flags)
__seterrno (); __seterrno ();
goto out; goto out;
} }
fifo_client_lock ();
for (int i = 0; i < nhandlers; i++) for (int i = 0; i < nhandlers; i++)
{ {
if (!DuplicateHandle (GetCurrentProcess (), fc_handler[i].fh->get_handle (), if (!DuplicateHandle (GetCurrentProcess (), fc_handler[i].fh->get_handle (),
@ -990,6 +993,7 @@ fhandler_fifo::dup (fhandler_base *child, int flags)
&fhf->fc_handler[i].fh->get_handle (), &fhf->fc_handler[i].fh->get_handle (),
0, true, DUPLICATE_SAME_ACCESS)) 0, true, DUPLICATE_SAME_ACCESS))
{ {
fifo_client_unlock ();
CloseHandle (fhf->read_ready); CloseHandle (fhf->read_ready);
CloseHandle (fhf->write_ready); CloseHandle (fhf->write_ready);
fhf->close (); fhf->close ();
@ -997,6 +1001,7 @@ fhandler_fifo::dup (fhandler_base *child, int flags)
goto out; goto out;
} }
} }
fifo_client_unlock ();
if (!reader || fhf->listen_client ()) if (!reader || fhf->listen_client ())
ret = 0; ret = 0;
if (reader) if (reader)
@ -1017,8 +1022,10 @@ fhandler_fifo::fixup_after_fork (HANDLE parent)
fhandler_base::fixup_after_fork (parent); fhandler_base::fixup_after_fork (parent);
fork_fixup (parent, read_ready, "read_ready"); fork_fixup (parent, read_ready, "read_ready");
fork_fixup (parent, write_ready, "write_ready"); fork_fixup (parent, write_ready, "write_ready");
fifo_client_lock ();
for (int i = 0; i < nhandlers; i++) for (int i = 0; i < nhandlers; i++)
fc_handler[i].fh->fhandler_base::fixup_after_fork (parent); fc_handler[i].fh->fhandler_base::fixup_after_fork (parent);
fifo_client_unlock ();
if (reader && !listen_client ()) if (reader && !listen_client ())
debug_printf ("failed to start lct, %E"); debug_printf ("failed to start lct, %E");
} }
@ -1037,6 +1044,8 @@ fhandler_fifo::set_close_on_exec (bool val)
fhandler_base::set_close_on_exec (val); fhandler_base::set_close_on_exec (val);
set_no_inheritance (read_ready, val); set_no_inheritance (read_ready, val);
set_no_inheritance (write_ready, val); set_no_inheritance (write_ready, val);
fifo_client_lock ();
for (int i = 0; i < nhandlers; i++) for (int i = 0; i < nhandlers; i++)
fc_handler[i].fh->fhandler_base::set_close_on_exec (val); fc_handler[i].fh->fhandler_base::set_close_on_exec (val);
fifo_client_unlock ();
} }