You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by sh...@apache.org on 2015/07/17 14:40:04 UTC

trafficserver git commit: TS-1007: SSLN Close called before TXN Close. This closes #249.

Repository: trafficserver
Updated Branches:
  refs/heads/master 6c2c31285 -> 2e0689748


TS-1007: SSLN Close called before TXN Close.  This closes #249.


Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/2e068974
Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/2e068974
Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/2e068974

Branch: refs/heads/master
Commit: 2e0689748b496e54977fa99d2f7f4c75915da1df
Parents: 6c2c312
Author: shinrich <sh...@yahoo-inc.com>
Authored: Mon Jul 13 11:09:37 2015 -0500
Committer: shinrich <sh...@yahoo-inc.com>
Committed: Fri Jul 17 07:38:50 2015 -0500

----------------------------------------------------------------------
 iocore/net/UnixNet.cc           |  5 ++++-
 proxy/http/HttpClientSession.cc |  8 ++++++++
 proxy/http/HttpSM.cc            | 32 +++++++++++++++++++++++---------
 proxy/spdy/SpdyClientSession.cc |  8 +++-----
 proxy/spdy/SpdyClientSession.h  |  7 ++-----
 5 files changed, 40 insertions(+), 20 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/trafficserver/blob/2e068974/iocore/net/UnixNet.cc
----------------------------------------------------------------------
diff --git a/iocore/net/UnixNet.cc b/iocore/net/UnixNet.cc
index 3c454a9..d35204f 100644
--- a/iocore/net/UnixNet.cc
+++ b/iocore/net/UnixNet.cc
@@ -672,7 +672,10 @@ NetHandler::_close_vc(UnixNetVConnection *vc, ink_hrtime now, int &handle_event,
     ++closed;
   } else {
     vc->next_inactivity_timeout_at = now;
-    keep_alive_queue.head->handleEvent(EVENT_IMMEDIATE, NULL);
+    // Found a case where the keep_alive_queue.head was NULL while running regression
+    // with non-standard configuration
+    if (keep_alive_queue.head)
+      keep_alive_queue.head->handleEvent(EVENT_IMMEDIATE, NULL);
     ++handle_event;
   }
 }

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/2e068974/proxy/http/HttpClientSession.cc
----------------------------------------------------------------------
diff --git a/proxy/http/HttpClientSession.cc b/proxy/http/HttpClientSession.cc
index f02834c..88b9e96 100644
--- a/proxy/http/HttpClientSession.cc
+++ b/proxy/http/HttpClientSession.cc
@@ -92,6 +92,11 @@ HttpClientSession::destroy()
     HTTP_DECREMENT_DYN_STAT(http_current_client_connections_stat);
     conn_decrease = false;
   }
+  // Make sure we clean up ua_session in our HttpSM
+  // Otherwise, we will try to double free it in HttpSM::kill_this
+  if (current_reader) {
+    current_reader->ua_session = NULL;
+  }
 
   super::destroy();
   THREAD_FREE(this, httpClientSessionAllocator, this_thread());
@@ -327,6 +332,9 @@ HttpClientSession::do_io_close(int alerrno)
     HTTP_SUM_DYN_STAT(http_transactions_per_client_con, transact_count);
     HTTP_DECREMENT_DYN_STAT(http_current_client_connections_stat);
     conn_decrease = false;
+
+    // Should only be triggering the hooks for session close
+    // if this is a SSLNetVConnection or a UnixNetVConnection
     do_api_callout(TS_HTTP_SSN_CLOSE_HOOK);
   }
 }

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/2e068974/proxy/http/HttpSM.cc
----------------------------------------------------------------------
diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc
index 8fd7304..700203f 100644
--- a/proxy/http/HttpSM.cc
+++ b/proxy/http/HttpSM.cc
@@ -3179,14 +3179,11 @@ HttpSM::tunnel_handler_ua(int event, HttpTunnelConsumer *c)
       ua_session->set_half_close_flag();
     }
 
