From f4a92734eac17dbfc7ff3541eef9611c87184ed0 Mon Sep 17 00:00:00 2001 From: Ken Brown Date: Fri, 23 Sep 2022 10:24:04 -0400 Subject: [PATCH] Cygwin: select: don't report read ready on a FIFO never opened for writing According to POSIX and the Linux man page, select(2) is supposed to report read ready if a file is at EOF. In the case of a FIFO, this means that the pipe is empty and there are no writers. But there seems to be an undocumented exception, observed on Linux and other platforms: If no writer has ever been opened, then select(2) does not report read ready. This can happen if a reader is opened with O_NONBLOCK before any writers have opened. This commit makes Cygwin consistent with those other platforms by introducing a special EOF test, fhandler_fifo::select_hit_eof, which returns false if there's never been a writer opened. To implement this we use a new variable '_writer_opened' in the FIFO's shared memory, which is set to 1 the first time a writer opens. New methods writer_opened() and set_writer_opened() are used to test and set this variable. Addresses: https://cygwin.com/pipermail/cygwin/2022-September/252223.html --- winsup/cygwin/fhandler/fifo.cc | 2 ++ winsup/cygwin/local_includes/fhandler.h | 9 +++++++++ winsup/cygwin/release/3.3.4 | 3 +++ winsup/cygwin/select.cc | 12 +++++++++++- 4 files changed, 25 insertions(+), 1 deletion(-) diff --git a/winsup/cygwin/fhandler/fifo.cc b/winsup/cygwin/fhandler/fifo.cc index 1d3e42908..ecfb33bff 100644 --- a/winsup/cygwin/fhandler/fifo.cc +++ b/winsup/cygwin/fhandler/fifo.cc @@ -1043,6 +1043,8 @@ writer_shmem: set_pipe_non_blocking (get_handle (), flags & O_NONBLOCK); nwriters_lock (); inc_nwriters (); + if (!writer_opened () ) + set_writer_opened (); SetEvent (write_ready); ResetEvent (writer_opening); nwriters_unlock (); diff --git a/winsup/cygwin/local_includes/fhandler.h b/winsup/cygwin/local_includes/fhandler.h index aad7f4c37..b012c6e8f 100644 --- a/winsup/cygwin/local_includes/fhandler.h +++ b/winsup/cygwin/local_includes/fhandler.h @@ -1327,6 +1327,8 @@ struct fifo_reader_id_t class fifo_shmem_t { LONG _nreaders, _nwriters; + /* Set to 1 the first time a writer opens. */ + LONG _writer_opened; fifo_reader_id_t _owner, _prev_owner, _pending_owner; af_unix_spinlock_t _owner_lock, _reading_lock, _nreaders_lock, _nwriters_lock; @@ -1343,6 +1345,9 @@ public: int inc_nwriters () { return (int) InterlockedIncrement (&_nwriters); } int dec_nwriters () { return (int) InterlockedDecrement (&_nwriters); } + bool writer_opened () const { return (bool) _writer_opened; } + void set_writer_opened () { InterlockedExchange (&_writer_opened, 1); } + fifo_reader_id_t get_owner () const { return _owner; } void set_owner (fifo_reader_id_t fr_id) { _owner = fr_id; } fifo_reader_id_t get_prev_owner () const { return _prev_owner; } @@ -1425,6 +1430,8 @@ class fhandler_fifo: public fhandler_pipe_fifo int nwriters () const { return shmem->nwriters (); } int inc_nwriters () { return shmem->inc_nwriters (); } int dec_nwriters () { return shmem->dec_nwriters (); } + bool writer_opened () const { return shmem->writer_opened (); } + void set_writer_opened () { shmem->set_writer_opened (); } void nreaders_lock () { shmem->nreaders_lock (); } void nreaders_unlock () { shmem->nreaders_unlock (); } void nwriters_lock () { shmem->nwriters_lock (); } @@ -1480,6 +1487,8 @@ public: /* Called if we appear to be at EOF after polling fc_handlers. */ bool hit_eof () const { return !nwriters () && !IsEventSignalled (writer_opening); } + /* Special EOF test needed by select.cc:peek_fifo(). */ + bool select_hit_eof () const { return hit_eof () && writer_opened (); } int get_nhandlers () const { return nhandlers; } fifo_client_handler &get_fc_handler (int i) { return fc_handler[i]; } PUNICODE_STRING get_pipe_name (); diff --git a/winsup/cygwin/release/3.3.4 b/winsup/cygwin/release/3.3.4 index bc74e6914..616f73ff0 100644 --- a/winsup/cygwin/release/3.3.4 +++ b/winsup/cygwin/release/3.3.4 @@ -37,3 +37,6 @@ Bug Fixes - Fix a permission problem when writing DOS attributes on Samba. Addresses: https://cygwin.com/pipermail/cygwin/2022-January/250629.html +- For Linux compatibility, select(2) no longer reports read ready on a + FIFO that has never been opened for writing. + Addresses: https://cygwin.com/pipermail/cygwin/2022-September/252223.html diff --git a/winsup/cygwin/select.cc b/winsup/cygwin/select.cc index 76ab91bbb..2fd7b72b6 100644 --- a/winsup/cygwin/select.cc +++ b/winsup/cygwin/select.cc @@ -950,7 +950,17 @@ peek_fifo (select_record *s, bool from_select) } } fh->fifo_client_unlock (); - if (!nconnected && fh->hit_eof ()) + /* According to POSIX and the Linux man page, we're supposed to + report read ready if the FIFO is at EOF, i.e., if the pipe is + empty and there are no writers. But there seems to be an + undocumented exception, observed on Linux and other platforms + (https://cygwin.com/pipermail/cygwin/2022-September/252223.html): + If no writer has ever been opened, then we do not report read + ready. This can happen if a reader is opened with O_NONBLOCK + before any writers have opened. To be consistent with other + platforms, we use a special EOF test that returns false if + there's never been a writer opened. */ + if (!nconnected && fh->select_hit_eof ()) { select_printf ("read: %s, saw EOF", fh->get_name ()); gotone += s->read_ready = true; -- 2.37.3