From 21054b5919e21b14627fac09e718262ae2812af4 Mon Sep 17 00:00:00 2001 From: "Eduardo Ramos Testillano (eramedu)" Date: Mon, 11 May 2020 18:53:32 +0200 Subject: [PATCH] Solve legacy problem with clear operation (coredump with running threads) 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 | 3 +++ include/anna/testing/TestManager.hpp | 2 +- include/anna/testing/TestStep.hpp | 16 +++++++++--- source/testing/TestCase.cpp | 17 ++++++++++++ source/testing/TestManager.cpp | 39 ++++++++++++++++++++++++---- source/testing/TestStep.cpp | 6 ++--- 6 files changed, 70 insertions(+), 13 deletions(-) diff --git a/include/anna/testing/TestCase.hpp b/include/anna/testing/TestCase.hpp index e87750c..3ebd535 100644 --- a/include/anna/testing/TestCase.hpp +++ b/include/anna/testing/TestCase.hpp @@ -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; } diff --git a/include/anna/testing/TestManager.hpp b/include/anna/testing/TestManager.hpp index 4f3bed2..40d5370 100644 --- a/include/anna/testing/TestManager.hpp +++ b/include/anna/testing/TestManager.hpp @@ -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; } diff --git a/include/anna/testing/TestStep.hpp b/include/anna/testing/TestStep.hpp index 4442f66..844b6be 100644 --- a/include/anna/testing/TestStep.hpp +++ b/include/anna/testing/TestStep.hpp @@ -13,6 +13,7 @@ #include #include #include +#include // Project #include @@ -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 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; } diff --git a/source/testing/TestCase.cpp b/source/testing/TestCase.cpp index fd4ed42..8c38d06 100644 --- a/source/testing/TestCase.cpp +++ b/source/testing/TestCase.cpp @@ -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::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::const_iterator it; + for (it = a_steps.begin(); it != a_steps.end(); it++) { + if ((*it)->getType() == TestStep::Type::Cmd) { + const TestStepCmd *tscmd = static_cast(*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); diff --git a/source/testing/TestManager.cpp b/source/testing/TestManager.cpp index a74ab6d..280a198 100644 --- a/source/testing/TestManager.cpp +++ b/source/testing/TestManager.cpp @@ -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); diff --git a/source/testing/TestStep.cpp b/source/testing/TestStep.cpp index d975ddd..5355024 100644 --- a/source/testing/TestStep.cpp +++ b/source/testing/TestStep.cpp @@ -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(); -- 2.20.1