Refs #15841: Return to monitor creation and add error handle

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

Author: jparisu

examples/cpp/HelloWorldExample/main.cpp

@@ -19,6 +19,8 @@
 
 #include <string>
 
+#include <fastdds/dds/log/Log.hpp>
+
 #include "arg_configuration.h"
 #include "HelloWorldPublisher.h"
 #include "HelloWorldSubscriber.h"

src/cpp/Monitor.hpp

@@ -22,23 +22,24 @@
 #include <map>
 #include <string>
 
-#include <fastdds/dds/domain/DomainParticipant.hpp>
-#include <fastdds/dds/domain/DomainParticipantFactory.hpp>
-#include <fastdds/dds/domain/DomainParticipantListener.hpp>
-#include <fastdds/dds/domain/qos/DomainParticipantQos.hpp>
-#include <fastdds/dds/subscriber/DataReader.hpp>
-#include <fastdds/dds/subscriber/DataReaderListener.hpp>
-#include <fastdds/dds/subscriber/qos/DataReaderQos.hpp>
-#include <fastdds/dds/subscriber/qos/SubscriberQos.hpp>
-#include <fastdds/dds/subscriber/Subscriber.hpp>
-#include <fastdds/dds/topic/qos/TopicQos.hpp>
-#include <fastdds/dds/topic/Topic.hpp>
-
 #include <fastdds_statistics_backend/listener/DomainListener.hpp>
 #include <fastdds_statistics_backend/listener/CallbackMask.hpp>
 #include <fastdds_statistics_backend/types/EntityId.hpp>
 
 namespace eprosima {
+namespace fastdds {
+namespace dds {
+
+class DomainParticipant;
+class DomainParticipantListener;
+class Subscriber;
+class Topic;
+class DataReader;
+class DataReaderListener;
+
+} // namespace dds
+} // namespace fastdds
+
 namespace statistics_backend {
 namespace details {
 
@@ -49,50 +50,6 @@ namespace details {
  */
 struct Monitor
 {
-    /**
-     * @brief Destroy the Monitor object
-     *
-     * Destroy every pointer that has been set.
-     * This method works even if the monitor creation has failed
-     *
-     * @warning this may not be the best way to implement the destruction of subentities, as they are not created
-     * under this class. But it is very convenience so it is reused during Monitor creation in case an error occurs
-     * and also it is used to normally destroy the Monitor.
-     */
-    ~Monitor()
-    {
-        // These values are not always set, as could come from an error creating Monitor, or for test sake.
-        if (participant)
-        {
-            if (subscriber)
-            {
-                for (auto& reader : readers)
-                {
-                    subscriber->delete_datareader(reader.second);
-                }
-
-                participant->delete_subscriber(subscriber);
-            }
-
-            for (auto& topic : topics)
-            {
-                participant->delete_topic(topic.second);
-            }
-
-            fastdds::dds::DomainParticipantFactory::get_instance()->delete_participant(participant);
-        }
-
-        if (reader_listener)
-        {
-            delete reader_listener;
-        }
-
-        if (participant_listener)
-        {
-            delete participant_listener;
-        }
-    }
-
     //! The EntityId of the monitored domain
     EntityId id{};
 
@@ -112,7 +69,6 @@ struct Monitor
     //! It will process the entity discoveries
     fastdds::dds::DomainParticipantListener* participant_listener = nullptr;
 
-
     //! The participant created to communicate with the statistics reporting publishers in this monitor
     fastdds::dds::Subscriber* subscriber = nullptr;
 

src/cpp/StatisticsBackend.cpp

@@ -185,14 +185,32 @@ EntityId create_and_register_monitor(
     details::StatisticsBackendData::get_instance()->lock();
 
     // Create monitor instance.
-    // NOTE: register in database at the end, in case any creation fails
     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;
+    }
+    // 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.
+
+    monitor->id = domain->id;
     monitor->domain_listener = domain_listener;
     monitor->domain_callback_mask = callback_mask;
     monitor->data_mask = data_mask;
+    details::StatisticsBackendData::get_instance()->monitors_by_entity_[domain->id] = monitor;
 
+    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_);
     monitor->reader_listener = new subscriber::StatisticsReaderListener(
         details::StatisticsBackendData::get_instance()->data_queue_);
 
@@ -207,6 +225,10 @@ 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()->unlock();
         throw Error("Error initializing monitor. Could not create participant");
     }
@@ -219,6 +241,11 @@ 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()->unlock();
         throw Error("Error initializing monitor. Could not create subscriber");
     }
@@ -230,6 +257,26 @@ EntityId create_and_register_monitor(
 
         if (monitor->topics[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()->unlock();
             throw Error("Error initializing monitor. Could not create topic " + std::string(topic));
         }
@@ -243,33 +290,31 @@ 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()->unlock();
             throw Error("Error initializing monitor. Could not create reader for topic " + std::string(topic));
         }
     }
 
-    // Insert domain entity in database
-    try
-    {
-        domain->id = details::StatisticsBackendData::get_instance()->database_->insert(domain);
-    }
-    catch (const std::exception&)
-    {
-        details::StatisticsBackendData::get_instance()->unlock();
-        throw;
-    }
-
-    // Insert monitor as a new monitor entity.
-    // NOTE: Monitor Id is only set after insert domain in database
-    monitor->id = domain->id;
-    details::StatisticsBackendData::get_instance()->monitors_by_entity_[domain->id] = monitor;
-
-    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_);
-
     details::StatisticsBackendData::get_instance()->unlock();
     return domain->id;
 }

