Improvement of Skill Memory and Skill Memory GUI

Together with @peller, we improved the skill memory and skill memory GUI. This MR ...

1. Adds various skills for testing, especially RandomChaining for extensive stress testing (when started multiple times)
2. Fixes various segmentation faults and deadlocks for the skill memory.
3. Adds 5 or 10 second timeouts for ice calls in the ARMARX skill memory gui, so it doesn't freeze.

Known open issue:

1. Skill memory still deadlocks in certain corner case during skill abortion. But this probably only happens very rarely. Reproduce by requesting execution of skill "InstantKill" and directly calling "Stop all executions".

source/RobotAPI/libraries/skills/manager/SkillManagerComponentPlugin.cpp

@@ -124,7 +124,7 @@ namespace armarx::plugins
 
                 auto async =
                     provider->begin_executeSkill(provider_executionRequest.toProviderIce());
-                l.unlock(); // allow parallel e.g. stopping
+                l.unlock(); // allow parallel e.g. stopping. Otherwise the manager would lock himself in nested calls
                 auto provider_statusUpdate_ice = provider->end_executeSkill(async);
 
                 // convert to manager view
@@ -132,17 +132,11 @@ namespace armarx::plugins
                     skills::SkillStatusUpdate::FromIce(provider_statusUpdate_ice, provderId);
                 return statusUpdate;
             }
-            catch (...)
+            catch (const std::exception &e)
             {
-                ARMARX_WARNING << __PRETTY_FUNCTION__
-                               << ": Found disconnected or buggy skill provider '" << provderId
-                               << "' during execution. Removing it from skills.";
-                skillProviderMap.erase(it);
+                l.lock();
 
-                throw skills::error::SkillException(
-                    __PRETTY_FUNCTION__,
-                    "Skill execution failed. Could not execute a skill of provider '" +
-                        provderId.toString() + "' because the provider does not exist.");
+                handleExceptionNonLockingThrow(__PRETTY_FUNCTION__, e, provderId);
             }
         }
         else
@@ -198,7 +192,7 @@ namespace armarx::plugins
 
                 auto async =
                     provider->begin_executeSkillAsync(provider_executionRequest.toProviderIce());
-                l.unlock(); // allow parallel e.g. stopping
+                l.unlock(); // allow parallel e.g. stopping. Otherwise the manager would lock himself in nested calls
                 auto provider_executionID_ice = provider->end_executeSkillAsync(async);
 
                 // convert to manager view
@@ -211,26 +205,7 @@ namespace armarx::plugins
             {
                 l.lock();
 
-                if (it = skillProviderMap.find(provderId); it != skillProviderMap.end())
-                {
-                    ARMARX_WARNING << __PRETTY_FUNCTION__
-                                << ": Found disconnected or buggy skill provider '" << provderId
-                                << "' during execution. Removing it from skills. "
-                                << "Error: " << e.what();
-                    skillProviderMap.erase(it);
-                }
-                else
-                {
-                    ARMARX_WARNING << __PRETTY_FUNCTION__
-                                << ": Found disconnected or buggy skill provider '" << provderId
-                                << "' during execution. However, it already got removed... "
-                                << "Error: " << e.what();
-                }
-
-                throw skills::error::SkillException(
-                    __PRETTY_FUNCTION__,
-                    "Skill execution failed. Could not execute a skill of provider '" +
-                        provderId.toString() + "' because the provider does not exist.");
+                handleExceptionNonLockingThrow(__PRETTY_FUNCTION__, e, provderId);
             }
         }
         else
@@ -270,7 +245,7 @@ namespace armarx::plugins
             {
                 auto async = provider->begin_updateSkillParameters(executionId.toProviderIce(),
                                                                    data->toAronDictDTO());
-                l.unlock(); // allow parallel e.g. stopping
+                l.unlock(); // allow parallel e.g. stopping. Otherwise the manager would lock himself in nested calls
                 auto r = provider->end_updateSkillParameters(async);
                 return r.success;
             }
@@ -278,21 +253,7 @@ namespace armarx::plugins
             {
                 l.lock();
 
-                if (it = skillProviderMap.find(providerId); it != skillProviderMap.end())
-                {
-                    ARMARX_WARNING << __PRETTY_FUNCTION__
-                                << ": Found disconnected or buggy skill provider '" << providerId
-                                << "' during execution. Removing it from skills. "
-                                << "Error: " << e.what();
-                    skillProviderMap.erase(it);
-                }
-                else
-                {
-                    ARMARX_WARNING << __PRETTY_FUNCTION__
-                                << ": Found disconnected or buggy skill provider '" << providerId
-                                << "' during execution. However, it already got removed... "
-                                << "Error: " << e.what();
-                }
+                handleExceptionNonLocking(__PRETTY_FUNCTION__, e, providerId);
 
                 return false;
             }
