Refs #15841: apply suggestions

Signed-off-by: jparisu <[email protected]>

Author: jparisu

.github/workflows/test.yml

@@ -257,7 +257,8 @@ jobs:
         uses: ./src/Fast-DDS-statistics-backend/.github/actions/install-python-packages
 
       - name: Fetch eProsima dependencies
-        uses: ./src/Fast-DDS-statistics-backend/.github/actions/fetch-fastdds-repos
+        run: |
+          vcs import src < ./src/Fast-DDS-statistics-backend/.github/workflows/ci.repos
 
       - name: Update colcon mixin
         run: |

src/cpp/StatisticsBackend.cpp

@@ -54,6 +54,7 @@
 #include "StatisticsBackendData.hpp"
 #include "detail/data_getters.hpp"
 #include "detail/data_aggregation.hpp"
+#include "detail/ScopeExit.hpp"
 
 using namespace eprosima::fastdds::dds;
 using namespace eprosima::fastdds::rtps;
@@ -187,20 +188,15 @@ EntityId create_and_register_monitor(
     // What should happen is that all this logic is moved to StatisticsBackendData. You know, some day...
 
     details::StatisticsBackendData::get_instance()->lock();
+    MAKE_UNNAMED_SCOPE_EXIT(details::StatisticsBackendData::get_instance()->unlock());
 
     // Create monitor instance.
     std::shared_ptr<details::Monitor> monitor = std::make_shared<details::Monitor>();
     std::shared_ptr<database::Domain> domain = std::make_shared<database::Domain>(domain_name);
 
-    try
-    {
-        domain->id = details::StatisticsBackendData::get_instance()->database_->insert(domain);
-    }
-    catch (const std::exception&)
-    {
-        details::StatisticsBackendData::get_instance()->unlock();
-        throw;
-    }
+    // Throw exception in fail case
+    domain->id = details::StatisticsBackendData::get_instance()->database_->insert(domain);
+
     // TODO: in case this function fails afterwards, the domain will be kept in the database without associated
     // Participant. There must exist a way in database to delete a domain, or to make a rollback.
 
@@ -209,14 +205,19 @@ EntityId create_and_register_monitor(
     monitor->domain_callback_mask = callback_mask;
     monitor->data_mask = data_mask;
     details::StatisticsBackendData::get_instance()->monitors_by_entity_[domain->id] = monitor;
+    auto se_erase_monitor_database_ =
+        MAKE_SCOPE_EXIT(details::StatisticsBackendData::get_instance()->monitors_by_entity_.erase(domain->id));
 
     monitor->participant_listener = new subscriber::StatisticsParticipantListener(
         domain->id,
         details::StatisticsBackendData::get_instance()->database_.get(),
         details::StatisticsBackendData::get_instance()->entity_queue_,
         details::StatisticsBackendData::get_instance()->data_queue_);
+    auto se_participant_listener_ = MAKE_SCOPE_EXIT(delete monitor->participant_listener);
+
     monitor->reader_listener = new subscriber::StatisticsReaderListener(
         details::StatisticsBackendData::get_instance()->data_queue_);
+    auto se_reader_listener_ = MAKE_SCOPE_EXIT(delete monitor->reader_listener);
 
     /* Create DomainParticipant */
     StatusMask participant_mask = StatusMask::all();
@@ -229,14 +230,9 @@ EntityId create_and_register_monitor(
 
     if (monitor->participant == nullptr)
     {
-        // Remove those elements that have been set
-        delete monitor->reader_listener;
-        delete monitor->participant_listener;
-        details::StatisticsBackendData::get_instance()->monitors_by_entity_.erase(domain->id);
-
-        details::StatisticsBackendData::get_instance()->unlock();
         throw Error("Error initializing monitor. Could not create participant");
     }
+    auto se_participant_ = MAKE_SCOPE_EXIT(DomainParticipantFactory::get_instance()->delete_participant(monitor->participant));
 
     /* Create Subscriber */
     monitor->subscriber = monitor->participant->create_subscriber(
@@ -246,15 +242,29 @@ EntityId create_and_register_monitor(
 
     if (monitor->subscriber == nullptr)
     {
-        // Remove those elements that have been set
-        DomainParticipantFactory::get_instance()->delete_participant(monitor->participant);
-        delete monitor->reader_listener;
-        delete monitor->participant_listener;
-        details::StatisticsBackendData::get_instance()->monitors_by_entity_.erase(domain->id);
-
-        details::StatisticsBackendData::get_instance()->unlock();
         throw Error("Error initializing monitor. Could not create subscriber");
     }
+    auto se_subscriber_ = MAKE_SCOPE_EXIT(monitor->participant->delete_subscriber(monitor->subscriber));
+
+    auto se_topics_datareaders_ =
+        MAKE_SCOPE_EXIT(
+            {
+                for (auto& it : monitor->readers)
+                {
+                    if (nullptr != it.second)
+                    {
+                        monitor->subscriber->delete_datareader(it.second);
+                    }
+                }
+                for (auto& it : monitor->topics)
+                {
+                    if (nullptr != it.second)
+                    {
+                        monitor->participant->delete_topic(it.second);
+                    }
+                }
+            }
+        );
 
     for (const auto& topic : topics)
     {
@@ -265,28 +275,6 @@ EntityId create_and_register_monitor(
         }
         catch (const std::exception& e)
         {
-            // Remove those elements that have been set
-            for (auto& it : monitor->readers)
-            {
-                if (nullptr != it.second)
-                {
-                    monitor->subscriber->delete_datareader(it.second);
-                }
-            }
-            for (auto& it : monitor->topics)
-            {
-                if (nullptr != it.second)
-                {
-                    monitor->participant->delete_topic(it.second);
-                }
-            }
-            monitor->participant->delete_subscriber(monitor->subscriber);
-            DomainParticipantFactory::get_instance()->delete_participant(monitor->participant);
-            delete monitor->reader_listener;
-            delete monitor->participant_listener;
-            details::StatisticsBackendData::get_instance()->monitors_by_entity_.erase(domain->id);
-
-            details::StatisticsBackendData::get_instance()->unlock();
             throw Error("Error registering topic " + std::string(topic) + " : " + e.what());
         }
 
@@ -299,33 +287,17 @@ EntityId create_and_register_monitor(
 
         if (monitor->readers[topic] == nullptr)
         {
-            // Remove those elements that have been set
-            for (auto& it : monitor->readers)
-            {
-                if (nullptr != it.second)
-                {
-                    monitor->subscriber->delete_datareader(it.second);
-                }
-            }
-            for (auto& it : monitor->topics)
-            {
-                if (nullptr != it.second)
-                {
-                    monitor->participant->delete_topic(it.second);
-                }
-            }
-            monitor->participant->delete_subscriber(monitor->subscriber);
-            DomainParticipantFactory::get_instance()->delete_participant(monitor->participant);
-            delete monitor->reader_listener;
-            delete monitor->participant_listener;
-            details::StatisticsBackendData::get_instance()->monitors_by_entity_.erase(domain->id);
-
-            details::StatisticsBackendData::get_instance()->unlock();
             throw Error("Error initializing monitor. Could not create reader for topic " + std::string(topic));
         }
     }
 
-    details::StatisticsBackendData::get_instance()->unlock();
+    se_erase_monitor_database_.cancel();
+    se_participant_listener_.cancel();
+    se_reader_listener_.cancel();
+    se_participant_.cancel();
+    se_subscriber_.cancel();
+    se_topics_datareaders_.cancel();
+
     return domain->id;
 }
 

src/cpp/StatisticsBackendData.cpp

@@ -16,6 +16,7 @@
  * @file StatisticsBackendData.cpp
  */
 
+#include "StatisticsBackendData.hpp"
 
 #include <map>
 #include <string>
@@ -31,8 +32,6 @@
 #include <fastdds_statistics_backend/listener/DomainListener.hpp>
 #include <fastdds_statistics_backend/listener/PhysicalListener.hpp>
 
-#include "StatisticsBackendData.hpp"
-
 #include "Monitor.hpp"
 #include <database/database_queue.hpp>
 #include <database/database.hpp>
@@ -52,10 +51,7 @@ StatisticsBackendData::StatisticsBackendData()
     , lock_(mutex_, std::defer_lock)
     , participant_factory_instance_(eprosima::fastdds::dds::DomainParticipantFactory::get_shared_instance())
 {
-    // Set in DomainParticipantFactory that entities are created disabled
-    eprosima::fastdds::dds::DomainParticipantFactoryQos qos;
-    participant_factory_instance_->get_qos(qos);
-    qos.entity_factory().autoenable_created_entities = false;
+    // Do nothing
 }
 
 StatisticsBackendData::~StatisticsBackendData()

src/cpp/detail/ScopeExit.hpp

@@ -0,0 +1,88 @@
+// Copyright 2015-2020 Open Source Robotics Foundation, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+// Copyright 2022 Proyectos y Sistemas de Mantenimiento SL (eProsima).
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef _EPROSIMA_FASTDDS_STATISTICS_BACKEND_DETAIL_SCOPE_EXIT_HPP_
+#define _EPROSIMA_FASTDDS_STATISTICS_BACKEND_DETAIL_SCOPE_EXIT_HPP_
+
+#include <utility>
+
+namespace eprosima {
+namespace statistics_backend {
+namespace details {
+
+template<typename CallableT>
+class ScopeExit final
+{
+public:
+    explicit ScopeExit(CallableT && callable)
+        : callable_(std::forward<CallableT>(callable))
+    {
+    }
+
+    ScopeExit(const ScopeExit &) = delete;
+    ScopeExit(ScopeExit &&) = default;
+
+    ScopeExit & operator=(const ScopeExit &) = delete;
+    ScopeExit & operator=(ScopeExit &&) = default;
+
+    ~ScopeExit()
+    {
+        if (!cancelled_) {
+        callable_();
+        }
+    }
+
+    void cancel()
+    {
+        cancelled_ = true;
+    }
+
+private:
+    CallableT callable_;
+    bool cancelled_{false};
+};
+
+template<typename CallableT>
+ScopeExit<CallableT>
+make_scope_exit(CallableT && callable)
+{
+    return ScopeExit<CallableT>(std::forward<CallableT>(callable));
+}
+
+}  // namespace details
+}  // namespace statistics_backend
+}  // namespace eprosima
+
+#define MAKE_SCOPE_EXIT(code) \
+  eprosima::statistics_backend::details::make_scope_exit([&]() {code;})
+
+#define JOIN_IMPL(arg1, arg2) arg1 ## arg2
+#define MAKE_UNNAMED_SCOPE_EXIT(code) \
+  auto JOIN_IMPL(scope_exit_, __LINE__) = eprosima::statistics_backend::details::make_scope_exit([&]() {code;})
+
+#endif  // _EPROSIMA_FASTDDS_STATISTICS_BACKEND_DETAIL_SCOPE_EXIT_HPP_