Cygwin: FIFO: change the fifo_client_connect_state enum

Make the values correspond to the possible return values of
fifo_client_handler::pipe_state().

When cleaning up the fc_handler list in listen_client_thread(), don't
delete handlers in the fc_closing state.  I think the pipe might still
have input to be read in that case.

Set the state to fc_closing later in the same function if a connection
is made and the status returned by NtFsControlFile is
STATUS_PIPE_CLOSING.

In raw_read, don't error out if NtReadFile returns an unexpected
status; just set the state of that handler to fc_error.  One writer in
a bad state doesn't justify giving up on reading.

2 files changed, 22 insertions, 17 deletions

diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
index e841f96ac..c1f47025a 100644
--- a/winsup/cygwin/fhandler.h
+++ b/winsup/cygwin/fhandler.h
@@ -1270,11 +1270,16 @@ public:
 #define CYGWIN_FIFO_PIPE_NAME_LEN     47
 #define MAX_CLIENTS 64
 
+/* The last three are the ones we try to read from. */
 enum fifo_client_connect_state
 {
   fc_unknown,
+  fc_error,
+  fc_disconnected,
+  fc_listening,
   fc_connected,
-  fc_invalid
+  fc_closing,
+  fc_input_avail,
 };
 
 enum
@@ -1316,7 +1321,8 @@ class fhandler_fifo: public fhandler_base
   bool listen_client ();
   int stop_listen_client ();
   int check_listen_client_thread ();
-  void record_connection (fifo_client_handler&);
+  void record_connection (fifo_client_handler&,
+			  fifo_client_connect_state = fc_connected);
 public:
   fhandler_fifo ();
   bool hit_eof ();
diff --git a/winsup/cygwin/fhandler_fifo.cc b/winsup/cygwin/fhandler_fifo.cc
index 6b71dd950..ba3dbb124 100644
--- a/winsup/cygwin/fhandler_fifo.cc
+++ b/winsup/cygwin/fhandler_fifo.cc
@@ -267,6 +267,7 @@ fhandler_fifo::add_client_handler ()
     {
       ret = 0;
       fc.h = ph;
+      fc.state = fc_listening;
       fc_handler[nhandlers++] = fc;
     }
 out:
@@ -311,10 +312,11 @@ fhandler_fifo::listen_client ()
 }
 
 void
-fhandler_fifo::record_connection (fifo_client_handler& fc)
+fhandler_fifo::record_connection (fifo_client_handler& fc,
+				  fifo_client_connect_state s)
 {
   SetEvent (write_ready);
-  fc.state = fc_connected;
+  fc.state = s;
   nconnected++;
   set_pipe_non_blocking (fc.h, true);
 }
@@ -330,15 +332,12 @@ fhandler_fifo::listen_client_thread ()
 
   while (1)
     {
-      /* At the beginning of the loop, all client handlers are
-	 in the fc_connected or fc_invalid state. */
-
-      /* Delete any invalid clients. */
+      /* Cleanup the fc_handler list. */
       fifo_client_lock ();
       int i = 0;
       while (i < nhandlers)
 	{
-	  if (fc_handler[i].state == fc_invalid)
+	  if (fc_handler[i].state < fc_connected)
 	    delete_client_handler (i);
 	  else
 	    i++;
@@ -393,6 +392,10 @@ fhandler_fifo::listen_client_thread ()
 	  record_connection (fc);
 	  ResetEvent (evt);
 	  break;
+	case STATUS_PIPE_CLOSING:
+	  record_connection (fc, fc_closing);
+	  ResetEvent (evt);
+	  break;
 	case STATUS_THREAD_IS_TERMINATING:
 	  /* Force NtFsControlFile to complete.  Otherwise the next
 	     writer to connect might not be recorded in the client
@@ -835,7 +838,7 @@ fhandler_fifo::raw_read (void *in_ptr, size_t& len)
       /* Poll the connected clients for input. */
       fifo_client_lock ();
       for (int i = 0; i < nhandlers; i++)
-	if (fc_handler[i].state == fc_connected)
+	if (fc_handler[i].state >= fc_connected)
 	  {
 	    NTSTATUS status;
 	    IO_STATUS_BLOCK io;
@@ -859,18 +862,14 @@ fhandler_fifo::raw_read (void *in_ptr, size_t& len)
 	      case STATUS_PIPE_EMPTY:
 		break;
 	      case STATUS_PIPE_BROKEN:
-		/* Client has disconnected.  Mark the client handler
-		   to be deleted when it's safe to do that. */
-		fc_handler[i].state = fc_invalid;
+		fc_handler[i].state = fc_disconnected;
 		nconnected--;
 		break;
 	      default:
 		debug_printf ("NtReadFile status %y", status);
-		__seterrno_from_nt_status (status);
-		fc_handler[i].state = fc_invalid;
+		fc_handler[i].state = fc_error;
 		nconnected--;
-		fifo_client_unlock ();
-		goto errout;
+		break;
 	      }
 	  }
       fifo_client_unlock ();