[trafficserver] 03/03: Set nullptr to ua_session after it is destoryed

[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://git-dual.apache.org/repos/asf/trafficserver.git

commit 959c2cc9cc7e65b7e2fac2a844e2c476b16c8ab5
Author: Masaori Koshiba <ma...@apache.org>
AuthorDate: Wed Feb 15 17:31:54 2017 +0900

    Set nullptr to ua_session after it is destoryed
    
    Issue:
    Crash by EXC_BAD_ACCESS in Http2ConnectionState::release_stream() under heavy load
    
    Cause:
    While total_connections_in is larger than max_connections_per_thread_in (in NetHandler::manage_keep_alive_queue()),
    Http2ConnectionState::release_stream() is called recurcively from add_to_keep_alive_queue().
    At the bottom of recursion, ua_session is destroyed and Http2ConnectionState::release_stream() access to it.
    
    Fix:
    1. Set nullptr to ua_session after it is destoryed
    2. Swap calls of add_to_keep_alive_queue() and cancel_active_timeout() for ua_session nullptr check
    3. Check m_active of ua_session to reduce recursion
    
    (cherry picked from commit b95271613fe9fe4dbb0b6592c454495865aac5b4)
---
 proxy/ProxyClientSession.h          |  6 ++++++
 proxy/http2/Http2ConnectionState.cc | 12 +++++++-----
 2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/proxy/ProxyClientSession.h b/proxy/ProxyClientSession.h
index aa5d129..30ee83c 100644
--- a/proxy/ProxyClientSession.h
+++ b/proxy/ProxyClientSession.h
@@ -107,6 +107,12 @@ public:
     return this->api_hooks.has_hooks() || http_global_hooks->has_hooks();
   }
 
+  bool
+  is_active() const
+  {
+    return m_active;
+  }
+
   // Initiate an API hook invocation.
   void do_api_callout(TSHttpHookID id);
 
diff --git a/proxy/http2/Http2ConnectionState.cc b/proxy/http2/Http2ConnectionState.cc
index 69a4b3b..65a694a 100644
--- a/proxy/http2/Http2ConnectionState.cc
+++ b/proxy/http2/Http2ConnectionState.cc
@@ -1055,18 +1055,20 @@ Http2ConnectionState::release_stream(Http2Stream *stream)
     stream_list.remove(stream);
   }
 
-  // If the number of clients is 0, then mark the connection as inactive
-  if (total_client_streams_count == 0 && ua_session) {
+  // If the number of clients is 0 and ua_session is active, then mark the connection as inactive
+  if (total_client_streams_count == 0 && ua_session && ua_session->is_active()) {
     ua_session->clear_session_active();
-    if (ua_session->get_netvc()) {
-      ua_session->get_netvc()->add_to_keep_alive_queue();
-      ua_session->get_netvc()->cancel_active_timeout();
+    UnixNetVConnection *vc = static_cast<UnixNetVConnection *>(ua_session->get_netvc());
+    if (vc) {
+      vc->cancel_active_timeout();
+      vc->add_to_keep_alive_queue();
     }
   }
 
   if (ua_session && fini_received && total_client_streams_count == 0) {
     // We were shutting down, go ahead and terminate the session
     ua_session->destroy();
+    ua_session = nullptr;
   }
 }
 

-- 
To stop receiving notification emails like this one, please contact
"commits@trafficserver.apache.org" <co...@trafficserver.apache.org>.