From 9ae951f4f7a7c49f2d36035d37dc9934deab2ab0 Mon Sep 17 00:00:00 2001
From: Patrick Dormanns <patrick.dormanns@student.kit.edu>
Date: Tue, 24 Oct 2023 21:55:35 +0200
Subject: [PATCH] implemented MemoryListener::(Scoped)SubscriptionHandle
correctly
---
.../armem/client/util/MemoryListener.cpp | 98 ++++++++++++++-----
.../armem/client/util/MemoryListener.h | 22 +++--
2 files changed, 91 insertions(+), 29 deletions(-)
diff --git a/source/RobotAPI/libraries/armem/client/util/MemoryListener.cpp b/source/RobotAPI/libraries/armem/client/util/MemoryListener.cpp
index 669be5db6..955075f35 100644
--- a/source/RobotAPI/libraries/armem/client/util/MemoryListener.cpp
+++ b/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));
+ }
}
}
diff --git a/source/RobotAPI/libraries/armem/client/util/MemoryListener.h b/source/RobotAPI/libraries/armem/client/util/MemoryListener.h
index 7c6bb2ef9..0ca15a23a 100644
--- a/source/RobotAPI/libraries/armem/client/util/MemoryListener.h
+++ b/source/RobotAPI/libraries/armem/client/util/MemoryListener.h
@@ -1,9 +1,7 @@
#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;
};
--
GitLab