Solve legacy problem with clear operation (coredump with running threads)
authorEduardo Ramos Testillano (eramedu) <eduardo.ramos.testillano@ericsson.com>
Mon, 11 May 2020 16:53:32 +0000 (18:53 +0200)
committerEduardo Ramos Testillano (eramedu) <eduardo.ramos.testillano@ericsson.com>
Mon, 11 May 2020 16:53:32 +0000 (18:53 +0200)
Modified the clear operation. Now is 'conditional', so, when is unsafe,
it will be ignored. A message on http interface advice about trying
later because some running threads could be ongoing (check test
summary to confirm). Sometimes, it could be due to stuck external
procedures. The user is responsible to kill those stuck procedures
or guarantee they are timedout (it is recommended to wrap procedures
into safe containers (subshells) if you don't trust too much those
procedures).

Normally, step commands are not so usual. ADML FSM (finite state
machine) is better used through REST API from other controllers
with their own FSM and interface endpoints (no need for ADML
dedicated command-step threads).

include/anna/testing/TestCase.hpp
include/anna/testing/TestManager.hpp
include/anna/testing/TestStep.hpp
source/testing/TestCase.cpp
source/testing/TestManager.cpp
source/testing/TestStep.cpp

index e87750c..3ebd535 100644 (file)
@@ -119,6 +119,9 @@ public:
   // Reset test case and underlaying information (steps context)
   bool reset(bool hard /* hard reset includes in-progress test cases */) throw();
 
+  // Check for clear (destroy steps)
+  bool safeToClear(); // false if something pending (running threads steps)
+
   // getters
   const unsigned int &getId() const throw() { return a_id; }
   const std::string &getDescription() const throw() { return a_description; }
index 4f3bed2..40d5370 100644 (file)
@@ -158,7 +158,7 @@ class TestManager : public anna::timex::TimeEventObserver, public anna::Singleto
     //  through the time manager. The first call to this method will start the time trigger system and check for new test cases to be launched.
     bool configureTTPS(int testTicksPerSecond) throw();
 
-    bool clearPool() throw();
+    bool clearPool(std::string &result) throw();
     bool resetPool(bool hard /* hard reset includes in-progress test cases */) throw();
     void setPoolRepeats(int repeats) throw() { a_poolRepeats = repeats; }
     int getPoolRepeats() const throw() { return a_poolRepeats; }
index 4442f66..844b6be 100644 (file)
@@ -13,6 +13,7 @@
 #include <string>
 #include <vector>
 #include <thread>
+#include <atomic>
 
 // Project
 #include <anna/core/DataBlock.hpp>
