You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by jp...@apache.org on 2016/05/02 18:55:51 UTC

[trafficserver] 03/06: TS-4388: Fix global hook handling in API tests.

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

jpeach pushed a commit to branch master
in repository https://git-dual.apache.org/repos/asf/trafficserver.git

commit 060b8e8bd0d794ee038595e42e3e104d6c89371a
Author: James Peach <jp...@apache.org>
AuthorDate: Thu Apr 28 20:02:38 2016 +0000

    TS-4388: Fix global hook handling in API tests.
    
    Many of the API regression tests work by trampolining off a global
    hook (which is really what you have to do). However, there's no way
    to unregister a global hook, so once the test is done, it needs to
    be careful to co-operate with the remaining tests. We clear the
    continuation data, and if is is clear, we either ignore the event
    or re-enable the HTTP transaction.
---
 proxy/InkAPITest.cc     | 110 ++++++++++++++++++++++++++----------------------
 proxy/InkAPITestTool.cc |  38 ++++++++++++-----
 2 files changed, 88 insertions(+), 60 deletions(-)

diff --git a/proxy/InkAPITest.cc b/proxy/InkAPITest.cc
index cb883ec..342e824 100644
--- a/proxy/InkAPITest.cc
+++ b/proxy/InkAPITest.cc
@@ -55,6 +55,39 @@
 
 #define UTDBG_TAG "sdk_ut"
 
+// Since there's no way to unregister global hooks, tests that register a hook
+// have to co-operate once they are complete by re-enabling and transactions
+// and getting out of the way.
+#define CHECK_SPURIOUS_EVENT(cont, event, edata)                     \
+  if (TSContDataGet(cont) == NULL) {                                 \
+    switch (event) {                                                 \
+    case TS_EVENT_IMMEDIATE:                                         \
+    case TS_EVENT_TIMEOUT:                                           \
+      return TS_EVENT_NONE;                                          \
+    case TS_EVENT_HTTP_SELECT_ALT:                                   \
+      return TS_EVENT_NONE;                                          \
+    case TS_EVENT_HTTP_READ_REQUEST_HDR:                             \
+    case TS_EVENT_HTTP_OS_DNS:                                       \
+    case TS_EVENT_HTTP_SEND_REQUEST_HDR:                             \
+    case TS_EVENT_HTTP_READ_CACHE_HDR:                               \
+    case TS_EVENT_HTTP_READ_RESPONSE_HDR:                            \
+    case TS_EVENT_HTTP_SEND_RESPONSE_HDR:                            \
+    case TS_EVENT_HTTP_REQUEST_TRANSFORM:                            \
+    case TS_EVENT_HTTP_RESPONSE_TRANSFORM:                           \
+    case TS_EVENT_HTTP_TXN_START:                                    \
+    case TS_EVENT_HTTP_TXN_CLOSE:                                    \
+    case TS_EVENT_HTTP_SSN_START:                                    \
+    case TS_EVENT_HTTP_SSN_CLOSE:                                    \
+    case TS_EVENT_HTTP_CACHE_LOOKUP_COMPLETE:                        \
+    case TS_EVENT_HTTP_PRE_REMAP:                                    \
+    case TS_EVENT_HTTP_POST_REMAP:                                   \
+      TSHttpTxnReenable((TSHttpTxn)(edata), TS_EVENT_HTTP_CONTINUE); \
+      return TS_EVENT_NONE;                                          \
+    default:                                                         \
+      break;                                                         \
+    }                                                                \
+  }
+
 /******************************************************************************/
 
 /* Use SDK_RPRINT to report failure or success for each test case */
@@ -2303,6 +2336,7 @@ mytest_handler(TSCont contp, TSEvent event, void *data)
       // transaction is over. clean up.
       synclient_txn_delete(test->browser);
       synserver_delete(test->os);
+      test->os = NULL;
 
       test->magic = MAGIC_DEAD;
       TSfree(test);
@@ -5804,6 +5838,7 @@ ssn_handler(TSCont contp, TSEvent event, void *edata)
       /* Don't need it as didn't initialize the server
          synserver_delete(data->os);
        */
+      data->os = NULL;
       data->magic = MAGIC_DEAD;
       TSfree(data);
       TSContDataSet(contp, NULL);
