From 7628893d2c3cd5afe173b34db15b0d1c3217c3de Mon Sep 17 00:00:00 2001
From: Rainer Kartmann <rainer.kartmann@kit.edu>
Date: Wed, 17 May 2023 11:59:02 +0200
Subject: [PATCH] Fix deadlock in skill provider when abortSkill() calls itself
indirectly
---
.../manager/SkillManagerComponentPlugin.cpp | 22 ++++++++++++++++---
1 file changed, 19 insertions(+), 3 deletions(-)
diff --git a/source/RobotAPI/libraries/skills/manager/SkillManagerComponentPlugin.cpp b/source/RobotAPI/libraries/skills/manager/SkillManagerComponentPlugin.cpp
index 38ca6ea50..455215311 100644
--- a/source/RobotAPI/libraries/skills/manager/SkillManagerComponentPlugin.cpp
+++ b/source/RobotAPI/libraries/skills/manager/SkillManagerComponentPlugin.cpp
@@ -82,6 +82,9 @@ namespace armarx
return "INVALID PROVIDER NAME";
}
+ using SkillProviderInterfacePrxMap =
+ std::map<std::string, skills::provider::dti::SkillProviderInterfacePrx>;
+
skills::provider::dto::SkillStatusUpdate
SkillManagerComponentPluginUser::executeSkill(
const skills::manager::dto::SkillExecutionRequest& info,
@@ -97,6 +100,12 @@ namespace armarx
providerName = info.skillId.providerName;
}
+ SkillProviderInterfacePrxMap skillProviderMap;
+ {
+ std::scoped_lock l(skillProviderMapMutex);
+ skillProviderMap = this->skillProviderMap;
+ }
+
bool remove = false;
if (auto it = skillProviderMap.find(providerName); it != skillProviderMap.end())
{
@@ -124,6 +133,8 @@ namespace armarx
if (remove)
{
std::scoped_lock l(skillProviderMapMutex);
+ // No copy!
+ SkillProviderInterfacePrxMap& skillProviderMap = this->skillProviderMap;
if (auto it = skillProviderMap.find(providerName); it != skillProviderMap.end())
{
ARMARX_WARNING << __PRETTY_FUNCTION__ << ": Found disconnected skill provider '"
@@ -149,7 +160,12 @@ namespace armarx
const std::string& skillName,
const Ice::Current& current)
{
- std::scoped_lock l(skillProviderMapMutex);
+ SkillProviderInterfacePrxMap skillProviderMap;
+ {
+ std::scoped_lock l(skillProviderMapMutex);
+ skillProviderMap = this->skillProviderMap;
+ }
+
if (auto it = skillProviderMap.find(providerName); it != skillProviderMap.end())
{
const auto& n = it->first;
@@ -160,8 +176,8 @@ namespace armarx
}
else
{
- ARMARX_WARNING << __PRETTY_FUNCTION__ << ": Found disconnected skill provider '" << n << "'. Removing it from skills.";
- it = skillProviderMap.erase(it);
+ ARMARX_WARNING << __PRETTY_FUNCTION__ << ": Found disconnected skill provider '"
+ << n << "'. Removing it from skills on next execute.";
}
}
}
--
GitLab