@@ -102,6 +103,7 @@ class TestStepTimeout : public TestStep {
 
   public:
     TestStepTimeout(TestCase *testCase) : TestStep(testCase), a_timeout(0), a_timer(NULL) { a_type = Type::Timeout; }
+    ~TestStepTimeout() { do_reset(); }
 
     // setter & getters
     void setTimeout(const anna::Millisecond &t) throw() { a_timeout = t; }
@@ -133,7 +135,7 @@ class TestStepSendDiameterXml : public TestStep {
       a_expired(false),
       a_originHost(NULL),
       a_waitForRequestStepNumber(-1) {;}
-    ~TestStepSendDiameterXml() {;}
+    ~TestStepSendDiameterXml() { do_reset(); }
 
     // setter & getters
     void setOriginHost(anna::diameter::comm::OriginHost *host) throw() { a_originHost = host; }
@@ -167,6 +169,7 @@ class TestStepDelay : public TestStep {
 
   public:
     TestStepDelay(TestCase *testCase) : TestStep(testCase), a_delay(0), a_timer(NULL) { a_type = Type::Delay; }
+    ~TestStepDelay() { do_reset(); }
 
     // setter & getters
     void setDelay(const anna::Millisecond &d) throw() { a_delay = d; }
@@ -193,7 +196,7 @@ class TestStepWaitDiameter : public TestStep {
       a_clientSession = NULL;
       a_serverSession = NULL;
     }
-    ~TestStepWaitDiameter() {;}
+    ~TestStepWaitDiameter() { do_reset(); }
 
     // setter & getters
     void setCondition(bool fromEntity,
@@ -226,7 +229,7 @@ class TestStepCmd : public TestStep {
 
   std::string a_script;
   std::thread a_thread;
-  bool a_threadRunning;
+  std::atomic<bool> a_threadRunning;
   bool a_threadDeprecated;
   int a_resultCode;
   std::string a_errorMsg;
@@ -236,9 +239,13 @@ class TestStepCmd : public TestStep {
 
   public:
     TestStepCmd(TestCase *testCase) : TestStep(testCase), a_threadRunning(false), a_threadDeprecated(false), a_resultCode(-2)/*, a_output("")*/, a_errorMsg(""), a_childPid(-1) { a_type = Type::Cmd; }
+    ~TestStepCmd() { do_reset(); }
 
     // setter & getters
-    void setThreadRunning(bool running) throw() { a_threadRunning = running; }
+    void setThreadRunning() throw() { a_threadRunning = true; }
+    void setThreadNotRunning() throw() { a_threadRunning = false; }
+    bool threadRunning() const throw() { return a_threadRunning; }
+    bool threadNotRunning() const throw() { return !a_threadRunning; }
 
     void setResultCode(int rc) throw() { a_resultCode = rc; }
     int getResultCode() const throw() { return a_resultCode; }
@@ -266,6 +273,7 @@ class TestStepIpLimit : public TestStep {
 
   public:
     TestStepIpLimit(TestCase *testCase) : TestStep(testCase), a_ipLimit(1) { a_type = Type::IpLimit; }
+    ~TestStepIpLimit() { do_reset(); }
 
     // setter & getters
     void setIpLimit(unsigned int limit) throw() { a_ipLimit = limit; }
index fd4ed42..8c38d06 100644 (file)
@@ -85,11 +85,13 @@ TestCase::TestCase(unsigned int id, const std::string &description) :
 }
 
 TestCase::~TestCase() {
+  // TestCase should not be deleted until is safeToClear()
   reset(true); // hard reset
   std::vector<TestStep*>::const_iterator it;
   for (it = a_steps.begin(); it != a_steps.end(); it++) delete (*it);
 }
 
+
 const char* TestCase::asText(const State::_v state)
 throw() {
   static const char* text [] = { "Initialized", "InProgress", "Failed", "Success" };
@@ -289,6 +291,21 @@ bool TestCase::reset(bool hard) throw() {
   return true;
 }
 
+bool TestCase::safeToClear() {
+
+  // Check if any step is running (command steps):
+  std::vector<TestStep*>::const_iterator it;
+  for (it = a_steps.begin(); it != a_steps.end(); it++) {
+    if ((*it)->getType() == TestStep::Type::Cmd) {
+      const TestStepCmd *tscmd = static_cast<const TestStepCmd*>(*it);
+      if(tscmd->threadRunning())
+        return false;
+    }
+  }
+
+  return true;
+}
+
 void TestCase::assertInitialized() const throw(anna::RuntimeException) {
   if (isFinished())
     throw anna::RuntimeException(anna::functions::asString("Cannot program anymore. The test case %llu (%s) has finished. You must reset it to append new steps (or do it during execution, which is also allowed).", a_id, a_description.c_str()), ANNA_FILE_LOCATION);
index a74ab6d..280a198 100644 (file)
@@ -295,10 +295,38 @@ TestCase *TestManager::getTestCase(unsigned int id, const std::string &descripti
   return result;
 }
 
-bool TestManager::clearPool() throw() {
-  if (!tests()) return false;
-  for (test_pool_it it = a_testPool.begin(); it != a_testPool.end(); it++) delete it->second;
-  // TODO: stop the possible command threads or there will be a core dump
+bool TestManager::clearPool(std::string &result) throw() {
+
+  result = "";
+
+  if (!tests()) {
+    result = "there are not programmed test cases to be removed";
+    return false;
+  }
+
+  int total = a_testPool.size();
+  int unsafe = 0;
+
+  test_pool_it it;
+  for (it = a_testPool.begin(); it != a_testPool.end(); it++) {
+    if (!it->second->safeToClear()) { // Check that non pending threads are running (command steps):
+      unsafe++;
+    }
+  }
+
+  if (unsafe > 0) {
+    result = "some test cases cannot be removed (";
+    result += anna::functions::asString(unsafe);
+    result += "/";
+    result += anna::functions::asString(total);
+    result += "), mainly those having running-thread steps. Check for stuck external procedures or try later.";
+    return false;
+  }
+
+  // Here is safe to clear:
+  for (it = a_testPool.begin(); it != a_testPool.end(); it++) {
+    delete it->second;
+  }
 
   a_testPool.clear();
   a_key1TestCaseMap.clear();
@@ -307,6 +335,7 @@ bool TestManager::clearPool() throw() {
   a_poolCycle = 1;
   configureTTPS(0); // stop
   a_statSummary.clear();
+
   return true;
 }
 
@@ -537,7 +566,7 @@ throw() {
   if (a_clock) {
     result->createAttribute("AsynchronousSendings", a_synchronousAmount);
     int ticksPerSecond = (a_synchronousAmount * 1000) / a_clock->getTimeout();
-    result->createAttribute("TicksPerSecond", ticksPerSecond);
+    result->createAttribute("TicksPerSecond", a_clock->isActive() ? ticksPerSecond : 0);
   }
   if (a_currentTestIt != a_testPool.end()) {
     result->createAttribute("CurrentTestCaseId", (*a_currentTestIt).first);
index d975ddd..5355024 100644 (file)
@@ -56,8 +56,8 @@ namespace {
 
   void cmdRunOnThread (TestStepCmd *step, const std::string &cmd) {
 
-    // Thread running:
-    step->setThreadRunning(true);
+    // Thread running (will be set to false at complete()):
+    step->setThreadRunning();
 
     int status = -2;
 
@@ -757,7 +757,7 @@ void TestStepCmd::do_complete() throw() {
     std::cout << "Executed Command Test Step (" << a_script << ") [rc=" << a_resultCode << "]" << std::endl;
   }
 
-  a_threadRunning = false;
+  setThreadNotRunning();
   if (a_threadDeprecated) {
     a_threadDeprecated = false;
     do_reset();