TS-3140: Traffic Server asserts during response redirect.

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

Branch: refs/heads/master
Commit: 9e2689b78a3ab6b768ec60506057b065206f4e0d
Parents: 179bb98
Author: shinrich <shinr...@yahoo-inc.com>
Authored: Thu Jan 22 08:15:21 2015 -0600
Committer: shinrich <shinr...@yahoo-inc.com>
Committed: Thu Jan 22 08:15:21 2015 -0600

----------------------------------------------------------------------
 CHANGES                    |  1 +
 proxy/InkAPI.cc            |  7 +++++--
 proxy/http/HttpSM.cc       | 24 ++++++++++++++++--------
 proxy/http/HttpTransact.cc |  3 ---
 proxy/http/HttpTransact.h  |  2 --
 proxy/http/HttpTunnel.cc   | 14 ++++++++++----
 6 files changed, 32 insertions(+), 19 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/trafficserver/blob/9e2689b7/CHANGES
----------------------------------------------------------------------
diff --git a/CHANGES b/CHANGES
index 70c038b..ddbd7e0 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,5 +1,6 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache Traffic Server 5.3.0
+  *) [TS-3140] Traffic Server asserts during response redirect.
 
   *) [TS-3309] Document TLS session ticket rotation.
    Author: Bin Zeng <bz...@linkedin.com>

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/9e2689b7/proxy/InkAPI.cc
----------------------------------------------------------------------
diff --git a/proxy/InkAPI.cc b/proxy/InkAPI.cc
index c59573e..7825eb7 100644
--- a/proxy/InkAPI.cc
+++ b/proxy/InkAPI.cc
@@ -4957,6 +4957,11 @@ TSHttpTxnSecondUrlTryLock(TSHttpTxn txnp)
   return TS_SUCCESS;
 }
 
