[nginx] SPDY: fixed possible request hang.

[Valentin Bartenev]

details:   http://hg.nginx.org/nginx/rev/916cb6d28f6a
branches:  
changeset: 5493:916cb6d28f6a
user:      Valentin Bartenev <vbart at nginx.com>
date:      Thu Dec 26 17:03:16 2013 +0400
description:
SPDY: fixed possible request hang.

Processing events from upstream connection can result in sending queued frames
from other streams.  In this case such streams were not added to handling queue
and properly handled.

A global per connection flag was replaced by a per stream flag that indicates
currently sending stream while all other streams can be added to handling
queue.

diffstat:

 src/http/ngx_http_spdy.c               |  17 +++++++++++++----
 src/http/ngx_http_spdy.h               |   3 ++-
 src/http/ngx_http_spdy_filter_module.c |  14 ++++++++------
 3 files changed, 23 insertions(+), 11 deletions(-)

diffs (110 lines):

diff -r 5c52ff68f380 -r 916cb6d28f6a src/http/ngx_http_spdy.c
--- a/src/http/ngx_http_spdy.c	Mon Dec 23 18:12:03 2013 +0400
+++ b/src/http/ngx_http_spdy.c	Thu Dec 26 17:03:16 2013 +0400
@@ -411,7 +411,7 @@ ngx_http_spdy_write_handler(ngx_event_t 
 
     ngx_log_debug0(NGX_LOG_DEBUG_HTTP, c->log, 0, "spdy write handler");
 
-    sc->blocked = 2;
+    sc->blocked = 1;
 
     rc = ngx_http_spdy_send_output_queue(sc);
 
@@ -430,8 +430,6 @@ ngx_http_spdy_write_handler(ngx_event_t 
 
     sc->last_stream = NULL;
 
-    sc->blocked = 1;
-
     for ( /* void */ ; stream; stream = sn) {
         sn = stream->next;
         stream->handled = 0;
@@ -2658,6 +2656,15 @@ ngx_http_spdy_close_stream(ngx_http_spdy
         }
     }
 
+    if (stream->handled) {
+        for (s = sc->last_stream; s; s = s->next) {
+            if (s->next == stream) {
+                s->next = stream->next;
+                break;
+            }
+        }
+    }
+
     sscf = ngx_http_get_module_srv_conf(sc->http_connection->conf_ctx,
                                         ngx_http_spdy_module);
 
@@ -2847,9 +2854,11 @@ ngx_http_spdy_finalize_connection(ngx_ht
         stream = sc->streams_index[i];
 
         while (stream) {
+            stream->handled = 0;
+
             r = stream->request;
-
             fc = r->connection;
+
             fc->error = 1;
 
             if (stream->waiting) {
diff -r 5c52ff68f380 -r 916cb6d28f6a src/http/ngx_http_spdy.h
--- a/src/http/ngx_http_spdy.h	Mon Dec 23 18:12:03 2013 +0400
+++ b/src/http/ngx_http_spdy.h	Thu Dec 26 17:03:16 2013 +0400
@@ -106,7 +106,7 @@ struct ngx_http_spdy_connection_s {
 
     ngx_uint_t                       last_sid;
 
-    unsigned                         blocked:2;
+    unsigned                         blocked:1;
     unsigned                         waiting:1; /* FIXME better name */
 };
 
@@ -125,6 +125,7 @@ struct ngx_http_spdy_stream_s {
 
     unsigned                         priority:2;
     unsigned                         handled:1;
+    unsigned                         blocked:1;
     unsigned                         in_closed:1;
     unsigned                         out_closed:1;
     unsigned                         skip_data:2;
diff -r 5c52ff68f380 -r 916cb6d28f6a src/http/ngx_http_spdy_filter_module.c
--- a/src/http/ngx_http_spdy_filter_module.c	Mon Dec 23 18:12:03 2013 +0400
+++ b/src/http/ngx_http_spdy_filter_module.c	Thu Dec 26 17:03:16 2013 +0400
@@ -795,11 +795,15 @@ ngx_http_spdy_filter_get_data_frame(ngx_
 static ngx_inline ngx_int_t
 ngx_http_spdy_filter_send(ngx_connection_t *fc, ngx_http_spdy_stream_t *stream)
 {
+    stream->blocked = 1;
+
     if (ngx_http_spdy_send_output_queue(stream->connection) == NGX_ERROR) {
         fc->error = 1;
         return NGX_ERROR;
     }
 
+    stream->blocked = 0;
+
     if (stream->waiting) {
         fc->buffered |= NGX_SPDY_WRITE_BUFFERED;
         fc->write->delayed = 1;
@@ -946,16 +950,14 @@ ngx_http_spdy_handle_stream(ngx_http_spd
 
     fc->write->delayed = 0;
 
-    if (stream->handled) {
+    if (stream->handled || stream->blocked) {
         return;
     }
 
-    if (sc->blocked == 2) {
-        stream->handled = 1;
+    stream->handled = 1;
 
-        stream->next = sc->last_stream;
-        sc->last_stream = stream;
-    }
+    stream->next = sc->last_stream;
+    sc->last_stream = stream;
 }