From a0ce5ea9aba0ae8801f3911b4765a84ffc9b42d5 Mon Sep 17 00:00:00 2001
From: Tatsuhiro Tsujikawa <tatsuhiro.t@gmail.com>
Date: Sat, 24 Dec 2016 22:50:02 +0900
Subject: [PATCH] nghttpx: Keep reading after backend write failed

Because of bidirectional nature of TCP, we may fail write(2), but have
still pending read in TCP buffer, which may contain response body.  To
forward them, we should keep reading until get EOF from backend.

To avoid stalling HTTP/1 upload when request buffer is full, and we
have received complete response from backend, drop connection in that
case.
---
 src/shrpx_http2_session.cc              | 14 ++++++++++++--
 src/shrpx_http_downstream_connection.cc | 14 ++++++++++++--
 src/shrpx_https_upstream.cc             |  4 +++-
 3 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/src/shrpx_http2_session.cc b/src/shrpx_http2_session.cc
index e2b4608c..24d9cbd5 100644
--- a/src/shrpx_http2_session.cc
+++ b/src/shrpx_http2_session.cc
@@ -2022,7 +2022,12 @@ int Http2Session::write_clear() {
       }
 
       if (nwrite < 0) {
-        return nwrite;
+        // We may have pending data in receive buffer which may
+        // contain part of response body.  So keep reading.  Invoke
+        // read event to get read(2) error just in case.
+        ev_feed_event(conn_.loop, &conn_.rev, EV_READ);
+        wb_.drain(wb_.rleft());
+        break;
       }
 
       wb_.drain(nwrite);
@@ -2132,7 +2137,12 @@ int Http2Session::write_tls() {
       }
 
       if (nwrite < 0) {
-        return nwrite;
+        // We may have pending data in receive buffer which may
+        // contain part of response body.  So keep reading.  Invoke
+        // read event to get read(2) error just in case.
+        ev_feed_event(conn_.loop, &conn_.rev, EV_READ);
+        wb_.drain(wb_.rleft());
+        break;
       }
 
       wb_.drain(nwrite);
diff --git a/src/shrpx_http_downstream_connection.cc b/src/shrpx_http_downstream_connection.cc
index 70333a09..f6cf7998 100644
--- a/src/shrpx_http_downstream_connection.cc
+++ b/src/shrpx_http_downstream_connection.cc
@@ -1116,7 +1116,12 @@ int HttpDownstreamConnection::write_clear() {
     }
 
     if (nwrite < 0) {
-      return nwrite;
+      // We may have pending data in receive buffer which may contain
+      // part of response body.  So keep reading.  Invoke read event
+      // to get read(2) error just in case.
+      ev_feed_event(conn_.loop, &conn_.rev, EV_READ);
+      input->drain(input->rleft());
+      break;
     }
 
     input->drain(nwrite);
@@ -1236,7 +1241,12 @@ int HttpDownstreamConnection::write_tls() {
     }
 
     if (nwrite < 0) {
-      return nwrite;
+      // We may have pending data in receive buffer which may contain
+      // part of response body.  So keep reading.  Invoke read event
+      // to get read(2) error just in case.
+      ev_feed_event(conn_.loop, &conn_.rev, EV_READ);
+      input->drain(input->rleft());
+      break;
     }
 
     input->drain(nwrite);
diff --git a/src/shrpx_https_upstream.cc b/src/shrpx_https_upstream.cc
index a12d1a40..4ea06aba 100644
--- a/src/shrpx_https_upstream.cc
+++ b/src/shrpx_https_upstream.cc
@@ -1184,7 +1184,9 @@ int HttpsUpstream::on_downstream_body_complete(Downstream *downstream) {
     resp.connection_close = true;
   }
 
-  if (req.connection_close || resp.connection_close) {
+  if (req.connection_close || resp.connection_close ||
+      // To avoid to stall upload body
+      downstream->get_request_state() != Downstream::MSG_COMPLETE) {
     auto handler = get_client_handler();
     handler->set_should_close_after_write(true);
   }