@@ -329,7 +290,7 @@ namespace armarx::plugins
             try
             {
                 auto async = provider->begin_abortSkill(executionId.toProviderIce());
-                l.unlock(); // allow parallel e.g. stopping
+                l.unlock(); // allow parallel e.g. stopping. Otherwise the manager would lock himself in nested calls
                 auto r = provider->end_abortSkill(async);
                 return r.success;
             }
@@ -337,21 +298,7 @@ namespace armarx::plugins
             {
                 l.lock();
 
-                if (it = skillProviderMap.find(providerId); it != skillProviderMap.end())
-                {
-                    ARMARX_WARNING << __PRETTY_FUNCTION__
-                                << ": Found disconnected or buggy skill provider '" << providerId
-                                << "' during execution. Removing it from skills. "
-                                << "Error: " << e.what();
-                    skillProviderMap.erase(it);
-                }
-                else
-                {
-                    ARMARX_WARNING << __PRETTY_FUNCTION__
-                                << ": Found disconnected or buggy skill provider '" << providerId
-                                << "' during execution. However, it already got removed... "
-                                << "Error: " << e.what();
-                }
+                handleExceptionNonLocking(__PRETTY_FUNCTION__, e, providerId);
 
                 return false;
             }
@@ -390,21 +337,7 @@ namespace armarx::plugins
             }
             catch (const std::exception &e)
             {
-                if (it = skillProviderMap.find(providerId); it != skillProviderMap.end())
-                {
-                    ARMARX_WARNING << __PRETTY_FUNCTION__
-                                << ": Found disconnected or buggy skill provider '" << providerId
-                                << "' during execution. Removing it from skills. "
-                                << "Error: " << e.what();
-                    skillProviderMap.erase(it);
-                }
-                else
-                {
-                    ARMARX_WARNING << __PRETTY_FUNCTION__
-                                << ": Found disconnected or buggy skill provider '" << providerId
-                                << "' during execution. However, it already got removed... "
-                                << "Error: " << e.what();
-                }
+                handleExceptionNonLocking(__PRETTY_FUNCTION__, e, providerId);
 
                 return false;
             }
@@ -444,7 +377,7 @@ namespace armarx::plugins
             try
             {
                 auto async = provider->begin_getSkillDescription(skillId.toProviderIce());
-                l.unlock(); // allow parallel e.g. stopping
+                l.unlock(); // allow parallel e.g. stopping. Otherwise the manager would lock himself in nested calls
                 auto provider_desc_ice = provider->end_getSkillDescription(async);
 
                 if (not provider_desc_ice)
@@ -459,28 +392,7 @@ namespace armarx::plugins
             }
             catch (const std::exception &e)
             {
-                if (it = skillProviderMap.find(providerId); it != skillProviderMap.end())
-                {
-                    ARMARX_WARNING << __PRETTY_FUNCTION__
-                                << ": Found disconnected or buggy skill provider '" << providerId
-                                << "' during execution. Removing it from skills. "
-                                << "Error: " << e.what();
-                    skillProviderMap.erase(it);
-                }
-                else
-                {
-                    ARMARX_WARNING << __PRETTY_FUNCTION__
-                                << ": Found disconnected or buggy skill provider '" << providerId
-                                << "' during execution. However, it already got removed... "
-                                << "Error: " << e.what();
-                }
-
-
-                throw skills::error::SkillException(__PRETTY_FUNCTION__,
-                                                    "Skill execution failed. Could not query a "
-                                                    "status update of a skill of provider '" +
-                                                        providerId.toString() +
-                                                        "' because the provider does not exist.");
+                handleExceptionNonLockingThrow(__PRETTY_FUNCTION__, e, providerId);
             }
         }
         else
