Fix cyclic references in shared_ptr (#174)

* Refs #15831: Remove acyclic shared_ptrs

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

* Refs #15831: Fix fragile ==operator

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

* Refs #15831: Fix documentation API spelling error

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

* Refs #15831: Fix memory leaks in some tests

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

* Refs #15831: Fix CI error when no ASAN failures

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

* Refs #15831: Add doxygen to fragile_ptr

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

* Refs #15831: apply suggestions over fragile_ptr

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

* Refs #15831: apply suggestions and add tests

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

* Refs #15831: uncrustify

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

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

Author: jparisu

.github/workflows/asan_log_parser.py

@@ -45,9 +45,13 @@
             dict({'ID': split_test[0], 'Name': split_test[2]}))
 
 # Convert python dict to markdown table
-md_table = Tomark.table(failed_tests)
-print(md_table)
-
-# Save table of failed test to GitHub action summary file
-with open(SUMMARY_FILE, 'a') as file:
-    file.write(f'\n{md_table}')
+if (failed_tests):
+    md_table = Tomark.table(failed_tests)
+    print(md_table)
+    # Save table of failed test to GitHub action summary file
+    with open(SUMMARY_FILE, 'a') as file:
+        file.write(f'\n{md_table}')
+else:
+    print('NO TESTS FAILED!')
+    with open(SUMMARY_FILE, 'a') as file:
+        file.write('\nNO TESTS FAILED!')

.github/workflows/test.yml

@@ -232,6 +232,7 @@ jobs:
         if: always()
 
       - name: Report ASAN errors
+        continue-on-error: true
         if: always()
         run: |
           echo -n "**ASAN Errors**: " >> $GITHUB_STEP_SUMMARY

docs/rst/api-reference/exception/exception_index.rst

@@ -9,5 +9,6 @@ Exception
     /rst/api-reference/exception/corruptedfile
     /rst/api-reference/exception/error
     /rst/api-reference/exception/exception
+    /rst/api-reference/exception/inconsistency
     /rst/api-reference/exception/preconditionnotmet
     /rst/api-reference/exception/unsupported

docs/rst/api-reference/exception/inconsistency.rst

@@ -0,0 +1,10 @@
+.. _api_exception_inconsistency:
+
+.. rst-class:: api-ref
+
+Inconsistency
+-------------
+
+.. doxygenclass:: eprosima::statistics_backend::Inconsistency
+    :project: fastdds_statistics_backend
+    :members:

docs/test/CMakeLists.txt

