[newlib-cygwin] Cygwin: select: don't report read ready on a FIFO never opened for writing

[newlib-cygwin] Cygwin: select: don't report read ready on a FIFO never opened for writing

[Ken Brown]

https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;h=5781871775fcba783938bc4ac645eba866711fc5

commit 5781871775fcba783938bc4ac645eba866711fc5
Author: Ken Brown <kbrown@cornell.edu>
Date:   Fri Sep 23 10:24:04 2022 -0400

    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

Diff:
---
 winsup/cygwin/fhandler/fifo.cc          |  1 +
 winsup/cygwin/local_includes/fhandler.h |  9 +++++++++
 winsup/cygwin/release/3.3.4             |  3 +++
 winsup/cygwin/select.cc                 | 12 +++++++++++-
 4 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/winsup/cygwin/fhandler/fifo.cc b/winsup/cygwin/fhandler/fifo.cc
index 1d3e42908..efea508ae 100644
--- a/winsup/cygwin/fhandler/fifo.cc
+++ b/winsup/cygwin/fhandler/fifo.cc
@@ -1043,6 +1043,7 @@ writer_shmem:
   set_pipe_non_blocking (get_handle (), flags & O_NONBLOCK);
   nwriters_lock ();
   inc_nwriters ();
+  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;