[newlib-cygwin] Cygwin: FIFO: designate one reader as owner
Ken Brown
kbrown@sourceware.org
Fri May 8 11:30:12 GMT 2020
https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;h=606baf5566d683ac68e3414c457d124c7c7bdcc5
commit 606baf5566d683ac68e3414c457d124c7c7bdcc5
Author: Ken Brown <kbrown@cornell.edu>
Date: Thu Mar 26 14:32:10 2020 -0400
Cygwin: FIFO: designate one reader as owner
Among all the open readers of a FIFO, one is declared to be the owner.
This is the only reader that listens for client connections, and it is
the only one that has an accurate fc_handler list.
Add shared data and methods for getting and setting the owner, as well
as a lock to prevent more than one reader from accessing these data
simultaneously.
Modify the fifo_reader_thread so that it checks the owner at the
beginning of its loop. If there is no owner, it takes ownership. If
there is an owner but it is a different reader, the thread just waits
to be canceled. Otherwise, it listens for client connections as
before.
Remove the 'first' argument from create_pipe_instance. It is not
needed, and it may be confusing in the future since only the owner
knows whether a pipe instance is the first.
When opening a reader, don't return until the fifo_reader_thread has
time to set an owner.
If the owner closes, indicate that there is no longer an owner.
Clear the child's fc_handler list in dup, and don't bother duplicating
the handles. The child never starts out as owner, so it can't use
those handles.
Do the same thing in fixup_after_fork in the close-on-exec case. In
the non-close-on-exec case, the child inherits an fc_handler list that
it can't use, but we can just leave it alone; the handles will be
closed when the child is closed.
Diff:
---
winsup/cygwin/fhandler.h | 13 ++-
winsup/cygwin/fhandler_fifo.cc | 237 ++++++++++++++++++++++-------------------
2 files changed, 141 insertions(+), 109 deletions(-)
diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
index 65aab4da3..bd44da5cd 100644
--- a/winsup/cygwin/fhandler.h
+++ b/winsup/cygwin/fhandler.h
@@ -1324,10 +1324,17 @@ struct fifo_reader_id_t
class fifo_shmem_t
{
LONG _nreaders;
+ fifo_reader_id_t _owner;
+ af_unix_spinlock_t _owner_lock;
public:
int inc_nreaders () { return (int) InterlockedIncrement (&_nreaders); }
int dec_nreaders () { return (int) InterlockedDecrement (&_nreaders); }
+
+ fifo_reader_id_t get_owner () const { return _owner; }
+ void set_owner (fifo_reader_id_t fr_id) { _owner = fr_id; }
+ void owner_lock () { _owner_lock.lock (); }
+ void owner_unlock () { _owner_lock.unlock (); }
};
class fhandler_fifo: public fhandler_base
@@ -1356,7 +1363,7 @@ class fhandler_fifo: public fhandler_base
bool __reg2 wait (HANDLE);
static NTSTATUS npfs_handle (HANDLE &);
- HANDLE create_pipe_instance (bool);
+ HANDLE create_pipe_instance ();
NTSTATUS open_pipe (HANDLE&);
NTSTATUS wait_open_pipe (HANDLE&);
int add_client_handler ();
@@ -1384,6 +1391,10 @@ public:
void fifo_client_unlock () { _fifo_client_lock.unlock (); }
fifo_reader_id_t get_me () const { return me; }
+ fifo_reader_id_t get_owner () const { return shmem->get_owner (); }
+ void set_owner (fifo_reader_id_t fr_id) { shmem->set_owner (fr_id); }
+ void owner_lock () { shmem->owner_lock (); }
+ void owner_unlock () { shmem->owner_unlock (); }
int open (int, mode_t);
off_t lseek (off_t offset, int whence);
diff --git a/winsup/cygwin/fhandler_fifo.cc b/winsup/cygwin/fhandler_fifo.cc
index 5676a2c97..0b9b33785 100644
--- a/winsup/cygwin/fhandler_fifo.cc
+++ b/winsup/cygwin/fhandler_fifo.cc
@@ -164,7 +164,7 @@ fhandler_fifo::npfs_handle (HANDLE &nph)
blocking mode so that we can easily wait for a connection. After
it is connected, it is put in nonblocking mode. */
HANDLE
-fhandler_fifo::create_pipe_instance (bool first)
+fhandler_fifo::create_pipe_instance ()
{
NTSTATUS status;
HANDLE npfsh;
@@ -187,14 +187,12 @@ fhandler_fifo::create_pipe_instance (bool first)
access = GENERIC_READ | FILE_READ_ATTRIBUTES | FILE_WRITE_ATTRIBUTES
| SYNCHRONIZE;
sharing = FILE_SHARE_READ | FILE_SHARE_WRITE;
- hattr = openflags & O_CLOEXEC ? 0 : OBJ_INHERIT;
- if (first)
- hattr |= OBJ_CASE_INSENSITIVE;
+ hattr = (openflags & O_CLOEXEC ? 0 : OBJ_INHERIT) | OBJ_CASE_INSENSITIVE;
InitializeObjectAttributes (&attr, get_pipe_name (),
hattr, npfsh, NULL);
timeout.QuadPart = -500000;
status = NtCreateNamedPipeFile (&ph, access, &attr, &io, sharing,
- first ? FILE_CREATE : FILE_OPEN, 0,
+ FILE_OPEN_IF, 0,
FILE_PIPE_MESSAGE_TYPE
| FILE_PIPE_REJECT_REMOTE_CLIENTS,
FILE_PIPE_MESSAGE_MODE,
@@ -292,14 +290,13 @@ fhandler_fifo::add_client_handler ()
int ret = -1;
fifo_client_handler fc;
HANDLE ph = NULL;
- bool first = (nhandlers == 0);
if (nhandlers == MAX_CLIENTS)
{
set_errno (EMFILE);
goto out;
}
- ph = create_pipe_instance (first);
+ ph = create_pipe_instance ();
if (!ph)
goto out;
else
@@ -349,92 +346,120 @@ fhandler_fifo::fifo_reader_thread_func ()
while (1)
{
- /* Cleanup the fc_handler list. */
- fifo_client_lock ();
- int i = 0;
- while (i < nhandlers)
+ fifo_reader_id_t cur_owner;
+
+ owner_lock ();
+ cur_owner = get_owner ();
+ if (!cur_owner)
{
- if (fc_handler[i].state < fc_connected)
- delete_client_handler (i);
- else
- i++;
+ set_owner (me);
+ owner_unlock ();
+ continue;
+ }
+ else if (cur_owner != me)
+ {
+ owner_unlock ();
+ WaitForSingleObject (cancel_evt, INFINITE);
+ goto canceled;
}
+ else
+ {
+ /* I'm the owner */
+ fifo_client_lock ();
- /* Create a new client handler. */
- if (add_client_handler () < 0)
- api_fatal ("Can't add a client handler, %E");
+ /* Cleanup the fc_handler list. */
+ fifo_client_lock ();
+ int i = 0;
+ while (i < nhandlers)
+ {
+ if (fc_handler[i].state < fc_connected)
+ delete_client_handler (i);
+ else
+ i++;
+ }
- /* Listen for a writer to connect to the new client handler. */
- fifo_client_handler& fc = fc_handler[nhandlers - 1];
- fifo_client_unlock ();
- NTSTATUS status;
- IO_STATUS_BLOCK io;
- bool cancel = false;
+ /* Create a new client handler. */
+ if (add_client_handler () < 0)
+ api_fatal ("Can't add a client handler, %E");
- status = NtFsControlFile (fc.h, conn_evt, NULL, NULL, &io,
- FSCTL_PIPE_LISTEN, NULL, 0, NULL, 0);
- if (status == STATUS_PENDING)
- {
- HANDLE w[2] = { conn_evt, cancel_evt };
- switch (WaitForMultipleObjects (2, w, false, INFINITE))
+ /* Listen for a writer to connect to the new client handler. */
+ fifo_client_handler& fc = fc_handler[nhandlers - 1];
+ fifo_client_unlock ();
+ owner_unlock ();
+ NTSTATUS status;
+ IO_STATUS_BLOCK io;
+ bool cancel = false;
+
+ status = NtFsControlFile (fc.h, conn_evt, NULL, NULL, &io,
+ FSCTL_PIPE_LISTEN, NULL, 0, NULL, 0);
+ if (status == STATUS_PENDING)
{
- case WAIT_OBJECT_0:
- status = io.Status;
+ HANDLE w[2] = { conn_evt, cancel_evt };
+ switch (WaitForMultipleObjects (2, w, false, INFINITE))
+ {
+ case WAIT_OBJECT_0:
+ status = io.Status;
+ debug_printf ("NtFsControlFile STATUS_PENDING, then %y",
+ status);
+ break;
+ case WAIT_OBJECT_0 + 1:
+ status = STATUS_THREAD_IS_TERMINATING;
+ cancel = true;
+ break;
+ default:
+ api_fatal ("WFMO failed, %E");
+ }
+ }
+ else
+ debug_printf ("NtFsControlFile status %y, no STATUS_PENDING",
+ status);
+ HANDLE ph = NULL;
+ NTSTATUS status1;
+
+ fifo_client_lock ();
+ switch (status)
+ {
+ case STATUS_SUCCESS:
+ case STATUS_PIPE_CONNECTED:
+ record_connection (fc);
break;
- case WAIT_OBJECT_0 + 1:
- status = STATUS_THREAD_IS_TERMINATING;
- cancel = true;
+ case STATUS_PIPE_CLOSING:
+ record_connection (fc, fc_closing);
+ break;
+ case STATUS_THREAD_IS_TERMINATING:
+ /* 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
+ /* 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:
- api_fatal ("WFMO failed, %E");
+ break;
}
+ fifo_client_unlock ();
+ if (ph)
+ NtClose (ph);
+ if (cancel)
+ goto canceled;
}
- HANDLE ph = NULL;
- NTSTATUS status1;
-
- fifo_client_lock ();
- switch (status)
- {
- case STATUS_SUCCESS:
- case STATUS_PIPE_CONNECTED:
- record_connection (fc);
- break;
- case STATUS_PIPE_CLOSING:
- record_connection (fc, fc_closing);
- break;
- case STATUS_THREAD_IS_TERMINATING:
- /* 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
- /* 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:
- break;
- }
- fifo_client_unlock ();
- if (ph)
- NtClose (ph);
- if (cancel)
- goto canceled;
}
canceled:
if (conn_evt)
@@ -580,6 +605,15 @@ fhandler_fifo::open (int flags, mode_t)
me.winpid = GetCurrentProcessId ();
me.fh = this;
new cygthread (fifo_reader_thread, this, "fifo_reader", thr_sync_evt);
+ /* Wait until there's an owner. */
+ owner_lock ();
+ while (!get_owner ())
+ {
+ owner_unlock ();
+ yield ();
+ owner_lock ();
+ }
+ owner_unlock ();
/* If we're a duplexer, we need a handle for writing. */
if (duplexer)
@@ -1014,6 +1048,10 @@ fhandler_fifo::close ()
if (dec_nreaders () == 0)
ResetEvent (read_ready);
cancel_reader_thread ();
+ owner_lock ();
+ if (get_owner () == me)
+ set_owner (null_fr_id);
+ owner_unlock ();
if (cancel_evt)
NtClose (cancel_evt);
if (thr_sync_evt)
@@ -1056,7 +1094,6 @@ fhandler_fifo::fcntl (int cmd, intptr_t arg)
int
fhandler_fifo::dup (fhandler_base *child, int flags)
{
- int i = 0;
fhandler_fifo *fhf = NULL;
if (get_flags () & O_PATH)
@@ -1092,6 +1129,9 @@ fhandler_fifo::dup (fhandler_base *child, int flags)
/* Make sure the child starts unlocked. */
fhf->fifo_client_unlock ();
+ /* Clear fc_handler list; the child never starts as owner. */
+ fhf->nhandlers = 0;
+
if (!DuplicateHandle (GetCurrentProcess (), shmem_handle,
GetCurrentProcess (), &fhf->shmem_handle,
0, !(flags & O_CLOEXEC), DUPLICATE_SAME_ACCESS))
@@ -1101,25 +1141,8 @@ fhandler_fifo::dup (fhandler_base *child, int flags)
}
if (fhf->reopen_shmem () < 0)
goto err_close_shmem_handle;
- fifo_client_lock ();
- for (i = 0; i < nhandlers; i++)
- {
- if (!DuplicateHandle (GetCurrentProcess (), fc_handler[i].h,
- GetCurrentProcess (), &fhf->fc_handler[i].h,
- 0, !(flags & O_CLOEXEC), DUPLICATE_SAME_ACCESS))
- {
- __seterrno ();
- break;
- }
- }
- if (i < nhandlers)
- {
- fifo_client_unlock ();
- goto err_close_handlers;
- }
- fifo_client_unlock ();
if (!(fhf->cancel_evt = create_event ()))
- goto err_close_handlers;
+ goto err_close_shmem;
if (!(fhf->thr_sync_evt = create_event ()))
goto err_close_cancel_evt;
inc_nreaders ();
@@ -1129,9 +1152,7 @@ fhandler_fifo::dup (fhandler_base *child, int flags)
return 0;
err_close_cancel_evt:
NtClose (fhf->cancel_evt);
-err_close_handlers:
- for (int j = 0; j < i; j++)
- fhf->fc_handler[j].close ();
+err_close_shmem:
NtUnmapViewOfSection (GetCurrentProcess (), fhf->shmem);
err_close_shmem_handle:
NtClose (fhf->shmem_handle);
@@ -1160,10 +1181,10 @@ fhandler_fifo::fixup_after_fork (HANDLE parent)
fork_fixup (parent, shmem_handle, "shmem_handle");
if (reopen_shmem () < 0)
api_fatal ("Can't reopen shared memory during fork, %E");
- fifo_client_lock ();
- for (int i = 0; i < nhandlers; i++)
- fork_fixup (parent, fc_handler[i].h, "fc_handler[].h");
- fifo_client_unlock ();
+ if (close_on_exec ())
+ /* Prevent a later attempt to close the non-inherited
+ pipe-instance handles copied from the parent. */
+ nhandlers = 0;
if (!(cancel_evt = create_event ()))
api_fatal ("Can't create reader thread cancel event during fork, %E");
if (!(thr_sync_evt = create_event ()))
More information about the Cygwin-cvs
mailing list