@@ -61,7 +61,7 @@ add_test(NAME documentation.code_block_check.cpp
 # Check docs spelling
 add_test(NAME documentation.spell_check
     COMMAND
-    ${SPHINX_EXECUTABLE} -Q -W --keep-going
+    ${SPHINX_EXECUTABLE} -W --keep-going
     -D breathe_projects.fastdds_statistics_backend=${DOXYGEN_OUTPUT_DIR}/xml
     -b spelling
     -d "${PROJECT_BINARY_DOCS_DIR}/doctrees"

include/fastdds_statistics_backend/exception/Exception.hpp

@@ -90,21 +90,8 @@ class Error : public Exception
 
 public:
 
-    /**
-     * @brief Construct a new statistics_backend::Error exception
-     *
-     * @param message The message to be returned by what()
-     */
-    FASTDDS_STATISTICS_BACKEND_DllAPI Error(
-            const char* message) noexcept;
-
-    /**
-     * @brief Construct a new statistics_backend::Error exception
-     *
-     * @param message The message to be returned by what()
-     */
-    FASTDDS_STATISTICS_BACKEND_DllAPI Error(
-            const std::string& message);
+    // Use parent constructors.
+    using Exception::Exception;
 
     /**
      * @brief Copies the statistics_backend::Error exception into a new one
@@ -132,21 +119,8 @@ class Unsupported : public Exception
 
 public:
 
-    /**
-     * @brief Construct a new statistics_backend::Unsupported exception
-     *
-     * @param message The message to be returned by what()
-     */
-    FASTDDS_STATISTICS_BACKEND_DllAPI Unsupported(
-            const char* message) noexcept;
-
-    /**
-     * @brief Construct a new statistics_backend::Unsupported exception
-     *
-     * @param message The message to be returned by what()
-     */
-    FASTDDS_STATISTICS_BACKEND_DllAPI Unsupported(
-            const std::string& message);
+    // Use parent constructors.
+    using Exception::Exception;
 
     /**
      * @brief Copies the statistics_backend::Unsupported exception into a new one
@@ -174,21 +148,8 @@ class BadParameter : public Exception
 
 public:
 
-    /**
-     * @brief Construct a new statistics_backend::BadParameter exception
-     *
-     * @param message The message to be returned by what()
-     */
-    FASTDDS_STATISTICS_BACKEND_DllAPI BadParameter(
-            const char* message) noexcept;
-
-    /**
-     * @brief Construct a new statistics_backend::BadParameter exception
-     *
-     * @param message The message to be returned by what()
-     */
-    FASTDDS_STATISTICS_BACKEND_DllAPI BadParameter(
-            const std::string& message);
+    // Use parent constructors.
+    using Exception::Exception;
 
     /**
      * @brief Copies the statistics_backend::BadParameter exception into a new one
@@ -216,21 +177,8 @@ class PreconditionNotMet : public Exception
 
 public:
 
-    /**
-     * @brief Construct a new statistics_backend::PreconditionNotMet exception
-     *
-     * @param message The message to be returned by what()
-     */
-    FASTDDS_STATISTICS_BACKEND_DllAPI PreconditionNotMet(
-            const char* message) noexcept;
-
-    /**
-     * @brief Construct a new statistics_backend::PreconditionNotMet exception
-     *
-     * @param message The message to be returned by what()
-     */
-    FASTDDS_STATISTICS_BACKEND_DllAPI PreconditionNotMet(
-            const std::string& message);
+    // Use parent constructors.
+    using Exception::Exception;
 
     /**
      * @brief Copies the statistics_backend::PreconditionNotMet exception into a new one
@@ -258,21 +206,8 @@ class CorruptedFile : public Exception
 
 public:
 
-    /**
-     * @brief Construct a new statistics_backend::CorruptedFile exception
-     *
-     * @param message The message to be returned by what()
-     */
-    FASTDDS_STATISTICS_BACKEND_DllAPI CorruptedFile(
-            const char* message) noexcept;
-
-    /**
-     * @brief Construct a new statistics_backend::CorruptedFile exception
-     *
-     * @param message The message to be returned by what()
-     */
-    FASTDDS_STATISTICS_BACKEND_DllAPI CorruptedFile(
-            const std::string& message);
+    // Use parent constructors.
+    using Exception::Exception;
 
     /**
      * @brief Copies the statistics_backend::CorruptedFile exception into a new one
@@ -292,9 +227,36 @@ class CorruptedFile : public Exception
             const CorruptedFile& other) = default;
 };
 
+/**
+ * @brief Exception to signal that an inconsistency inside the database has been found.
+ */
+class Inconsistency : public Exception
+{
+
+public:
+
+    // Use parent constructors.
+    using Exception::Exception;
+
+    /**
+     * @brief Copies the statistics_backend::Inconsistency exception into a new one
+     *
+     * @param other The original exception object to copy
+     */
+    FASTDDS_STATISTICS_BACKEND_DllAPI Inconsistency(
+            const Inconsistency& other) = default;
+
+    /**
+     * @brief Copies the statistics_backend::Inconsistency exception into the current one
+     *
+     * @param other The original statistics_backend::Inconsistency exception to copy
+     * @return the current statistics_backend::Inconsistency exception after the copy
+     */
+    FASTDDS_STATISTICS_BACKEND_DllAPI Inconsistency& operator =(
+            const Inconsistency& other) = default;
+};
+
 } // namespace statistics_backend
 } // namespace eprosima
 
-
 #endif // _EPROSIMA_FASTDDS_STATISTICS_BACKEND_EXCEPTION_EXCEPTION_HPP_
