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 [email protected]
> 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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html