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.
This commit is contained in:
Ken Brown 2020-05-06 18:39:26 -04:00
parent ce23e97640
commit 32dbc3d215
2 changed files with 22 additions and 17 deletions

View File

@ -1270,11 +1270,16 @@ public:
#define CYGWIN_FIFO_PIPE_NAME_LEN 47 #define CYGWIN_FIFO_PIPE_NAME_LEN 47
#define MAX_CLIENTS 64 #define MAX_CLIENTS 64
/* The last three are the ones we try to read from. */
enum fifo_client_connect_state enum fifo_client_connect_state
{ {
fc_unknown, fc_unknown,
fc_error,
fc_disconnected,
fc_listening,
fc_connected, fc_connected,
fc_invalid fc_closing,
fc_input_avail,
}; };
enum enum
@ -1316,7 +1321,8 @@ class fhandler_fifo: public fhandler_base
bool listen_client (); bool listen_client ();
int stop_listen_client (); int stop_listen_client ();
int check_listen_client_thread (); int check_listen_client_thread ();
void record_connection (fifo_client_handler&); void record_connection (fifo_client_handler&,
fifo_client_connect_state = fc_connected);
public: public:
fhandler_fifo (); fhandler_fifo ();
bool hit_eof (); bool hit_eof ();

View File

@ -267,6 +267,7 @@ fhandler_fifo::add_client_handler ()
{ {
ret = 0; ret = 0;
fc.h = ph; fc.h = ph;
fc.state = fc_listening;
fc_handler[nhandlers++] = fc; fc_handler[nhandlers++] = fc;
} }
out: out:
@ -311,10 +312,11 @@ fhandler_fifo::listen_client ()
} }
void 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); SetEvent (write_ready);
fc.state = fc_connected; fc.state = s;
nconnected++; nconnected++;
set_pipe_non_blocking (fc.h, true); set_pipe_non_blocking (fc.h, true);
} }
@ -330,15 +332,12 @@ fhandler_fifo::listen_client_thread ()
while (1) while (1)
{ {
/* At the beginning of the loop, all client handlers are /* Cleanup the fc_handler list. */
in the fc_connected or fc_invalid state. */
/* Delete any invalid clients. */
fifo_client_lock (); fifo_client_lock ();
int i = 0; int i = 0;
while (i < nhandlers) while (i < nhandlers)
{ {
if (fc_handler[i].state == fc_invalid) if (fc_handler[i].state < fc_connected)
delete_client_handler (i); delete_client_handler (i);
else else
i++; i++;
@ -393,6 +392,10 @@ fhandler_fifo::listen_client_thread ()
record_connection (fc); record_connection (fc);
ResetEvent (evt); ResetEvent (evt);
break; break;
case STATUS_PIPE_CLOSING:
record_connection (fc, fc_closing);
ResetEvent (evt);
break;
case STATUS_THREAD_IS_TERMINATING: case STATUS_THREAD_IS_TERMINATING:
/* Force NtFsControlFile to complete. Otherwise the next /* Force NtFsControlFile to complete. Otherwise the next
writer to connect might not be recorded in the client 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. */ /* Poll the connected clients for input. */
fifo_client_lock (); fifo_client_lock ();
for (int i = 0; i < nhandlers; i++) for (int i = 0; i < nhandlers; i++)
if (fc_handler[i].state == fc_connected) if (fc_handler[i].state >= fc_connected)
{ {
NTSTATUS status; NTSTATUS status;
IO_STATUS_BLOCK io; IO_STATUS_BLOCK io;
@ -859,18 +862,14 @@ fhandler_fifo::raw_read (void *in_ptr, size_t& len)
case STATUS_PIPE_EMPTY: case STATUS_PIPE_EMPTY:
break; break;
case STATUS_PIPE_BROKEN: case STATUS_PIPE_BROKEN:
/* Client has disconnected. Mark the client handler fc_handler[i].state = fc_disconnected;
to be deleted when it's safe to do that. */
fc_handler[i].state = fc_invalid;
nconnected--; nconnected--;
break; break;
default: default:
debug_printf ("NtReadFile status %y", status); debug_printf ("NtReadFile status %y", status);
__seterrno_from_nt_status (status); fc_handler[i].state = fc_error;
fc_handler[i].state = fc_invalid;
nconnected--; nconnected--;
fifo_client_unlock (); break;
goto errout;
} }
} }
fifo_client_unlock (); fifo_client_unlock ();