From 9961e6118323f35f6b45056cc4c9849edb453e49 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: Tue, 2 Jul 2024 14:18:36 +0200 Subject: [PATCH] Fix flaky latency tests on mac (#5009) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Refs #21232: Fix Latency test destruction Signed-off-by: Mario Dominguez * Refs #21232: Change local_reader() to return a BaseReader Signed-off-by: Mario Domínguez López * Refs #21232: Typo Signed-off-by: Mario Dominguez --------- Signed-off-by: Mario Dominguez Signed-off-by: Mario Domínguez López Signed-off-by: Mario Dominguez --- src/cpp/rtps/writer/ReaderLocator.cpp | 2 +- src/cpp/rtps/writer/ReaderLocator.hpp | 8 +-- src/cpp/rtps/writer/ReaderProxy.hpp | 3 +- src/cpp/rtps/writer/StatefulWriter.cpp | 4 +- .../rtps/writer/ReaderLocator.hpp | 4 +- .../latency/LatencyTestPublisher.cpp | 49 ++++++++++--------- .../latency/LatencyTestPublisher.hpp | 2 + .../latency/LatencyTestSubscriber.cpp | 49 ++++++++++--------- .../latency/LatencyTestSubscriber.hpp | 2 + test/performance/latency/main_LatencyTest.cpp | 9 ++++ 10 files changed, 76 insertions(+), 56 deletions(-) diff --git a/src/cpp/rtps/writer/ReaderLocator.cpp b/src/cpp/rtps/writer/ReaderLocator.cpp index 312a12b2d0c..8265b8d30a5 100644 --- a/src/cpp/rtps/writer/ReaderLocator.cpp +++ b/src/cpp/rtps/writer/ReaderLocator.cpp @@ -207,7 +207,7 @@ bool ReaderLocator::send( return true; } -RTPSReader* ReaderLocator::local_reader() +BaseReader* ReaderLocator::local_reader() { if (!local_reader_) { diff --git a/src/cpp/rtps/writer/ReaderLocator.hpp b/src/cpp/rtps/writer/ReaderLocator.hpp index 173966ec9bd..a1a0eccf811 100644 --- a/src/cpp/rtps/writer/ReaderLocator.hpp +++ b/src/cpp/rtps/writer/ReaderLocator.hpp @@ -31,7 +31,7 @@ namespace rtps { class RTPSParticipantImpl; class RTPSWriter; -class RTPSReader; +class BaseReader; class IDataSharingNotifier; /** @@ -67,10 +67,10 @@ class ReaderLocator : public RTPSMessageSenderInterface return is_local_reader_; } - RTPSReader* local_reader(); + BaseReader* local_reader(); void local_reader( - RTPSReader* local_reader) + BaseReader* local_reader) { local_reader_ = local_reader; } @@ -260,7 +260,7 @@ class ReaderLocator : public RTPSMessageSenderInterface LocatorSelectorEntry async_locator_info_; bool expects_inline_qos_; bool is_local_reader_; - RTPSReader* local_reader_; + BaseReader* local_reader_; std::vector guid_prefix_as_vector_; std::vector guid_as_vector_; IDataSharingNotifier* datasharing_notifier_; diff --git a/src/cpp/rtps/writer/ReaderProxy.hpp b/src/cpp/rtps/writer/ReaderProxy.hpp index 1883416f62e..8ebd60639cc 100644 --- a/src/cpp/rtps/writer/ReaderProxy.hpp +++ b/src/cpp/rtps/writer/ReaderProxy.hpp @@ -40,6 +40,7 @@ namespace eprosima { namespace fastdds { namespace rtps { +class BaseReader; class StatefulWriter; class TimedEvent; class RTPSReader; @@ -289,7 +290,7 @@ class ReaderProxy * Get the local reader on the same process (if any). * @return The local reader on the same process. */ - inline RTPSReader* local_reader() + inline BaseReader* local_reader() { return locator_info_.local_reader(); } diff --git a/src/cpp/rtps/writer/StatefulWriter.cpp b/src/cpp/rtps/writer/StatefulWriter.cpp index aa674715998..308be3b86e6 100644 --- a/src/cpp/rtps/writer/StatefulWriter.cpp +++ b/src/cpp/rtps/writer/StatefulWriter.cpp @@ -402,14 +402,14 @@ bool StatefulWriter::intraprocess_delivery( CacheChange_t* change, ReaderProxy* reader_proxy) { - RTPSReader* reader = reader_proxy->local_reader(); + BaseReader* reader = reader_proxy->local_reader(); if (reader) { if (change->write_params.related_sample_identity() != SampleIdentity::unknown()) { change->write_params.sample_identity(change->write_params.related_sample_identity()); } - return BaseReader::downcast(reader)->process_data_msg(change); + return reader->process_data_msg(change); } return false; } diff --git a/test/mock/rtps/ReaderLocator/rtps/writer/ReaderLocator.hpp b/test/mock/rtps/ReaderLocator/rtps/writer/ReaderLocator.hpp index 6e964c39127..4886ee24e26 100644 --- a/test/mock/rtps/ReaderLocator/rtps/writer/ReaderLocator.hpp +++ b/test/mock/rtps/ReaderLocator/rtps/writer/ReaderLocator.hpp @@ -33,7 +33,7 @@ namespace rtps { class RTPSParticipantImpl; class RTPSWriter; -class RTPSReader; +class BaseReader; class IDataSharingNotifier; /** @@ -202,7 +202,7 @@ class ReaderLocator : public RTPSMessageSenderInterface return false; } - RTPSReader* local_reader() + BaseReader* local_reader() { return nullptr; } diff --git a/test/performance/latency/LatencyTestPublisher.cpp b/test/performance/latency/LatencyTestPublisher.cpp index 3fa0212b8f6..9e1f79149b0 100644 --- a/test/performance/latency/LatencyTestPublisher.cpp +++ b/test/performance/latency/LatencyTestPublisher.cpp @@ -57,29 +57,6 @@ LatencyTestPublisher::LatencyTestPublisher() LatencyTestPublisher::~LatencyTestPublisher() { - // Static type endpoints shpuld have been removed for each payload iteration - if (dynamic_types_) - { - destroy_data_endpoints(); - } - else if (nullptr != data_writer_ - || nullptr != data_reader_ - || nullptr != latency_data_pub_topic_ - || nullptr != latency_data_sub_topic_ - || !latency_data_type_) - { - EPROSIMA_LOG_ERROR(LATENCYPUBLISHER, "ERROR unregistering the DATA type and/or removing the endpoints"); - } - - subscriber_->delete_datareader(command_reader_); - participant_->delete_subscriber(subscriber_); - - publisher_->delete_datawriter(command_writer_); - participant_->delete_publisher(publisher_); - - participant_->delete_topic(latency_command_sub_topic_); - participant_->delete_topic(latency_command_pub_topic_); - std::string TestCommandType("TestCommandType"); participant_->unregister_type(TestCommandType); @@ -682,6 +659,32 @@ void LatencyTestPublisher::run() } } +void LatencyTestPublisher::destroy_user_entities() +{ + // Static type endpoints should have been removed for each payload iteration + if (dynamic_types_) + { + destroy_data_endpoints(); + } + else if (nullptr != data_writer_ + || nullptr != data_reader_ + || nullptr != latency_data_pub_topic_ + || nullptr != latency_data_sub_topic_ + || !latency_data_type_) + { + EPROSIMA_LOG_ERROR(LATENCYPUBLISHER, "ERROR unregistering the DATA type and/or removing the endpoints"); + } + + subscriber_->delete_datareader(command_reader_); + participant_->delete_subscriber(subscriber_); + + publisher_->delete_datawriter(command_writer_); + participant_->delete_publisher(publisher_); + + participant_->delete_topic(latency_command_sub_topic_); + participant_->delete_topic(latency_command_pub_topic_); +} + void LatencyTestPublisher::export_csv( const std::string& data_name, const std::string& str_reliable, diff --git a/test/performance/latency/LatencyTestPublisher.hpp b/test/performance/latency/LatencyTestPublisher.hpp index 0c2866cdf0c..89008c73bc1 100644 --- a/test/performance/latency/LatencyTestPublisher.hpp +++ b/test/performance/latency/LatencyTestPublisher.hpp @@ -100,6 +100,8 @@ class LatencyTestPublisher void run(); + void destroy_user_entities(); + private: bool init_dynamic_types(); diff --git a/test/performance/latency/LatencyTestSubscriber.cpp b/test/performance/latency/LatencyTestSubscriber.cpp index 1a0723206a5..5217e3fbd72 100644 --- a/test/performance/latency/LatencyTestSubscriber.cpp +++ b/test/performance/latency/LatencyTestSubscriber.cpp @@ -51,29 +51,6 @@ LatencyTestSubscriber::LatencyTestSubscriber() LatencyTestSubscriber::~LatencyTestSubscriber() { - // Static type endpoints should have been remove for each payload iteration - if (dynamic_types_) - { - destroy_data_endpoints(); - } - else if (nullptr != data_writer_ - || nullptr != data_reader_ - || nullptr != latency_data_pub_topic_ - || nullptr != latency_data_sub_topic_ - || !latency_data_type_) - { - EPROSIMA_LOG_ERROR(LATENCYSUBSCRIBER, "ERROR unregistering the DATA type and/or removing the endpoints"); - } - - subscriber_->delete_datareader(command_reader_); - participant_->delete_subscriber(subscriber_); - - publisher_->delete_datawriter(command_writer_); - participant_->delete_publisher(publisher_); - - participant_->delete_topic(latency_command_sub_topic_); - participant_->delete_topic(latency_command_pub_topic_); - std::string TestCommandType("TestCommandType"); participant_->unregister_type(TestCommandType); @@ -641,6 +618,32 @@ void LatencyTestSubscriber::run() } } +void LatencyTestSubscriber::destroy_user_entities() +{ + // Static type endpoints should have been remove for each payload iteration + if (dynamic_types_) + { + destroy_data_endpoints(); + } + else if (nullptr != data_writer_ + || nullptr != data_reader_ + || nullptr != latency_data_pub_topic_ + || nullptr != latency_data_sub_topic_ + || !latency_data_type_) + { + EPROSIMA_LOG_ERROR(LATENCYSUBSCRIBER, "ERROR unregistering the DATA type and/or removing the endpoints"); + } + + subscriber_->delete_datareader(command_reader_); + participant_->delete_subscriber(subscriber_); + + publisher_->delete_datawriter(command_writer_); + participant_->delete_publisher(publisher_); + + participant_->delete_topic(latency_command_sub_topic_); + participant_->delete_topic(latency_command_pub_topic_); +} + bool LatencyTestSubscriber::test( uint32_t datasize) { diff --git a/test/performance/latency/LatencyTestSubscriber.hpp b/test/performance/latency/LatencyTestSubscriber.hpp index e3545feb3fe..778502d946e 100644 --- a/test/performance/latency/LatencyTestSubscriber.hpp +++ b/test/performance/latency/LatencyTestSubscriber.hpp @@ -60,6 +60,8 @@ class LatencyTestSubscriber void run(); + void destroy_user_entities(); + bool test( uint32_t datasize); diff --git a/test/performance/latency/main_LatencyTest.cpp b/test/performance/latency/main_LatencyTest.cpp index 99744551997..c87bf782fc2 100644 --- a/test/performance/latency/main_LatencyTest.cpp +++ b/test/performance/latency/main_LatencyTest.cpp @@ -503,6 +503,7 @@ int main( dynamic_types, data_sharing, data_loans, shared_memory, forced_domain, data_sizes)) { latency_publisher.run(); + latency_publisher.destroy_user_entities(); } else { @@ -518,6 +519,7 @@ int main( xml_config_file, dynamic_types, data_sharing, data_loans, shared_memory, forced_domain, data_sizes)) { latency_subscriber.run(); + latency_subscriber.destroy_user_entities(); } else { @@ -568,6 +570,13 @@ int main( { sub.join(); } + + for (auto& sub : latency_subscribers) + { + sub->destroy_user_entities(); + } + + latency_publisher.destroy_user_entities(); } else {