Is there a bug in ngx_http_upstream_module's broken connection check code?

[Maxim Dounin]

Hello!

On Thu, Jun 23, 2011 at 12:14:47AM -0400, speedfirst wrote:

> In the function "ngx_http_upstream_check_broken_connection" of
> ngx_http_upstream.c, there is a code segment like below:
> 
> 
>  n = recv(c->fd, buf, 1, MSG_PEEK);
> ...
>  else { /* n == 0 */
>         err = 0;
>     }
> 
> Here use whether receiving 0 bytes from downstream client to judge
> whether the connection has been closed. However, if the downstream
> connection is https, and the connection is cosed, this n will never be
> 0. So the code fails to check the broken connection for https.
> 
> My test is create a connection by firefox and click the "Stop" button in
> the firefox. If the connection is http, n will be 0; but if the
> connection is https, n is always 1. I try to increase the peek read
> limit from 1 to 1000 and then n will get 37.
> 
> So is it a bug or I misunderstand something?

Yes, the code in question won't be able to detect SSL connection 
close with appropriate shutdown records sent by a client (as well 
as connection close with some other pending data, e.g. pipelined 
request).  It is not possible to detect connection close with 
standard socket interface if there are pending data without 
reading that data first, which isn't always desired/possible.

In the particular case of SSL it should be possible to use 
SSL_peek() here, but better aproach is to use OS-provided hint, 
e.g. as EV_EOF provided by kqueue interface (and used by nginx).

Maxim Dounin