Re: RFC loop at EOF from client socket

[Steven Dake <sdake@redhat.com>]

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