Improved locking of subskillsMutex during callSubskillAsync to avoid possible...

Improved locking of subskillsMutex during callSubskillAsync to avoid possible deadlock during abortion of skills

source/RobotAPI/libraries/skills/core/Skill.cpp

@@ -48,10 +48,11 @@ namespace armarx
         skills::SkillExecutionID
         Skill::callSubskillAsync(const skills::SkillProxy& prx, const aron::data::DictPtr& params)
         {
-            std::unique_lock l(subskillsMutex);
-
             std::string executorHistory = this->executorName + "->" + getSkillId().toString();
             auto eid = prx.executeSkillAsync(executorHistory, params);
+
+            std::unique_lock l(subskillsMutex);
+            throwIfSkillShouldTerminate([&](){prx.abortSkillAsync(eid);}); // also notify newly added skill as it was not added to subskills list yet
             this->subskills.push_back(eid);
             return eid;
         }
@@ -338,6 +339,11 @@ namespace armarx
         void
         Skill::notifySkillToStop()
         {
+            if (stopped)
+            {
+                // skill already got stopped. Ignore
+                return;
+            }
             std::scoped_lock l(subskillsMutex);
             stopped = true;
             _onStopRequested();
@@ -347,6 +353,12 @@ namespace armarx
         void
         Skill::notifyTimeoutReached()
         {
+            if (stopped || timeoutReached)
+            {
+                // skill already got timeoutReached. Ignore
+                return;
+            }
+
             std::scoped_lock l(subskillsMutex);
             timeoutReached = true;
             _onTimeoutReached();
@@ -363,10 +375,13 @@ namespace armarx
         void
         Skill::_onTimeoutReached()
         {
+            // WE ASSUME THAT THE LOCK IS ALREADY TAKEN
+
             if (!manager)
             {
                 return;
             }
+
             for (const auto& execId : subskills)
             {
                 manager->abortSkillAsync(execId.toManagerIce());
@@ -376,10 +391,13 @@ namespace armarx
         void
         Skill::_onStopRequested()
         {
+            // WE ASSUME THAT THE LOCK IS ALREADY TAKEN
+
             if (!manager)
             {
                 return;
             }
+
             for (const auto& execId : subskills)
             {
                 manager->abortSkillAsync(execId.toManagerIce());