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

[sh...@apache.org]

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

shinrich pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git


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

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

    Fix lost pending_actions causing actions on stale objects.
---
 iocore/net/P_UnixNetVConnection.h |  7 +++++++
 proxy/http/HttpSM.cc              | 29 +++++++++++++++++++++--------
 2 files changed, 28 insertions(+), 8 deletions(-)

diff --git a/iocore/net/P_UnixNetVConnection.h b/iocore/net/P_UnixNetVConnection.h
index 90ce23c..1516ddd 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;
@@ -436,6 +437,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 7a974f9..0dd7ca5 100644
--- a/proxy/http/HttpSM.cc
+++ b/proxy/http/HttpSM.cc
@@ -1729,15 +1729,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;
   NetVConnection *netvc = nullptr;
 
   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();
@@ -1746,7 +1747,12 @@ HttpSM::state_http_server_open(int event, void *data)
 
     netvc = static_cast<NetVConnection *>(data);
     session->attach_hostname(t_state.current.server->name);
-    session->new_connection(netvc);
+    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;
     ats_ip_copy(&t_state.server_info.src_addr, netvc->get_local_addr());
 
@@ -1777,7 +1783,7 @@ HttpSM::state_http_server_open(int event, void *data)
       handle_http_server_open();
     }
     return 0;
-
+  }
   case VC_EVENT_WRITE_READY:
   case VC_EVENT_WRITE_COMPLETE:
     // Update the time out to the regular connection timeout.
@@ -1826,6 +1832,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;
@@ -2312,6 +2321,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) {
@@ -5380,7 +5390,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();
   }
 }
@@ -6842,7 +6852,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();
     }