implemented MemoryListener::(Scoped)SubscriptionHandle correctly

source/RobotAPI/libraries/armem/client/util/MemoryListener.cpp

@@ -15,12 +15,39 @@ namespace armarx::armem::client::util
 
 
     MemoryListener::SubscriptionHandle::SubscriptionHandle(MemoryListener* memoryListener,
-                                                           MemoryID memoryID,
-                                                           long id)
+                                                           const MemoryID& memoryID,
+                                                           long id) :
+        valid{true}, memoryListener{memoryListener}, memoryID(memoryID), id{id}
     {
-        this->memoryListener = memoryListener;
-        this->memoryID = memoryID;
-        this->id = id;
+    }
+
+    MemoryListener::SubscriptionHandle::SubscriptionHandle() : valid{false}
+    {
+    }
+
+    MemoryListener::SubscriptionHandle::SubscriptionHandle(SubscriptionHandle&& other) :
+        valid{other.valid},
+        memoryListener{other.memoryListener},
+        memoryID(std::move(other.memoryID)),
+        id{other.id}
+    {
+        other.valid = false;
+    }
+
+    MemoryListener::SubscriptionHandle&
+    MemoryListener::SubscriptionHandle::operator=(SubscriptionHandle other)
+    {
+        swap(*this, other);
+        return *this;
+    }
+
+    void
+    swap(MemoryListener::SubscriptionHandle& first, MemoryListener::SubscriptionHandle& second)
+    {
+        std::swap(first.valid, second.valid);
+        std::swap(first.memoryListener, second.memoryListener);
+        std::swap(first.memoryID, second.memoryID);
+        std::swap(first.id, second.id);
     }
 
     void
@@ -29,24 +56,26 @@ namespace armarx::armem::client::util
         memoryListener->unsubscribe(*this);
     }
 
-    MemoryListener::ScopedSubscriptionHandle::ScopedSubscriptionHandle(SubscriptionHandle handle) :
-        handle(handle)
+    MemoryListener::ScopedSubscriptionHandle::ScopedSubscriptionHandle()
+    {
+    }
+
+    MemoryListener::ScopedSubscriptionHandle::ScopedSubscriptionHandle(
+        SubscriptionHandle&& handle) :
+        handle(std::move(handle))
     {
     }
 
-    MemoryListener::ScopedSubscriptionHandle*
-    MemoryListener::ScopedSubscriptionHandle::operator=(const SubscriptionHandle& other)
+    MemoryListener::ScopedSubscriptionHandle&
+    MemoryListener::ScopedSubscriptionHandle::operator=(MemoryListener::SubscriptionHandle handle)
     {
-        handle.emplace(other);
-        return this;
+        std::swap(this->handle, handle);
+        return *this;
     }
 
     MemoryListener::ScopedSubscriptionHandle::~ScopedSubscriptionHandle()
     {
-        if (handle)
-        {
-            handle->release();
-        }
+        handle.release();
     }
 
     std::string
@@ -140,7 +169,6 @@ namespace armarx::armem::client::util
                 }
             }
         }
-
         if (error.str().size() > 0)
         {
             ARMARX_WARNING << "The following issues were encountered during MemoryListener::"
@@ -152,14 +180,19 @@ namespace armarx::armem::client::util
     MemoryListener::SubscriptionHandle
     MemoryListener::subscribe(const MemoryID& memoryID, Callback callback)
     {
-        if (component && callbacks.count(memoryID) == 0)
+        ARMARX_CHECK_NOT_EMPTY(memoryID.memoryName)
+            << "The memoryName must be specified to subscribe";
+
+        if (component && memoryRefCount[memoryID.memoryName] == 0)
         {
             component->usingTopic(MakeMemoryTopicName(memoryID));
         }
 
-        long id = next_id++;
+        auto id = next_id++;
         callbacks[memoryID].push_back({id, callback});
 
+        memoryRefCount[memoryID.memoryName]++;
+
         return SubscriptionHandle(this, memoryID, id);
     }
 
@@ -175,15 +208,36 @@ namespace armarx::armem::client::util
     void
     MemoryListener::unsubscribe(SubscriptionHandle& handle)
     {
+        if (not handle.valid)
+        {
+            return;
+        }
+        handle.valid = false;
+
         // Remove ManagedCallback with ManagedCallback.id == handle.id from callbacks[handle.memoryID]
         auto it = std::find_if(callbacks[handle.memoryID].begin(),
                                callbacks[handle.memoryID].end(),
-                               [handle](ManagedCallback mCb) { return mCb.id == handle.id; });
+                               [&handle](ManagedCallback& mCb) { return mCb.id == handle.id; });
 
-        if (it->id == handle.id)
+        if (it->id != handle.id)
         {
-            std::iter_swap(it, callbacks[handle.memoryID].end());
-            callbacks[handle.memoryID].pop_back();
+            return;
+        }
+
+        std::iter_swap(it, callbacks[handle.memoryID].end() - 1);
+        callbacks[handle.memoryID].pop_back();
+
+        memoryRefCount[handle.memoryID.memoryName]--;
+
+        if (callbacks[handle.memoryID].size() == 0)
+        {
+            callbacks.erase(handle.memoryID);
+
+            // unsubscribe from memory topic if no remainig callback needs it
+            if (component && memoryRefCount[handle.memoryID.memoryName] == 0)
+            {
+                component->unsubscribeFromTopic(MakeMemoryTopicName(handle.memoryID));
+            }
         }
     }
 

source/RobotAPI/libraries/armem/client/util/MemoryListener.h

 #pragma once
 
-
 // STD/STL
 #include <functional>
-#include <list>
 #include <unordered_map>
 #include <vector>
 
@@ -48,13 +46,19 @@ namespace armarx::armem::client::util
             friend class MemoryListener;
 
         public:
-            //SubscriptionHandle() = delete;
+            SubscriptionHandle();
+            SubscriptionHandle(SubscriptionHandle&& other);
+            SubscriptionHandle& operator=(SubscriptionHandle other);
+
+            friend void swap(SubscriptionHandle& first, SubscriptionHandle& second);
+
             void release();
 
         private:
-            SubscriptionHandle(MemoryListener* memoryListener, MemoryID memoryID, long id);
+            SubscriptionHandle(MemoryListener* memoryListener, const MemoryID& memoryID, long id);
 
         private:
+            bool valid;
             MemoryListener* memoryListener;
             MemoryID memoryID;
             long id;
@@ -63,13 +67,14 @@ namespace armarx::armem::client::util
         class ScopedSubscriptionHandle
         {
         public:
-            ScopedSubscriptionHandle(SubscriptionHandle handle);
-            ScopedSubscriptionHandle* operator=(SubscriptionHandle const& other);
+            ScopedSubscriptionHandle();
+            ScopedSubscriptionHandle(SubscriptionHandle&& handle);
+            ScopedSubscriptionHandle& operator=(SubscriptionHandle handle);
 
             ~ScopedSubscriptionHandle();
 
         private:
-            std::optional<SubscriptionHandle> handle;
+            SubscriptionHandle handle;
         };
 
 
@@ -132,6 +137,9 @@ namespace armarx::armem::client::util
 
         std::unordered_map<MemoryID, std::vector<ManagedCallback>> callbacks;
 
+        /// memoryName -> #callbacks needing memory topic
+        std::unordered_map<std::string, int> memoryRefCount;
+
     private:
         armarx::ManagedIceObject* component;
     };