@@ -5871,9 +5906,12 @@ struct ParentTest {
 static int
 parent_proxy_handler(TSCont contp, TSEvent event, void *edata)
 {
-  ParentTest *ptest = (ParentTest *)TSContDataGet(contp);
+  ParentTest *ptest = NULL;
   TSHttpTxn txnp = (TSHttpTxn)edata;
 
+  CHECK_SPURIOUS_EVENT(contp, event, edata);
+  ptest = (ParentTest *)TSContDataGet(contp);
+
   switch (event) {
   case TS_EVENT_HTTP_READ_REQUEST_HDR:
     rprintf(ptest->regtest, "setting synserver parent proxy to %s:%d\n", "127.0.0.1", SYNSERVER_LISTEN_PORT);
@@ -5914,11 +5952,13 @@ parent_proxy_handler(TSCont contp, TSEvent event, void *edata)
       // Otherwise the test completed so clean up.
       RecSetRecordInt("proxy.config.http.parent_proxy_routing_enable", ptest->parent_proxy_routing_enable, REC_SOURCE_EXPLICIT);
 
-      ptest->magic = MAGIC_DEAD;
+      TSContDataSet(contp, NULL);
+
       synclient_txn_delete(ptest->browser);
       synserver_delete(ptest->os);
+      ptest->os = NULL;
+      ptest->magic = MAGIC_DEAD;
       TSfree(ptest);
-      TSContDataSet(contp, NULL);
     }
     break;
 
@@ -6017,20 +6057,10 @@ static int
 cache_hook_handler(TSCont contp, TSEvent event, void *edata)
 {
   TSHttpTxn txnp = NULL;
-  CacheTestData *data = (CacheTestData *)TSContDataGet(contp);
+  CacheTestData *data = NULL;
 
-  if (data == NULL) {
-    switch (event) {
-    case TS_EVENT_IMMEDIATE:
-    case TS_EVENT_TIMEOUT:
-      break;
-    case TS_EVENT_HTTP_READ_CACHE_HDR:
-    default:
-      TSHttpTxnReenable((TSHttpTxn)edata, TS_EVENT_HTTP_CONTINUE);
-      break;
-    }
-    return 0;
-  }
+  CHECK_SPURIOUS_EVENT(contp, event, edata);
+  data = (CacheTestData *)TSContDataGet(contp);
 
   switch (event) {
   case TS_EVENT_HTTP_READ_REQUEST_HDR:
@@ -6139,6 +6169,7 @@ cache_hook_handler(TSCont contp, TSEvent event, void *edata)
         data->first_time = false;
         /* Kill the origin server */
         synserver_delete(data->os);
+        data->os = NULL;
 
         /* Send another similar client request */
         synclient_txn_send_request(data->browser2, data->request);
@@ -6523,19 +6554,9 @@ transform_hook_handler(TSCont contp, TSEvent event, void *edata)
 {
   TSHttpTxn txnp = NULL;
   TransformTestData *data = NULL;
+
+  CHECK_SPURIOUS_EVENT(contp, event, edata);
   data = (TransformTestData *)TSContDataGet(contp);
-  if (data == NULL) {
-    switch (event) {
-    case TS_EVENT_IMMEDIATE:
-    case TS_EVENT_TIMEOUT:
-      break;
-    case TS_EVENT_HTTP_READ_RESPONSE_HDR:
-    default:
-      TSHttpTxnReenable((TSHttpTxn)edata, TS_EVENT_HTTP_CONTINUE);
-      break;
-    }
-    return 0;
-  }
 
   switch (event) {
   case TS_EVENT_HTTP_READ_REQUEST_HDR:
@@ -6651,6 +6672,7 @@ transform_hook_handler(TSCont contp, TSEvent event, void *edata)
         return 0;
       }
       synserver_delete(data->os);
+      data->os = NULL;
       data->req_no++;
       TSfree(data->request1);
       TSfree(data->request2);
@@ -6823,20 +6845,8 @@ altinfo_hook_handler(TSCont contp, TSEvent event, void *edata)
   AltInfoTestData *data = NULL;
   TSHttpTxn txnp = NULL;
 
+  CHECK_SPURIOUS_EVENT(contp, event, edata);
   data = (AltInfoTestData *)TSContDataGet(contp);
-  if (data == NULL) {
-    switch (event) {
-    case TS_EVENT_IMMEDIATE:
-    case TS_EVENT_TIMEOUT:
-      break;
-    case TS_EVENT_HTTP_SELECT_ALT:
-      break;
-    default:
-      TSHttpTxnReenable((TSHttpTxn)edata, TS_EVENT_HTTP_CONTINUE);
-      break;
-    }
-    return 0;
-  }
 
   switch (event) {
   case TS_EVENT_HTTP_READ_REQUEST_HDR:
@@ -6928,6 +6938,7 @@ altinfo_hook_handler(TSCont contp, TSEvent event, void *edata)
         data->first_time = false;
         /* Kill the origin server */
         synserver_delete(data->os);
+        data->os = NULL;
         // ink_release_assert(0);
         /* Send another similar client request */
         synclient_txn_send_request(data->browser3, data->request3);
@@ -7044,8 +7055,6 @@ EXCLUSIVE_REGRESSION_TEST(SDK_API_HttpAltInfo)(RegressionTest *test, int /* atyp
 #define TEST_CASE_CONNECT_ID1 9  // TSHttpTxnIntercept
 #define TEST_CASE_CONNECT_ID2 10 // TSHttpTxnServerIntercept
 
-#define SYNSERVER_DUMMY_PORT -1
-
 typedef struct {
   RegressionTest *test;
   int *pstatus;
@@ -7061,9 +7070,12 @@ static int
 cont_test_handler(TSCont contp, TSEvent event, void *edata)
 {
   TSHttpTxn txnp = (TSHttpTxn)edata;
-  ConnectTestData *data = (ConnectTestData *)TSContDataGet(contp);
+  ConnectTestData *data = NULL;
   int request_id = -1;
 
+  CHECK_SPURIOUS_EVENT(contp, event, edata);
+  data = (ConnectTestData *)TSContDataGet(contp);
+
   TSReleaseAssert(data->magic == MAGIC_ALIVE);
   TSReleaseAssert((data->test_case == TEST_CASE_CONNECT_ID1) || (data->test_case == TEST_CASE_CONNECT_ID2));
 
@@ -7139,12 +7151,10 @@ cont_test_handler(TSCont contp, TSEvent event, void *edata)
       // transaction is over. clean it up.
       synclient_txn_delete(data->browser);
       synserver_delete(data->os);
-
-      // As we registered to a global hook, we may be called back again.
-      // Do not destroy the continuation...
-      // data->magic = MAGIC_DEAD;
-      // TSfree(data);
-      // TSContDataSet(contp, NULL);
+      data->os = NULL;
+      data->magic = MAGIC_DEAD;
+      TSfree(data);
+      TSContDataSet(contp, NULL);
     }
     break;
 
diff --git a/proxy/InkAPITestTool.cc b/proxy/InkAPITestTool.cc
index eb399fd..1ebac13 100644
--- a/proxy/InkAPITestTool.cc
+++ b/proxy/InkAPITestTool.cc
@@ -42,6 +42,7 @@
 #define MAGIC_DEAD 0xdeadbeef
 
 #define SYNSERVER_LISTEN_PORT 3300
+#define SYNSERVER_DUMMY_PORT -1
 
 #define PROXY_CONFIG_NAME_HTTP_PORT "proxy.config.http.server_port"
 #define PROXY_HTTP_DEFAULT_PORT 8080
@@ -762,6 +763,11 @@ synclient_txn_main_handler(TSCont contp, TSEvent event, void *data)
 SocketServer *
 synserver_create(int port)
 {
+  if (port != SYNSERVER_DUMMY_PORT) {
+    TSAssert(port > 0);
+    TSAssert(port < INT16_MAX);
+  }
+
   SocketServer *s = (SocketServer *)TSmalloc(sizeof(SocketServer));
   s->magic = MAGIC_ALIVE;
   s->accept_port = port;
@@ -775,7 +781,15 @@ static int
 synserver_start(SocketServer *s)
 {
   TSAssert(s->magic == MAGIC_ALIVE);
-  s->accept_action = TSNetAccept(s->accept_cont, s->accept_port, -1, 0);
+  TSAssert(s->accept_action == NULL);
+
+  if (s->accept_port != SYNSERVER_DUMMY_PORT) {
+    TSAssert(s->accept_port > 0);
+    TSAssert(s->accept_port < INT16_MAX);
+
+    s->accept_action = TSNetAccept(s->accept_cont, s->accept_port, AF_INET, 0);
+  }
+
   return 1;
 }
 
@@ -795,17 +809,21 @@ synserver_stop(SocketServer *s)
 static int
 synserver_delete(SocketServer *s)
 {
-  TSAssert(s->magic == MAGIC_ALIVE);
-  synserver_stop(s);
+  if (s != NULL) {
+    TSAssert(s->magic == MAGIC_ALIVE);
+    synserver_stop(s);
+
+    if (s->accept_cont) {
+      TSContDestroy(s->accept_cont);
+      s->accept_cont = NULL;
+      TSDebug(SDBG_TAG, "destroyed accept cont");
+    }
 
-  if (s->accept_cont) {
-    TSContDestroy(s->accept_cont);
-    s->accept_cont = NULL;
-    TSDebug(SDBG_TAG, "destroyed accept cont");
+    s->magic = MAGIC_DEAD;
+    TSfree(s);
+    TSDebug(SDBG_TAG, "deleted server");
   }
-  s->magic = MAGIC_DEAD;
-  TSfree(s);
-  TSDebug(SDBG_TAG, "deleted server");
+
   return 1;
 }
 

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