Fix SegFault in delete_contained_entities() when compiled with statis…

…tics (eProsima#3048) * Refs 14723. Initial debg msgs Signed-off-by: Mario Dominguez <mariodominguez@eprosima.com> * Refs 14723. Issue solved, still to remove some debugging msgs Signed-off-by: Mario Dominguez <mariodominguez@eprosima.com> * Refs 14723. BasicConfigurationExample HelloWorldDestructordestructor simplified Signed-off-by: Mario Dominguez <mariodominguez@eprosima.com> * Refs 14723. reformat for uncrustify Signed-off-by: Mario Dominguez <mariodominguez@eprosima.com> * Refs 14723. Added DeleteParticipantAfterDeleteContainedEntitiesFailure to statistics tests Signed-off-by: Mario Dominguez <mariodominguez@eprosima.com> * Refs 14723. Uncrustify stuff Signed-off-by: Mario Dominguez <mariodominguez@eprosima.com> * Refs 14723. Required Test changes Signed-off-by: Mario Dominguez <mariodominguez@eprosima.com> Signed-off-by: Mario Dominguez <mariodominguez@eprosima.com>
simonmcqueen · Nov 3, 2022 · b0d0e9a · b0d0e9a
1 parent 118dda9
commit b0d0e9a
Show file tree

Hide file tree

Showing 6 changed files with 66 additions and 14 deletions.
diff --git a/examples/cpp/dds/BasicConfigurationExample/BasicConfigurationPublisher.cpp b/examples/cpp/dds/BasicConfigurationExample/BasicConfigurationPublisher.cpp
@@ -214,18 +214,7 @@ HelloWorldPublisher::~HelloWorldPublisher()
 {
     if (participant_ != nullptr)
     {
-        if (publisher_ != nullptr)
-        {
-            if (writer_ != nullptr)
-            {
-                publisher_->delete_datawriter(writer_);
-            }
-            participant_->delete_publisher(publisher_);
-        }
-        if (topic_ != nullptr)
-        {
-            participant_->delete_topic(topic_);
-        }
+        participant_->delete_contained_entities();
         DomainParticipantFactory::get_instance()->delete_participant(participant_);
     }
 }

diff --git a/src/cpp/fastdds/domain/DomainParticipantImpl.hpp b/src/cpp/fastdds/domain/DomainParticipantImpl.hpp
@@ -332,7 +332,7 @@ class DomainParticipantImpl
 
     DomainId_t get_domain_id() const;
 
-    ReturnCode_t delete_contained_entities();
+    virtual ReturnCode_t delete_contained_entities();
 
     ReturnCode_t assert_liveliness();
 

diff --git a/src/cpp/statistics/fastdds/domain/DomainParticipantImpl.cpp b/src/cpp/statistics/fastdds/domain/DomainParticipantImpl.cpp
@@ -267,6 +267,19 @@ void DomainParticipantImpl::disable()
     efd::DomainParticipantImpl::disable();
 }
 
+ReturnCode_t DomainParticipantImpl::delete_contained_entities()
+{
+    ReturnCode_t ret = efd::DomainParticipantImpl::delete_contained_entities();
+
+    if (ret == ReturnCode_t::RETCODE_OK)
+    {
+        builtin_publisher_impl_ = nullptr;
+        builtin_publisher_ = nullptr;
+    }
+
+    return ret;
+}
+
 efd::PublisherImpl* DomainParticipantImpl::create_publisher_impl(
         const efd::PublisherQos& qos,
         efd::PublisherListener* listener)

diff --git a/src/cpp/statistics/fastdds/domain/DomainParticipantImpl.hpp b/src/cpp/statistics/fastdds/domain/DomainParticipantImpl.hpp
@@ -115,6 +115,13 @@ class DomainParticipantImpl : public efd::DomainParticipantImpl
     static bool is_statistics_topic_name(
             const std::string& topic_name) noexcept;
 
+    /**
+     * @brief This override calls the parent method and returns builtin publishers to nullptr
+     *
+     * @return RETCODE_OK if successful
+     */
+    ReturnCode_t delete_contained_entities() override;
+
 protected:
 
     /**

diff --git a/test/mock/dds/DomainParticipantImpl/fastdds/domain/DomainParticipantImpl.hpp b/test/mock/dds/DomainParticipantImpl/fastdds/domain/DomainParticipantImpl.hpp
@@ -607,7 +607,7 @@ class DomainParticipantImpl
         return false;
     }
 
-    ReturnCode_t delete_contained_entities()
+    virtual ReturnCode_t delete_contained_entities()
     {
         bool can_be_deleted = true;
 

diff --git a/test/unittest/statistics/dds/StatisticsDomainParticipantTests.cpp b/test/unittest/statistics/dds/StatisticsDomainParticipantTests.cpp
@@ -698,6 +698,49 @@ TEST_F(StatisticsDomainParticipantTests, EnableStatisticsDataWriterFailureIncomp
 #endif // FASTDDS_STATISTICS
 }
 
+/**
+
+ * This test checks that a participant is correctly deleted by the factory
+ * after calling delete_contained_entities() method
+ * 1. Create a statistics participant, register type
+ * 2. Create a sample topic
+ * 3. Perform a delete_contained_entities() in the statistics participant
+ * 4. Delete the participant
+ */
+TEST_F(StatisticsDomainParticipantTests, DeleteParticipantAfterDeleteContainedEntitiesFailure)
+{
+#ifdef FASTDDS_STATISTICS
+
+    eprosima::fastdds::dds::TypeSupport count_type(new EntityCountPubSubType);
+
+    // 1. Create DomainParticipant with statistics
+    eprosima::fastdds::dds::DomainParticipant* participant =
+            eprosima::fastdds::dds::DomainParticipantFactory::get_instance()->
+                    create_participant(0, eprosima::fastdds::dds::PARTICIPANT_QOS_DEFAULT);
+    ASSERT_NE(participant, nullptr);
+
+    // Register type
+    participant->register_type(count_type);
+
+    // 2. Create a sample topic
+    participant->create_topic(HEARTBEAT_COUNT_TOPIC,
+            count_type->getName(), eprosima::fastdds::dds::TOPIC_QOS_DEFAULT);
+
+    DomainParticipant* statistics_participant = DomainParticipant::narrow(participant);
+    ASSERT_NE(statistics_participant, nullptr);
+
+    EXPECT_EQ(ReturnCode_t::RETCODE_OK, statistics_participant->enable_statistics_datawriter(HEARTBEAT_COUNT_TOPIC,
+            STATISTICS_DATAWRITER_QOS));
+
+    // 3. Perform a delete_contained_entities() in the statistics participant
+    EXPECT_EQ(participant->delete_contained_entities(), ReturnCode_t::RETCODE_OK);
+
+    // 4. Delete the participant
+    EXPECT_EQ(eprosima::fastdds::dds::DomainParticipantFactory::get_instance()->
+                    delete_participant(participant), ReturnCode_t::RETCODE_OK);
+#endif // FASTDDS_STATISTICS
+}
+
 } // namespace dds
 } // namespace statistics
 } // namespace fastdds