[trafficserver] branch 8.1.x updated: Fix lost pending_actions causing actions on stale objects.

[bc...@apache.org]

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

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


The following commit(s) were added to refs/heads/8.1.x by this push:
     new 912000f  Fix lost pending_actions causing actions on stale objects.
912000f is described below

commit 912000fb764d0df73eacb7077fd3e37eaaf2782d
Author: Susan Hinrichs <sh...@oath.com>
AuthorDate: Wed Aug 22 23:50:54 2018 +0000

    Fix lost pending_actions causing actions on stale objects.
    
    (cherry picked from commit 493803c4e26348a9c707320a134709d832f9a182)
    
    Conflicts:
    	proxy/http/HttpSM.cc
---
 iocore/net/P_UnixNetVConnection.h |  7 +++++++
 proxy/http/HttpSM.cc              | 29 ++++++++++++++++++++++-------
 2 files changed, 29 insertions(+), 7 deletions(-)

diff --git a/iocore/net/P_UnixNetVConnection.h b/iocore/net/P_UnixNetVConnection.h
index d258e7c..c454b49 100644
--- a/iocore/net/P_UnixNetVConnection.h
+++ b/iocore/net/P_UnixNetVConnection.h
@@ -155,6 +155,7 @@ public:
   void cancel_active_timeout() override;
   void cancel_inactivity_timeout() override;
   void set_action(Continuation *c) override;
+  const Action *get_action() const;
   void add_to_keep_alive_queue() override;
   void remove_from_keep_alive_queue() override;
   bool add_to_active_queue() override;
@@ -403,6 +404,12 @@ UnixNetVConnection::set_action(Continuation *c)
   action_ = c;
 }
 
+TS_INLINE const Action *
+UnixNetVConnection::get_action() const
+{
+  return &action_;
+}
+
 // declarations for local use (within the net module)
 
 void write_to_net(NetHandler *nh, UnixNetVConnection *vc, EThread *thread);
diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc
index b0cfc1a..08bfa06 100644
--- a/proxy/http/HttpSM.cc
+++ b/proxy/http/HttpSM.cc
@@ -1725,14 +1725,16 @@ HttpSM::state_http_server_open(int event, void *data)
 {
   SMDebug("http_track", "entered inside state_http_server_open");
   STATE_ENTER(&HttpSM::state_http_server_open, event);
-  // TODO decide whether to uncomment after finish testing redirect
-  // ink_assert(server_entry == NULL);
-  pending_action                              = nullptr;
+  ink_release_assert(event == EVENT_INTERVAL || event == NET_EVENT_OPEN || event == NET_EVENT_OPEN_FAILED ||
+                     pending_action == nullptr);
+  if (event != NET_EVENT_OPEN) {
+    pending_action = nullptr;
+  }
   milestones[TS_MILESTONE_SERVER_CONNECT_END] = Thread::get_hrtime();
   HttpServerSession *session;
 
   switch (event) {
-  case NET_EVENT_OPEN:
+  case NET_EVENT_OPEN: {
     session = (TS_SERVER_SESSION_SHARING_POOL_THREAD == t_state.http_config_param->server_session_sharing_pool) ?
                 THREAD_ALLOC_INIT(httpServerSessionAllocator, mutex->thread_holding) :
                 httpServerSessionAllocator.alloc();
@@ -1752,7 +1754,12 @@ HttpSM::state_http_server_open(int event, void *data)
        printf("client fd is :%d , server fd is %d\n",vc->con.fd,
        server_vc->con.fd); */
     session->attach_hostname(t_state.current.server->name);
-    session->new_connection(static_cast<NetVConnection *>(data));
+    UnixNetVConnection *vc = static_cast<UnixNetVConnection *>(data);
+    ink_release_assert(pending_action == nullptr || pending_action == vc->get_action());
+    pending_action = nullptr;
+
+    session->new_connection(vc);
+
     session->state = HSS_ACTIVE;
 
     attach_server_session(session);
@@ -1766,6 +1773,7 @@ HttpSM::state_http_server_open(int event, void *data)
     }
     handle_http_server_open();
     return 0;
+  }
   case EVENT_INTERVAL: // Delayed call from another thread
     if (server_session == nullptr) {
       do_http_server_open();
@@ -1799,6 +1807,9 @@ HttpSM::state_http_server_open(int event, void *data)
       HTTP_INCREMENT_DYN_STAT(http_origin_connections_throttled_stat);
       send_origin_throttled_response();
     } else {
+      // Go ahead and release the failed server session.  Since it didn't receive a response, the release logic will
+      // see that it didn't get a valid response and it will close it rather than returning it to the server session pool
+      release_server_session();
       call_transact_and_set_next_state(HttpTransact::HandleResponse);
     }
     return 0;
@@ -2285,6 +2296,7 @@ HttpSM::state_hostdb_reverse_lookup(int event, void *data)
 int
 HttpSM::state_mark_os_down(int event, void *data)
 {
+  STATE_ENTER(&HttpSM::state_mark_os_down, event);
   HostDBInfo *mark_down = nullptr;
 
   if (event == EVENT_HOST_DB_LOOKUP && data) {
@@ -5399,7 +5411,7 @@ HttpSM::handle_http_server_open()
       (t_state.hdr_info.request_content_length > 0 || t_state.client_info.transfer_encoding == HttpTransact::CHUNKED_ENCODING) &&
       do_post_transform_open()) {
     do_setup_post_tunnel(HTTP_TRANSFORM_VC);
-  } else {
+  } else if (server_session != nullptr) {
     setup_server_send_request_api();
   }
 }
@@ -6861,7 +6873,10 @@ HttpSM::kill_this()
   //   then the value of kill_this_async_done has changed so
   //   we must check it again
   if (kill_this_async_done == true) {
-    ink_assert(pending_action == nullptr);
+    if (pending_action) {
+      pending_action->cancel();
+      pending_action = nullptr;
+    }
     if (t_state.http_config_param->enable_http_stats) {
       update_stats();
     }