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 2013/10/30 21:21:27 UTC
[2/2] git commit: TS-2296: fix the ProxyConfig regression tests
TS-2296: fix the ProxyConfig regression tests
The ProxyConfig_Set and ProxyConfig_Release tests held doown an
event thread while waiting for the ConfigProcessor to garbage collect
stale configs. The fix is to go fully async and build a mini-semaphore
so that we can check the released object states at the correct time.
Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/6ae6b3f4
Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/6ae6b3f4
Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/6ae6b3f4
Branch: refs/heads/master
Commit: 6ae6b3f491ab37003770878fc26e4afbc0962572
Parents: ffc8428
Author: James Peach <jp...@apache.org>
Authored: Mon Oct 28 21:20:48 2013 -0700
Committer: James Peach <jp...@apache.org>
Committed: Wed Oct 30 13:20:53 2013 -0700
----------------------------------------------------------------------
mgmt/ProxyConfig.cc | 98 +++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 85 insertions(+), 13 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/trafficserver/blob/6ae6b3f4/mgmt/ProxyConfig.cc
----------------------------------------------------------------------
diff --git a/mgmt/ProxyConfig.cc b/mgmt/ProxyConfig.cc
index 2c6f7be..1052a58 100644
--- a/mgmt/ProxyConfig.cc
+++ b/mgmt/ProxyConfig.cc
@@ -105,7 +105,7 @@ public:
{
configProcessor.release(m_id, m_info);
delete this;
- return 0;
+ return EVENT_DONE;
}
public:
@@ -221,6 +221,36 @@ enum {
struct RegressionConfig : public ConfigInfo
{
+ static volatile int nobjects; // count of outstanding RegressionConfig objects (not-reentrant)
+
+ // DeferredCall is a simple function call wrapper that defers itself until the RegressionConfig
+ // object count drops below the specified count.
+ template <typename CallType>
+ struct DeferredCall : public Continuation
+ {
+ DeferredCall(int _r, CallType _c) : remain(_r), call(_c) {
+ SET_HANDLER(&DeferredCall::handleEvent);
+ }
+
+ int handleEvent(int event, Event * e) {
+ if (RegressionConfig::nobjects > this->remain) {
+ e->schedule_in(HRTIME_MSECONDS(500));
+ return EVENT_CONT;
+ }
+
+ call();
+ delete this;
+ return EVENT_DONE;
+ }
+
+ int remain; // Number of remaining RegressionConfig objects to wait for.
+ CallType call;
+ };
+
+ template <typename CallType>
+ static void defer(int count, CallType call) {
+ eventProcessor.schedule_in(NEW(new RegressionConfig::DeferredCall<CallType>(count, call)), HRTIME_MSECONDS(500));
+ }
RegressionConfig(RegressionTest * r, int * ps, unsigned f)
: test(r), pstatus(ps), flags(f) {
@@ -228,6 +258,8 @@ struct RegressionConfig : public ConfigInfo
TestBox box(this->test, this->pstatus);
box.check(this->refcount() == 1, "invalid refcount %d (should be 1)", this->refcount());
}
+
+ ink_atomic_increment(&nobjects, 1);
}
~RegressionConfig() {
@@ -240,8 +272,10 @@ struct RegressionConfig : public ConfigInfo
if (REGRESSION_CONFIG_LAST & flags) {
*this->pstatus = REGRESSION_TEST_PASSED;
} else {
- box.check(*this->pstatus == REGRESSION_TEST_INPROGRESS, "intermediate config out of sequence");
+ box.check(*this->pstatus == REGRESSION_TEST_INPROGRESS, "intermediate config out of sequence, *pstatus is %d", *pstatus);
}
+
+ ink_atomic_increment(&nobjects, -1);
}
RegressionTest * test;
@@ -249,31 +283,68 @@ struct RegressionConfig : public ConfigInfo
unsigned flags;
};
+volatile int RegressionConfig::nobjects = 0;
+
+struct ProxyConfig_Set_Completion
+{
+ ProxyConfig_Set_Completion(int _id, RegressionConfig * _c)
+ : configid(_id), config(_c) {
+ }
+
+ void operator() (void) const {
+ // Push one more RegressionConfig to force the LAST-tagged one to get destroyed.
+ rprintf(config->test, "setting LAST config object %p\n", config);
+ configProcessor.set(configid, config, 1);
+ }
+
+ int configid;
+ RegressionConfig * config;
+};
+
// Test that ConfigProcessor::set() correctly releases the old ConfigInfo after a timeout.
-REGRESSION_TEST(ProxyConfig_Set)(RegressionTest * test, int /* atype ATS_UNUSED */, int * pstatus)
+EXCLUSIVE_REGRESSION_TEST(ProxyConfig_Set)(RegressionTest * test, int /* atype ATS_UNUSED */, int * pstatus)
{
int configid = 0;
+
*pstatus = REGRESSION_TEST_INPROGRESS;
+ RegressionConfig::nobjects = 0;
+
configid = configProcessor.set(configid, new RegressionConfig(test, pstatus, REGRESSION_CONFIG_FIRST), 1);
configid = configProcessor.set(configid, new RegressionConfig(test, pstatus, REGRESSION_CONFIG_FIRST), 1);
configid = configProcessor.set(configid, new RegressionConfig(test, pstatus, REGRESSION_CONFIG_FIRST), 1);
- configid = configProcessor.set(configid, new RegressionConfig(test, pstatus, REGRESSION_CONFIG_LAST), 2);
-
- // Push one more RegressionConfig to force the tagged one to
- // get destroyed.
- configid = configProcessor.set(configid, new RegressionConfig(test, pstatus, 0), 4);
+ configid = configProcessor.set(configid, new RegressionConfig(test, pstatus, REGRESSION_CONFIG_FIRST), 1);
+ configid = configProcessor.set(configid, new RegressionConfig(test, pstatus, REGRESSION_CONFIG_FIRST), 1);
+ configid = configProcessor.set(configid, new RegressionConfig(test, pstatus, REGRESSION_CONFIG_FIRST), 1);
+ configid = configProcessor.set(configid, new RegressionConfig(test, pstatus, REGRESSION_CONFIG_LAST), 1);
- return;
+ // Wait until there's only 2 objects remaining, the one in ConfigProcessor, and the one we make here.
+ RegressionConfig::defer(2, ProxyConfig_Set_Completion(configid, new RegressionConfig(test, pstatus, 0)));
}
+struct ProxyConfig_Release_Completion
+{
+ ProxyConfig_Release_Completion(int _id, RegressionConfig * _c)
+ : configid(_id), config(_c) {
+ }
+
+ void operator() (void) const {
+ // Release the reference count. Since we were keeping this alive, it should be the last to die.
+ configProcessor.release(configid, config);
+ }
+
+ int configid;
+ RegressionConfig * config;
+};
+
// Test that ConfigProcessor::release() correctly releases the old ConfigInfo across an implicit
// release timeout.
-REGRESSION_TEST(ProxyConfig_Release)(RegressionTest * test, int /* atype ATS_UNUSED */, int * pstatus)
+EXCLUSIVE_REGRESSION_TEST(ProxyConfig_Release)(RegressionTest * test, int /* atype ATS_UNUSED */, int * pstatus)
{
int configid = 0;
RegressionConfig * config;
*pstatus = REGRESSION_TEST_INPROGRESS;
+ RegressionConfig::nobjects = 0;
// Set an initial config, then get it back to hold a reference count.
configid = configProcessor.set(configid, new RegressionConfig(test, pstatus, REGRESSION_CONFIG_LAST), 1);
@@ -284,10 +355,11 @@ REGRESSION_TEST(ProxyConfig_Release)(RegressionTest * test, int /* atype ATS_UNU
configid = configProcessor.set(configid, new RegressionConfig(test, pstatus, REGRESSION_CONFIG_FIRST), 1);
configid = configProcessor.set(configid, new RegressionConfig(test, pstatus, REGRESSION_CONFIG_FIRST), 1);
- sleep(2);
+ configid = configProcessor.set(configid, new RegressionConfig(test, pstatus, 0), 1);
- // Release the reference count. Since we were keeping this alive, it should be the last to die.
- configProcessor.release(configid, config);
+ // Defer the release of the object that we held back until there are only 2 left. The one we are holding
+ // and the one in the ConfigProcessor. Then releasing the one we hold will trigger the LAST check
+ RegressionConfig::defer(2, ProxyConfig_Release_Completion(configid, config));
}
#endif /* TS_HAS_TESTS */