+/*
+ * TSHttpTxnRedirectRequest is very odd.  It is only in experimental.h.  
+ * It is not used in any checked in code.  We should probably remove this.
+ * SKH 1/15/2015
+ */
 TSReturnCode
 TSHttpTxnRedirectRequest(TSHttpTxn txnp, TSMBuffer bufp, TSMLoc url_loc)
 {
@@ -4991,8 +4996,6 @@ TSHttpTxnRedirectRequest(TSHttpTxn txnp, TSMBuffer bufp, 
TSMLoc url_loc)
   r_url->copy(&u);
 
   s->hdr_info.server_request.destroy();
-  // we want to close the server session
-  s->api_release_server_session = true;
 
   s->request_sent_time = 0;
   s->response_received_time = 0;

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/9e2689b7/proxy/http/HttpSM.cc
----------------------------------------------------------------------
diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc
index 99da799..3998771 100644
--- a/proxy/http/HttpSM.cc
+++ b/proxy/http/HttpSM.cc
@@ -1515,13 +1515,9 @@ HttpSM::handle_api_return()
   case HttpTransact::SM_ACTION_API_READ_CACHE_HDR:
   case HttpTransact::SM_ACTION_API_READ_RESPONSE_HDR:
   case HttpTransact::SM_ACTION_API_CACHE_LOOKUP_COMPLETE:
-    // this part is added for automatic redirect
-    if (t_state.api_next_action == 
HttpTransact::SM_ACTION_API_READ_RESPONSE_HDR && 
t_state.api_release_server_session) {
-      t_state.api_release_server_session = false;
-      release_server_session();
-    } else if (t_state.api_next_action == 
HttpTransact::SM_ACTION_API_CACHE_LOOKUP_COMPLETE &&
-               t_state.api_cleanup_cache_read &&
-               t_state.api_update_cached_object != 
HttpTransact::UPDATE_CACHED_OBJECT_PREPARE) {
+    if (t_state.api_next_action == 
HttpTransact::SM_ACTION_API_CACHE_LOOKUP_COMPLETE &&
+        t_state.api_cleanup_cache_read &&
+        t_state.api_update_cached_object != 
HttpTransact::UPDATE_CACHED_OBJECT_PREPARE) {
       t_state.api_cleanup_cache_read = false;
       t_state.cache_info.object_read = NULL;
       t_state.request_sent_time = UNDEFINED_TIME;
@@ -1603,6 +1599,11 @@ HttpSM::handle_api_return()
 
   case HttpTransact::SM_ACTION_REDIRECT_READ:
     {
+      // Clean up from any communication with previous servers
+      release_server_session();
+      cache_sm.close_write();
+      //tunnel.deallocate_redirect_postdata_buffers();
+    
       call_transact_and_set_next_state(HttpTransact::HandleRequest);
       break;
     }
@@ -5059,6 +5060,7 @@ HttpSM::release_server_session(bool serve_from_cache)
   if (TS_SERVER_SESSION_SHARING_MATCH_NONE != 
t_state.txn_conf->server_session_sharing_match &&
       t_state.current.server->keep_alive == HTTP_KEEPALIVE &&
       t_state.hdr_info.server_response.valid() &&
+      t_state.hdr_info.server_request.valid() &&
       (t_state.hdr_info.server_response.status_get() == 
HTTP_STATUS_NOT_MODIFIED ||
        (t_state.hdr_info.server_request.method_get_wksidx() == HTTP_WKSIDX_HEAD
         && t_state.www_auth_content != HttpTransact::CACHE_AUTH_NONE)) &&
@@ -7600,8 +7602,10 @@ HttpSM::redirect_request(const char *redirect_url, const 
int redirect_len)
     valid_origHost = false;
 
   t_state.hdr_info.server_request.destroy();
+
   // we want to close the server session
-  t_state.api_release_server_session = true;
+  // will do that in handle_api_return under the 
+  // HttpTransact::SM_ACTION_REDIRECT_READ state
   t_state.parent_result.r = PARENT_UNDEFINED;
   t_state.request_sent_time = 0;
   t_state.response_received_time = 0;
@@ -7611,6 +7615,10 @@ HttpSM::redirect_request(const char *redirect_url, const 
int redirect_len)
   t_state.dns_info.lookup_success = false;
   t_state.force_dns = false;
 
+  if (t_state.txn_conf->cache_http) {
+    t_state.cache_info.object_read = NULL;
+  }
+
   bool noPortInHost = HttpConfig::m_master.redirection_host_no_port;
 
   // check to see if the client request passed a host header, if so copy the 
host and port from the redirect url and

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/9e2689b7/proxy/http/HttpTransact.cc
----------------------------------------------------------------------
diff --git a/proxy/http/HttpTransact.cc b/proxy/http/HttpTransact.cc
index c6f55ed..987a084 100644
--- a/proxy/http/HttpTransact.cc
+++ b/proxy/http/HttpTransact.cc
@@ -1242,9 +1242,6 @@ HttpTransact::HandleRequest(State* s)
     HTTP_INCREMENT_TRANS_STAT(https_incoming_requests_stat);
   }
 
-  if (s->api_release_server_session == true) {
-    s->api_release_server_session = false;
-  }
   ///////////////////////////////////////////////
   // if request is bad, return error response  //
   ///////////////////////////////////////////////

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/9e2689b7/proxy/http/HttpTransact.h
----------------------------------------------------------------------
diff --git a/proxy/http/HttpTransact.h b/proxy/http/HttpTransact.h
index b2b6b42..f30f19c 100644
--- a/proxy/http/HttpTransact.h
+++ b/proxy/http/HttpTransact.h
@@ -969,7 +969,6 @@ public:
     // These ptrs are deallocate when transaction is over.
     HdrHeapSDKHandle *cache_req_hdr_heap_handle;
     HdrHeapSDKHandle *cache_resp_hdr_heap_handle;
-    bool api_release_server_session;
     bool api_cleanup_cache_read;
     bool api_server_response_no_store;
     bool api_server_response_ignore;
@@ -1082,7 +1081,6 @@ public:
         api_txn_no_activity_timeout_value(-1),
         cache_req_hdr_heap_handle(NULL),
         cache_resp_hdr_heap_handle(NULL),
-        api_release_server_session(false),
         api_cleanup_cache_read(false),
         api_server_response_no_store(false),
         api_server_response_ignore(false),

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/9e2689b7/proxy/http/HttpTunnel.cc
----------------------------------------------------------------------
diff --git a/proxy/http/HttpTunnel.cc b/proxy/http/HttpTunnel.cc
index a043b63..d75b5a1 100644
--- a/proxy/http/HttpTunnel.cc
+++ b/proxy/http/HttpTunnel.cc
@@ -1638,11 +1638,17 @@ 
HttpTunnel::allocate_redirect_postdata_buffers(IOBufferReader * ua_reader)
   // If fixed, obviously also fix the deallocator.
   if (postbuf == NULL) {
     postbuf = new PostDataBuffers();
+    postbuf->ua_buffer_reader = ua_reader;
+    postbuf->postdata_copy_buffer = new_MIOBuffer(alloc_index);
+    postbuf->postdata_copy_buffer_start = 
postbuf->postdata_copy_buffer->alloc_reader();
+    allocate_redirect_postdata_producer_buffer();
+  } else {
+    // Reset the buffer readers
+    
postbuf->postdata_copy_buffer->dealloc_reader(postbuf->postdata_copy_buffer_start);
+    postbuf->postdata_copy_buffer_start = 
postbuf->postdata_copy_buffer->alloc_reader();
+    
postbuf->postdata_producer_buffer->dealloc_reader(postbuf->postdata_producer_reader);
+    postbuf->postdata_producer_reader = 
postbuf->postdata_producer_buffer->alloc_reader();
   }
-  postbuf->ua_buffer_reader = ua_reader;
-  postbuf->postdata_copy_buffer = new_MIOBuffer(alloc_index);
-  postbuf->postdata_copy_buffer_start = 
postbuf->postdata_copy_buffer->alloc_reader();
-  allocate_redirect_postdata_producer_buffer();
 }
 
 

Reply via email to