From 75dad742e547f766ff17e38deda44d8dcb2caf9b Mon Sep 17 00:00:00 2001 From: Valentin Bartenev Date: Thu, 26 Dec 2013 17:03:16 +0400 Subject: [PATCH] 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. --- 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(-) diff --git a/src/http/ngx_http_spdy.c b/src/http/ngx_http_spdy.c index 2346ad7a1..9a3550ad1 100644 --- a/src/http/ngx_http_spdy.c +++ b/src/http/ngx_http_spdy.c @@ -411,7 +411,7 @@ ngx_http_spdy_write_handler(ngx_event_t *wev) 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 *wev) 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_stream_t *stream, ngx_int_t rc) } } + 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_http_spdy_connection_t *sc, stream = sc->streams_index[i]; while (stream) { - r = stream->request; + stream->handled = 0; + r = stream->request; fc = r->connection; + fc->error = 1; if (stream->waiting) { diff --git a/src/http/ngx_http_spdy.h b/src/http/ngx_http_spdy.h index c47243fb0..c011a3d6f 100644 --- a/src/http/ngx_http_spdy.h +++ b/src/http/ngx_http_spdy.h @@ -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 --git a/src/http/ngx_http_spdy_filter_module.c b/src/http/ngx_http_spdy_filter_module.c index 4497e8b16..c6926b20b 100644 --- a/src/http/ngx_http_spdy_filter_module.c +++ b/src/http/ngx_http_spdy_filter_module.c @@ -795,11 +795,15 @@ ngx_http_spdy_filter_get_data_frame(ngx_http_spdy_stream_t *stream, 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_spdy_connection_t *sc, 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; }