src/cpp/StatisticsBackendData.cpp

@@ -22,6 +22,7 @@
 
 #include <fastdds/dds/domain/DomainParticipant.hpp>
 #include <fastdds/dds/domain/DomainParticipantFactory.hpp>
+#include <fastdds/dds/domain/qos/DomainParticipantFactoryQos.hpp>
 #include <fastdds/dds/domain/DomainParticipantListener.hpp>
 #include <fastdds/dds/subscriber/DataReader.hpp>
 #include <fastdds/dds/subscriber/Subscriber.hpp>
@@ -51,7 +52,10 @@ StatisticsBackendData::StatisticsBackendData()
     , lock_(mutex_, std::defer_lock)
     , participant_factory_instance_(eprosima::fastdds::dds::DomainParticipantFactory::get_shared_instance())
 {
-    // Do nothing
+    // 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;
 }
 
 StatisticsBackendData::~StatisticsBackendData()
@@ -363,7 +367,36 @@ void StatisticsBackendData::stop_monitor(
     monitors_by_entity_.erase(it);
 
     // Delete everything created during monitor initialization
-    monitor.reset();
+    // These values are not always set, as could come from an error creating Monitor, or for test sake.
+    if (monitor->participant)
+    {
+        if (monitor->subscriber)
+        {
+            for (auto& reader : monitor->readers)
+            {
+                monitor->subscriber->delete_datareader(reader.second);
+            }
+
+            monitor->participant->delete_subscriber(monitor->subscriber);
+        }
+
+        for (auto& topic : monitor->topics)
+        {
+            monitor->participant->delete_topic(topic.second);
+        }
+
+        fastdds::dds::DomainParticipantFactory::get_instance()->delete_participant(monitor->participant);
+    }
+
+    if (monitor->reader_listener)
+    {
+        delete monitor->reader_listener;
+    }
+
+    if (monitor->participant_listener)
+    {
+        delete monitor->participant_listener;
+    }
 
     // The monitor is inactive
     // NOTE: for test sake, this is not always set

test/mock/dds/DomainParticipant/fastdds/dds/domain/DomainParticipant.hpp

@@ -187,6 +187,11 @@ class DomainParticipant
             const std::string& type_name
         ));
 
+    MOCK_METHOD0(
+        enable,
+        ReturnCode_t
+        ());
+
     DomainParticipantQos qos_;
     DomainId_t domain_id_;
     eprosima::fastrtps::rtps::GUID_t guid_;

test/mock/dds/DomainParticipantFactory/fastdds/dds/domain/DomainParticipantFactory.hpp

@@ -26,6 +26,7 @@
 
 #include <fastdds/dds/core/status/StatusMask.hpp>
 #include <fastdds/dds/domain/DomainParticipant.hpp>
+#include <fastdds/dds/domain/qos/DomainParticipantFactoryQos.hpp>
 #include <fastdds/dds/domain/qos/DomainParticipantQos.hpp>
 #include <fastrtps/types/TypesBase.h>
 
@@ -96,16 +97,34 @@ class DomainParticipantFactory
     const DomainParticipantQos& get_default_participant_qos()
     {
         get_default_participant_qos_count++;
-        return qos;
+        return participant_qos;
     }
 
-    DomainParticipantQos qos{};
+    ReturnCode_t get_qos(DomainParticipantFactoryQos& qos) const
+    {
+        get_qos_count++;
+        qos = factory_qos;
+        return fastrtps::types::ReturnCode_t::RETCODE_OK;
+    }
+
+    ReturnCode_t set_qos(
+            const DomainParticipantFactoryQos& qos)
+    {
+        set_qos_count++;
+        factory_qos = qos;
+        return fastrtps::types::ReturnCode_t::RETCODE_OK;
+    }
+
+    DomainParticipantFactoryQos factory_qos{};
+    DomainParticipantQos participant_qos{};
     DomainParticipant* domain_participant = nullptr;
 
-    unsigned int load_profiles_count = 0;
-    unsigned int create_participant_count = 0;
-    unsigned int delete_participant_count = 0;
-    unsigned int get_default_participant_qos_count = 0;
+    mutable unsigned int load_profiles_count = 0;
+    mutable unsigned int create_participant_count = 0;
+    mutable unsigned int delete_participant_count = 0;
+    mutable unsigned int get_default_participant_qos_count = 0;
+    mutable unsigned int get_qos_count = 0;
+    mutable unsigned int set_qos_count = 0;
 
 };