-

src/cpp/StatisticsBackendData.cpp

@@ -399,7 +399,7 @@ void StatisticsBackendData::stop_monitor(
 
     // The monitor is inactive
     // NOTE: for test sake, this is not always set
-    if (database_->is_entity_present(monitor_id))
+    if (database_ && database_->is_entity_present(monitor_id))
     {
         database_->change_entity_status(monitor_id, false);
     }

src/cpp/database/database.hpp

@@ -79,6 +79,13 @@ class Database
 {
 public:
 
+    /**
+     * @brief Destructor of Database.
+     *
+     * @note It requires a virtual dtor for test sake.
+     */
+    virtual ~Database() = default;
+
     /**
      * @brief Insert a new entity into the database.
      * @param entity The entity object to be inserted.
@@ -465,7 +472,7 @@ class Database
             throw BadParameter("Endpoint locators cannot be empty");
         }
 
-        /* Check that participant exits */
+        /* Check that participant exists */
         bool participant_exists = false;
         auto domain_participants = participants_.find(endpoint->participant->domain->id);
         if (domain_participants != participants_.end())
@@ -585,11 +592,11 @@ class Database
             }
         }
 
-        // Change the curent locators map for the new updated one
-        endpoint->locators = actual_locators_map;
+        // Remove the current locator map as it will be filled in the following loop
+        endpoint->locators.clear();
 
         /* Add to x_by_y_ collections and to locators_ */
-        for (auto& locator_it : endpoint->locators)
+        for (auto& locator_it : actual_locators_map)
         {
             /* Check that locator exists */
             if (locators_.find(locator_it.first) == locators_.end())
@@ -599,6 +606,9 @@ class Database
 
             // Add endpoint to locator's collection
             insert_ddsendpoint_to_locator(endpoint, locator_it.second);
+
+            // Add this locator to the locators map
+            endpoint->locators[locator_it.first] = locator_it.second;
         }
 
         /* Add endpoint to topics's collection */

src/cpp/database/entities.cpp

@@ -58,8 +58,8 @@ DDSEndpoint::DDSEndpoint(
         std::string endpoint_name, /* "INVALID" */
         Qos endpoint_qos, /* {} */
         std::string endpoint_guid, /* "|GUID UNKNOWN|" */
-        std::shared_ptr<DomainParticipant> endpoint_participant, /* nullptr */
-        std::shared_ptr<Topic> endpoint_topic /* nullptr */) noexcept
+        details::fragile_ptr<DomainParticipant> endpoint_participant, /* nullptr */
+        details::fragile_ptr<Topic> endpoint_topic /* nullptr */) noexcept
     : DDSEntity(entity_kind, endpoint_name, endpoint_qos, endpoint_guid)
     , participant(endpoint_participant)
     , topic(endpoint_topic)
@@ -121,25 +121,25 @@ void Locator::clear()
 }
 
 template<>
-std::map<EntityId, std::shared_ptr<DataReader>>& DomainParticipant::ddsendpoints<DataReader>()
+std::map<EntityId, details::fragile_ptr<DataReader>>& DomainParticipant::ddsendpoints<DataReader>()
 {
     return data_readers;
 }
 
 template<>
-std::map<EntityId, std::shared_ptr<DataWriter>>& DomainParticipant::ddsendpoints<DataWriter>()
+std::map<EntityId, details::fragile_ptr<DataWriter>>& DomainParticipant::ddsendpoints<DataWriter>()
 {
     return data_writers;
 }
 
 template<>
-std::map<EntityId, std::shared_ptr<DataReader>>& Topic::ddsendpoints<DataReader>()
+std::map<EntityId, details::fragile_ptr<DataReader>>& Topic::ddsendpoints<DataReader>()
 {
     return data_readers;
 }
 
 template<>
-std::map<EntityId, std::shared_ptr<DataWriter>>& Topic::ddsendpoints<DataWriter>()
+std::map<EntityId, details::fragile_ptr<DataWriter>>& Topic::ddsendpoints<DataWriter>()
 {
     return data_writers;
 }