From b707cfe9864dc2be0eb9521eb32b77111de0d9ce Mon Sep 17 00:00:00 2001
From: Tatsuhiro Tsujikawa <tatsuhiro.t@gmail.com>
Date: Tue, 3 Feb 2015 01:47:04 +0900
Subject: [PATCH] nghttpx: Fix busy loop when HTTP/2 backend reset after
 connection established

We have now Downstream retry count to be limited to 5 times.  At 6th
failure, we send 503 message to client.
---
 src/shrpx_downstream.cc     | 8 ++++++--
 src/shrpx_downstream.h      | 7 +++++++
 src/shrpx_http2_upstream.cc | 4 +++-
 src/shrpx_https_upstream.cc | 8 +++++---
 src/shrpx_spdy_upstream.cc  | 4 +++-
 5 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/src/shrpx_downstream.cc b/src/shrpx_downstream.cc
index 0a6450ad..d1985f87 100644
--- a/src/shrpx_downstream.cc
+++ b/src/shrpx_downstream.cc
@@ -112,8 +112,8 @@ Downstream::Downstream(Upstream *upstream, int32_t stream_id, int32_t priority)
       request_bodylen_(0), response_bodylen_(0), response_sent_bodylen_(0),
       request_content_length_(-1), response_content_length_(-1),
       upstream_(upstream), request_headers_sum_(0), response_headers_sum_(0),
-      request_datalen_(0), response_datalen_(0), stream_id_(stream_id),
-      priority_(priority), downstream_stream_id_(-1),
+      request_datalen_(0), response_datalen_(0), num_retry_(0),
+      stream_id_(stream_id), priority_(priority), downstream_stream_id_(-1),
       response_rst_stream_error_code_(NGHTTP2_NO_ERROR),
       request_state_(INITIAL), request_major_(1), request_minor_(1),
       response_state_(INITIAL), response_http_status_(0), response_major_(1),
@@ -1020,4 +1020,8 @@ void Downstream::disable_downstream_wtimer() {
 
 bool Downstream::accesslog_ready() const { return response_http_status_ > 0; }
 
+void Downstream::add_retry() { ++num_retry_; }
+
+bool Downstream::no_more_retry() const { return num_retry_ > 5; }
+
 } // namespace shrpx
diff --git a/src/shrpx_downstream.h b/src/shrpx_downstream.h
index 4a0f0077..7d4e7025 100644
--- a/src/shrpx_downstream.h
+++ b/src/shrpx_downstream.h
@@ -289,6 +289,11 @@ public:
   // Returns true if accesslog can be written for this downstream.
   bool accesslog_ready() const;
 
+  // Increment retry count
+  void add_retry();
+  // true if retry attempt should not be done.
+  bool no_more_retry() const;
+
   enum {
     EVENT_ERROR = 0x1,
     EVENT_TIMEOUT = 0x2,
@@ -338,6 +343,8 @@ private:
   size_t request_datalen_;
   size_t response_datalen_;
 
+  size_t num_retry_;
+
   int32_t stream_id_;
   int32_t priority_;
   // stream ID in backend connection
diff --git a/src/shrpx_http2_upstream.cc b/src/shrpx_http2_upstream.cc
index 033580ec..f7c8ee7f 100644
--- a/src/shrpx_http2_upstream.cc
+++ b/src/shrpx_http2_upstream.cc
@@ -1420,7 +1420,9 @@ int Http2Upstream::on_downstream_reset(bool no_retry) {
 
     downstream->pop_downstream_connection();
 
-    if (no_retry) {
+    downstream->add_retry();
+
+    if (no_retry || downstream->no_more_retry()) {
       if (on_downstream_abort_request(downstream, 503) != 0) {
         return -1;
       }
diff --git a/src/shrpx_https_upstream.cc b/src/shrpx_https_upstream.cc
index 0198a737..e2d5b2eb 100644
--- a/src/shrpx_https_upstream.cc
+++ b/src/shrpx_https_upstream.cc
@@ -836,15 +836,17 @@ int HttpsUpstream::on_downstream_reset(bool no_retry) {
     return -1;
   }
 
-  if (no_retry) {
+  downstream_->pop_downstream_connection();
+
+  downstream_->add_retry();
+
+  if (no_retry || downstream_->no_more_retry()) {
     if (on_downstream_abort_request(downstream_.get(), 503) != 0) {
       return -1;
     }
     return 0;
   }
 
-  downstream_->pop_downstream_connection();
-
   rv = downstream_->attach_downstream_connection(
       handler_->get_downstream_connection());
   if (rv != 0) {
diff --git a/src/shrpx_spdy_upstream.cc b/src/shrpx_spdy_upstream.cc
index 09cddf4c..3a202756 100644
--- a/src/shrpx_spdy_upstream.cc
+++ b/src/shrpx_spdy_upstream.cc
@@ -1051,7 +1051,9 @@ int SpdyUpstream::on_downstream_reset(bool no_retry) {
 
     downstream->pop_downstream_connection();
 
-    if (no_retry) {
+    downstream->add_retry();
+
+    if (no_retry || downstream->no_more_retry()) {
       if (on_downstream_abort_request(downstream, 503) != 0) {
         return -1;
       }