iox-#1150 Protect findService with a mutex

Signed-off-by: Matthias Killat <[email protected]>
eclipse-iceoryx · Feb 23, 2022 · 1879568 · 1879568
1 parent 1592b46
commit 1879568
Show file tree

Hide file tree

Showing 4 changed files with 25 additions and 12 deletions.
diff --git a/iceoryx_posh/include/iceoryx_posh/runtime/service_discovery.hpp b/iceoryx_posh/include/iceoryx_posh/runtime/service_discovery.hpp
@@ -23,6 +23,7 @@
 #include "iceoryx_posh/runtime/posh_runtime.hpp"
 
 #include <memory>
+#include <mutex>
 
 namespace iox
 {
@@ -43,10 +44,7 @@ enum class ServiceDiscoveryEvent : popo::EventEnumIdentifier
 class ServiceDiscovery
 {
   public:
-    ServiceDiscovery()
-    {
-    }
-
+    ServiceDiscovery() = default;
     ServiceDiscovery(const ServiceDiscovery&) = delete;
     ServiceDiscovery& operator=(const ServiceDiscovery&) = delete;
     ServiceDiscovery(ServiceDiscovery&&) = delete;
@@ -73,13 +71,16 @@ class ServiceDiscovery
     iox::popo::WaitSetIsConditionSatisfiedCallback
     getCallbackForIsStateConditionSatisfied(const popo::SubscriberState state);
 
-    //  use dynamic memory to reduce stack usage,
-    /// @todo improve solution to avoid stack usage without using dynamic memory
-    std::unique_ptr<roudi::ServiceRegistry> m_serviceRegistry{new roudi::ServiceRegistry};
+    // use dynamic memory to reduce stack usage
+    /// @todo #1155 improve solution to avoid stack usage without using dynamic memory
+    std::unique_ptr<roudi::ServiceRegistry> m_serviceRegistry{new iox::roudi::ServiceRegistry};
+    std::mutex m_serviceRegistryMutex;
 
     popo::Subscriber<roudi::ServiceRegistry> m_serviceRegistrySubscriber{
         {SERVICE_DISCOVERY_SERVICE_NAME, SERVICE_DISCOVERY_INSTANCE_NAME, SERVICE_DISCOVERY_EVENT_NAME},
         {1U, 1U, iox::NodeName_t("Service Registry"), true}};
+
+    void update();
 };
 
 } // namespace runtime

diff --git a/iceoryx_posh/source/runtime/service_discovery.cpp b/iceoryx_posh/source/runtime/service_discovery.cpp
@@ -21,6 +21,15 @@ namespace iox
 {
 namespace runtime
 {
+void ServiceDiscovery::update()
+{
+    // allows us to use update and hence findService concurrently
+    std::lock_guard<std::mutex> lock(m_serviceRegistryMutex);
+    m_serviceRegistrySubscriber.take().and_then([&](popo::Sample<const roudi::ServiceRegistry>& serviceRegistrySample) {
+        *m_serviceRegistry = *serviceRegistrySample;
+    });
+}
+
 void ServiceDiscovery::findService(const cxx::optional<capro::IdString_t>& service,
                                    const cxx::optional<capro::IdString_t>& instance,
                                    const cxx::optional<capro::IdString_t>& event,
@@ -32,9 +41,7 @@ void ServiceDiscovery::findService(const cxx::optional<capro::IdString_t>& servi
         return;
     }
 
-    m_serviceRegistrySubscriber.take().and_then([&](popo::Sample<const roudi::ServiceRegistry>& serviceRegistrySample) {
-        *m_serviceRegistry = *serviceRegistrySample;
-    });
+    update();
 
     switch (pattern)
     {

diff --git a/iceoryx_posh/test/integrationtests/test_service_discovery.cpp b/iceoryx_posh/test/integrationtests/test_service_discovery.cpp
@@ -208,6 +208,13 @@ TYPED_TEST(ServiceDiscovery_test, FindSameServiceMultipleTimesReturnsSingleInsta
                       SERVICE_DESCRIPTION.getEventIDString());
     ASSERT_THAT(serviceContainer.size(), Eq(1U));
     EXPECT_THAT(*serviceContainer.begin(), Eq(SERVICE_DESCRIPTION));
+
+    this->findService(SERVICE_DESCRIPTION.getServiceIDString(),
+                      SERVICE_DESCRIPTION.getInstanceIDString(),
+                      SERVICE_DESCRIPTION.getEventIDString());
+
+    ASSERT_THAT(serviceContainer.size(), Eq(1U));
+    EXPECT_THAT(*serviceContainer.begin(), Eq(SERVICE_DESCRIPTION));
 }
 
 TYPED_TEST(ServiceDiscovery_test, OfferDifferentServicesWithSameInstanceAndEvent)

diff --git a/iceoryx_posh/test/moduletests/test_roudi_service_registry.cpp b/iceoryx_posh/test/moduletests/test_roudi_service_registry.cpp
@@ -639,8 +639,6 @@ TYPED_TEST(ServiceRegistry_test, SearchInFullRegistryWorks)
     ASSERT_EQ(this->searchResult.size(), 1);
 }
 
-using Entry = iox::roudi::ServiceRegistry::ServiceDescriptionEntry;
-
 TYPED_TEST(ServiceRegistry_test, FunctionIsAppliedToAllEntriesInSearchResult)
 {
     ::testing::Test::RecordProperty("TEST_ID", "b7828085-d879-43b7-9fee-e5e88cf36995");