Skip to content

Commit

Permalink
Properly delete builtin statistics writers upon `delete_contained_ent…
Browse files Browse the repository at this point in the history
…ities()` (#4891)

* Refs #20816: Add BB test

Signed-off-by: Mario Dominguez <mariodominguez@eprosima.com>

* Refs #20816: Fix

Signed-off-by: Mario Dominguez <mariodominguez@eprosima.com>

* Refs #20816: Apply Edu's suggestion

Signed-off-by: Mario Dominguez <mariodominguez@eprosima.com>

---------

Signed-off-by: Mario Dominguez <mariodominguez@eprosima.com>
(cherry picked from commit 0d62335)

Signed-off-by: eduponz <eduardoponz@eprosima.com>
  • Loading branch information
Mario-DL authored and EduPonz committed Jun 8, 2024
1 parent a1d4cd9 commit 911278e
Show file tree
Hide file tree
Showing 3 changed files with 98 additions and 9 deletions.
11 changes: 2 additions & 9 deletions src/cpp/statistics/fastdds/domain/DomainParticipantImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
2 changes: 2 additions & 0 deletions src/cpp/statistics/fastdds/domain/DomainParticipantImpl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
94 changes: 94 additions & 0 deletions test/blackbox/common/DDSBlackboxTestsStatistics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<HelloWorldPubSubType>
{
public:

CustomStatisticsParticipantSubscriber(
const std::string& topic_name)
: PubSubReader<HelloWorldPubSubType>(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<std::shared_ptr<PubSubWriter<HelloWorldPubSubType>>> writers;
std::vector<std::shared_ptr<CustomStatisticsParticipantSubscriber>> readers;

readers.reserve(n_participants);
writers.reserve(n_participants);

std::vector<std::shared_ptr<std::thread>> 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<PubSubWriter<HelloWorldPubSubType>>(TEST_TOPIC_NAME + std::to_string(
topic_number));
auto reader =
std::make_shared<CustomStatisticsParticipantSubscriber>(TEST_TOPIC_NAME + std::to_string(topic_number));

std::shared_ptr<std::list<HelloWorld>> data = std::make_shared<std::list<HelloWorld>>(default_helloworld_data_generator(
10));

threads.emplace_back(std::make_shared<std::thread>([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<std::thread>([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
}

0 comments on commit 911278e

Please sign in to comment.