using std::scoped_lock instead of shared and unique locks

source/RobotAPI/components/RobotHealth/RobotHealth.cpp

@@ -35,6 +35,7 @@
 
 #include <ArmarXCore/core/time/ice_conversions.h>
 #include <ArmarXCore/util/CPPUtility/trace.h>
+#include <mutex>
 #include <optional>
 
 namespace armarx
@@ -71,17 +72,15 @@ namespace armarx
     void
     RobotHealth::monitorHealthUpdateTaskClb()
     {
-        std::shared_lock lock(updateMutex);
+        const std::scoped_lock lock(updateMutex);
 
         ARMARX_TRACE;
         auto now = armarx::core::time::DateTime::Now();
 
-        for (size_t i = 0; i < updateEntries.size(); i++)
+        for (auto & e : updateEntries)
         {
             ARMARX_TRACE;
-            UpdateEntry& e = updateEntries.at(i);
-
-            std::unique_lock elock(e.mutex);
+            const std::scoped_lock elock(e.mutex);
 
             if (!e.enabled)
             {
@@ -129,12 +128,10 @@ namespace armarx
 
 
         // get aggregated status
-        for (size_t i = 0; i < updateEntries.size(); i++)
+        for (auto & e : updateEntries)
         {
             ARMARX_TRACE;
-            UpdateEntry& e = updateEntries.at(i);
-
-            std::shared_lock elock(e.mutex);
+             std::scoped_lock elock(e.mutex);
 
             // trim history
             while (e.history.size() > 20)
@@ -218,11 +215,11 @@ namespace armarx
     {
         ARMARX_TRACE;
 
-        for (size_t i = 0; i < updateEntries.size(); i++)
+        for (auto & updateEntrie : updateEntries)
         {
-            if (updateEntries.at(i).name == name)
+            if (updateEntrie.name == name)
             {
-                return &updateEntries.at(i);
+                return &updateEntrie;
             }
         }
 
@@ -234,11 +231,11 @@ namespace armarx
     {
         ARMARX_TRACE;
         {
-            for (size_t i = 0; i < updateEntries.size(); i++)
+            for (auto & updateEntrie : updateEntries)
             {
-                if (updateEntries.at(i).name == name)
+                if (updateEntrie.name == name)
                 {
-                    return {false, updateEntries.at(i)};
+                    return {false, updateEntrie};
                 }
             }
         }
@@ -254,12 +251,12 @@ namespace armarx
     RobotHealth::signUp(const RobotHealthHeartbeatArgs& args, const Ice::Current& current)
     {
         ARMARX_TRACE;
-        std::unique_lock lockU(updateMutex);
+        std::scoped_lock lockU(updateMutex);
 
         auto e = findOrCreateUpdateEntry(args.identifier);
 
         {
-            std::unique_lock lock(e.second.mutex);
+            std::scoped_lock lock(e.second.mutex);
 
             if (e.first)
             {
@@ -294,7 +291,7 @@ namespace armarx
 
         // We hold a reference to 'o' which is an element in a vector.
         // If we don't lock until the end of this scope, the vector size might change and 'o' will be invalidated. 
-        std::shared_lock lockU(updateMutex);  
+        std::scoped_lock lockU(updateMutex);  
         auto* const entry = findUpdateEntry(identifier);
 
         if (entry == nullptr)
@@ -306,7 +303,7 @@ namespace armarx
 
         ARMARX_CHECK_NOT_NULL(entry);
 
-        std::unique_lock lock(entry->mutex);
+        std::scoped_lock lock(entry->mutex);
 
         if (not entry->enabled)
         {
@@ -325,21 +322,18 @@ namespace armarx
     }
 
     void
-    armarx::RobotHealth::unregister(const std::string& componentName, const Ice::Current&)
+    armarx::RobotHealth::unregister(const std::string& identifier, const Ice::Current&)
     {
         ARMARX_TRACE;
-        std::shared_lock lock(updateMutex);
+        std::scoped_lock lock(updateMutex);
 
         bool found = false;
         {
-            std::shared_lock lock(updateMutex);
-
-            for (size_t i = 0; i < updateEntries.size(); i++)
+            for (auto & e : updateEntries)
             {
-                auto& e = updateEntries.at(i);
-                if (e.name == componentName)
+                if (e.name == identifier)
                 {
-                    std::unique_lock elock(e.mutex);
+                    std::scoped_lock elock(e.mutex);
                     e.required = false;
                     e.enabled = false;
                     found = true;
@@ -351,11 +345,11 @@ namespace armarx
 
         if (found)
         {
-            ARMARX_INFO << "Component " << componentName << " disabled from heartbeat";
+            ARMARX_INFO << "Component " << identifier << " disabled from heartbeat";
         }
         else
         {
-            ARMARX_INFO << "Could not unregister component " << componentName << ". Not found.";
+            ARMARX_INFO << "Could not unregister component " << identifier << ". Not found.";
         }
 
         updateRequiredElements();
@@ -366,7 +360,7 @@ namespace armarx
                      const std::vector<std::string>& tags,
                      const Ice::Current& current)
     {
-        std::shared_lock lock(updateMutex);
+        std::scoped_lock lock(updateMutex);
 
         // add newly requested tags
         for(const auto& tag : tags)
@@ -429,7 +423,7 @@ namespace armarx
                        const std::vector<std::string>& tags,
                        const Ice::Current& current)
     {
-        std::shared_lock lock(updateMutex);
+        std::scoped_lock lock(updateMutex);
 
         // update the required tags list / map
         for(const auto& tag : tags)
@@ -443,7 +437,7 @@ namespace armarx
     void
     armarx::RobotHealth::resetRequiredTags(const Ice::Current& current)
     {
-        std::shared_lock lock(updateMutex);
+        std::scoped_lock lock(updateMutex);
 
         // treat the special provider (see onInitComponent()) appropriately  
         ARMARX_CHECK(tagsPerRequester.count(PROPERTY_REQUESTER_ID) > 0);
@@ -457,7 +451,7 @@ namespace armarx
     RobotHealthInfo
     RobotHealth::getSummary(const Ice::Current&)
     {
-        std::shared_lock lock(updateMutex);
+        std::scoped_lock lock(updateMutex);
 
         ARMARX_TRACE;
         RobotHealthInfo ret;

source/RobotAPI/components/RobotHealth/RobotHealth.h

@@ -70,6 +70,29 @@ namespace armarx
             return "RobotHealth";
         }
 
+        // RobotHealthInterface interface
+        void signUp(const RobotHealthHeartbeatArgs& args, const Ice::Current& current) override;
+        void unregister(const std::string& identifier, const Ice::Current&) override;
+        
+
+        void addRequiredTags(const std::string& requesterIdentifier,
+                     const std::vector<std::string>& tags,
+                     const Ice::Current& current) override;
+        void removeRequiredTags(const std::string& requesterIdentifier,
+                       const std::vector<std::string>& tags,
+                       const Ice::Current& current) override;
+        void resetRequiredTags(const Ice::Current& current) override;
+
+
+        void heartbeat(const std::string& identifier,
+                       const core::time::dto::DateTime& referenceTime,
+                       const Ice::Current& current) override;
+
+        std::string getTopicName(const Ice::Current& current) override;
+
+        RobotHealthInfo getSummary(const Ice::Current& current) override;
+
+
     protected:
         /**
          * @see armarx::ManagedIceObject::onInitComponent()
@@ -116,7 +139,7 @@ namespace armarx
             bool required = false;
             bool enabled = false;
 
-            mutable std::shared_mutex mutex;
+            mutable std::mutex mutex;
 
             struct TimeInfo
             {
@@ -140,36 +163,13 @@ namespace armarx
 
         void reportDebugObserver();
 
-        // RobotHealthInterface interface
-    public:
-        void signUp(const RobotHealthHeartbeatArgs& args, const Ice::Current& current) override;
-        void unregister(const std::string& identifier, const Ice::Current&) override;
-        
-
-        void addRequiredTags(const std::string& requesterIdentifier,
-                     const std::vector<std::string>& tags,
-                     const Ice::Current& current) override;
-        void removeRequiredTags(const std::string& requesterIdentifier,
-                       const std::vector<std::string>& tags,
-                       const Ice::Current& current) override;
-        void resetRequiredTags(const Ice::Current& current) override;
-
-
-        void heartbeat(const std::string& identifier,
-                       const core::time::dto::DateTime& referenceTime,
-                       const Ice::Current& current) override;
-
-        std::string getTopicName(const Ice::Current& current) override;
-
-        // RobotHealthComponentInterface interface
-    public:
-        RobotHealthInfo getSummary(const Ice::Current& current) override;
-
-    private:
+   
         void updateRequiredElements();
         std::set<std::string> requestedTags() const;
 
-        mutable std::shared_mutex updateMutex;
+        // Mutex to restrict access to all interface / public methods. This ensures that all requests are
+        // handled sequentially. 
+        mutable std::mutex updateMutex;
 
         std::deque<UpdateEntry> updateEntries;