[trafficserver] 02/02: Revert "Expanding the cases where the server_entry gets cleared on error."

[zw...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

zwoop pushed a commit to branch 7.1.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git

commit 3454647ec5e70bcd59168f4ee0f401d45ae88ae1
Author: Leif Hedstrom <le...@ogre.com>
AuthorDate: Mon Jul 16 17:11:30 2018 -0400

    Revert "Expanding the cases where the server_entry gets cleared on error."
    
    This reverts commit f1e3e4e0afee68a33848864c4a54038efddfaab4.
    
    Seeing a crasher with this commit, and 29aad24249b, for details
    on the crasher see issue
    
       https://github.com/apache/trafficserver/issues/3837
---
 proxy/http/HttpSM.cc       | 18 +++++++-----------
 proxy/http/HttpTransact.cc |  5 +++--
 2 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc
index 5a7d5f4..e9e70cd 100644
--- a/proxy/http/HttpSM.cc
+++ b/proxy/http/HttpSM.cc
@@ -5484,16 +5484,14 @@ HttpSM::handle_server_setup_error(int event, void *data)
         ua_producer->alive         = false;
         ua_producer->handler_state = HTTP_SM_POST_SERVER_FAIL;
         tunnel.handleEvent(VC_EVENT_ERROR, c->write_vio);
-        return;
       }
     } else {
       // c could be null here as well
       if (c != nullptr) {
         tunnel.handleEvent(event, c->write_vio);
-        return;
       }
     }
-    // If there is no consumer, let the event pass through to shutdown
+    return;
   } else {
     if (post_transform_info.vc) {
       HttpTunnelConsumer *c = tunnel.get_consumer(post_transform_info.vc);
@@ -5530,17 +5528,15 @@ HttpSM::handle_server_setup_error(int event, void *data)
     // if (vio->op == VIO::WRITE && vio->ndone == 0) {
     if (server_entry->write_vio && server_entry->write_vio->nbytes > 0 && server_entry->write_vio->ndone == 0) {
       t_state.current.state = HttpTransact::CONNECTION_ERROR;
+      if (server_entry) {
+        ink_assert(server_entry->vc_type == HTTP_SERVER_VC);
+        vc_table.cleanup_entry(server_entry);
+        server_entry   = nullptr;
+        server_session = nullptr;
+      }
     } else {
       t_state.current.state = HttpTransact::INACTIVE_TIMEOUT;
     }
-    // Clean up the vc_table entry so any events in play to the timed out server vio
-    // don't get handled.  The connection isn't there.
-    if (server_entry) {
-      ink_assert(server_entry->vc_type == HTTP_SERVER_VC);
-      vc_table.cleanup_entry(server_entry);
-      server_entry   = nullptr;
-      server_session = nullptr;
-    }
     break;
   default:
     ink_release_assert(0);
diff --git a/proxy/http/HttpTransact.cc b/proxy/http/HttpTransact.cc
index 58031d9..ace91d8 100644
--- a/proxy/http/HttpTransact.cc
+++ b/proxy/http/HttpTransact.cc
@@ -6587,9 +6587,10 @@ HttpTransact::is_request_retryable(State *s)
 {
   // If safe requests are  retryable, it should be safe to retry safe requests irrespective of bytes sent or connection state
   // according to RFC the following methods are safe (https://tools.ietf.org/html/rfc7231#section-4.2.1)
-  // Otherwise, if there was no error establishing the connection (and we sent bytes)-- we cannot retry
+  // If there was no error establishing the connection (and we sent bytes)-- we cannot retry
   if (!(s->txn_conf->safe_requests_retryable && HttpTransactHeaders::is_method_safe(s->method)) &&
-      s->current.state != CONNECTION_ERROR && s->state_machine->server_request_hdr_bytes > 0) {
+      (s->current.state != CONNECTION_ERROR && s->state_machine->server_request_hdr_bytes > 0 &&
+       s->state_machine->get_server_session()->get_netvc()->outstanding() != s->state_machine->server_request_hdr_bytes)) {
     return false;
   }