From 0a22a276cfff2155cbaf4939d75c257dfdb46932 Mon Sep 17 00:00:00 2001 From: Stefan Radomski Date: Tue, 26 Aug 2014 17:11:47 +0200 Subject: Identify InterpreterIssues at runtime --- src/uscxml/Interpreter.cpp | 14 +++++++++ src/uscxml/Interpreter.h | 5 +++- src/uscxml/debug/InterpreterIssue.cpp | 35 +++++++++++++++++------ src/uscxml/interpreter/InterpreterRC.cpp | 5 ++-- src/uscxml/transform/FlatStateIdentifier.h | 46 ++++++++++++++++++++++-------- test/src/test-flat-stateid.cpp | 4 +-- test/src/test-issue-reporting.cpp | 32 +++++++++++++++++++++ 7 files changed, 115 insertions(+), 26 deletions(-) diff --git a/src/uscxml/Interpreter.cpp b/src/uscxml/Interpreter.cpp index b87efed..aa4475c 100644 --- a/src/uscxml/Interpreter.cpp +++ b/src/uscxml/Interpreter.cpp @@ -768,6 +768,17 @@ InterpreterState InterpreterImpl::step(int waitForMS) { microstep(enabledTransitions); setInterpreterState(USCXML_MICROSTEPPED); + + // check whether we run in cycles + FlatStateIdentifier flat(_configuration, _alreadyEntered, _historyValue); + if (_microstepConfigurations.find(flat.getStateId()) != _microstepConfigurations.end()) { + USCXML_MONITOR_CALLBACK2(reportIssue, + InterpreterIssue("Reentering during microstep " + flat.getFlatActive() + " - possible endless loop", + Arabica::DOM::Node(), + InterpreterIssue::USCXML_ISSUE_WARNING)); + } + _microstepConfigurations.insert(flat.getStateId()); + return _state; } _stable = true; @@ -800,6 +811,7 @@ InterpreterState InterpreterImpl::step(int waitForMS) { } else { _stable = true; + _microstepConfigurations.clear(); } if (_state != USCXML_MACROSTEPPED && _state != USCXML_IDLE) @@ -953,6 +965,8 @@ void InterpreterImpl::stabilize() { assert(initialTransitions.size() > 0); enterStates(initialTransitions); } + + std::set configurationsSeen; do { // process microsteps for enabled transitions until there are no more left diff --git a/src/uscxml/Interpreter.h b/src/uscxml/Interpreter.h index 1f62255..7732bfb 100644 --- a/src/uscxml/Interpreter.h +++ b/src/uscxml/Interpreter.h @@ -517,7 +517,8 @@ protected: InterpreterHTTPServlet* _httpServlet; InterpreterWebSocketServlet* _wsServlet; std::set _monitors; - + std::set _microstepConfigurations; + long _lastRunOnMainThread; std::string _name; std::string _sessionId; @@ -836,6 +837,8 @@ public: virtual void beforeCompletion(Interpreter interpreter) {} virtual void afterCompletion(Interpreter interpreter) {} + virtual void reportIssue(Interpreter interpreter, const InterpreterIssue& issue) {} + }; } diff --git a/src/uscxml/debug/InterpreterIssue.cpp b/src/uscxml/debug/InterpreterIssue.cpp index 60f8f1b..fedb40f 100644 --- a/src/uscxml/debug/InterpreterIssue.cpp +++ b/src/uscxml/debug/InterpreterIssue.cpp @@ -407,7 +407,19 @@ std::list InterpreterIssue::forInterpreter(InterpreterImpl* in for (int i = 0; i < invokes.size(); i++) { Element invoke = Element(invokes[i]); if (HAS_ATTR(invoke, "type") && !_factory->hasInvoker(ATTR(invoke, "type"))) { - issues.push_back(InterpreterIssue("Invoke with unknown type '" + ATTR(invoke, "type") + "'", invoke, InterpreterIssue::USCXML_ISSUE_FATAL)); + // unknown at factory - adhoc extension? + if (HAS_ATTR(invoke, "id") && interpreter->_invokers.find(ATTR(invoke, "id")) != interpreter->_invokers.end()) + continue; // not an issue + + IssueSeverity severity; + if (HAS_ATTR(invoke, "idlocation")) { + // we might still resolve at runtime + severity = InterpreterIssue::USCXML_ISSUE_WARNING; + } else { + // fatality! + severity = InterpreterIssue::USCXML_ISSUE_FATAL; + } + issues.push_back(InterpreterIssue("Invoke with unknown type '" + ATTR(invoke, "type") + "'", invoke, severity)); continue; } } @@ -418,6 +430,9 @@ std::list InterpreterIssue::forInterpreter(InterpreterImpl* in for (int i = 0; i < sends.size(); i++) { Element send = Element(sends[i]); if (HAS_ATTR(send, "type") && !_factory->hasIOProcessor(ATTR(send, "type"))) { + if (interpreter->_ioProcessors.find(ATTR(send, "type")) != interpreter->_ioProcessors.end()) + continue; // not an issue, available ad-hoc + issues.push_back(InterpreterIssue("Send to unknown IO Processor '" + ATTR(send, "type") + "'", send, InterpreterIssue::USCXML_ISSUE_FATAL)); continue; } @@ -470,16 +485,18 @@ std::list InterpreterIssue::forInterpreter(InterpreterImpl* in } - // check that the datamodel is known - if (HAS_ATTR(_scxml, "datamodel")) { - if (!_factory->hasDataModel(ATTR(_scxml, "datamodel"))) { - issues.push_back(InterpreterIssue("SCXML document requires unknown datamodel '" + ATTR(_scxml, "datamodel") + "'", _scxml, InterpreterIssue::USCXML_ISSUE_FATAL)); - - // we cannot even check the rest as we require a datamodel - return issues; + // check that the datamodel is known if not already instantiated + if (!interpreter->_dataModel) { + if (HAS_ATTR(_scxml, "datamodel")) { + if (!_factory->hasDataModel(ATTR(_scxml, "datamodel"))) { + issues.push_back(InterpreterIssue("SCXML document requires unknown datamodel '" + ATTR(_scxml, "datamodel") + "'", _scxml, InterpreterIssue::USCXML_ISSUE_FATAL)); + + // we cannot even check the rest as we require a datamodel + return issues; + } } } - + bool instantiatedDataModel = false; // instantiate datamodel if not explicitly set if (!_dataModel) { diff --git a/src/uscxml/interpreter/InterpreterRC.cpp b/src/uscxml/interpreter/InterpreterRC.cpp index d91eadb..4d2b4f6 100644 --- a/src/uscxml/interpreter/InterpreterRC.cpp +++ b/src/uscxml/interpreter/InterpreterRC.cpp @@ -33,8 +33,8 @@ namespace uscxml { using namespace Arabica::XPath; using namespace Arabica::DOM; +#ifdef VERBOSE size_t padding = 0; - std::string getPadding() { std::string pad = ""; for (int i = 0; i < padding; i++) { @@ -42,7 +42,8 @@ std::string getPadding() { } return pad; } - +#endif + Arabica::XPath::NodeSet InterpreterRC::removeConflictingTransitions(const Arabica::XPath::NodeSet& enabledTransitions) { Arabica::XPath::NodeSet filteredTransitions; diff --git a/src/uscxml/transform/FlatStateIdentifier.h b/src/uscxml/transform/FlatStateIdentifier.h index 08451c2..0957e34 100644 --- a/src/uscxml/transform/FlatStateIdentifier.h +++ b/src/uscxml/transform/FlatStateIdentifier.h @@ -135,57 +135,79 @@ public: const std::list& getActive() { return active; } + const std::string& getFlatActive() { + return flatActive; + } const std::list& getVisited() { return visited; } + const std::string& getFlatVisited() { + return flatVisited; + } const std::map > & getHistory() { return histories; } + const std::string& getFlatHistory() { + return flatHistories; + } protected: std::list active; std::list visited; std::map > histories; + + std::string flatActive; + std::string flatVisited; + std::string flatHistories; + std::string stateId; void initStateId() { std::stringstream stateIdSS; - std::string seperator; - stateIdSS << "active:{"; + std::stringstream flatActiveSS; + flatActiveSS << "active:{"; for (std::list::const_iterator actIter = active.begin(); actIter != active.end(); actIter++) { - stateIdSS << seperator << *actIter; + flatActiveSS << seperator << *actIter; seperator = ","; } - stateIdSS << "}"; + flatActiveSS << "}"; + flatActive = flatActiveSS.str(); + stateIdSS << flatActive; if (visited.size() > 0) { + std::stringstream flatVisitedSS; seperator = ""; - stateIdSS << ";visited:{"; + flatVisitedSS << "visited:{"; for (std::list::const_iterator visitIter = visited.begin(); visitIter != visited.end(); visitIter++) { - stateIdSS << seperator << *visitIter; + flatVisitedSS << seperator << *visitIter; seperator = ","; } - stateIdSS << "}"; + flatVisitedSS << "}"; + flatVisited = flatVisitedSS.str(); + stateIdSS << ";" << flatVisited; } if (histories.size() > 0) { + std::stringstream flatHistorySS; seperator = ""; - stateIdSS << ";history:{"; + flatHistorySS << "history:{"; for (std::map >::const_iterator histIter = histories.begin(); histIter != histories.end(); histIter++) { - stateIdSS << seperator << histIter->first << ":{"; + flatHistorySS << seperator << histIter->first << ":{"; seperator = ","; std::string itemSeperator; for (std::list::const_iterator histItemIter = histIter->second.begin(); histItemIter != histIter->second.end(); histItemIter++) { - stateIdSS << itemSeperator << *histItemIter; + flatHistorySS << itemSeperator << *histItemIter; itemSeperator = ","; } - stateIdSS << "}"; + flatHistorySS << "}"; } - stateIdSS << "}"; + flatHistorySS << "}"; + flatHistories = flatHistorySS.str(); + stateIdSS << ";" << flatHistories; } stateId = stateIdSS.str(); diff --git a/test/src/test-flat-stateid.cpp b/test/src/test-flat-stateid.cpp index 6eb1ed8..719e4ee 100644 --- a/test/src/test-flat-stateid.cpp +++ b/test/src/test-flat-stateid.cpp @@ -16,7 +16,7 @@ int main(int argc, char** argv) { } { - std::string stateId = "active:{s1};entered:{s1,s2}"; + std::string stateId = "active:{s1};visited:{s1,s2}"; uscxml::FlatStateIdentifier flat1(stateId); assert(flat1.getActive().size() == 1); assert(flat1.getVisited().size() == 2); @@ -28,7 +28,7 @@ int main(int argc, char** argv) { { - std::string stateId = "active:{s0,s1,s2};entered:{s0,s1,s2};history:{h0:{s1,s2},h1:{s2,s3}}"; + std::string stateId = "active:{s0,s1,s2};visited:{s0,s1,s2};history:{h0:{s1,s2},h1:{s2,s3}}"; uscxml::FlatStateIdentifier flat1(stateId); listIter = flat1.getActive().begin(); diff --git a/test/src/test-issue-reporting.cpp b/test/src/test-issue-reporting.cpp index 3cdb141..cd1687c 100644 --- a/test/src/test-issue-reporting.cpp +++ b/test/src/test-issue-reporting.cpp @@ -17,6 +17,15 @@ std::set issueLocationsForXML(const std::string xml) { return issueLocations; } +size_t runtimeIssues; +class IssueMonitor : public InterpreterMonitor { +public: + IssueMonitor() { runtimeIssues = 0; } + void reportIssue(Interpreter interpreter, const InterpreterIssue& issue) { + runtimeIssues++; + } +}; + int main(int argc, char** argv) { google::InitGoogleLogging(argv[0]); @@ -27,6 +36,29 @@ int main(int argc, char** argv) { while(iterations--) { if (1) { + // Potential endless loop + + const char* xml = + "" + " " + " " + " " + " 0\" />" + " " + " " + " " + ""; + + IssueMonitor monitor; + Interpreter interpreter = Interpreter::fromXML(xml); + interpreter.addMonitor(&monitor); + interpreter.interpret(); + + // first reiteration is not counted as it might be valid when raising internal errors + assert(runtimeIssues == 3); + } + + if (1) { // Unreachable states const char* xml = -- cgit v0.12