From 44cbc785fcd116c924dd086c7e78dceb6ad86513 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Wed, 22 Jul 2015 21:41:16 +0900 Subject: [PATCH] nghttpx: Don't reuse backend connection if it is not clean --- src/shrpx_downstream.cc | 6 ++++++ src/shrpx_downstream.h | 3 +++ src/shrpx_http2_upstream.cc | 25 +++++++------------------ src/shrpx_https_upstream.cc | 8 +++----- src/shrpx_spdy_upstream.cc | 22 ++++++---------------- 5 files changed, 25 insertions(+), 39 deletions(-) diff --git a/src/shrpx_downstream.cc b/src/shrpx_downstream.cc index bfd7aa4b..a37c7f2f 100644 --- a/src/shrpx_downstream.cc +++ b/src/shrpx_downstream.cc @@ -1207,4 +1207,10 @@ void Downstream::add_request_headers_sum(size_t amount) { request_headers_sum_ += amount; } +bool Downstream::can_detach_downstream_connection() const { + return dconn_ && response_state_ == Downstream::MSG_COMPLETE && + request_state_ == Downstream::MSG_COMPLETE && !upgraded_ && + !response_connection_close_; +} + } // namespace shrpx diff --git a/src/shrpx_downstream.h b/src/shrpx_downstream.h index 3486621a..c64aba4f 100644 --- a/src/shrpx_downstream.h +++ b/src/shrpx_downstream.h @@ -332,6 +332,9 @@ public: void attach_blocked_link(BlockedLink *l); BlockedLink *detach_blocked_link(); + // Returns true if downstream_connection can be detached and reused. + bool can_detach_downstream_connection() const; + enum { EVENT_ERROR = 0x1, EVENT_TIMEOUT = 0x2, diff --git a/src/shrpx_http2_upstream.cc b/src/shrpx_http2_upstream.cc index 51146e8c..db57b9db 100644 --- a/src/shrpx_http2_upstream.cc +++ b/src/shrpx_http2_upstream.cc @@ -74,22 +74,13 @@ int on_stream_close_callback(nghttp2_session *session, int32_t stream_id, return 0; } - downstream->set_request_state(Downstream::STREAM_CLOSED); - - if (downstream->get_response_state() == Downstream::MSG_COMPLETE) { - // At this point, downstream response was read - if (!downstream->get_upgraded() && - !downstream->get_response_connection_close()) { - // Keep-alive - downstream->detach_downstream_connection(); - } - - upstream->remove_downstream(downstream); - // downstream was deleted - - return 0; + if (downstream->can_detach_downstream_connection()) { + // Keep-alive + downstream->detach_downstream_connection(); } + downstream->set_request_state(Downstream::STREAM_CLOSED); + // At this point, downstream read may be paused. // If shrpx_downstream::push_request_headers() failed, the @@ -915,10 +906,8 @@ int Http2Upstream::downstream_read(DownstreamConnection *dconn) { } return downstream_error(dconn, Downstream::EVENT_ERROR); } - // Detach downstream connection early so that it could be reused - // without hitting server's request timeout. - if (downstream->get_response_state() == Downstream::MSG_COMPLETE && - !downstream->get_response_connection_close()) { + + if (downstream->can_detach_downstream_connection()) { // Keep-alive downstream->detach_downstream_connection(); } diff --git a/src/shrpx_https_upstream.cc b/src/shrpx_https_upstream.cc index 4893c9fd..40a29593 100644 --- a/src/shrpx_https_upstream.cc +++ b/src/shrpx_https_upstream.cc @@ -540,7 +540,8 @@ int HttpsUpstream::on_write() { // We need to postpone detachment until all data are sent so that // we can notify nghttp2 library all data consumed. if (downstream->get_response_state() == Downstream::MSG_COMPLETE) { - if (downstream->get_response_connection_close()) { + if (downstream->get_response_connection_close() || + downstream->get_request_state() != Downstream::MSG_COMPLETE) { // Connection close downstream->pop_downstream_connection(); // dconn was deleted @@ -607,10 +608,7 @@ int HttpsUpstream::downstream_read(DownstreamConnection *dconn) { goto end; } - // Detach downstream connection early so that it could be reused - // without hitting server's request timeout. - if (downstream->get_response_state() == Downstream::MSG_COMPLETE && - !downstream->get_response_connection_close()) { + if (downstream->can_detach_downstream_connection()) { // Keep-alive downstream->detach_downstream_connection(); } diff --git a/src/shrpx_spdy_upstream.cc b/src/shrpx_spdy_upstream.cc index 745cc0b5..89fcb678 100644 --- a/src/shrpx_spdy_upstream.cc +++ b/src/shrpx_spdy_upstream.cc @@ -109,20 +109,13 @@ void on_stream_close_callback(spdylay_session *session, int32_t stream_id, return; } - downstream->set_request_state(Downstream::STREAM_CLOSED); - if (downstream->get_response_state() == Downstream::MSG_COMPLETE) { - // At this point, downstream response was read - if (!downstream->get_upgraded() && - !downstream->get_response_connection_close()) { - // Keep-alive - downstream->detach_downstream_connection(); - } - upstream->remove_downstream(downstream); - // downstrea was deleted - - return; + if (downstream->can_detach_downstream_connection()) { + // Keep-alive + downstream->detach_downstream_connection(); } + downstream->set_request_state(Downstream::STREAM_CLOSED); + // At this point, downstream read may be paused. // If shrpx_downstream::push_request_headers() failed, the @@ -589,10 +582,7 @@ int SpdyUpstream::downstream_read(DownstreamConnection *dconn) { } return downstream_error(dconn, Downstream::EVENT_ERROR); } - // Detach downstream connection early so that it could be reused - // without hitting server's request timeout. - if (downstream->get_response_state() == Downstream::MSG_COMPLETE && - !downstream->get_response_connection_close()) { + if (downstream->can_detach_downstream_connection()) { // Keep-alive downstream->detach_downstream_connection(); }