-    ua_session->do_io_close();
-    ua_session = NULL;
-  } else {
-    ink_assert(ua_buffer_reader != NULL);
-    ua_session->release(ua_buffer_reader);
-    ua_buffer_reader = NULL;
-    ua_session = NULL;
-  }
+    // TS-1007, delaying ua_session->do_io_close to kill_this
+    // so the session_close hook occurs after the transaction close hook
+    // Also delaying the session release to kill_this in the keep_alive case
+    // so we don't lose any keep-alive opportunities
+  } 
 
   return 0;
 }
@@ -6535,7 +6532,6 @@ HttpSM::kill_this()
       plugin_tunnel = NULL;
     }
 
-    ua_session = NULL;
     server_session = NULL;
 
     // So we don't try to nuke the state machine
@@ -6560,6 +6556,24 @@ HttpSM::kill_this()
   reentrancy_count--;
   ink_release_assert(reentrancy_count == 0);
 
+  // Delay the close of the user agent session, so the close session
+  // occurs after the close transaction
+  if (ua_session) {
+    // If this is a keep-alive client connection, just relase the client
+    // session rather than closing it.
+    if (t_state.client_info.keep_alive == HTTP_KEEPALIVE &&
+      (t_state.www_auth_content != HttpTransact::CACHE_AUTH_SERVE || ua_session->get_bound_ss())) {
+      // successful keep-alive, release the client session instead of destroying it
+      ink_assert(ua_buffer_reader != NULL);
+      ua_session->release(ua_buffer_reader);
+      ua_buffer_reader = NULL;
+    } else {
+      // Not keep alive, go ahead and shut it down
+      ua_session->do_io_close();
+    }
+    ua_session = NULL;
+  }
+
   // If the api shutdown & list removeal was synchronous
   //   then the value of kill_this_async_done has changed so
   //   we must check it again

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/2e068974/proxy/spdy/SpdyClientSession.cc
----------------------------------------------------------------------
diff --git a/proxy/spdy/SpdyClientSession.cc b/proxy/spdy/SpdyClientSession.cc
index c6f1822..2f8720e 100644
--- a/proxy/spdy/SpdyClientSession.cc
+++ b/proxy/spdy/SpdyClientSession.cc
@@ -114,7 +114,6 @@ SpdyClientSession::init(NetVConnection *netvc)
 
   this->vc->set_inactivity_timeout(HRTIME_SECONDS(spdy_accept_no_activity_timeout));
   vc->add_to_keep_alive_queue();
-  SET_HANDLER(&SpdyClientSession::state_session_start);
 }
 
 void
@@ -193,11 +192,11 @@ SpdyClientSession::new_connection(NetVConnection *new_vc, MIOBuffer *iobuf, IOBu
   sm->resp_buffer = TSIOBufferCreate();
   sm->resp_reader = TSIOBufferReaderAlloc(sm->resp_buffer);
 
-  eventProcessor.schedule_imm(sm, ET_NET);
+  do_api_callout(TS_HTTP_SSN_START_HOOK);
 }
 
-int
-SpdyClientSession::state_session_start(int /* event */, void * /* edata */)
+void
+SpdyClientSession::start()
 {
   const spdylay_settings_entry entries[] = {
     {SPDYLAY_SETTINGS_MAX_CONCURRENT_STREAMS, SPDYLAY_ID_FLAG_SETTINGS_NONE, spdy_max_concurrent_streams},
@@ -224,7 +223,6 @@ SpdyClientSession::state_session_start(int /* event */, void * /* edata */)
   }
 
   TSVIOReenable(this->write_vio);
-  return EVENT_CONT;
 }
 
 int

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/2e068974/proxy/spdy/SpdyClientSession.h
----------------------------------------------------------------------
diff --git a/proxy/spdy/SpdyClientSession.h b/proxy/spdy/SpdyClientSession.h
index 8b46c60..2ff5a30 100644
--- a/proxy/spdy/SpdyClientSession.h
+++ b/proxy/spdy/SpdyClientSession.h
@@ -117,11 +117,8 @@ public:
     ink_release_assert(false);
     return NULL;
   }
-  void
-  start()
-  {
-    ink_release_assert(false);
-  }
+  void start();
+  
   void do_io_close(int lerrno = -1);
   void
   do_io_shutdown(ShutdownHowTo_t howto)