[PATCH] ngx_http_upstream state machine fixes

[agentzh]

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 :)

Thanks!
-agentzh

diff -ur nginx-1.0.11/src/http/ngx_http_upstream.c
nginx-1.0.11-patched/src/http/ngx_http_upstream.c
--- nginx-1.0.11/src/http/ngx_http_upstream.c	2011-12-14
02:34:34.000000000 +0800
+++ nginx-1.0.11-patched/src/http/ngx_http_upstream.c	2012-03-21
21:20:17.333111806 +0800
@@ -1385,6 +1385,8 @@

     /* rc == NGX_OK */

+    u->request_all_sent = 1;
+
     if (c->tcp_nopush == NGX_TCP_NOPUSH_SET) {
         if (ngx_tcp_push(c->fd) == NGX_ERROR) {
             ngx_log_error(NGX_LOG_CRIT, c->log, ngx_socket_errno,
@@ -1451,7 +1453,7 @@

 #endif

-    if (u->header_sent) {
+    if (u->request_all_sent) {
         u->write_event_handler = ngx_http_upstream_dummy_handler;

         (void) ngx_handle_write_event(c->write, 0);
Only in nginx-1.0.11-patched/src/http: ngx_http_upstream.c~
diff -ur nginx-1.0.11/src/http/ngx_http_upstream.h
nginx-1.0.11-patched/src/http/ngx_http_upstream.h
--- nginx-1.0.11/src/http/ngx_http_upstream.h	2011-11-01
22:18:10.000000000 +0800
+++ nginx-1.0.11-patched/src/http/ngx_http_upstream.h	2012-03-21
21:18:21.041237173 +0800
@@ -313,6 +313,7 @@
     unsigned                         buffering:1;

     unsigned                         request_sent:1;
+    unsigned                         request_all_sent:1;
     unsigned                         header_sent:1;
 };
-------------- next part --------------
A non-text attachment was scrubbed...
Name: nginx-1.0.11-upstream_pipelining.patch
Type: application/octet-stream
Size: 1310 bytes
Desc: not available
URL: <http://mailman.nginx.org/pipermail/nginx-devel/attachments/20120321/90a92047/attachment.obj>