From a5aa54c6cd50f295be9e672d425e51d45726c0a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Dom=C3=ADnguez=20L=C3=B3pez?= <116071334+Mario-DL@users.noreply.github.com> Date: Sat, 8 Jun 2024 07:42:42 +0200 Subject: [PATCH] Properly delete builtin statistics writers upon `delete_contained_entities()` (#4891) * Refs #20816: Add BB test Signed-off-by: Mario Dominguez * Refs #20816: Fix Signed-off-by: Mario Dominguez * Refs #20816: Apply Edu's suggestion Signed-off-by: Mario Dominguez --------- Signed-off-by: Mario Dominguez (cherry picked from commit 0d62335cc86dbe8cb43d347acf9bde547de179d4) --- .../fastdds/domain/DomainParticipantImpl.cpp | 11 +-- .../fastdds/domain/DomainParticipantImpl.hpp | 2 + .../common/DDSBlackboxTestsStatistics.cpp | 94 +++++++++++++++++++ 3 files changed, 98 insertions(+), 9 deletions(-) diff --git a/src/cpp/statistics/fastdds/domain/DomainParticipantImpl.cpp b/src/cpp/statistics/fastdds/domain/DomainParticipantImpl.cpp index afd8aa662ec..e2ea20c9f5d 100644 --- a/src/cpp/statistics/fastdds/domain/DomainParticipantImpl.cpp +++ b/src/cpp/statistics/fastdds/domain/DomainParticipantImpl.cpp @@ -287,15 +287,8 @@ void 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; + delete_statistics_builtin_entities(); + return efd::DomainParticipantImpl::delete_contained_entities(); } ReturnCode_t DomainParticipantImpl::enable_monitor_service() diff --git a/src/cpp/statistics/fastdds/domain/DomainParticipantImpl.hpp b/src/cpp/statistics/fastdds/domain/DomainParticipantImpl.hpp index cec232c6ec0..a1a2ffda81c 100644 --- a/src/cpp/statistics/fastdds/domain/DomainParticipantImpl.hpp +++ b/src/cpp/statistics/fastdds/domain/DomainParticipantImpl.hpp @@ -121,6 +121,8 @@ class DomainParticipantImpl : public efd::DomainParticipantImpl, * @brief This override calls the parent method and returns builtin publishers to nullptr * * @return RETCODE_OK if successful + * @note This method is meant to be used followed by a deletion of the participant as it implies + * the deletion of the builtin statistics publishers. */ ReturnCode_t delete_contained_entities() override; diff --git a/test/blackbox/common/DDSBlackboxTestsStatistics.cpp b/test/blackbox/common/DDSBlackboxTestsStatistics.cpp index a9dcd8ebcbb..8e7c0018a22 100644 --- a/test/blackbox/common/DDSBlackboxTestsStatistics.cpp +++ b/test/blackbox/common/DDSBlackboxTestsStatistics.cpp @@ -686,3 +686,97 @@ TEST(DDSStatistics, discovery_topic_physical_data_delete_physical_properties) test_discovery_topic_physical_data(DiscoveryTopicPhysicalDataTest::NO_PHYSICAL_DATA); #endif // FASTDDS_STATISTICS } + +class CustomStatisticsParticipantSubscriber : public PubSubReader +{ +public: + + CustomStatisticsParticipantSubscriber( + const std::string& topic_name) + : PubSubReader(topic_name) + { + } + + void destroy() override + { + participant_->delete_contained_entities(); + DomainParticipantFactory::get_instance()->delete_participant(participant_); + participant_ = nullptr; + } + +}; + +// Regression test for #20816. When an application is terminated with delete_contained_entities() +// it has to properly finish. The test creates a number of participants with some of them sharing the same topic. +// Each participant asynchronously sends and receive a number of samples. In the readers, when a minumm number of samples +// is received the destroy() method is called (abruptly). The test checks that the application finishes successfully +TEST(DDSStatistics, correct_deletion_upon_delete_contained_entities) +{ +#ifdef FASTDDS_STATISTICS + + //! Set environment variable and create participant using Qos set by code + const char* value = "HISTORY_LATENCY_TOPIC;NETWORK_LATENCY_TOPIC;" + "PUBLICATION_THROUGHPUT_TOPIC;SUBSCRIPTION_THROUGHPUT_TOPIC;RTPS_SENT_TOPIC;" + "RTPS_LOST_TOPIC;HEARTBEAT_COUNT_TOPIC;ACKNACK_COUNT_TOPIC;NACKFRAG_COUNT_TOPIC;" + "GAP_COUNT_TOPIC;DATA_COUNT_TOPIC;RESENT_DATAS_TOPIC;SAMPLE_DATAS_TOPIC;" + "PDP_PACKETS_TOPIC;EDP_PACKETS_TOPIC;DISCOVERY_TOPIC;PHYSICAL_DATA_TOPIC;"; + + #ifdef _WIN32 + ASSERT_EQ(0, _putenv_s("FASTDDS_STATISTICS", value)); + #else + ASSERT_EQ(0, setenv("FASTDDS_STATISTICS", value, 1)); + #endif // ifdef _WIN32 + + size_t n_participants = 5; + size_t n_participants_same_topic = 2; + + std::vector>> writers; + std::vector> readers; + + readers.reserve(n_participants); + writers.reserve(n_participants); + + std::vector> threads; + threads.reserve(2 * n_participants); + + for (size_t i = 0; i < n_participants; ++i) + { + size_t topic_number = (i < n_participants_same_topic) ? 0 : i; + + auto writer = std::make_shared>(TEST_TOPIC_NAME + std::to_string( + topic_number)); + auto reader = + std::make_shared(TEST_TOPIC_NAME + std::to_string(topic_number)); + + std::shared_ptr> data = std::make_shared>(default_helloworld_data_generator( + 10)); + + threads.emplace_back(std::make_shared([reader, data]() + { + reader->init(); + ASSERT_TRUE(reader->isInitialized()); + reader->startReception(data->size()); + reader->block_for_at_least(3); + reader->destroy(); + })); + + threads.emplace_back(std::make_shared([writer, data]() + { + writer->init(); + ASSERT_TRUE(writer->isInitialized()); + writer->wait_discovery(); + writer->send(*data, 10); + writer->destroy(); + })); + + writers.push_back(writer); + readers.push_back(reader); + } + + for (auto& thread : threads) + { + thread->join(); + } + +#endif // FASTDDS_STATISTICS +}