You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by so...@apache.org on 2016/05/31 21:55:17 UTC
[trafficserver] branch 6.2.x updated: Fix ParentProxySet
configuration race.
This is an automated email from the ASF dual-hosted git repository.
sorber pushed a commit to branch 6.2.x
in repository https://git-dual.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/6.2.x by this push:
new dcffefa Fix ParentProxySet configuration race.
dcffefa is described below
commit dcffefa27247c07d0daa31c470f3431488eb2a55
Author: James Peach <jp...@apache.org>
AuthorDate: Thu May 26 11:42:17 2016 -0700
Fix ParentProxySet configuration race.
The first part of the problem occurs because the HTTP state machine
samples the parent proxy configuration at the start of a transaction.
We have to delay the start of the test until we know the configuration
has been applied.
The second pars occurs because the regression driver starts the
next exclusive test as soon as the current one has set its status.
This means that we need to tear down the synserver and any other
exclusive state before setting the test status.
(cherry picked from commit 55c57b39ce0bf51ec2b10fc97e401c2b6588f1ae)
---
proxy/InkAPITest.cc | 102 ++++++++++++++++++++++--------------------------
proxy/InkAPITestTool.cc | 77 +++++++++++++++++++++---------------
2 files changed, 91 insertions(+), 88 deletions(-)
diff --git a/proxy/InkAPITest.cc b/proxy/InkAPITest.cc
index 68ff2cc..8d3ba09 100644
--- a/proxy/InkAPITest.cc
+++ b/proxy/InkAPITest.cc
@@ -5900,8 +5900,8 @@ struct ParentTest {
this->regtest = test;
this->pstatus = pstatus;
this->magic = MAGIC_ALIVE;
- this->deferred.event = TS_EVENT_NONE;
- this->deferred.edata = NULL;
+ this->configured = false;
+ this->browser = synclient_txn_create();
/* If parent proxy routing is not enabled, enable it for the life of the test. */
RecGetRecordBool("proxy.config.http.parent_proxy_routing_enable", &this->parent_proxy_routing_enable);
@@ -5916,6 +5916,7 @@ struct ParentTest {
~ParentTest()
{
+ synclient_txn_close(this->browser);
synclient_txn_delete(this->browser);
synserver_delete(this->os);
this->os = NULL;
@@ -5936,11 +5937,7 @@ struct ParentTest {
RegressionTest *regtest;
int *pstatus;
-
- struct {
- TSEvent event;
- void *edata;
- } deferred;
+ bool configured;
const char *testcase;
SocketServer *os;
@@ -5959,6 +5956,7 @@ parent_proxy_success(TSCont contp, TSEvent event, void *edata)
int expected;
int received;
+ int status;
switch (event) {
case TS_EVENT_HTTP_SEND_RESPONSE_HDR:
@@ -5966,19 +5964,19 @@ parent_proxy_success(TSCont contp, TSEvent event, void *edata)
received = get_response_id(txnp);
if (expected != received) {
- *(ptest->pstatus) = REGRESSION_TEST_FAILED;
+ status = REGRESSION_TEST_FAILED;
SDK_RPRINT(ptest->regtest, "TSHttpTxnParentProxySet", "TestCase", TC_FAIL, "Expected response ID %d, received %d", expected,
received);
} else {
- *(ptest->pstatus) = REGRESSION_TEST_PASSED;
+ status = REGRESSION_TEST_PASSED;
SDK_RPRINT(ptest->regtest, "TSHttpTxnParentProxySet", "TestCase", TC_PASS, "Received expected response ID %d", expected);
}
TSHttpTxnReenable(txnp, TS_EVENT_HTTP_CONTINUE);
- return TS_EVENT_NONE;
+ return status;
default:
SDK_RPRINT(ptest->regtest, "TSHttpTxnParentProxySet", ptest->testcase, TC_FAIL, "Unexpected event %d", event);
- return TS_EVENT_ERROR;
+ return REGRESSION_TEST_FAILED;
}
}
@@ -5992,6 +5990,7 @@ parent_proxy_fail(TSCont contp, TSEvent event, void *edata)
TSMLoc hdr;
TSHttpStatus expected = TS_HTTP_STATUS_BAD_GATEWAY;
TSHttpStatus received;
+ int status;
switch (event) {
case TS_EVENT_HTTP_SEND_RESPONSE_HDR:
@@ -5999,20 +5998,20 @@ parent_proxy_fail(TSCont contp, TSEvent event, void *edata)
received = TSHttpHdrStatusGet(mbuf, hdr);
if (expected != received) {
- *(ptest->pstatus) = REGRESSION_TEST_FAILED;
+ status = REGRESSION_TEST_FAILED;
SDK_RPRINT(ptest->regtest, "TSHttpTxnParentProxySet", "TestCase", TC_FAIL, "Expected response status %d, received %d",
expected, received);
} else {
- *(ptest->pstatus) = REGRESSION_TEST_PASSED;
+ status = REGRESSION_TEST_PASSED;
SDK_RPRINT(ptest->regtest, "TSHttpTxnParentProxySet", "TestCase", TC_PASS, "Received expected response status %d", expected);
}
TSHandleMLocRelease(mbuf, TS_NULL_MLOC, hdr);
- return TS_EVENT_NONE;
+ return status;
default:
SDK_RPRINT(ptest->regtest, "TSHttpTxnParentProxySet", ptest->testcase, TC_FAIL, "Unexpected event %d", event);
- return TS_EVENT_ERROR;
+ return REGRESSION_TEST_FAILED;
}
}
@@ -6025,28 +6024,10 @@ parent_proxy_handler(TSCont contp, TSEvent event, void *edata)
ptest = (ParentTest *)TSContDataGet(contp);
ink_release_assert(ptest);
- if (ptest->deferred.event != TS_EVENT_NONE) {
- event = ptest->deferred.event;
- edata = ptest->deferred.edata;
- ptest->deferred.event = TS_EVENT_NONE;
- ptest->deferred.edata = NULL;
- }
-
TSHttpTxn txnp = (TSHttpTxn)edata;
switch (event) {
case TS_EVENT_HTTP_READ_REQUEST_HDR:
- // Keep deferring the test start until the parent configuration
- // has taken effect.
- if (!ptest->parent_routing_enabled()) {
- rprintf(ptest->regtest, "waiting for parent proxy configuration\n");
-
- ptest->deferred.event = event;
- ptest->deferred.edata = edata;
- TSContSchedule(contp, 100, TS_THREAD_POOL_NET);
- break;
- }
-
rprintf(ptest->regtest, "setting synserver parent proxy to %s:%d\n", "127.0.0.1", SYNSERVER_LISTEN_PORT);
// Since we chose a request format with a hostname of trafficserver.apache.org, it won't get
@@ -6062,9 +6043,29 @@ parent_proxy_handler(TSCont contp, TSEvent event, void *edata)
case TS_EVENT_TIMEOUT:
if (*(ptest->pstatus) == REGRESSION_TEST_INPROGRESS) {
- // If we are still in progress, reschedule.
- rprintf(ptest->regtest, "waiting for response\n");
- TSContSchedule(contp, 100, TS_THREAD_POOL_DEFAULT);
+ if (ptest->configured) {
+ // If we are still in progress, reschedule.
+ rprintf(ptest->regtest, "waiting for response\n");
+ TSContSchedule(contp, 100, TS_THREAD_POOL_DEFAULT);
+ break;
+ }
+
+ if (!ptest->parent_routing_enabled()) {
+ rprintf(ptest->regtest, "waiting for configuration\n");
+ TSContSchedule(contp, 100, TS_THREAD_POOL_DEFAULT);
+ break;
+ }
+
+ // Now that the configuration is applied, it is safe to create a request.
+ // HTTP_REQUEST_FORMAT11 is a hostname with a no-cache response, so
+ // we will need to set the parent to the synserver to get a
+ // response.
+ char *request = generate_request(11);
+ synclient_txn_send_request(ptest->browser, request);
+ TSfree(request);
+
+ ptest->configured = true;
+
} else {
// Otherwise the test completed so clean up.
RecSetRecordInt("proxy.config.http.parent_proxy_routing_enable", ptest->parent_proxy_routing_enable, REC_SOURCE_EXPLICIT);
@@ -6084,13 +6085,20 @@ parent_proxy_handler(TSCont contp, TSEvent event, void *edata)
break;
default:
- if (ptest->handler(contp, event, edata) == TS_ERROR) {
- *(ptest->pstatus) = REGRESSION_TEST_FAILED;
+ {
+ int status = ptest->handler(contp, event, edata);
+ if (status != REGRESSION_TEST_INPROGRESS) {
+ int *pstatus = ptest->pstatus;
+
+ RecSetRecordInt("proxy.config.http.parent_proxy_routing_enable", ptest->parent_proxy_routing_enable, REC_SOURCE_EXPLICIT);
TSContDataSet(contp, NULL);
delete ptest;
+
+ *pstatus = status;
}
}
+ }
return TS_EVENT_NONE;
}
@@ -6119,15 +6127,6 @@ EXCLUSIVE_REGRESSION_TEST(SDK_API_HttpParentProxySet_Fail)(RegressionTest *test,
ptest->os = synserver_create(SYNSERVER_LISTEN_PORT, TSContCreate(synserver_vc_refuse, TSMutexCreate()));
synserver_start(ptest->os);
- /* Create a client transaction */
- ptest->browser = synclient_txn_create();
-
- // HTTP_REQUEST_FORMAT11 is a hostname with a no-cache response, so we will need to set the parent to the synserver to get a
- // response.
- char *request = generate_request(11);
- synclient_txn_send_request(ptest->browser, request);
- TSfree(request);
-
TSContSchedule(cont, 25, TS_THREAD_POOL_DEFAULT);
}
@@ -6155,15 +6154,6 @@ EXCLUSIVE_REGRESSION_TEST(SDK_API_HttpParentProxySet_Success)(RegressionTest *te
ptest->os = synserver_create(SYNSERVER_LISTEN_PORT, TSContCreate(synserver_vc_accept, TSMutexCreate()));
synserver_start(ptest->os);
- /* Create a client transaction */
- ptest->browser = synclient_txn_create();
-
- // HTTP_REQUEST_FORMAT11 is a hostname with a no-cache response, so we will need to set the parent to the synserver to get a
- // response.
- char *request = generate_request(11);
- synclient_txn_send_request(ptest->browser, request);
- TSfree(request);
-
TSContSchedule(cont, 25, TS_THREAD_POOL_DEFAULT);
}
diff --git a/proxy/InkAPITestTool.cc b/proxy/InkAPITestTool.cc
index 4fc5d10..3e508bb 100644
--- a/proxy/InkAPITestTool.cc
+++ b/proxy/InkAPITestTool.cc
@@ -139,7 +139,7 @@ static int get_request_id(TSHttpTxn txnp);
/* client side */
static ClientTxn *synclient_txn_create(void);
static int synclient_txn_delete(ClientTxn *txn);
-static int synclient_txn_close(TSCont contp);
+static void synclient_txn_close(ClientTxn *txn);
static int synclient_txn_send_request(ClientTxn *txn, char *request);
static int synclient_txn_send_request_to_vc(ClientTxn *txn, char *request, TSVConn vc);
static int synclient_txn_read_response(TSCont contp);
@@ -364,11 +364,11 @@ generate_response(const char *request)
"\r\n" \
"Body for response 10"
-#define HTTP_RESPONSE_FORMAT11 \
- "HTTP/1.0 200 OK\r\n" \
- "Cache-Control: no-cache\r\n" \
- "X-Response-ID: %d\r\n" \
- "\r\n" \
+#define HTTP_RESPONSE_FORMAT11 \
+ "HTTP/1.0 200 OK\r\n" \
+ "Cache-Control: private,no-store\r\n" \
+ "X-Response-ID: %d\r\n" \
+ "\r\n" \
"Body for response 11"
int test_case, match, http_version;
@@ -521,26 +521,27 @@ synclient_txn_delete(ClientTxn *txn)
return 1;
}
-static int
-synclient_txn_close(TSCont contp)
+static void
+synclient_txn_close(ClientTxn *txn)
{
- ClientTxn *txn = (ClientTxn *)TSContDataGet(contp);
- TSAssert(txn->magic == MAGIC_ALIVE);
+ if (txn) {
+ if (txn->vconn != NULL) {
+ TSVConnClose(txn->vconn);
+ txn->vconn = NULL;
+ }
- if (txn->vconn != NULL) {
- TSVConnClose(txn->vconn);
- }
- if (txn->req_buffer != NULL) {
- TSIOBufferDestroy(txn->req_buffer);
- }
- if (txn->resp_buffer != NULL) {
- TSIOBufferDestroy(txn->resp_buffer);
- }
+ if (txn->req_buffer != NULL) {
+ TSIOBufferDestroy(txn->req_buffer);
+ txn->req_buffer = NULL;
+ }
- TSContDestroy(contp);
+ if (txn->resp_buffer != NULL) {
+ TSIOBufferDestroy(txn->resp_buffer);
+ txn->resp_buffer = NULL;
+ }
- TSDebug(CDBG_TAG, "Client Txn destroyed");
- return TS_EVENT_IMMEDIATE;
+ TSDebug(CDBG_TAG, "Client Txn destroyed");
+ }
}
static int
@@ -636,14 +637,16 @@ synclient_txn_read_response_handler(TSCont contp, TSEvent event, void * /* data
TSDebug(CDBG_TAG, "READ_EOS");
// Connection closed. In HTTP/1.0 it means we're done for this request.
txn->status = REQUEST_SUCCESS;
- return synclient_txn_close(contp);
- break;
+ synclient_txn_close((ClientTxn *)TSContDataGet(contp));
+ TSContDestroy(contp);
+ return 1;
case TS_EVENT_ERROR:
TSDebug(CDBG_TAG, "READ_ERROR");
txn->status = REQUEST_FAILURE;
- return synclient_txn_close(contp);
- break;
+ synclient_txn_close((ClientTxn *)TSContDataGet(contp));
+ TSContDestroy(contp);
+ return 1;
default:
TSAssert(!"Invalid event");
@@ -708,13 +711,15 @@ synclient_txn_write_request_handler(TSCont contp, TSEvent event, void * /* data
case TS_EVENT_VCONN_EOS:
TSDebug(CDBG_TAG, "WRITE_EOS");
txn->status = REQUEST_FAILURE;
- return synclient_txn_close(contp);
+ synclient_txn_close((ClientTxn *)TSContDataGet(contp));
+ TSContDestroy(contp);
break;
case TS_EVENT_ERROR:
TSDebug(CDBG_TAG, "WRITE_ERROR");
txn->status = REQUEST_FAILURE;
- return synclient_txn_close(contp);
+ synclient_txn_close((ClientTxn *)TSContDataGet(contp));
+ TSContDestroy(contp);
break;
default:
@@ -757,7 +762,8 @@ synclient_txn_connect_handler(TSCont contp, TSEvent event, void *data)
} else {
TSDebug(CDBG_TAG, "NET_CONNECT_FAILED");
txn->status = REQUEST_FAILURE;
- synclient_txn_close(contp);
+ synclient_txn_close((ClientTxn *)TSContDataGet(contp));
+ TSContDestroy(contp);
}
return TS_EVENT_IMMEDIATE;
@@ -858,7 +864,14 @@ synserver_vc_refuse(TSCont contp, TSEvent event, void *data)
SocketServer *s = (SocketServer *)TSContDataGet(contp);
TSAssert(s->magic == MAGIC_ALIVE);
- TSDebug(SDBG_TAG, "NET_ACCEPT");
+ TSDebug(SDBG_TAG, "%s: NET_ACCEPT", __func__);
+
+ if (event == TS_EVENT_NET_ACCEPT_FAILED) {
+ Warning("Synserver failed to bind to port %d.", ntohs(s->accept_port));
+ ink_release_assert(!"Synserver must be able to bind to a port, check system netstat");
+ TSDebug(SDBG_TAG, "%s: NET_ACCEPT_FAILED", __func__);
+ return TS_EVENT_IMMEDIATE;
+ }
TSVConnClose((TSVConn)data);
return TS_EVENT_IMMEDIATE;
@@ -875,11 +888,11 @@ synserver_vc_accept(TSCont contp, TSEvent event, void *data)
if (event == TS_EVENT_NET_ACCEPT_FAILED) {
Warning("Synserver failed to bind to port %d.", ntohs(s->accept_port));
ink_release_assert(!"Synserver must be able to bind to a port, check system netstat");
- TSDebug(SDBG_TAG, "NET_ACCEPT_FAILED");
+ TSDebug(SDBG_TAG, "%s: NET_ACCEPT_FAILED", __func__);
return TS_EVENT_IMMEDIATE;
}
- TSDebug(SDBG_TAG, "NET_ACCEPT");
+ TSDebug(SDBG_TAG, "%s: NET_ACCEPT", __func__);
/* Create a new transaction */
ServerTxn *txn = (ServerTxn *)TSmalloc(sizeof(ServerTxn));
--
To stop receiving notification emails like this one, please contact
['"commits@trafficserver.apache.org" <co...@trafficserver.apache.org>'].