[PATCH] ngx_http_upstream state machine fixes

[Maxim Dounin]

Hello!

On Wed, Mar 21, 2012 at 09:42:02PM +0800, agentzh wrote:

> Hello!
> 
> I've just found that ngx_http_upstream seems to assume that the
> upstream request is already fully sent (i.e., fully flushed into the
> socket send buffers) after the first read event on the upstream
> connection, and in this case, ngx_http_upstream_send_request_handler
> will just silently refuse to re-send the requests on subsequent write
> events even if the request has not been fully sent out yet, hence
> leading to a hang.
> 
> This was observed when doing pipelined requests atop ngx_http_upstream
> where the remote server can start sending back response data *before*
> all the request data is flushed out (the latter can be trivially
> emulated by our mockeagain tool: https://github.com/agentzh/mockeagain
> ).
> 
> I've attached a patch that (hopefully) fixes this issue by introducing
> an extra 1-bit "request_all_sent" flag to ngx_http_upstream_t, which
> can be set to 1 after ngx_output_chain returns NGX_OK in
> ngx_http_upstream_send_request. I know there is already a
> "request_sent" flag which indicates whether there is the first attempt
> of sending out the request) :)
> 
> Feedback will be highly appreciated :)

I've looked into this a while ago (the same problem appears e.g. 
if fastcgi app returns response header before reading body, and 
then tries to read body).

The major question seems to be "what should be done if we've not 
yet sent full request, but already got response"?  Right now line 
is drawn at "if we've got response header, we stop sending 
request".  And if we move the line - we should clearly understand 
where the new line is and why.

As for the patch from coding point of view - I belive adding 
another flag isn't really needed, it should be enough to reset 
r->write_event_handler just after we've done writing.

Maxim Dounin