From decbee75e229af2c8bb9aa762b6327a304284fa5 Mon Sep 17 00:00:00 2001 From: Irene Bandera Date: Tue, 30 Jan 2024 15:28:34 +0100 Subject: [PATCH 1/6] Add listener classes to handle cache change acknowledgments Signed-off-by: Irene Bandera --- .../builtin/typelookup/TypeLookupManager.hpp | 10 ++++++ .../typelookup/TypeLookupReplyListener.hpp | 32 +++++++++++++++++ .../typelookup/TypeLookupRequestListener.hpp | 32 +++++++++++++++++ .../builtin/typelookup/TypeLookupManager.cpp | 34 ++++++++++++++++--- .../typelookup/TypeLookupReplyListener.cpp | 18 ++++++++++ .../typelookup/TypeLookupRequestListener.cpp | 18 ++++++++++ 6 files changed, 140 insertions(+), 4 deletions(-) diff --git a/include/fastdds/dds/builtin/typelookup/TypeLookupManager.hpp b/include/fastdds/dds/builtin/typelookup/TypeLookupManager.hpp index 7c955fc95d0..42ec2b277cf 100644 --- a/include/fastdds/dds/builtin/typelookup/TypeLookupManager.hpp +++ b/include/fastdds/dds/builtin/typelookup/TypeLookupManager.hpp @@ -60,7 +60,9 @@ extern const fastrtps::rtps::SampleIdentity INVALID_SAMPLE_IDENTITY; class TypeLookupManager { friend class TypeLookupRequestListener; + friend class TypeLookupRequestWListener; friend class TypeLookupReplyListener; + friend class TypeLookupReplyWListener; public: @@ -243,9 +245,11 @@ class TypeLookupManager //!Request Listener object. TypeLookupRequestListener* request_listener_; + TypeLookupRequestWListener* request_wlistener_; //!Reply Listener object. TypeLookupReplyListener* reply_listener_; + TypeLookupReplyWListener* reply_wlistener_; std::mutex temp_data_lock_; fastrtps::rtps::ReaderProxyData temp_reader_proxy_data_; @@ -286,6 +290,12 @@ class TypeLookupManager bool create_secure_endpoints(); #endif */ + + void request_cache_change_acked( + fastrtps::rtps::CacheChange_t* change); + + void reply_cache_change_acked( + fastrtps::rtps::CacheChange_t* change); }; } /* namespace builtin */ diff --git a/include/fastdds/dds/builtin/typelookup/TypeLookupReplyListener.hpp b/include/fastdds/dds/builtin/typelookup/TypeLookupReplyListener.hpp index 3509f6fa1e4..f1aa8eb8bba 100644 --- a/include/fastdds/dds/builtin/typelookup/TypeLookupReplyListener.hpp +++ b/include/fastdds/dds/builtin/typelookup/TypeLookupReplyListener.hpp @@ -21,6 +21,8 @@ #define TYPELOOKUP_REPLY_LISTENER_HPP_ #ifndef DOXYGEN_SHOULD_SKIP_THIS_PUBLIC #include +#include + namespace eprosima { namespace fastrtps { @@ -81,6 +83,36 @@ class TypeLookupReplyListener : public fastrtps::rtps::ReaderListener fastrtps::types::TypeObjectFactory* factory_; }; +class TypeLookupReplyWListener : public fastrtps::rtps::WriterListener +{ +public: + + /** + * @brief Constructor + * @param pwlp Pointer to the writer liveliness protocol + */ + TypeLookupReplyWListener( + TypeLookupManager* pwlp); + + /** + * @brief Destructor + */ + virtual ~TypeLookupReplyWListener() override; + + void onWriterChangeReceivedByAll( + fastrtps::rtps::RTPSWriter*, + fastrtps::rtps::CacheChange_t* change) override; + +private: + + //! A pointer to the typelookup manager + TypeLookupManager* tlm_; + + //! A pointer to the TypeObject factory. + fastrtps::types::TypeObjectFactory* factory_; + +}; + } /* namespace builtin */ } /* namespace dds */ } /* namespace fastdds */ diff --git a/include/fastdds/dds/builtin/typelookup/TypeLookupRequestListener.hpp b/include/fastdds/dds/builtin/typelookup/TypeLookupRequestListener.hpp index 23ffd24139b..581abec7d25 100644 --- a/include/fastdds/dds/builtin/typelookup/TypeLookupRequestListener.hpp +++ b/include/fastdds/dds/builtin/typelookup/TypeLookupRequestListener.hpp @@ -21,6 +21,8 @@ #define TYPELOOKUP_REQUEST_LISTENER_HPP_ #ifndef DOXYGEN_SHOULD_SKIP_THIS_PUBLIC #include +#include + namespace eprosima { namespace fastrtps { @@ -82,6 +84,36 @@ class TypeLookupRequestListener : public fastrtps::rtps::ReaderListener }; +class TypeLookupRequestWListener : public fastrtps::rtps::WriterListener +{ +public: + + /** + * @brief Constructor + * @param pwlp Pointer to the writer liveliness protocol + */ + TypeLookupRequestWListener( + TypeLookupManager* pwlp); + + /** + * @brief Destructor + */ + virtual ~TypeLookupRequestWListener() override; + + void onWriterChangeReceivedByAll( + fastrtps::rtps::RTPSWriter*, + fastrtps::rtps::CacheChange_t* change) override; + +private: + + //! A pointer to the typelookup manager + TypeLookupManager* tlm_; + + //! A pointer to the TypeObject factory. + fastrtps::types::TypeObjectFactory* factory_; + +}; + } /* namespace builtin */ } /* namespace dds */ } /* namespace fastdds */ diff --git a/src/cpp/fastdds/builtin/typelookup/TypeLookupManager.cpp b/src/cpp/fastdds/builtin/typelookup/TypeLookupManager.cpp index 8ae2c618c54..0432945e766 100644 --- a/src/cpp/fastdds/builtin/typelookup/TypeLookupManager.cpp +++ b/src/cpp/fastdds/builtin/typelookup/TypeLookupManager.cpp @@ -348,6 +348,7 @@ bool TypeLookupManager::create_endpoints() // Built-in request writer if (builtin_protocols_->m_att.typelookup_config.use_client) { + request_wlistener_ = new TypeLookupRequestWListener(this); builtin_request_writer_history_ = new WriterHistory(hatt); RTPSWriter* req_writer; @@ -355,7 +356,7 @@ bool TypeLookupManager::create_endpoints() &req_writer, watt, builtin_request_writer_history_, - nullptr, + request_wlistener_, fastrtps::rtps::c_EntityId_TypeLookup_request_writer, true)) { @@ -374,6 +375,7 @@ bool TypeLookupManager::create_endpoints() // Built-in reply writer if (builtin_protocols_->m_att.typelookup_config.use_server) { + reply_wlistener_ = new TypeLookupReplyWListener(this); builtin_reply_writer_history_ = new WriterHistory(hatt); RTPSWriter* rep_writer; @@ -381,7 +383,7 @@ bool TypeLookupManager::create_endpoints() &rep_writer, watt, builtin_reply_writer_history_, - nullptr, + reply_wlistener_, fastrtps::rtps::c_EntityId_TypeLookup_reply_writer, true)) { @@ -567,7 +569,13 @@ bool TypeLookupManager::send_request( SerializedPayload_t payload; payload.max_size = change->serializedPayload.max_size - 4; payload.data = change->serializedPayload.data + 4; - if (valid && request_type_.serialize(&req, &payload, DataRepresentationId_t::XCDR2_DATA_REPRESENTATION)) + + bool serialize_ret = request_type_.serialize(&req, &payload, DataRepresentationId_t::XCDR_DATA_REPRESENTATION); + if (!serialize_ret) + { + payload.data = nullptr; + } + else if (valid) { change->serializedPayload.length += payload.length; change->serializedPayload.pos += payload.pos; @@ -610,7 +618,13 @@ bool TypeLookupManager::send_reply( SerializedPayload_t payload; payload.max_size = change->serializedPayload.max_size - 4; payload.data = change->serializedPayload.data + 4; - if (valid && reply_type_.serialize(&rep, &payload, DataRepresentationId_t::XCDR2_DATA_REPRESENTATION)) + + bool serialize_ret = reply_type_.serialize(&rep, &payload, DataRepresentationId_t::XCDR_DATA_REPRESENTATION); + if (!serialize_ret) + { + payload.data = nullptr; + } + else if (valid) { change->serializedPayload.length += payload.length; change->serializedPayload.pos += payload.pos; @@ -695,6 +709,18 @@ const fastrtps::rtps::GUID_t& TypeLookupManager::get_builtin_request_writer_guid return c_Guid_Unknown; } +void TypeLookupManager::request_cache_change_acked( + fastrtps::rtps::CacheChange_t* change) +{ + builtin_request_writer_history_->remove_change(change); +} + +void TypeLookupManager::reply_cache_change_acked( +fastrtps::rtps::CacheChange_t* change) +{ + builtin_reply_writer_history_->remove_change(change); +} + } // namespace builtin } // namespace dds } // namespace fastdds diff --git a/src/cpp/fastdds/builtin/typelookup/TypeLookupReplyListener.cpp b/src/cpp/fastdds/builtin/typelookup/TypeLookupReplyListener.cpp index 7afc3f194ce..1c894e45b0f 100644 --- a/src/cpp/fastdds/builtin/typelookup/TypeLookupReplyListener.cpp +++ b/src/cpp/fastdds/builtin/typelookup/TypeLookupReplyListener.cpp @@ -116,6 +116,24 @@ void TypeLookupReplyListener::onNewCacheChangeAdded( reader->getHistory()->remove_change(change); } +TypeLookupReplyWListener::TypeLookupReplyWListener( + TypeLookupManager* manager) + : tlm_(manager) + , factory_(TypeObjectFactory::get_instance()) +{ +} + +TypeLookupReplyWListener::~TypeLookupReplyWListener() +{ +} + +void TypeLookupReplyWListener::onWriterChangeReceivedByAll( + fastrtps::rtps::RTPSWriter*, + fastrtps::rtps::CacheChange_t* change) +{ + tlm_->reply_cache_change_acked(change); +} + } // namespace builtin } // namespace dds } // namespace fastdds diff --git a/src/cpp/fastdds/builtin/typelookup/TypeLookupRequestListener.cpp b/src/cpp/fastdds/builtin/typelookup/TypeLookupRequestListener.cpp index 835eae0b350..c1bc81071cf 100644 --- a/src/cpp/fastdds/builtin/typelookup/TypeLookupRequestListener.cpp +++ b/src/cpp/fastdds/builtin/typelookup/TypeLookupRequestListener.cpp @@ -150,6 +150,24 @@ void TypeLookupRequestListener::onNewCacheChangeAdded( reader->getHistory()->remove_change(change); } +TypeLookupRequestWListener::TypeLookupRequestWListener( + TypeLookupManager* manager) + : tlm_(manager) + , factory_(TypeObjectFactory::get_instance()) +{ +} + +TypeLookupRequestWListener::~TypeLookupRequestWListener() +{ +} + +void TypeLookupRequestWListener::onWriterChangeReceivedByAll( + fastrtps::rtps::RTPSWriter*, + fastrtps::rtps::CacheChange_t* change) +{ + tlm_->request_cache_change_acked(change); +} + } // namespace builtin } // namespace dds } // namespace fastdds From 484b608bab3066cafb329656b9da7baa013f5d83 Mon Sep 17 00:00:00 2001 From: Irene Bandera Date: Wed, 31 Jan 2024 08:59:19 +0100 Subject: [PATCH 2/6] Uncrustify Signed-off-by: Irene Bandera --- src/cpp/fastdds/builtin/typelookup/TypeLookupManager.cpp | 2 +- .../fastdds/builtin/typelookup/TypeLookupReplyListener.cpp | 4 ++-- .../fastdds/builtin/typelookup/TypeLookupRequestListener.cpp | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/cpp/fastdds/builtin/typelookup/TypeLookupManager.cpp b/src/cpp/fastdds/builtin/typelookup/TypeLookupManager.cpp index 0432945e766..6fe1df5d7e7 100644 --- a/src/cpp/fastdds/builtin/typelookup/TypeLookupManager.cpp +++ b/src/cpp/fastdds/builtin/typelookup/TypeLookupManager.cpp @@ -716,7 +716,7 @@ void TypeLookupManager::request_cache_change_acked( } void TypeLookupManager::reply_cache_change_acked( -fastrtps::rtps::CacheChange_t* change) + fastrtps::rtps::CacheChange_t* change) { builtin_reply_writer_history_->remove_change(change); } diff --git a/src/cpp/fastdds/builtin/typelookup/TypeLookupReplyListener.cpp b/src/cpp/fastdds/builtin/typelookup/TypeLookupReplyListener.cpp index 1c894e45b0f..bdd1c6f6e79 100644 --- a/src/cpp/fastdds/builtin/typelookup/TypeLookupReplyListener.cpp +++ b/src/cpp/fastdds/builtin/typelookup/TypeLookupReplyListener.cpp @@ -128,8 +128,8 @@ TypeLookupReplyWListener::~TypeLookupReplyWListener() } void TypeLookupReplyWListener::onWriterChangeReceivedByAll( - fastrtps::rtps::RTPSWriter*, - fastrtps::rtps::CacheChange_t* change) + fastrtps::rtps::RTPSWriter*, + fastrtps::rtps::CacheChange_t* change) { tlm_->reply_cache_change_acked(change); } diff --git a/src/cpp/fastdds/builtin/typelookup/TypeLookupRequestListener.cpp b/src/cpp/fastdds/builtin/typelookup/TypeLookupRequestListener.cpp index c1bc81071cf..cb3a49c98b6 100644 --- a/src/cpp/fastdds/builtin/typelookup/TypeLookupRequestListener.cpp +++ b/src/cpp/fastdds/builtin/typelookup/TypeLookupRequestListener.cpp @@ -162,8 +162,8 @@ TypeLookupRequestWListener::~TypeLookupRequestWListener() } void TypeLookupRequestWListener::onWriterChangeReceivedByAll( - fastrtps::rtps::RTPSWriter*, - fastrtps::rtps::CacheChange_t* change) + fastrtps::rtps::RTPSWriter*, + fastrtps::rtps::CacheChange_t* change) { tlm_->request_cache_change_acked(change); } From e0b9c234a0cce0751e5653d333ee15883fb91f92 Mon Sep 17 00:00:00 2001 From: Irene Bandera Date: Wed, 31 Jan 2024 09:03:36 +0100 Subject: [PATCH 3/6] Fix memory leaks in TypeLookupManager.cpp --- .../fastdds/dds/builtin/typelookup/TypeLookupManager.hpp | 4 ++-- .../dds/builtin/typelookup/TypeLookupReplyListener.hpp | 4 ++-- .../dds/builtin/typelookup/TypeLookupRequestListener.hpp | 4 ++-- src/cpp/fastdds/builtin/typelookup/TypeLookupManager.cpp | 9 +++++++++ 4 files changed, 15 insertions(+), 6 deletions(-) diff --git a/include/fastdds/dds/builtin/typelookup/TypeLookupManager.hpp b/include/fastdds/dds/builtin/typelookup/TypeLookupManager.hpp index 42ec2b277cf..edaa012a416 100644 --- a/include/fastdds/dds/builtin/typelookup/TypeLookupManager.hpp +++ b/include/fastdds/dds/builtin/typelookup/TypeLookupManager.hpp @@ -292,10 +292,10 @@ class TypeLookupManager */ void request_cache_change_acked( - fastrtps::rtps::CacheChange_t* change); + fastrtps::rtps::CacheChange_t* change); void reply_cache_change_acked( - fastrtps::rtps::CacheChange_t* change); + fastrtps::rtps::CacheChange_t* change); }; } /* namespace builtin */ diff --git a/include/fastdds/dds/builtin/typelookup/TypeLookupReplyListener.hpp b/include/fastdds/dds/builtin/typelookup/TypeLookupReplyListener.hpp index f1aa8eb8bba..b3fa030ea89 100644 --- a/include/fastdds/dds/builtin/typelookup/TypeLookupReplyListener.hpp +++ b/include/fastdds/dds/builtin/typelookup/TypeLookupReplyListener.hpp @@ -72,7 +72,7 @@ class TypeLookupReplyListener : public fastrtps::rtps::ReaderListener */ void onNewCacheChangeAdded( fastrtps::rtps::RTPSReader* reader, - const fastrtps::rtps::CacheChange_t* const change) override; + const fastrtps::rtps::CacheChange_t* const change) override; private: @@ -117,5 +117,5 @@ class TypeLookupReplyWListener : public fastrtps::rtps::WriterListener } /* namespace dds */ } /* namespace fastdds */ } /* namespace eprosima */ -#endif +#endif // ifndef DOXYGEN_SHOULD_SKIP_THIS_PUBLIC #endif /* TYPELOOKUP_REPLY_LISTENER_HPP_*/ diff --git a/include/fastdds/dds/builtin/typelookup/TypeLookupRequestListener.hpp b/include/fastdds/dds/builtin/typelookup/TypeLookupRequestListener.hpp index 581abec7d25..ff98d553d5b 100644 --- a/include/fastdds/dds/builtin/typelookup/TypeLookupRequestListener.hpp +++ b/include/fastdds/dds/builtin/typelookup/TypeLookupRequestListener.hpp @@ -72,7 +72,7 @@ class TypeLookupRequestListener : public fastrtps::rtps::ReaderListener */ void onNewCacheChangeAdded( fastrtps::rtps::RTPSReader* reader, - const fastrtps::rtps::CacheChange_t* const change) override; + const fastrtps::rtps::CacheChange_t* const change) override; private: @@ -118,5 +118,5 @@ class TypeLookupRequestWListener : public fastrtps::rtps::WriterListener } /* namespace dds */ } /* namespace fastdds */ } /* namespace eprosima */ -#endif +#endif // ifndef DOXYGEN_SHOULD_SKIP_THIS_PUBLIC #endif /* TYPELOOKUP_REQUEST_LISTENER_HPP_*/ diff --git a/src/cpp/fastdds/builtin/typelookup/TypeLookupManager.cpp b/src/cpp/fastdds/builtin/typelookup/TypeLookupManager.cpp index 6fe1df5d7e7..a167dea6a05 100644 --- a/src/cpp/fastdds/builtin/typelookup/TypeLookupManager.cpp +++ b/src/cpp/fastdds/builtin/typelookup/TypeLookupManager.cpp @@ -68,7 +68,9 @@ TypeLookupManager::TypeLookupManager( , builtin_request_reader_history_(nullptr) , builtin_reply_reader_history_(nullptr) , request_listener_(nullptr) + , request_wlistener_(nullptr) , reply_listener_(nullptr) + , reply_wlistener_(nullptr) , temp_reader_proxy_data_( prot->mp_participantImpl->getRTPSParticipantAttributes().allocation.locators.max_unicast_locators, prot->mp_participantImpl->getRTPSParticipantAttributes().allocation.locators.max_multicast_locators) @@ -125,6 +127,9 @@ TypeLookupManager::~TypeLookupManager() delete builtin_request_reader_history_; delete builtin_reply_reader_history_; + delete request_wlistener_; + delete reply_wlistener_; + delete reply_listener_; delete request_listener_; } @@ -368,6 +373,8 @@ bool TypeLookupManager::create_endpoints() EPROSIMA_LOG_ERROR(TYPELOOKUP_SERVICE, "Typelookup request writer creation failed."); delete builtin_request_writer_history_; builtin_request_writer_history_ = nullptr; + delete request_wlistener_; + request_wlistener_ = nullptr; return false; } } @@ -395,6 +402,8 @@ bool TypeLookupManager::create_endpoints() EPROSIMA_LOG_ERROR(TYPELOOKUP_SERVICE, "Typelookup reply writer creation failed."); delete builtin_reply_writer_history_; builtin_reply_writer_history_ = nullptr; + delete reply_wlistener_; + reply_wlistener_ = nullptr; return false; } } From cf01f8782e298ad5e1ab355073a59cd6257da68a Mon Sep 17 00:00:00 2001 From: Irene Bandera Date: Wed, 21 Feb 2024 11:24:31 +0100 Subject: [PATCH 4/6] Apply changes Signed-off-by: Irene Bandera --- .../builtin/typelookup/TypeLookupManager.hpp | 4 --- .../typelookup/TypeLookupReplyListener.hpp | 29 +++---------------- .../typelookup/TypeLookupRequestListener.hpp | 29 +++---------------- .../builtin/typelookup/TypeLookupManager.cpp | 25 +++++++--------- .../typelookup/TypeLookupReplyListener.cpp | 13 +-------- .../typelookup/TypeLookupRequestListener.cpp | 13 +-------- 6 files changed, 20 insertions(+), 93 deletions(-) diff --git a/include/fastdds/dds/builtin/typelookup/TypeLookupManager.hpp b/include/fastdds/dds/builtin/typelookup/TypeLookupManager.hpp index edaa012a416..c6465905c80 100644 --- a/include/fastdds/dds/builtin/typelookup/TypeLookupManager.hpp +++ b/include/fastdds/dds/builtin/typelookup/TypeLookupManager.hpp @@ -60,9 +60,7 @@ extern const fastrtps::rtps::SampleIdentity INVALID_SAMPLE_IDENTITY; class TypeLookupManager { friend class TypeLookupRequestListener; - friend class TypeLookupRequestWListener; friend class TypeLookupReplyListener; - friend class TypeLookupReplyWListener; public: @@ -245,11 +243,9 @@ class TypeLookupManager //!Request Listener object. TypeLookupRequestListener* request_listener_; - TypeLookupRequestWListener* request_wlistener_; //!Reply Listener object. TypeLookupReplyListener* reply_listener_; - TypeLookupReplyWListener* reply_wlistener_; std::mutex temp_data_lock_; fastrtps::rtps::ReaderProxyData temp_reader_proxy_data_; diff --git a/include/fastdds/dds/builtin/typelookup/TypeLookupReplyListener.hpp b/include/fastdds/dds/builtin/typelookup/TypeLookupReplyListener.hpp index b3fa030ea89..adc4b6c6499 100644 --- a/include/fastdds/dds/builtin/typelookup/TypeLookupReplyListener.hpp +++ b/include/fastdds/dds/builtin/typelookup/TypeLookupReplyListener.hpp @@ -49,7 +49,7 @@ class TypeLookupManager; * Class TypeLookupReplyListener that receives the typelookup request messages of remote endpoints. * @ingroup TYPES_MODULE */ -class TypeLookupReplyListener : public fastrtps::rtps::ReaderListener +class TypeLookupReplyListener : public fastrtps::rtps::ReaderListener, public fastrtps::rtps::WriterListener { public: @@ -74,31 +74,11 @@ class TypeLookupReplyListener : public fastrtps::rtps::ReaderListener fastrtps::rtps::RTPSReader* reader, const fastrtps::rtps::CacheChange_t* const change) override; -private: - - //! A pointer to the typelookup manager - TypeLookupManager* tlm_; - - //! A pointer to the TypeObject factory. - fastrtps::types::TypeObjectFactory* factory_; -}; - -class TypeLookupReplyWListener : public fastrtps::rtps::WriterListener -{ -public: - /** - * @brief Constructor - * @param pwlp Pointer to the writer liveliness protocol - */ - TypeLookupReplyWListener( - TypeLookupManager* pwlp); - - /** - * @brief Destructor + * @brief This method is called when all the readers matched with this Writer acknowledge that a cache + * change has been received. + * @param change The cache change */ - virtual ~TypeLookupReplyWListener() override; - void onWriterChangeReceivedByAll( fastrtps::rtps::RTPSWriter*, fastrtps::rtps::CacheChange_t* change) override; @@ -110,7 +90,6 @@ class TypeLookupReplyWListener : public fastrtps::rtps::WriterListener //! A pointer to the TypeObject factory. fastrtps::types::TypeObjectFactory* factory_; - }; } /* namespace builtin */ diff --git a/include/fastdds/dds/builtin/typelookup/TypeLookupRequestListener.hpp b/include/fastdds/dds/builtin/typelookup/TypeLookupRequestListener.hpp index ff98d553d5b..87741fc84c5 100644 --- a/include/fastdds/dds/builtin/typelookup/TypeLookupRequestListener.hpp +++ b/include/fastdds/dds/builtin/typelookup/TypeLookupRequestListener.hpp @@ -49,7 +49,7 @@ class TypeLookupManager; * Class TypeLookupRequestListener that receives the typelookup request messages of remote endpoints. * @ingroup TYPES_MODULE */ -class TypeLookupRequestListener : public fastrtps::rtps::ReaderListener +class TypeLookupRequestListener : public fastrtps::rtps::ReaderListener, public fastrtps::rtps::WriterListener { public: @@ -74,32 +74,11 @@ class TypeLookupRequestListener : public fastrtps::rtps::ReaderListener fastrtps::rtps::RTPSReader* reader, const fastrtps::rtps::CacheChange_t* const change) override; -private: - - //! A pointer to the typelookup manager - TypeLookupManager* tlm_; - - //! A pointer to the TypeObject factory. - fastrtps::types::TypeObjectFactory* factory_; - -}; - -class TypeLookupRequestWListener : public fastrtps::rtps::WriterListener -{ -public: - /** - * @brief Constructor - * @param pwlp Pointer to the writer liveliness protocol - */ - TypeLookupRequestWListener( - TypeLookupManager* pwlp); - - /** - * @brief Destructor + * @brief This method is called when all the readers matched with this Writer acknowledge that a cache + * change has been received. + * @param change The cache change */ - virtual ~TypeLookupRequestWListener() override; - void onWriterChangeReceivedByAll( fastrtps::rtps::RTPSWriter*, fastrtps::rtps::CacheChange_t* change) override; diff --git a/src/cpp/fastdds/builtin/typelookup/TypeLookupManager.cpp b/src/cpp/fastdds/builtin/typelookup/TypeLookupManager.cpp index a167dea6a05..71b3b064b23 100644 --- a/src/cpp/fastdds/builtin/typelookup/TypeLookupManager.cpp +++ b/src/cpp/fastdds/builtin/typelookup/TypeLookupManager.cpp @@ -68,9 +68,7 @@ TypeLookupManager::TypeLookupManager( , builtin_request_reader_history_(nullptr) , builtin_reply_reader_history_(nullptr) , request_listener_(nullptr) - , request_wlistener_(nullptr) , reply_listener_(nullptr) - , reply_wlistener_(nullptr) , temp_reader_proxy_data_( prot->mp_participantImpl->getRTPSParticipantAttributes().allocation.locators.max_unicast_locators, prot->mp_participantImpl->getRTPSParticipantAttributes().allocation.locators.max_multicast_locators) @@ -127,9 +125,6 @@ TypeLookupManager::~TypeLookupManager() delete builtin_request_reader_history_; delete builtin_reply_reader_history_; - delete request_wlistener_; - delete reply_wlistener_; - delete reply_listener_; delete request_listener_; } @@ -353,7 +348,7 @@ bool TypeLookupManager::create_endpoints() // Built-in request writer if (builtin_protocols_->m_att.typelookup_config.use_client) { - request_wlistener_ = new TypeLookupRequestWListener(this); + request_listener_ = new TypeLookupRequestListener(this); builtin_request_writer_history_ = new WriterHistory(hatt); RTPSWriter* req_writer; @@ -361,7 +356,7 @@ bool TypeLookupManager::create_endpoints() &req_writer, watt, builtin_request_writer_history_, - request_wlistener_, + request_listener_, fastrtps::rtps::c_EntityId_TypeLookup_request_writer, true)) { @@ -373,8 +368,8 @@ bool TypeLookupManager::create_endpoints() EPROSIMA_LOG_ERROR(TYPELOOKUP_SERVICE, "Typelookup request writer creation failed."); delete builtin_request_writer_history_; builtin_request_writer_history_ = nullptr; - delete request_wlistener_; - request_wlistener_ = nullptr; + delete request_listener_; + request_listener_ = nullptr; return false; } } @@ -382,7 +377,7 @@ bool TypeLookupManager::create_endpoints() // Built-in reply writer if (builtin_protocols_->m_att.typelookup_config.use_server) { - reply_wlistener_ = new TypeLookupReplyWListener(this); + reply_listener_ = new TypeLookupReplyListener(this); builtin_reply_writer_history_ = new WriterHistory(hatt); RTPSWriter* rep_writer; @@ -390,7 +385,7 @@ bool TypeLookupManager::create_endpoints() &rep_writer, watt, builtin_reply_writer_history_, - reply_wlistener_, + reply_listener_, fastrtps::rtps::c_EntityId_TypeLookup_reply_writer, true)) { @@ -402,8 +397,8 @@ bool TypeLookupManager::create_endpoints() EPROSIMA_LOG_ERROR(TYPELOOKUP_SERVICE, "Typelookup reply writer creation failed."); delete builtin_reply_writer_history_; builtin_reply_writer_history_ = nullptr; - delete reply_wlistener_; - reply_wlistener_ = nullptr; + delete reply_listener_; + reply_listener_ = nullptr; return false; } } @@ -579,7 +574,7 @@ bool TypeLookupManager::send_request( payload.max_size = change->serializedPayload.max_size - 4; payload.data = change->serializedPayload.data + 4; - bool serialize_ret = request_type_.serialize(&req, &payload, DataRepresentationId_t::XCDR_DATA_REPRESENTATION); + bool serialize_ret = request_type_.serialize(&req, &payload, DataRepresentationId_t::XCDR2_DATA_REPRESENTATION); if (!serialize_ret) { payload.data = nullptr; @@ -628,7 +623,7 @@ bool TypeLookupManager::send_reply( payload.max_size = change->serializedPayload.max_size - 4; payload.data = change->serializedPayload.data + 4; - bool serialize_ret = reply_type_.serialize(&rep, &payload, DataRepresentationId_t::XCDR_DATA_REPRESENTATION); + bool serialize_ret = reply_type_.serialize(&rep, &payload, DataRepresentationId_t::XCDR2_DATA_REPRESENTATION); if (!serialize_ret) { payload.data = nullptr; diff --git a/src/cpp/fastdds/builtin/typelookup/TypeLookupReplyListener.cpp b/src/cpp/fastdds/builtin/typelookup/TypeLookupReplyListener.cpp index bdd1c6f6e79..3efe307b383 100644 --- a/src/cpp/fastdds/builtin/typelookup/TypeLookupReplyListener.cpp +++ b/src/cpp/fastdds/builtin/typelookup/TypeLookupReplyListener.cpp @@ -116,18 +116,7 @@ void TypeLookupReplyListener::onNewCacheChangeAdded( reader->getHistory()->remove_change(change); } -TypeLookupReplyWListener::TypeLookupReplyWListener( - TypeLookupManager* manager) - : tlm_(manager) - , factory_(TypeObjectFactory::get_instance()) -{ -} - -TypeLookupReplyWListener::~TypeLookupReplyWListener() -{ -} - -void TypeLookupReplyWListener::onWriterChangeReceivedByAll( +void TypeLookupReplyListener::onWriterChangeReceivedByAll( fastrtps::rtps::RTPSWriter*, fastrtps::rtps::CacheChange_t* change) { diff --git a/src/cpp/fastdds/builtin/typelookup/TypeLookupRequestListener.cpp b/src/cpp/fastdds/builtin/typelookup/TypeLookupRequestListener.cpp index cb3a49c98b6..f2ccfe37cc8 100644 --- a/src/cpp/fastdds/builtin/typelookup/TypeLookupRequestListener.cpp +++ b/src/cpp/fastdds/builtin/typelookup/TypeLookupRequestListener.cpp @@ -150,18 +150,7 @@ void TypeLookupRequestListener::onNewCacheChangeAdded( reader->getHistory()->remove_change(change); } -TypeLookupRequestWListener::TypeLookupRequestWListener( - TypeLookupManager* manager) - : tlm_(manager) - , factory_(TypeObjectFactory::get_instance()) -{ -} - -TypeLookupRequestWListener::~TypeLookupRequestWListener() -{ -} - -void TypeLookupRequestWListener::onWriterChangeReceivedByAll( +void TypeLookupRequestListener::onWriterChangeReceivedByAll( fastrtps::rtps::RTPSWriter*, fastrtps::rtps::CacheChange_t* change) { From a9ac2a4d5687d14e938a43bc6cb232f5dd779c08 Mon Sep 17 00:00:00 2001 From: Irene Bandera Date: Thu, 22 Feb 2024 08:36:38 +0100 Subject: [PATCH 5/6] Apply changes Signed-off-by: Irene Bandera --- .../builtin/typelookup/TypeLookupManager.cpp | 41 ++++++++++++------- 1 file changed, 27 insertions(+), 14 deletions(-) diff --git a/src/cpp/fastdds/builtin/typelookup/TypeLookupManager.cpp b/src/cpp/fastdds/builtin/typelookup/TypeLookupManager.cpp index 71b3b064b23..629409605db 100644 --- a/src/cpp/fastdds/builtin/typelookup/TypeLookupManager.cpp +++ b/src/cpp/fastdds/builtin/typelookup/TypeLookupManager.cpp @@ -326,6 +326,8 @@ ReaderHistory* TypeLookupManager::get_builtin_reply_reader_history() */ bool TypeLookupManager::create_endpoints() { + bool ret = true; + const RTPSParticipantAttributes& pattr = participant_->getRTPSParticipantAttributes(); // Built-in history attributes. @@ -368,9 +370,7 @@ bool TypeLookupManager::create_endpoints() EPROSIMA_LOG_ERROR(TYPELOOKUP_SERVICE, "Typelookup request writer creation failed."); delete builtin_request_writer_history_; builtin_request_writer_history_ = nullptr; - delete request_listener_; - request_listener_ = nullptr; - return false; + ret = false; } } @@ -397,9 +397,7 @@ bool TypeLookupManager::create_endpoints() EPROSIMA_LOG_ERROR(TYPELOOKUP_SERVICE, "Typelookup reply writer creation failed."); delete builtin_reply_writer_history_; builtin_reply_writer_history_ = nullptr; - delete reply_listener_; - reply_listener_ = nullptr; - return false; + ret = false; } } @@ -418,7 +416,10 @@ bool TypeLookupManager::create_endpoints() // Built-in request reader if (builtin_protocols_->m_att.typelookup_config.use_server) { - request_listener_ = new TypeLookupRequestListener(this); + if (nullptr == request_listener_) + { + request_listener_ = new TypeLookupRequestListener(this); + } builtin_request_reader_history_ = new ReaderHistory(hatt); RTPSReader* req_reader; @@ -438,16 +439,17 @@ bool TypeLookupManager::create_endpoints() EPROSIMA_LOG_ERROR(TYPELOOKUP_SERVICE, "Typelookup request reader creation failed."); delete builtin_request_reader_history_; builtin_request_reader_history_ = nullptr; - delete request_listener_; - request_listener_ = nullptr; - return false; + ret = false; } } // Built-in reply reader if (builtin_protocols_->m_att.typelookup_config.use_client) { - reply_listener_ = new TypeLookupReplyListener(this); + if (nullptr == reply_listener_) + { + reply_listener_ = new TypeLookupReplyListener(this); + } builtin_reply_reader_history_ = new ReaderHistory(hatt); RTPSReader* rep_reader; @@ -467,13 +469,24 @@ bool TypeLookupManager::create_endpoints() EPROSIMA_LOG_ERROR(TYPELOOKUP_SERVICE, "Typelookup reply reader creation failed."); delete builtin_reply_reader_history_; builtin_reply_reader_history_ = nullptr; + ret = false; + } + } + + if (!ret) + { + if (request_listener_ != nullptr) + { + delete request_listener_; + request_listener_ = nullptr; + } + if (reply_listener_ != nullptr) + { delete reply_listener_; reply_listener_ = nullptr; - return false; } } - - return true; + return ret; } /* TODO Implement if security is needed. From dd7575ad788aaa0b276626185778084d575d72d9 Mon Sep 17 00:00:00 2001 From: EduPonz Date: Thu, 22 Feb 2024 12:09:02 +0100 Subject: [PATCH 6/6] Refs #20311: Further refactor TypeLookupManager::create_endpoints Signed-off-by: EduPonz --- .../builtin/typelookup/TypeLookupManager.cpp | 44 +++++++++++++------ 1 file changed, 31 insertions(+), 13 deletions(-) diff --git a/src/cpp/fastdds/builtin/typelookup/TypeLookupManager.cpp b/src/cpp/fastdds/builtin/typelookup/TypeLookupManager.cpp index 629409605db..40ffd150fb2 100644 --- a/src/cpp/fastdds/builtin/typelookup/TypeLookupManager.cpp +++ b/src/cpp/fastdds/builtin/typelookup/TypeLookupManager.cpp @@ -368,14 +368,12 @@ bool TypeLookupManager::create_endpoints() else { EPROSIMA_LOG_ERROR(TYPELOOKUP_SERVICE, "Typelookup request writer creation failed."); - delete builtin_request_writer_history_; - builtin_request_writer_history_ = nullptr; ret = false; } } // Built-in reply writer - if (builtin_protocols_->m_att.typelookup_config.use_server) + if (ret && builtin_protocols_->m_att.typelookup_config.use_server) { reply_listener_ = new TypeLookupReplyListener(this); builtin_reply_writer_history_ = new WriterHistory(hatt); @@ -395,8 +393,6 @@ bool TypeLookupManager::create_endpoints() else { EPROSIMA_LOG_ERROR(TYPELOOKUP_SERVICE, "Typelookup reply writer creation failed."); - delete builtin_reply_writer_history_; - builtin_reply_writer_history_ = nullptr; ret = false; } } @@ -414,7 +410,7 @@ bool TypeLookupManager::create_endpoints() ratt.endpoint.durabilityKind = fastrtps::rtps::VOLATILE; // Built-in request reader - if (builtin_protocols_->m_att.typelookup_config.use_server) + if (ret && builtin_protocols_->m_att.typelookup_config.use_server) { if (nullptr == request_listener_) { @@ -437,14 +433,12 @@ bool TypeLookupManager::create_endpoints() else { EPROSIMA_LOG_ERROR(TYPELOOKUP_SERVICE, "Typelookup request reader creation failed."); - delete builtin_request_reader_history_; - builtin_request_reader_history_ = nullptr; ret = false; } } // Built-in reply reader - if (builtin_protocols_->m_att.typelookup_config.use_client) + if (ret && builtin_protocols_->m_att.typelookup_config.use_client) { if (nullptr == reply_listener_) { @@ -467,25 +461,49 @@ bool TypeLookupManager::create_endpoints() else { EPROSIMA_LOG_ERROR(TYPELOOKUP_SERVICE, "Typelookup reply reader creation failed."); - delete builtin_reply_reader_history_; - builtin_reply_reader_history_ = nullptr; ret = false; } } + // Clean up if something failed. if (!ret) { - if (request_listener_ != nullptr) + if (nullptr != builtin_request_writer_history_) + { + delete builtin_request_writer_history_; + builtin_request_writer_history_ = nullptr; + } + + if (nullptr != builtin_reply_writer_history_) + { + delete builtin_reply_writer_history_; + builtin_reply_writer_history_ = nullptr; + } + + if (nullptr != builtin_request_reader_history_) + { + delete builtin_request_reader_history_; + builtin_request_reader_history_ = nullptr; + } + + if (nullptr != builtin_reply_reader_history_) + { + delete builtin_reply_reader_history_; + builtin_reply_reader_history_ = nullptr; + } + + if (nullptr != request_listener_) { delete request_listener_; request_listener_ = nullptr; } - if (reply_listener_ != nullptr) + if (nullptr != reply_listener_) { delete reply_listener_; reply_listener_ = nullptr; } } + return ret; }