On Mon, 2010-05-10 at 22:16 -0600, Pete Zaitcev wrote: > Just recently I hit a combination of clients, chunks, and possibly the > tabled server machine that reliably reproduces tabled looping on CPU > after the client quits. It enters a state when the read returns 0 > (in cli_read), then it changes state to parse_req, fails fetching > anything meaningful, goes back to reading, and so it loops. > > The problem seems too obvious: any network server should close > a client socket which returns EOF... except for one little question: > is it possible to receive zero bytes from an open socket that was > set to O_NONBLOCK? If it is, we must either rely on libevent properly According to Posix specifications - return from read of 0 indicates EOF; doesn't matter on socket options. If O_NONBLOCK is set and the read would block, result is -1 / errno=EAGAIN (or number of bytes read, or 0 = EOF). There is no condition in which O_NONBLOCK should have any effect on read returning the value 0 to indicate an EOF. YMMV with BSD platforms, they have some non-compliant return values around poll when EOF occurs if I recall correctly. Regards -steve > waiting and never calling the read callback unless some data is > still available, or use some kind of ioctl or other to verify > if the connection is still open. If it isn't then something like > the attached patch might work (it assumes that when an asynchronous > socket has no data, it returns -EAGAIN instead of 0 bytes). > > Thoughts? The attached patch seems to work so far, but may fail > in a different environment. > > -- Pete > > diff --git a/server/server.c b/server/server.c > index a28965c..d6a193a 100644 > --- a/server/server.c > +++ b/server/server.c > @@ -671,6 +671,12 @@ size_t cli_wqueued(struct client *cli) > return cli->write_cnt; > } > > +/* > + * Return: > + * 0: progress was NOT made (EOF) > + * >0: some data was gotten > + * <0: an error happened (equals to nevative of system error) > + */ > static int cli_read(struct client *cli) > { > ssize_t rc; > @@ -683,7 +689,7 @@ do_read: > if (errno == EINTR) > goto do_read; > if (errno == EAGAIN) > - return 0; > + return 1; > return -errno; > } > > @@ -699,7 +705,7 @@ do_read: > if (cli->req_used == CLI_REQ_BUF_SZ) > return -ENOSPC; > > - return 0; > + return rc != 0; > } > > bool cli_cb_free(struct client *cli, void *cb_data, bool done) > @@ -1186,7 +1192,7 @@ static bool cli_evt_parse_hdr(struct client *cli, unsigned int events) > static bool cli_evt_read_hdr(struct client *cli, unsigned int events) > { > int rc = cli_read(cli); > - if (rc < 0) { > + if (rc <= 0) { > if (rc == -ENOSPC) > return cli_err(cli, InvalidArgument); > > @@ -1268,7 +1274,7 @@ err_out: > static bool cli_evt_read_req(struct client *cli, unsigned int events) > { > int rc = cli_read(cli); > - if (rc < 0) { > + if (rc <= 0) { > if (rc == -ENOSPC) > return cli_err(cli, InvalidArgument); > > -- > To unsubscribe from this list: send the line "unsubscribe hail-devel" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe hail-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html