@@ -523,20 +435,9 @@ namespace armarx::plugins
             }
             catch (const std::exception &e)
             {
-                if (it = skillProviderMap.find(providerId); it != skillProviderMap.end())
-                {
-                    ARMARX_WARNING << __PRETTY_FUNCTION__
-                                << ": Found disconnected or buggy skill provider '" << providerId
-                                << "' during execution. Removing it from skills. "
-                                << "Error: " << e.what();
-                    skillProviderMap.erase(it);
-                }
-                else
+                if (auto _it = handleExceptionNonLocking(__PRETTY_FUNCTION__, e, providerId))
                 {
-                    ARMARX_WARNING << __PRETTY_FUNCTION__
-                                << ": Found disconnected or buggy skill provider '" << providerId
-                                << "' during execution. However, it already got removed... "
-                                << "Error: " << e.what();
+                    it = _it.value(); // next element
                 }
             }
         }
@@ -573,7 +474,7 @@ namespace armarx::plugins
             try
             {
                 auto async = provider->begin_getSkillExecutionStatus(executionId.toProviderIce());
-                l.unlock(); // allow parallel e.g. stopping
+                l.unlock(); // allow parallel e.g. stopping. Otherwise the manager would lock himself in nested calls
                 auto provider_statusUpdate_ice = provider->end_getSkillExecutionStatus(async);
 
                 if (not provider_statusUpdate_ice)
@@ -590,27 +491,7 @@ namespace armarx::plugins
             {
                 l.lock();
 
-                if (it = skillProviderMap.find(providerId); it != skillProviderMap.end())
-                {
-                    ARMARX_WARNING << __PRETTY_FUNCTION__
-                                << ": Found disconnected or buggy skill provider '" << providerId
-                                << "' during execution. Removing it from skills. "
-                                << "Error: " << e.what();
-                    skillProviderMap.erase(it);
-                }
-                else
-                {
-                    ARMARX_WARNING << __PRETTY_FUNCTION__
-                                << ": Found disconnected or buggy skill provider '" << providerId
-                                << "' during execution. However, it already got removed... "
-                                << "Error: " << e.what();
-                }
-
-                throw skills::error::SkillException(__PRETTY_FUNCTION__,
-                                                    "Skill execution failed. Could not query a "
-                                                    "status update of a skill of provider '" +
-                                                        providerId.toString() +
-                                                        "' because the provider does not exist.");
+                handleExceptionNonLockingThrow(__PRETTY_FUNCTION__, e, providerId);
             }
         }
         else
@@ -651,16 +532,53 @@ namespace armarx::plugins
                 }
                 it++;
             }
-            catch (...)
+            catch (const std::exception &e)
             {
-                ARMARX_WARNING << __PRETTY_FUNCTION__ << ": Found buggy skill provider '"
-                               << providerId << "'. Removing it from skills.";
-                it = skillProviderMap.erase(it);
+                if (auto _it = handleExceptionNonLocking(__PRETTY_FUNCTION__, e, providerId))
+                {
+                    it = _it.value(); // next element
+                }
             }
         }
         return ret;
     }
 
+    std::optional<std::map<skills::ProviderID, skills::provider::dti::SkillProviderInterfacePrx>::iterator>
+    SkillManagerComponentPlugin::handleExceptionNonLocking(
+        const char* funcName, const std::exception& e, skills::ProviderID providerId, bool eraseSkillProvider)
+    {
+        // NON LOCKING! WE ASSERT THAT THE CALLER HOLDS LOCK
+        if (auto it = skillProviderMap.find(providerId); eraseSkillProvider and it != skillProviderMap.end())
+        {
+            ARMARX_WARNING << funcName
+                        << ": Found disconnected or buggy skill provider '" << providerId
+                        << "' during execution. Removing it from skills. "
+                        << "Error: " << e.what();
+            return skillProviderMap.erase(it);
+        }
+        else
+        {
+            ARMARX_WARNING << funcName
+                        << ": Found disconnected or buggy skill provider '" << providerId
+                        << "' during execution. However, it already got removed... "
+                        << "Error: " << e.what();
+            return std::nullopt;
+        }
+    }
+
+    void 
+    SkillManagerComponentPlugin::handleExceptionNonLockingThrow(
+        const char* funcName, const std::exception& e, skills::ProviderID providerId, bool eraseSkillProvider)
+    {
+        // NON LOCKING! WE ASSERT THAT THE CALLER HOLDS LOCK
+        handleExceptionNonLocking(funcName, e, providerId, eraseSkillProvider);
+
+        throw skills::error::SkillException(
+            funcName,
+            "Skill execution failed. Could not execute a skill of provider '" +
+                providerId.toString() + "' because the provider does not exist.");
+    }
+
 } // namespace armarx::plugins
 
 namespace armarx