From 010464069ff346ccbd79f60241cfce351b0af1ec Mon Sep 17 00:00:00 2001
From: Meixner <andre.meixner@kit.edu>
Date: Mon, 12 Aug 2024 12:30:09 +0200
Subject: [PATCH] Improved locking of subskillsMutex during callSubskillAsync
to avoid possible deadlock during abortion of skills
---
.../RobotAPI/libraries/skills/core/Skill.cpp | 22 +++++++++++++++++--
1 file changed, 20 insertions(+), 2 deletions(-)
diff --git a/source/RobotAPI/libraries/skills/core/Skill.cpp b/source/RobotAPI/libraries/skills/core/Skill.cpp
index 7075ae86d..ba6fca085 100644
--- a/source/RobotAPI/libraries/skills/core/Skill.cpp
+++ b/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());
--
GitLab