From d3ca40cbce231c4bdb4fc88a4b3911afaaae4f6f Mon Sep 17 00:00:00 2001 From: Miguel Company Date: Thu, 27 Jun 2024 15:21:53 +0200 Subject: [PATCH] RTPS `WriterHistory` refactor (#4966) * Refs #21082. Remove templated version of `new_change`. Signed-off-by: Miguel Company * Refs #21082. Call simple version of `new_change` in rtps examples. Signed-off-by: Miguel Company * Refs #21082. Call simple version of `new_change` in `TypeLookupManager`. Signed-off-by: Miguel Company * Refs #21082. Refactor on EDP. Signed-off-by: Miguel Company * Refs #21082. Refactor on PDP. Signed-off-by: Miguel Company * Refs #21082. Refactor on SecurityManager. Signed-off-by: Miguel Company * Refs #21082. Refactor on unit tests. Signed-off-by: Miguel Company * Refs #21082. Refactor on WLP. Signed-off-by: Miguel Company * Refs #21082. Refactor on MonitorService. Signed-off-by: Miguel Company * Refs #21082. Refactor on blackbox tests. Signed-off-by: Miguel Company * Refs #21082. Fix blackbox tests. Signed-off-by: Miguel Company * Refs #21082. Remove `History::do_reserve_cache`. Signed-off-by: Miguel Company * Refs #21082. Remove `new_change` overload receiving `std::function`. Signed-off-by: Miguel Company * Refs #21082. Remove `is_pool_initialized`. Signed-off-by: Miguel Company * Refs #21082. WriterHistory keeps a change pool. Signed-off-by: Miguel Company * Refs #21082. Implementation of `release_change` moved to WriterHistory. Signed-off-by: Miguel Company * Refs #21082. Remove `release_change` from `RTPSWriter`. Signed-off-by: Miguel Company * Refs #21082. Add `create_change` to `WriterHistory`. Signed-off-by: Miguel Company * Refs #21082. Change calls from `new_change` to `create_change`. Signed-off-by: Miguel Company * Refs #21082. Remove `new_change` from `RTPSWriter`. Signed-off-by: Miguel Company * Refs #21082. Remove `remove_older_changes` from `RTPSWriter`. Signed-off-by: Miguel Company * Refs #21082. Add payload pool to `WriterHistory`. Signed-off-by: Miguel Company * Refs #21082. Add new `create_change` overload. Signed-off-by: Miguel Company * Refs #21082. Using new `create_change` overload where relevant. Signed-off-by: Miguel Company * Refs #21082. Refactor on `IPersistenceService`. Signed-off-by: Miguel Company * Refs #21082. Refactor on `DataWriterHistory`. Signed-off-by: Miguel Company * Refs #21082. Several methods to create writers removed. Signed-off-by: Miguel Company * Refs #21082. Move data sharing pool initialization to DataWriterImpl. Signed-off-by: Miguel Company * Refs #21082. Refactor creation of rtps writer. Signed-off-by: Miguel Company * Refs #21082. Refactor PersistentWriter. Signed-off-by: Miguel Company * Refs #21082. Remove constructors taking pools. Signed-off-by: Miguel Company * Refs #21082. RTPSWriter does not handle pools. Signed-off-by: Miguel Company * Refs #21082. Add pool getters to WriterHistory. Signed-off-by: Miguel Company * Refs #21082. Remove pool references on StatexxxWriter. Signed-off-by: Miguel Company * Refs #21082. Move pools from `Endpoint` to `BaseReader`. Signed-off-by: Miguel Company * Refs #21082. Avoid accessing history attributes. Signed-off-by: Miguel Company * Refs #21082. Fixes on SecurityManager. Signed-off-by: Miguel Company * Refs #21082. Fix build after rebase. Signed-off-by: Miguel Company * Refs #21082. Please linters. Signed-off-by: Miguel Company * Refs #21082. Fix release order in `WriterHistory` destructor. Signed-off-by: Miguel Company * Refs #21082. Move datasharing pool initialization to `RTPSWriter`. Signed-off-by: Miguel Company * Refs #21082. Fix negative tests. Signed-off-by: Miguel Company * Refs #21137. Apply review suggestions. Signed-off-by: Miguel Company * Refs #21137. Apply suggestions from code review Signed-off-by: Miguel Company Co-authored-by: Eduardo Ponz Segrelles * Refs #21137. Add note to versions.md Signed-off-by: eduponz --------- Signed-off-by: Miguel Company Signed-off-by: eduponz Co-authored-by: Eduardo Ponz Segrelles --- .../cpp/rtps/AsSocket/TestWriterSocket.cpp | 5 +- .../rtps/Persistent/TestWriterPersistent.cpp | 12 +- .../rtps/Registered/TestWriterRegistered.cpp | 12 +- include/fastdds/rtps/Endpoint.hpp | 11 -- include/fastdds/rtps/RTPSDomain.hpp | 83 ++------- include/fastdds/rtps/history/History.hpp | 4 - .../fastdds/rtps/history/ReaderHistory.hpp | 4 - .../fastdds/rtps/history/WriterHistory.hpp | 115 +++++++++++- include/fastdds/rtps/writer/RTPSWriter.hpp | 82 --------- .../type_lookup_service/TypeLookupManager.cpp | 33 ++-- .../type_lookup_service/TypeLookupManager.hpp | 1 - .../fastdds/publisher/DataWriterHistory.cpp | 6 +- .../fastdds/publisher/DataWriterHistory.hpp | 10 ++ src/cpp/fastdds/publisher/DataWriterImpl.cpp | 130 ++++++++------ src/cpp/fastdds/publisher/DataWriterImpl.hpp | 11 +- src/cpp/rtps/RTPSDomain.cpp | 60 +------ src/cpp/rtps/RTPSDomainImpl.hpp | 4 +- .../builtin/discovery/endpoint/EDPClient.cpp | 16 +- .../builtin/discovery/endpoint/EDPServer.cpp | 18 +- .../builtin/discovery/endpoint/EDPSimple.cpp | 48 ++--- .../builtin/discovery/endpoint/EDPSimple.h | 8 +- .../builtin/discovery/endpoint/EDPUtils.hpp | 27 ++- .../builtin/discovery/participant/PDP.cpp | 14 +- .../rtps/builtin/discovery/participant/PDP.h | 3 +- .../discovery/participant/PDPClient.cpp | 15 +- .../discovery/participant/PDPServer.cpp | 24 +-- .../discovery/participant/PDPSimple.cpp | 14 +- src/cpp/rtps/builtin/liveliness/WLP.cpp | 31 ++-- src/cpp/rtps/builtin/liveliness/WLP.hpp | 6 + src/cpp/rtps/history/ReaderHistory.cpp | 7 - src/cpp/rtps/history/WriterHistory.cpp | 156 +++++++++++++++-- src/cpp/rtps/messages/RTPSMessageGroup.cpp | 8 +- .../rtps/participant/RTPSParticipantImpl.cpp | 133 ++++---------- .../rtps/participant/RTPSParticipantImpl.h | 42 ++--- src/cpp/rtps/persistence/PersistenceService.h | 14 +- .../persistence/SQLite3PersistenceService.cpp | 21 +-- .../persistence/SQLite3PersistenceService.h | 7 - src/cpp/rtps/reader/BaseReader.cpp | 6 + src/cpp/rtps/reader/BaseReader.hpp | 6 + src/cpp/rtps/security/SecurityManager.cpp | 75 ++++---- src/cpp/rtps/writer/PersistentWriter.cpp | 9 +- src/cpp/rtps/writer/PersistentWriter.hpp | 2 - src/cpp/rtps/writer/RTPSWriter.cpp | 165 +----------------- .../rtps/writer/StatefulPersistentWriter.cpp | 31 +--- .../rtps/writer/StatefulPersistentWriter.hpp | 21 --- src/cpp/rtps/writer/StatefulWriter.cpp | 81 +-------- src/cpp/rtps/writer/StatefulWriter.hpp | 19 -- .../rtps/writer/StatelessPersistentWriter.cpp | 31 +--- .../rtps/writer/StatelessPersistentWriter.hpp | 21 --- src/cpp/rtps/writer/StatelessWriter.cpp | 41 +---- src/cpp/rtps/writer/StatelessWriter.hpp | 19 -- .../rtps/monitor-service/MonitorService.cpp | 33 ++-- .../rtps/monitor-service/MonitorService.hpp | 1 - test/blackbox/common/RTPSAsSocketWriter.hpp | 20 +-- .../common/RTPSWithRegistrationWriter.hpp | 34 ++-- .../fastdds/publisher/DataWriterHistory.hpp | 44 ++--- .../DataSharing/DataSharingPayloadPool.hpp | 10 ++ .../rtps/DataSharing/WriterPool.hpp | 61 +++++++ .../RTPSDomain/fastdds/rtps/RTPSDomain.hpp | 12 -- .../RTPSDomainImpl/rtps/RTPSDomainImpl.hpp | 7 +- .../rtps/participant/RTPSParticipantImpl.h | 8 +- .../fastdds/rtps/writer/RTPSWriter.hpp | 15 -- .../fastdds/rtps/history/WriterHistory.hpp | 37 +++- .../dds/publisher/DataWriterTests.cpp | 2 +- .../rtps/DataSharing/SHMSegmentTests.cpp | 5 +- .../rtps/persistence/PersistenceTests.cpp | 33 +++- .../SecurityHandshakeProcessTests.cpp | 12 +- test/unittest/rtps/security/SecurityTests.cpp | 12 +- .../SecurityValidationRemoteTests.cpp | 10 +- test/unittest/rtps/writer/RTPSWriterTests.cpp | 19 +- .../mock/fastdds/publisher/DataWriterImpl.hpp | 35 ++-- test/unittest/statistics/rtps/CMakeLists.txt | 1 + .../statistics/rtps/MonitorServiceTests.cpp | 1 - .../statistics/rtps/RTPSStatisticsTests.cpp | 16 +- .../fastdds/publisher/DataWriterHistory.hpp | 12 +- .../rtps/monitor-service/MonitorService.hpp | 1 - versions.md | 11 +- 77 files changed, 867 insertions(+), 1307 deletions(-) create mode 100644 test/mock/rtps/DataSharingPayloadPool/rtps/DataSharing/WriterPool.hpp diff --git a/examples/cpp/rtps/AsSocket/TestWriterSocket.cpp b/examples/cpp/rtps/AsSocket/TestWriterSocket.cpp index 5e4d3d1d9e4..4af2bda3d4d 100644 --- a/examples/cpp/rtps/AsSocket/TestWriterSocket.cpp +++ b/examples/cpp/rtps/AsSocket/TestWriterSocket.cpp @@ -89,10 +89,7 @@ void TestWriterSocket::run( { for (int i = 0; i < nmsgs; ++i ) { - CacheChange_t* ch = mp_writer->new_change([]() -> uint32_t - { - return 255; - }, ALIVE); + CacheChange_t* ch = mp_history->create_change(255, ALIVE); #if defined(_WIN32) ch->serializedPayload.length = sprintf_s((char*)ch->serializedPayload.data, 255, "My example string %d", i) + 1; diff --git a/examples/cpp/rtps/Persistent/TestWriterPersistent.cpp b/examples/cpp/rtps/Persistent/TestWriterPersistent.cpp index 2781c017237..3701b5770f2 100644 --- a/examples/cpp/rtps/Persistent/TestWriterPersistent.cpp +++ b/examples/cpp/rtps/Persistent/TestWriterPersistent.cpp @@ -112,18 +112,12 @@ void TestWriterPersistent::run( for (int i = 0; i < samples; ++i ) { - CacheChange_t* ch = mp_writer->new_change([]() -> uint32_t - { - return 255; - }, ALIVE); + CacheChange_t* ch = mp_history->create_change(255, ALIVE); if (!ch) // In the case history is full, remove some old changes { std::cout << "cleaning history..."; - mp_writer->remove_older_changes(20); - ch = mp_writer->new_change([]() -> uint32_t - { - return 255; - }, ALIVE); + mp_history->remove_min_change(); + ch = mp_history->create_change(255, ALIVE); } #if defined(_WIN32) diff --git a/examples/cpp/rtps/Registered/TestWriterRegistered.cpp b/examples/cpp/rtps/Registered/TestWriterRegistered.cpp index 8ebb8cf6fa2..31e7d898a6c 100644 --- a/examples/cpp/rtps/Registered/TestWriterRegistered.cpp +++ b/examples/cpp/rtps/Registered/TestWriterRegistered.cpp @@ -103,18 +103,12 @@ void TestWriterRegistered::run( for (int i = 0; i < samples; ++i ) { - CacheChange_t* ch = mp_writer->new_change([]() -> uint32_t - { - return 255; - }, ALIVE); + CacheChange_t* ch = mp_history->create_change(255, ALIVE); if (!ch) // In the case history is full, remove some old changes { std::cout << "cleaning history..."; - mp_writer->remove_older_changes(20); - ch = mp_writer->new_change([]() -> uint32_t - { - return 255; - }, ALIVE); + mp_history->remove_min_change(); + ch = mp_history->create_change(255, ALIVE); } #if defined(_WIN32) diff --git a/include/fastdds/rtps/Endpoint.hpp b/include/fastdds/rtps/Endpoint.hpp index f1b89dd9caf..0db7ad2affb 100644 --- a/include/fastdds/rtps/Endpoint.hpp +++ b/include/fastdds/rtps/Endpoint.hpp @@ -62,11 +62,6 @@ class Endpoint virtual ~Endpoint() { - // As releasing the change pool will delete the cache changes it owns, - // the payload pool may be called to release their payloads, so we should - // ensure that the payload pool is destroyed after the change pool. - change_pool_.reset(); - payload_pool_.reset(); } public: @@ -120,12 +115,6 @@ class Endpoint //!Endpoint Mutex mutable RecursiveTimedMutex mp_mutex; - //!Pool of serialized payloads. - std::shared_ptr payload_pool_; - - //!Pool of cache changes. - std::shared_ptr change_pool_; - //!Fixed size of payloads uint32_t fixed_payload_size_ = 0; diff --git a/include/fastdds/rtps/RTPSDomain.hpp b/include/fastdds/rtps/RTPSDomain.hpp index 808577c998b..394c684a022 100644 --- a/include/fastdds/rtps/RTPSDomain.hpp +++ b/include/fastdds/rtps/RTPSDomain.hpp @@ -113,70 +113,12 @@ class RTPSDomain /** * Create a RTPSWriter in a participant. - * @param p Pointer to the RTPSParticipant. - * @param watt Writer Attributes. - * @param hist Pointer to the WriterHistory. - * @param listen Pointer to the WriterListener. - * @return Pointer to the created RTPSWriter. * - * \warning The returned pointer is invalidated after a call to removeRTPSWriter() or stopAll(), - * so its use may result in undefined behaviour. - */ - FASTDDS_EXPORTED_API static RTPSWriter* createRTPSWriter( - RTPSParticipant* p, - WriterAttributes& watt, - WriterHistory* hist, - WriterListener* listen = nullptr); - - /** - * Create a RTPSWriter in a participant using a custom payload pool. - * @param p Pointer to the RTPSParticipant. - * @param watt Writer Attributes. - * @param payload_pool Shared pointer to the IPayloadPool - * @param hist Pointer to the WriterHistory. - * @param listen Pointer to the WriterListener. - * @return Pointer to the created RTPSWriter. - * - * \warning The returned pointer is invalidated after a call to removeRTPSWriter() or stopAll(), - * so its use may result in undefined behaviour. - */ - FASTDDS_EXPORTED_API static RTPSWriter* createRTPSWriter( - RTPSParticipant* p, - WriterAttributes& watt, - const std::shared_ptr& payload_pool, - WriterHistory* hist, - WriterListener* listen = nullptr); - - /** - * Create a RTPSWriter in a participant using a custom payload pool. - * @param p Pointer to the RTPSParticipant. - * @param watt Writer Attributes. - * @param payload_pool Shared pointer to the IPayloadPool - * @param change_pool Shared pointer to the IChangePool - * @param hist Pointer to the WriterHistory. - * @param listen Pointer to the WriterListener. - * @return Pointer to the created RTPSWriter. + * @param p Pointer to the RTPSParticipant. + * @param watt Writer Attributes. + * @param hist Pointer to the WriterHistory. + * @param listen Pointer to the WriterListener. * - * \warning The returned pointer is invalidated after a call to removeRTPSWriter() or stopAll(), - * so its use may result in undefined behaviour. - */ - FASTDDS_EXPORTED_API static RTPSWriter* createRTPSWriter( - RTPSParticipant* p, - WriterAttributes& watt, - const std::shared_ptr& payload_pool, - const std::shared_ptr& change_pool, - WriterHistory* hist, - WriterListener* listen = nullptr); - - /** - * Create a RTPSWriter in a participant using a custom payload pool. - * @param p Pointer to the RTPSParticipant. - * @param entity_id Specific entity id to use for the created writer. - * @param watt Writer Attributes. - * @param payload_pool Shared pointer to the IPayloadPool - * @param change_pool Shared pointer to the IChangePool - * @param hist Pointer to the WriterHistory. - * @param listen Pointer to the WriterListener. * @return Pointer to the created RTPSWriter. * * \warning The returned pointer is invalidated after a call to removeRTPSWriter() or stopAll(), @@ -184,21 +126,19 @@ class RTPSDomain */ FASTDDS_EXPORTED_API static RTPSWriter* createRTPSWriter( RTPSParticipant* p, - const EntityId_t& entity_id, WriterAttributes& watt, - const std::shared_ptr& payload_pool, - const std::shared_ptr& change_pool, WriterHistory* hist, WriterListener* listen = nullptr); /** * Create a RTPSWriter in a participant. - * @param p Pointer to the RTPSParticipant. - * @param entity_id Specific entity id to use for the created writer. - * @param watt Writer Attributes. - * @param payload_pool Shared pointer to the IPayloadPool - * @param hist Pointer to the WriterHistory. - * @param listen Pointer to the WriterListener. + * + * @param p Pointer to the RTPSParticipant. + * @param entity_id Specific entity id to use for the created writer. + * @param watt Writer Attributes. + * @param hist Pointer to the WriterHistory. + * @param listen Pointer to the WriterListener. + * * @return Pointer to the created RTPSWriter. * * \warning The returned pointer is invalidated after a call to removeRTPSWriter() or stopAll(), @@ -208,7 +148,6 @@ class RTPSDomain RTPSParticipant* p, const EntityId_t& entity_id, WriterAttributes& watt, - const std::shared_ptr& payload_pool, WriterHistory* hist, WriterListener* listen = nullptr); diff --git a/include/fastdds/rtps/history/History.hpp b/include/fastdds/rtps/history/History.hpp index 7950976a6da..bd64e1e8bdd 100644 --- a/include/fastdds/rtps/history/History.hpp +++ b/include/fastdds/rtps/history/History.hpp @@ -271,10 +271,6 @@ class History //!Print the seqNum of the changes in the History (for debuggisi, mng purposes). void print_changes_seqNum2(); - FASTDDS_EXPORTED_API virtual bool do_reserve_cache( - CacheChange_t** change, - uint32_t size) = 0; - FASTDDS_EXPORTED_API virtual void do_release_cache( CacheChange_t* ch) = 0; diff --git a/include/fastdds/rtps/history/ReaderHistory.hpp b/include/fastdds/rtps/history/ReaderHistory.hpp index cdc1286ad87..511a0d36152 100644 --- a/include/fastdds/rtps/history/ReaderHistory.hpp +++ b/include/fastdds/rtps/history/ReaderHistory.hpp @@ -233,10 +233,6 @@ class ReaderHistory : public History protected: - FASTDDS_EXPORTED_API bool do_reserve_cache( - CacheChange_t** change, - uint32_t size) override; - FASTDDS_EXPORTED_API void do_release_cache( CacheChange_t* ch) override; diff --git a/include/fastdds/rtps/history/WriterHistory.hpp b/include/fastdds/rtps/history/WriterHistory.hpp index 5af83c16748..09cac628062 100644 --- a/include/fastdds/rtps/history/WriterHistory.hpp +++ b/include/fastdds/rtps/history/WriterHistory.hpp @@ -20,13 +20,23 @@ #ifndef FASTDDS_RTPS_HISTORY__WRITERHISTORY_HPP #define FASTDDS_RTPS_HISTORY__WRITERHISTORY_HPP -#include +#include +#include + +#include #include +#include +#include +#include +#include +#include +#include namespace eprosima { namespace fastdds { namespace rtps { +class HistoryAttributes; class RTPSWriter; class WriteParams; @@ -48,12 +58,81 @@ class WriterHistory : public rtps::History public: /** - * Constructor of the WriterHistory. + * @brief Construct a WriterHistory. + * + * @param att Attributes configuring the WriterHistory. + */ + FASTDDS_EXPORTED_API WriterHistory( + const HistoryAttributes& att); + + /** + * @brief Construct a WriterHistory with a custom payload pool. + * + * @param att Attributes configuring the WriterHistory. + * @param payload_pool Pool of payloads to be used by the WriterHistory. + */ + FASTDDS_EXPORTED_API WriterHistory( + const HistoryAttributes& att, + const std::shared_ptr& payload_pool); + + /** + * @brief Construct a WriterHistory with custom payload and change pools. + * + * @param att Attributes configuring the WriterHistory. + * @param payload_pool Pool of payloads to be used by the WriterHistory. + * @param change_pool Pool of changes to be used by the WriterHistory. */ FASTDDS_EXPORTED_API WriterHistory( - const HistoryAttributes& att); + const HistoryAttributes& att, + const std::shared_ptr& payload_pool, + const std::shared_ptr& change_pool); + FASTDDS_EXPORTED_API virtual ~WriterHistory() override; + /** + * @brief Get the payload pool used by this history. + * + * @return Reference to the payload pool used by this history. + */ + FASTDDS_EXPORTED_API const std::shared_ptr& get_payload_pool() const; + + /** + * @brief Get the change pool used by this history. + * + * @return Reference to the change pool used by this history. + */ + FASTDDS_EXPORTED_API const std::shared_ptr& get_change_pool() const; + + /** + * @brief Create a new CacheChange_t object. + * + * @param change_kind Kind of the change. + * @param handle InstanceHandle_t of the change. + * + * @return Pointer to the new CacheChange_t object. + * + * @pre A writer has been associated with this history + */ + FASTDDS_EXPORTED_API CacheChange_t* create_change( + ChangeKind_t change_kind, + InstanceHandle_t handle = c_InstanceHandle_Unknown); + + /** + * @brief Create a new CacheChange_t object with a specific payload size. + * + * @param payload_size Size of the payload. + * @param change_kind Kind of the change. + * @param handle InstanceHandle_t of the change. + * + * @return Pointer to the new CacheChange_t object. + * + * @pre A writer has been associated with this history + */ + FASTDDS_EXPORTED_API CacheChange_t* create_change( + uint32_t payload_size, + ChangeKind_t change_kind, + InstanceHandle_t handle = c_InstanceHandle_Unknown); + /** * Add a CacheChange_t to the WriterHistory. * @param a_change Pointer to the CacheChange_t to be added. @@ -141,11 +220,24 @@ class WriterHistory : public rtps::History return m_lastCacheChangeSeqNum + 1; } -protected: + /** + * Release a change when it is not being used anymore. + * + * @param ch Pointer to the cache change to be released. + * + * @returns whether the operation succeeded or not + * + * @pre + * @li A writer has been associated with this history + * @li @c ch is not @c nullptr + * @li @c ch points to a cache change obtained from a call to @c this->create_change + * + * @post memory pointed to by @c ch is not accessed + */ + FASTDDS_EXPORTED_API bool release_change( + CacheChange_t* ch); - FASTDDS_EXPORTED_API bool do_reserve_cache( - CacheChange_t** change, - uint32_t size) override; +protected: FASTDDS_EXPORTED_API void do_release_cache( CacheChange_t* ch) override; @@ -211,9 +303,9 @@ class WriterHistory : public rtps::History } //!Last CacheChange Sequence Number added to the History. - SequenceNumber_t m_lastCacheChangeSeqNum; + SequenceNumber_t m_lastCacheChangeSeqNum {}; //!Pointer to the associated RTPSWriter; - RTPSWriter* mp_writer; + RTPSWriter* mp_writer = nullptr; uint32_t high_mark_for_frag_ = 0; @@ -248,6 +340,11 @@ class WriterHistory : public rtps::History void set_fragments( CacheChange_t* change); + + /// Reference to the change pool used by this history. + std::shared_ptr change_pool_; + /// Reference to the payload pool used by this history. + std::shared_ptr payload_pool_; }; } // namespace rtps diff --git a/include/fastdds/rtps/writer/RTPSWriter.hpp b/include/fastdds/rtps/writer/RTPSWriter.hpp index dfb6580da5b..a0bd20a47ab 100644 --- a/include/fastdds/rtps/writer/RTPSWriter.hpp +++ b/include/fastdds/rtps/writer/RTPSWriter.hpp @@ -86,80 +86,10 @@ class RTPSWriter WriterHistory* hist, WriterListener* listen = nullptr); - RTPSWriter( - RTPSParticipantImpl* impl, - const GUID_t& guid, - const WriterAttributes& att, - const std::shared_ptr& payload_pool, - fastdds::rtps::FlowController* flow_controller, - WriterHistory* hist, - WriterListener* listen = nullptr); - - RTPSWriter( - RTPSParticipantImpl* impl, - const GUID_t& guid, - const WriterAttributes& att, - const std::shared_ptr& payload_pool, - const std::shared_ptr& change_pool, - fastdds::rtps::FlowController* flow_controller, - WriterHistory* hist, - WriterListener* listen = nullptr); - virtual ~RTPSWriter(); public: - /** - * Create a new change based with the provided changeKind. - * @param data Data of the change. - * @param changeKind The type of change. - * @param handle InstanceHandle to assign. - * @return Pointer to the CacheChange or nullptr if incorrect. - */ - template - CacheChange_t* new_change( - T& data, - ChangeKind_t changeKind, - InstanceHandle_t handle = c_InstanceHandle_Unknown) - { - return new_change([data]() -> uint32_t - { -#if FASTCDR_VERSION_MAJOR == 1 - return (uint32_t)T::getCdrSerializedSize(data); -#else - eprosima::fastcdr::CdrSizeCalculator calculator( - eprosima::fastdds::rtps::DEFAULT_XCDR_VERSION); - size_t current_alignment {0}; - return (uint32_t)calculator.calculate_serialized_size(data, current_alignment); -#endif // if FASTCDR_VERSION_MAJOR == 1 - }, changeKind, handle); - } - - FASTDDS_EXPORTED_API CacheChange_t* new_change( - const std::function& dataCdrSerializedSize, - ChangeKind_t changeKind, - InstanceHandle_t handle = c_InstanceHandle_Unknown); - - FASTDDS_EXPORTED_API CacheChange_t* new_change( - ChangeKind_t changeKind, - InstanceHandle_t handle = c_InstanceHandle_Unknown); - - /** - * Release a change when it is not being used anymore. - * - * @param change Pointer to the cache change to be released. - * - * @returns whether the operation succeeded or not - * - * @pre - * @li @c change is not @c nullptr - * @li @c change points to a cache change obtained from a call to @c this->new_change - * - * @post memory pointed to by @c change is not accessed - */ - FASTDDS_EXPORTED_API bool release_change( - CacheChange_t* change); - /** * Add a matched reader. * @param data Pointer to the ReaderProxyData object added. @@ -295,14 +225,6 @@ class RTPSWriter return is_async_; } - /** - * Remove an specified max number of changes - * @param max Maximum number of changes to remove. - * @return at least one change has been removed - */ - FASTDDS_EXPORTED_API bool remove_older_changes( - unsigned int max = 0); - /** * @brief Returns if disable positive ACKs QoS is enabled. * @@ -588,8 +510,6 @@ class RTPSWriter bool is_datasharing_compatible_with( const ReaderProxyData& rdata) const; - bool is_pool_initialized() const; - void add_statistics_sent_submessage( CacheChange_t* change, size_t num_locators); @@ -607,8 +527,6 @@ class RTPSWriter const RTPSWriter&) = delete; void init( - const std::shared_ptr& payload_pool, - const std::shared_ptr& change_pool, const WriterAttributes& att); diff --git a/src/cpp/fastdds/builtin/type_lookup_service/TypeLookupManager.cpp b/src/cpp/fastdds/builtin/type_lookup_service/TypeLookupManager.cpp index a9f49af5965..c8bdd3be946 100644 --- a/src/cpp/fastdds/builtin/type_lookup_service/TypeLookupManager.cpp +++ b/src/cpp/fastdds/builtin/type_lookup_service/TypeLookupManager.cpp @@ -680,7 +680,7 @@ TypeLookup_Request* TypeLookupManager::create_request( bool TypeLookupManager::send( TypeLookup_Request& request) const { - if (!send_impl(request, &request_type_, builtin_request_writer_, builtin_request_writer_history_)) + if (!send_impl(request, &request_type_, builtin_request_writer_history_)) { EPROSIMA_LOG_WARNING(TYPELOOKUP_SERVICE, "Error sending request."); return false; @@ -691,7 +691,7 @@ bool TypeLookupManager::send( bool TypeLookupManager::send( TypeLookup_Reply& reply) const { - if (!send_impl(reply, &reply_type_, builtin_reply_writer_, builtin_reply_writer_history_)) + if (!send_impl(reply, &reply_type_, builtin_reply_writer_history_)) { EPROSIMA_LOG_WARNING(TYPELOOKUP_SERVICE, "Error sending reply."); return false; @@ -703,19 +703,15 @@ template bool TypeLookupManager::send_impl( Type& msg, PubSubType* pubsubtype, - fastdds::rtps::StatefulWriter* writer, fastdds::rtps::WriterHistory* writer_history) const { + // Calculate the serialized size of the message using a CdrSizeCalculator + eprosima::fastcdr::CdrSizeCalculator calculator(eprosima::fastcdr::CdrVersion::XCDRv2); + size_t current_alignment {0}; + uint32_t payload_size = static_cast(calculator.calculate_serialized_size(msg, current_alignment) + 4); + // Create a new CacheChange_t using the provided StatefulWriter - CacheChange_t* change = writer->new_change( - [&msg]() - { - // Calculate the serialized size of the message using a CdrSizeCalculator - eprosima::fastcdr::CdrSizeCalculator calculator(eprosima::fastcdr::CdrVersion::XCDRv2); - size_t current_alignment {0}; - return static_cast(calculator.calculate_serialized_size(msg, current_alignment) + 4); - }, - ALIVE); + CacheChange_t* change = writer_history->create_change(payload_size, ALIVE); // Check if the creation of CacheChange_t was successful if (!change) @@ -723,23 +719,14 @@ bool TypeLookupManager::send_impl( return false; } - // Prepare the payload for sending the message - SerializedPayload_t payload; - payload.max_size = change->serializedPayload.max_size; - payload.data = change->serializedPayload.data; - // Serialize the message using the provided PubSubType - bool result = pubsubtype->serialize(&msg, &payload, DataRepresentationId_t::XCDR2_DATA_REPRESENTATION); + bool result = pubsubtype->serialize(&msg, &change->serializedPayload, + DataRepresentationId_t::XCDR2_DATA_REPRESENTATION); // If serialization was successful, update the change and add it to the WriterHistory if (result) { - change->serializedPayload.length += payload.length; - change->serializedPayload.pos += payload.pos; result = writer_history->add_change(change); } - // Release the payload data - payload.data = nullptr; - // If adding the change to WriterHistory failed, remove the change if (!result) { diff --git a/src/cpp/fastdds/builtin/type_lookup_service/TypeLookupManager.hpp b/src/cpp/fastdds/builtin/type_lookup_service/TypeLookupManager.hpp index 25a87106c09..99ba4abc1aa 100644 --- a/src/cpp/fastdds/builtin/type_lookup_service/TypeLookupManager.hpp +++ b/src/cpp/fastdds/builtin/type_lookup_service/TypeLookupManager.hpp @@ -284,7 +284,6 @@ class TypeLookupManager bool send_impl( Type& msg, PubSubType* pubsubtype, - fastdds::rtps::StatefulWriter* writer, fastdds::rtps::WriterHistory* writer_history) const; /** diff --git a/src/cpp/fastdds/publisher/DataWriterHistory.cpp b/src/cpp/fastdds/publisher/DataWriterHistory.cpp index 47c3578ed22..109f3e323cd 100644 --- a/src/cpp/fastdds/publisher/DataWriterHistory.cpp +++ b/src/cpp/fastdds/publisher/DataWriterHistory.cpp @@ -32,7 +32,7 @@ namespace dds { using namespace eprosima::fastdds::rtps; -static HistoryAttributes to_history_attributes( +HistoryAttributes DataWriterHistory::to_history_attributes( const TopicAttributes& topic_att, uint32_t payloadMaxSize, MemoryManagementPolicy_t mempolicy) @@ -56,11 +56,13 @@ static HistoryAttributes to_history_attributes( } DataWriterHistory::DataWriterHistory( + const std::shared_ptr& payload_pool, + const std::shared_ptr& change_pool, const TopicAttributes& topic_att, uint32_t payloadMaxSize, MemoryManagementPolicy_t mempolicy, std::function unack_sample_remove_functor) - : WriterHistory(to_history_attributes(topic_att, payloadMaxSize, mempolicy)) + : WriterHistory(to_history_attributes(topic_att, payloadMaxSize, mempolicy), payload_pool, change_pool) , history_qos_(topic_att.historyQos) , resource_limited_qos_(topic_att.resourceLimitsQos) , topic_att_(topic_att) diff --git a/src/cpp/fastdds/publisher/DataWriterHistory.hpp b/src/cpp/fastdds/publisher/DataWriterHistory.hpp index bcbc125bf84..cd0ee87227c 100644 --- a/src/cpp/fastdds/publisher/DataWriterHistory.hpp +++ b/src/cpp/fastdds/publisher/DataWriterHistory.hpp @@ -27,6 +27,8 @@ #include #include #include +#include +#include #include #include @@ -42,8 +44,14 @@ namespace dds { */ class DataWriterHistory : public rtps::WriterHistory { + public: + static rtps::HistoryAttributes to_history_attributes( + const TopicAttributes& topic_att, + uint32_t payloadMaxSize, + rtps::MemoryManagementPolicy_t mempolicy); + /** * Constructor of the DataWriterHistory. * @param topic_att TopicAttributed @@ -52,6 +60,8 @@ class DataWriterHistory : public rtps::WriterHistory * @param unack_sample_remove_functor Functor to call DDS listener callback on_unacknowledged_sample_removed */ DataWriterHistory( + const std::shared_ptr& payload_pool, + const std::shared_ptr& change_pool, const TopicAttributes& topic_att, uint32_t payloadMax, rtps::MemoryManagementPolicy_t mempolicy, diff --git a/src/cpp/fastdds/publisher/DataWriterImpl.cpp b/src/cpp/fastdds/publisher/DataWriterImpl.cpp index 77732e1e805..3d40705230d 100644 --- a/src/cpp/fastdds/publisher/DataWriterImpl.cpp +++ b/src/cpp/fastdds/publisher/DataWriterImpl.cpp @@ -42,6 +42,7 @@ #include #include +#include #include #include #include @@ -154,15 +155,7 @@ DataWriterImpl::DataWriterImpl( , topic_(topic) , qos_(get_datawriter_qos_from_settings(qos)) , listener_(listen) - , history_(get_topic_attributes(qos_, *topic_, type_), type_->m_typeSize, qos_.endpoint().history_memory_policy, - [this]( - const InstanceHandle_t& handle) -> void - { - if (nullptr != listener_) - { - listener_->on_unacknowledged_sample_removed(user_datawriter_, handle); - } - }) + , history_() #pragma warning (disable : 4355 ) , writer_listener_(this) , deadline_duration_us_(qos_.deadline().period.to_ns() * 1e-3) @@ -197,15 +190,7 @@ DataWriterImpl::DataWriterImpl( , topic_(topic) , qos_(get_datawriter_qos_from_settings(qos)) , listener_(listen) - , history_(get_topic_attributes(qos_, *topic_, type_), type_->m_typeSize, qos_.endpoint().history_memory_policy, - [this]( - const InstanceHandle_t& handle) -> void - { - if (nullptr != listener_) - { - listener_->on_unacknowledged_sample_removed(user_datawriter_, handle); - } - }) + , history_() #pragma warning (disable : 4355 ) , writer_listener_(this) , deadline_duration_us_(qos_.deadline().period.to_ns() * 1e-3) @@ -235,10 +220,34 @@ DataWriterQos DataWriterImpl::get_datawriter_qos_from_settings( return return_qos; } +void DataWriterImpl::create_history( + const std::shared_ptr& payload_pool, + const std::shared_ptr& change_pool) +{ + history_.reset(new DataWriterHistory( + payload_pool, change_pool, + get_topic_attributes(qos_, *topic_, type_), + type_->m_typeSize, + qos_.endpoint().history_memory_policy, + [this]( + const InstanceHandle_t& handle) -> void + { + if (nullptr != listener_) + { + listener_->on_unacknowledged_sample_removed(user_datawriter_, handle); + } + })); +} + ReturnCode_t DataWriterImpl::enable() { assert(writer_ == nullptr); + auto topic_att = get_topic_attributes(qos_, *topic_, type_); + auto history_att = DataWriterHistory::to_history_attributes( + topic_att, type_->m_typeSize, qos_.endpoint().history_memory_policy); + pool_config_ = PoolConfig::from_history_attributes(history_att); + WriterAttributes w_att; w_att.endpoint.durabilityKind = qos_.durability().durabilityKind(); w_att.endpoint.endpointKind = WRITER; @@ -347,19 +356,32 @@ ReturnCode_t DataWriterImpl::enable() return RETCODE_ERROR; } + create_history(pool, change_pool); + RTPSWriter* writer = RTPSDomainImpl::create_rtps_writer( publisher_->rtps_participant(), guid_.entityId, w_att, - pool, - change_pool, - static_cast(&history_), + history_.get(), static_cast(&writer_listener_)); + if (writer != nullptr && + w_att.endpoint.data_sharing_configuration().kind() != DataSharingKind::OFF) + { + auto writer_pool = std::dynamic_pointer_cast(pool); + if (!writer_pool || !writer_pool->is_initialized()) + { + EPROSIMA_LOG_ERROR(DATA_WRITER, "Could not initialize DataSharing writer pool"); + RTPSDomain::removeRTPSWriter(writer); + writer = nullptr; + } + } + if (writer == nullptr && w_att.endpoint.data_sharing_configuration().kind() == DataSharingKind::AUTO) { EPROSIMA_LOG_INFO(DATA_WRITER, "Trying with a non-datasharing pool"); + history_.reset(); release_payload_pool(); is_data_sharing_compatible_ = false; DataSharingQosPolicy datasharing; @@ -373,17 +395,17 @@ ReturnCode_t DataWriterImpl::enable() return RETCODE_ERROR; } + create_history(pool, change_pool); writer = RTPSDomainImpl::create_rtps_writer( publisher_->rtps_participant(), guid_.entityId, w_att, - pool, - change_pool, - static_cast(&history_), + history_.get(), static_cast(&writer_listener_)); } if (writer == nullptr) { + history_.reset(); release_payload_pool(); EPROSIMA_LOG_ERROR(DATA_WRITER, "Problem creating associated Writer"); return RETCODE_ERROR; @@ -396,7 +418,7 @@ ReturnCode_t DataWriterImpl::enable() } // In case it has been loaded from the persistence DB, rebuild instances on history - history_.rebuild_instances(); + history_->rebuild_instances(); deadline_timer_ = new TimedEvent(publisher_->rtps_participant()->get_resource_event(), [&]() -> bool @@ -799,7 +821,7 @@ InstanceHandle_t DataWriterImpl::do_register_instance( #endif // if HAVE_STRICT_REALTIME { SerializedPayload_t* payload = nullptr; - if (history_.register_instance(instance_handle, lock, max_blocking_time, payload)) + if (history_->register_instance(instance_handle, lock, max_blocking_time, payload)) { // Keep serialization of sample inside the instance assert(nullptr != payload); @@ -814,7 +836,7 @@ InstanceHandle_t DataWriterImpl::do_register_instance( // Serialization of the sample failed. Remove the instance to keep original state. // Note that we will only end-up here if the instance has just been created, so it will be empty // and removing its changes will remove the instance completely. - history_.remove_instance_changes(instance_handle, fastdds::rtps::SequenceNumber_t()); + history_->remove_instance_changes(instance_handle, rtps::SequenceNumber_t()); } } return instance_handle; @@ -832,7 +854,7 @@ ReturnCode_t DataWriterImpl::unregister_instance( // Preconditions InstanceHandle_t ih; ReturnCode_t returned_value = check_instance_preconditions(instance, handle, ih); - if (RETCODE_OK == returned_value && !history_.is_key_registered(ih)) + if (RETCODE_OK == returned_value && !history_->is_key_registered(ih)) { returned_value = RETCODE_PRECONDITION_NOT_MET; } @@ -865,7 +887,7 @@ ReturnCode_t DataWriterImpl::unregister_instance_w_timestamp( { ret = check_instance_preconditions(instance, handle, instance_handle); } - if (RETCODE_OK == ret && !history_.is_key_registered(instance_handle)) + if (RETCODE_OK == ret && !history_->is_key_registered(instance_handle)) { ret = RETCODE_PRECONDITION_NOT_MET; } @@ -917,7 +939,7 @@ ReturnCode_t DataWriterImpl::get_key_value( std::lock_guard lock(writer_->getMutex()); #endif // if HAVE_STRICT_REALTIME - SerializedPayload_t* payload = history_.get_key_value(handle); + SerializedPayload_t* payload = history_->get_key_value(handle); if (nullptr == payload) { return RETCODE_BAD_PARAMETER; @@ -997,7 +1019,7 @@ ReturnCode_t DataWriterImpl::perform_create_new_change( } } - CacheChange_t* ch = writer_->new_change(change_kind, handle); + CacheChange_t* ch = history_->create_change(change_kind, handle); if (ch != nullptr) { ch->serializedPayload = std::move(payload); @@ -1011,11 +1033,11 @@ ReturnCode_t DataWriterImpl::perform_create_new_change( reader_filters_->update_filter_info(static_cast(ch), related_sample_identity); }; - added = history_.add_pub_change_with_commit_hook(ch, wparams, filter_hook, lock, max_blocking_time); + added = history_->add_pub_change_with_commit_hook(ch, wparams, filter_hook, lock, max_blocking_time); } else { - added = history_.add_pub_change(ch, wparams, lock, max_blocking_time); + added = history_->add_pub_change(ch, wparams, lock, max_blocking_time); } if (!added) @@ -1025,13 +1047,13 @@ ReturnCode_t DataWriterImpl::perform_create_new_change( payload = std::move(ch->serializedPayload); add_loan(data, payload); } - writer_->release_change(ch); + history_->release_change(ch); return RETCODE_TIMEOUT; } if (qos_.deadline().period != c_TimeInfinite) { - if (!history_.set_next_deadline( + if (!history_->set_next_deadline( handle, steady_clock::now() + duration_cast(deadline_duration_us_))) { @@ -1105,13 +1127,13 @@ ReturnCode_t DataWriterImpl::create_new_change_with_params( bool DataWriterImpl::remove_min_seq_change() { - return history_.removeMinChange(); + return history_->removeMinChange(); } ReturnCode_t DataWriterImpl::clear_history( size_t* removed) { - return (history_.removeAllChange(removed) ? RETCODE_OK : RETCODE_ERROR); + return (history_->removeAllChange(removed) ? RETCODE_OK : RETCODE_ERROR); } ReturnCode_t DataWriterImpl::get_sending_locators( @@ -1301,11 +1323,11 @@ void DataWriterImpl::InnerDataWriterListener::onWriterChangeReceivedByAll( (NOT_ALIVE_UNREGISTERED == ch->kind || NOT_ALIVE_DISPOSED_UNREGISTERED == ch->kind)) { - data_writer_->history_.remove_instance_changes(ch->instanceHandle, ch->sequenceNumber); + data_writer_->history_->remove_instance_changes(ch->instanceHandle, ch->sequenceNumber); } else if (data_writer_->qos_.durability().kind == VOLATILE_DURABILITY_QOS) { - data_writer_->history_.remove_change_g(ch); + data_writer_->history_->remove_change_g(ch); } } @@ -1415,12 +1437,12 @@ ReturnCode_t DataWriterImpl::wait_for_acknowledgments( std::unique_lock lock(writer_->getMutex()); #endif // HAVE_STRICT_REALTIME - if (!history_.is_key_registered(ih)) + if (!history_->is_key_registered(ih)) { return RETCODE_PRECONDITION_NOT_MET; } - if (history_.wait_for_acknowledgement_last_change(ih, lock, max_blocking_time)) + if (history_->wait_for_acknowledgement_last_change(ih, lock, max_blocking_time)) { return RETCODE_OK; } @@ -1469,7 +1491,7 @@ bool DataWriterImpl::deadline_timer_reschedule() std::unique_lock lock(writer_->getMutex()); steady_clock::time_point next_deadline_us; - if (!history_.get_next_deadline(timer_owner_, next_deadline_us)) + if (!history_->get_next_deadline(timer_owner_, next_deadline_us)) { EPROSIMA_LOG_ERROR(DATA_WRITER, "Could not get the next deadline from the history"); return false; @@ -1503,7 +1525,7 @@ bool DataWriterImpl::deadline_missed() user_datawriter_->get_statuscondition().get_impl()->set_status(notify_status, true); - if (!history_.set_next_deadline( + if (!history_->set_next_deadline( timer_owner_, steady_clock::now() + duration_cast(deadline_duration_us_))) { @@ -1556,7 +1578,7 @@ bool DataWriterImpl::lifespan_expired() std::unique_lock lock(writer_->getMutex()); CacheChange_t* earliest_change; - while (history_.get_earliest_change(&earliest_change)) + while (history_->get_earliest_change(&earliest_change)) { auto source_timestamp = system_clock::time_point() + nanoseconds(earliest_change->sourceTimestamp.to_ns()); auto now = system_clock::now(); @@ -1570,10 +1592,10 @@ bool DataWriterImpl::lifespan_expired() } // The earliest change has expired - history_.remove_change_pub(earliest_change); + history_->remove_change_pub(earliest_change); // Set the timer for the next change if there is one - if (!history_.get_earliest_change(&earliest_change)) + if (!history_->get_earliest_change(&earliest_change)) { return false; } @@ -2021,30 +2043,29 @@ DataWriterListener* DataWriterImpl::get_listener_for( std::shared_ptr DataWriterImpl::get_change_pool() const { - PoolConfig config = PoolConfig::from_history_attributes(history_.m_att); if (reader_filters_) { return std::make_shared( - config, qos_.writer_resource_limits().reader_filters_allocation); + pool_config_, qos_.writer_resource_limits().reader_filters_allocation); } - return std::make_shared(config); + return std::make_shared(pool_config_); } std::shared_ptr DataWriterImpl::get_payload_pool() { if (!payload_pool_) { + PoolConfig config = pool_config_; + // When the user requested PREALLOCATED_WITH_REALLOC, but we know the type cannot // grow, we translate the policy into bare PREALLOCATED - if (PREALLOCATED_WITH_REALLOC_MEMORY_MODE == history_.m_att.memoryPolicy && + if (PREALLOCATED_WITH_REALLOC_MEMORY_MODE == config.memory_policy && (type_->is_bounded() || type_->is_plain(data_representation_))) { - history_.m_att.memoryPolicy = PREALLOCATED_MEMORY_MODE; + config.memory_policy = PREALLOCATED_MEMORY_MODE; } - PoolConfig config = PoolConfig::from_history_attributes(history_.m_att); - // Avoid calling the serialization size functors on PREALLOCATED mode fixed_payload_size_ = config.memory_policy == PREALLOCATED_MEMORY_MODE ? config.payload_initial_size : 0u; @@ -2086,9 +2107,8 @@ bool DataWriterImpl::release_payload_pool() } else { - PoolConfig config = PoolConfig::from_history_attributes(history_.m_att); auto topic_pool = std::static_pointer_cast(payload_pool_); - result = topic_pool->release_history(config, false); + result = topic_pool->release_history(pool_config_, false); } payload_pool_.reset(); diff --git a/src/cpp/fastdds/publisher/DataWriterImpl.hpp b/src/cpp/fastdds/publisher/DataWriterImpl.hpp index d5b4310c138..1b4397f068e 100644 --- a/src/cpp/fastdds/publisher/DataWriterImpl.hpp +++ b/src/cpp/fastdds/publisher/DataWriterImpl.hpp @@ -34,6 +34,7 @@ #include #include #include +#include #include #include #include @@ -41,9 +42,9 @@ #include #include -#include #include #include +#include namespace eprosima { namespace fastdds { @@ -403,7 +404,7 @@ class DataWriterImpl : protected rtps::IReaderDataFilter std::mutex listener_mutex_; //!History - DataWriterHistory history_; + std::unique_ptr history_; //!Listener to capture the events of the Writer class InnerDataWriterListener : public fastdds::rtps::WriterListener @@ -488,6 +489,8 @@ class DataWriterImpl : protected rtps::IReaderDataFilter uint32_t fixed_payload_size_ = 0u; + rtps::PoolConfig pool_config_ {}; + std::shared_ptr payload_pool_; bool is_custom_payload_pool_ = false; @@ -715,6 +718,10 @@ class DataWriterImpl : protected rtps::IReaderDataFilter private: + void create_history( + const std::shared_ptr& payload_pool, + const std::shared_ptr& change_pool); + DataWriterQos get_datawriter_qos_from_settings( const DataWriterQos& qos); diff --git a/src/cpp/rtps/RTPSDomain.cpp b/src/cpp/rtps/RTPSDomain.cpp index 07c6b2d21ab..779d1feacd9 100644 --- a/src/cpp/rtps/RTPSDomain.cpp +++ b/src/cpp/rtps/RTPSDomain.cpp @@ -331,64 +331,10 @@ RTPSWriter* RTPSDomain::createRTPSWriter( return nullptr; } -RTPSWriter* RTPSDomain::createRTPSWriter( - RTPSParticipant* p, - WriterAttributes& watt, - const std::shared_ptr& payload_pool, - WriterHistory* hist, - WriterListener* listen) -{ - RTPSParticipantImpl* impl = RTPSDomainImpl::find_local_participant(p->getGuid()); - if (impl) - { - RTPSWriter* ret_val = nullptr; - if (impl->createWriter(&ret_val, watt, payload_pool, hist, listen)) - { - return ret_val; - } - } - - return nullptr; -} - -RTPSWriter* RTPSDomain::createRTPSWriter( - RTPSParticipant* p, - WriterAttributes& watt, - const std::shared_ptr& payload_pool, - const std::shared_ptr& change_pool, - WriterHistory* hist, - WriterListener* listen) -{ - RTPSParticipantImpl* impl = RTPSDomainImpl::find_local_participant(p->getGuid()); - if (impl) - { - RTPSWriter* ret_val = nullptr; - if (impl->create_writer(&ret_val, watt, payload_pool, change_pool, hist, listen)) - { - return ret_val; - } - } - - return nullptr; -} - RTPSWriter* RTPSDomain::createRTPSWriter( RTPSParticipant* p, const EntityId_t& entity_id, WriterAttributes& watt, - const std::shared_ptr& payload_pool, - const std::shared_ptr& change_pool, - WriterHistory* hist, - WriterListener* listen) -{ - return RTPSDomainImpl::create_rtps_writer(p, entity_id, watt, payload_pool, change_pool, hist, listen); -} - -RTPSWriter* RTPSDomain::createRTPSWriter( - RTPSParticipant* p, - const EntityId_t& entity_id, - WriterAttributes& watt, - const std::shared_ptr& payload_pool, WriterHistory* hist, WriterListener* listen) { @@ -396,7 +342,7 @@ RTPSWriter* RTPSDomain::createRTPSWriter( if (impl) { RTPSWriter* ret_val = nullptr; - if (impl->createWriter(&ret_val, watt, payload_pool, hist, listen, entity_id)) + if (impl->createWriter(&ret_val, watt, hist, listen, entity_id)) { return ret_val; } @@ -409,8 +355,6 @@ RTPSWriter* RTPSDomainImpl::create_rtps_writer( RTPSParticipant* p, const EntityId_t& entity_id, WriterAttributes& watt, - const std::shared_ptr& payload_pool, - const std::shared_ptr& change_pool, WriterHistory* hist, WriterListener* listen) { @@ -418,7 +362,7 @@ RTPSWriter* RTPSDomainImpl::create_rtps_writer( if (impl) { RTPSWriter* ret_val = nullptr; - if (impl->create_writer(&ret_val, watt, payload_pool, change_pool, hist, listen, entity_id)) + if (impl->create_writer(&ret_val, watt, hist, listen, entity_id, false)) { return ret_val; } diff --git a/src/cpp/rtps/RTPSDomainImpl.hpp b/src/cpp/rtps/RTPSDomainImpl.hpp index 9643b601fab..945cbeda76f 100644 --- a/src/cpp/rtps/RTPSDomainImpl.hpp +++ b/src/cpp/rtps/RTPSDomainImpl.hpp @@ -144,10 +144,8 @@ class RTPSDomainImpl RTPSParticipant* p, const EntityId_t& entity_id, WriterAttributes& watt, - const std::shared_ptr& payload_pool, - const std::shared_ptr& change_pool, WriterHistory* hist, - WriterListener* listen = nullptr); + WriterListener* listen); /** * Creates the guid of a participant given its identifier. diff --git a/src/cpp/rtps/builtin/discovery/endpoint/EDPClient.cpp b/src/cpp/rtps/builtin/discovery/endpoint/EDPClient.cpp index ad32ae88f86..a14528453a5 100644 --- a/src/cpp/rtps/builtin/discovery/endpoint/EDPClient.cpp +++ b/src/cpp/rtps/builtin/discovery/endpoint/EDPClient.cpp @@ -122,12 +122,8 @@ bool EDPClient::removeLocalWriter( { InstanceHandle_t iH; iH = W->getGuid(); - CacheChange_t* change = writer->first->new_change( - [this]() -> uint32_t - { - return mp_PDP->builtin_attributes().writerPayloadSize; - }, - NOT_ALIVE_DISPOSED_UNREGISTERED, iH); + CacheChange_t* change = EDPUtils::create_change(*writer, NOT_ALIVE_DISPOSED_UNREGISTERED, iH, + mp_PDP->builtin_attributes().writerPayloadSize); if (change != nullptr) { { @@ -175,12 +171,8 @@ bool EDPClient::removeLocalReader( { InstanceHandle_t iH; iH = (R->getGuid()); - CacheChange_t* change = writer->first->new_change( - [this]() -> uint32_t - { - return mp_PDP->builtin_attributes().writerPayloadSize; - }, - NOT_ALIVE_DISPOSED_UNREGISTERED, iH); + CacheChange_t* change = EDPUtils::create_change(*writer, NOT_ALIVE_DISPOSED_UNREGISTERED, iH, + mp_PDP->builtin_attributes().writerPayloadSize); if (change != nullptr) { { diff --git a/src/cpp/rtps/builtin/discovery/endpoint/EDPServer.cpp b/src/cpp/rtps/builtin/discovery/endpoint/EDPServer.cpp index 8386a4a6027..3ec267d785c 100644 --- a/src/cpp/rtps/builtin/discovery/endpoint/EDPServer.cpp +++ b/src/cpp/rtps/builtin/discovery/endpoint/EDPServer.cpp @@ -237,12 +237,8 @@ bool EDPServer::removeLocalReader( { // We need to create a DATA(Ur) here to added it to the discovery database, so that the disposal can be // propagated to remote clients - CacheChange_t* change = writer->first->new_change( - [this]() -> uint32_t - { - return mp_PDP->builtin_attributes().readerPayloadSize; - }, - NOT_ALIVE_DISPOSED_UNREGISTERED, guid); + CacheChange_t* change = EDPUtils::create_change(*writer, NOT_ALIVE_DISPOSED_UNREGISTERED, guid, + mp_PDP->builtin_attributes().readerPayloadSize); // Populate the DATA(Ur) if (change != nullptr) @@ -297,12 +293,8 @@ bool EDPServer::removeLocalWriter( { // We need to create a DATA(Uw) here to added it to the discovery database, so that the disposal can be // propagated to remote clients - CacheChange_t* change = writer->first->new_change( - [this]() -> uint32_t - { - return mp_PDP->builtin_attributes().writerPayloadSize; - }, - NOT_ALIVE_DISPOSED_UNREGISTERED, guid); + CacheChange_t* change = EDPUtils::create_change(*writer, NOT_ALIVE_DISPOSED_UNREGISTERED, guid, + mp_PDP->builtin_attributes().writerPayloadSize); // Populate the DATA(Uw) if (change != nullptr) @@ -504,7 +496,7 @@ bool EDPServer::process_and_release_change( { auto builtin_to_release = builtin_to_remove_from; - builtin_to_release.first->release_change(change); + builtin_to_release.second->release_change(change); ret_val = true; } } diff --git a/src/cpp/rtps/builtin/discovery/endpoint/EDPSimple.cpp b/src/cpp/rtps/builtin/discovery/endpoint/EDPSimple.cpp index b5474d76de5..991a3e3b48c 100644 --- a/src/cpp/rtps/builtin/discovery/endpoint/EDPSimple.cpp +++ b/src/cpp/rtps/builtin/discovery/endpoint/EDPSimple.cpp @@ -78,13 +78,12 @@ static void delete_reader( static void delete_writer( RTPSParticipantImpl* participant, - std::pair& writer_pair, - std::shared_ptr& pool) + EDPUtils::WriterHistoryPair& writer_pair) { if (nullptr != writer_pair.first) { participant->deleteUserEndpoint(writer_pair.first->getGuid()); - EDPUtils::release_payload_pool(pool, writer_pair.second->m_att, false); + EDPUtils::release_payload_pool(writer_pair.payload_pool, writer_pair.second->m_att, false); delete(writer_pair.second); } } @@ -104,15 +103,15 @@ EDPSimple::~EDPSimple() delete_reader(mp_RTPSParticipant, publications_secure_reader_, sec_pub_reader_payload_pool_); delete_reader(mp_RTPSParticipant, subscriptions_secure_reader_, sec_sub_reader_payload_pool_); - delete_writer(mp_RTPSParticipant, publications_secure_writer_, sec_pub_writer_payload_pool_); - delete_writer(mp_RTPSParticipant, subscriptions_secure_writer_, sec_sub_writer_payload_pool_); + delete_writer(mp_RTPSParticipant, publications_secure_writer_); + delete_writer(mp_RTPSParticipant, subscriptions_secure_writer_); #endif // if HAVE_SECURITY delete_reader(mp_RTPSParticipant, publications_reader_, pub_reader_payload_pool_); delete_reader(mp_RTPSParticipant, subscriptions_reader_, sub_reader_payload_pool_); - delete_writer(mp_RTPSParticipant, publications_writer_, pub_writer_payload_pool_); - delete_writer(mp_RTPSParticipant, subscriptions_writer_, sub_writer_payload_pool_); + delete_writer(mp_RTPSParticipant, publications_writer_); + delete_writer(mp_RTPSParticipant, subscriptions_writer_); if (nullptr != publications_listener_) { @@ -305,7 +304,7 @@ void EDPSimple::processPersistentData( EDPSimple::t_p_StatefulWriter EDPSimple::get_builtin_writer_history_pair_by_entity( const EntityId_t& entity_id) { - t_p_StatefulWriter ret{nullptr, nullptr}; + t_p_StatefulWriter ret{}; if (entity_id == c_EntityId_SEDPPubWriter) { @@ -421,7 +420,7 @@ bool EDPSimple::createSEDPEndpoints() if (m_discovery.discovery_config.m_simpleEDP.use_PublicationWriterANDSubscriptionReader) { if (!EDPUtils::create_edp_writer(mp_RTPSParticipant, "DCPSPublications", c_EntityId_SEDPPubWriter, - writer_history_att, watt, publications_listener_, pub_writer_payload_pool_, publications_writer_)) + writer_history_att, watt, publications_listener_, publications_writer_)) { return false; } @@ -448,7 +447,7 @@ bool EDPSimple::createSEDPEndpoints() EPROSIMA_LOG_INFO(RTPS_EDP, "SEDP Publication Reader created"); if (!EDPUtils::create_edp_writer(mp_RTPSParticipant, "DCPSSubscriptions", c_EntityId_SEDPSubWriter, - writer_history_att, watt, subscriptions_listener_, sub_writer_payload_pool_, subscriptions_writer_)) + writer_history_att, watt, subscriptions_listener_, subscriptions_writer_)) { return false; } @@ -478,7 +477,7 @@ bool EDPSimple::create_sedp_secure_endpoints() { if (!EDPUtils::create_edp_writer(mp_RTPSParticipant, "DCPSPublicationsSecure", sedp_builtin_publications_secure_writer, writer_history_att, watt, publications_listener_, - sec_pub_writer_payload_pool_, publications_secure_writer_)) + publications_secure_writer_)) { return false; } @@ -508,7 +507,7 @@ bool EDPSimple::create_sedp_secure_endpoints() if (!EDPUtils::create_edp_writer(mp_RTPSParticipant, "DCPSSubscriptionsSecure", sedp_builtin_subscriptions_secure_writer, writer_history_att, watt, subscriptions_listener_, - sec_sub_writer_payload_pool_, subscriptions_secure_writer_)) + subscriptions_secure_writer_)) { return false; } @@ -602,13 +601,8 @@ bool EDPSimple::serialize_proxy_data( if (writer.first != nullptr) { uint32_t cdr_size = data.get_serialized_size(true); - CacheChange_t* change = writer.first->new_change( - [cdr_size]() -> uint32_t - { - return cdr_size; - }, - ALIVE, data.key()); - if (change != nullptr) + CacheChange_t* change = EDPUtils::create_change(writer, ALIVE, data.key(), cdr_size); + if (nullptr != change) { CDRMessage_t aux_msg(change->serializedPayload); @@ -661,12 +655,8 @@ bool EDPSimple::removeLocalWriter( { InstanceHandle_t iH; iH = W->getGuid(); - CacheChange_t* change = writer->first->new_change( - [this]() -> uint32_t - { - return mp_PDP->builtin_attributes().writerPayloadSize; - }, - NOT_ALIVE_DISPOSED_UNREGISTERED, iH); + CacheChange_t* change = EDPUtils::create_change(*writer, NOT_ALIVE_DISPOSED_UNREGISTERED, iH, + mp_PDP->builtin_attributes().writerPayloadSize); if (change != nullptr) { { @@ -715,12 +705,8 @@ bool EDPSimple::removeLocalReader( { InstanceHandle_t iH; iH = (R->getGuid()); - CacheChange_t* change = writer->first->new_change( - [this]() -> uint32_t - { - return mp_PDP->builtin_attributes().writerPayloadSize; - }, - NOT_ALIVE_DISPOSED_UNREGISTERED, iH); + CacheChange_t* change = EDPUtils::create_change(*writer, NOT_ALIVE_DISPOSED_UNREGISTERED, iH, + mp_PDP->builtin_attributes().writerPayloadSize); if (change != nullptr) { { diff --git a/src/cpp/rtps/builtin/discovery/endpoint/EDPSimple.h b/src/cpp/rtps/builtin/discovery/endpoint/EDPSimple.h index 231acb52de8..42dee37c1d3 100644 --- a/src/cpp/rtps/builtin/discovery/endpoint/EDPSimple.h +++ b/src/cpp/rtps/builtin/discovery/endpoint/EDPSimple.h @@ -26,6 +26,8 @@ #include +#include "EDPUtils.hpp" + namespace eprosima { namespace fastdds { namespace rtps { @@ -50,7 +52,7 @@ struct CacheChange_t; */ class EDPSimple : public EDP { - using t_p_StatefulWriter = std::pair; + using t_p_StatefulWriter = EDPUtils::WriterHistoryPair; using t_p_StatefulReader = std::pair; public: @@ -244,15 +246,11 @@ class EDPSimple : public EDP t_p_StatefulReader get_builtin_reader_history_pair_by_entity( const EntityId_t& entity_id); - std::shared_ptr pub_writer_payload_pool_; std::shared_ptr pub_reader_payload_pool_; - std::shared_ptr sub_writer_payload_pool_; std::shared_ptr sub_reader_payload_pool_; #if HAVE_SECURITY - std::shared_ptr sec_pub_writer_payload_pool_; std::shared_ptr sec_pub_reader_payload_pool_; - std::shared_ptr sec_sub_writer_payload_pool_; std::shared_ptr sec_sub_reader_payload_pool_; #endif // if HAVE_SECURITY diff --git a/src/cpp/rtps/builtin/discovery/endpoint/EDPUtils.hpp b/src/cpp/rtps/builtin/discovery/endpoint/EDPUtils.hpp index f11861dcc39..416a0d34dbf 100644 --- a/src/cpp/rtps/builtin/discovery/endpoint/EDPUtils.hpp +++ b/src/cpp/rtps/builtin/discovery/endpoint/EDPUtils.hpp @@ -45,7 +45,12 @@ class EDPUtils { public: - using WriterHistoryPair = std::pair; + struct WriterHistoryPair + { + StatefulWriter* first = nullptr; + WriterHistory* second = nullptr; + std::shared_ptr payload_pool {}; + }; using ReaderHistoryPair = std::pair; static std::shared_ptr create_payload_pool( @@ -111,16 +116,13 @@ class EDPUtils const HistoryAttributes& history_att, WriterAttributes& watt, WriterListener* listener, - std::shared_ptr& payload_pool, WriterHistoryPair& edp_writer) { RTPSWriter* waux = nullptr; - payload_pool = create_payload_pool(topic_name, history_att, false); - edp_writer.second = new WriterHistory(history_att); - bool created = - participant->createWriter(&waux, watt, payload_pool, edp_writer.second, listener, entity_id, - true); + edp_writer.payload_pool = create_payload_pool(topic_name, history_att, false); + edp_writer.second = new WriterHistory(history_att, edp_writer.payload_pool); + bool created = participant->createWriter(&waux, watt, edp_writer.second, listener, entity_id, true); if (created) { @@ -130,12 +132,21 @@ class EDPUtils { delete(edp_writer.second); edp_writer.second = nullptr; - release_payload_pool(payload_pool, history_att, false); + release_payload_pool(edp_writer.payload_pool, history_att, false); } return created; } + static CacheChange_t* create_change( + const WriterHistoryPair& writer, + ChangeKind_t change_kind, + InstanceHandle_t handle, + uint32_t cdr_size) + { + return writer.second->create_change(cdr_size, change_kind, handle); + } + }; } /* namespace rtps */ diff --git a/src/cpp/rtps/builtin/discovery/participant/PDP.cpp b/src/cpp/rtps/builtin/discovery/participant/PDP.cpp index b6939588de9..971bf9ef3f2 100644 --- a/src/cpp/rtps/builtin/discovery/participant/PDP.cpp +++ b/src/cpp/rtps/builtin/discovery/participant/PDP.cpp @@ -479,7 +479,6 @@ void PDP::announceParticipantState( } void PDP::announceParticipantState( - RTPSWriter& writer, WriterHistory& history, bool new_change, bool dispose, @@ -505,12 +504,7 @@ void PDP::announceParticipantState( history.remove_min_change(); } uint32_t cdr_size = proxy_data_copy.get_serialized_size(true); - change = writer.new_change( - [cdr_size]() -> uint32_t - { - return cdr_size; - }, - ALIVE, key); + change = history.create_change(cdr_size, ALIVE, key); if (nullptr != change) { @@ -551,11 +545,7 @@ void PDP::announceParticipantState( history.remove_min_change(); } uint32_t cdr_size = proxy_data_copy.get_serialized_size(true); - change = writer.new_change([cdr_size]() -> uint32_t - { - return cdr_size; - }, - NOT_ALIVE_DISPOSED_UNREGISTERED, key); + change = history.create_change(cdr_size, NOT_ALIVE_DISPOSED_UNREGISTERED, key); if (nullptr != change) { diff --git a/src/cpp/rtps/builtin/discovery/participant/PDP.h b/src/cpp/rtps/builtin/discovery/participant/PDP.h index 17f5a53ef25..ed95dedfe42 100644 --- a/src/cpp/rtps/builtin/discovery/participant/PDP.h +++ b/src/cpp/rtps/builtin/discovery/participant/PDP.h @@ -32,6 +32,7 @@ #include #include #include +#include #include #include #include @@ -585,14 +586,12 @@ class PDP : public fastdds::statistics::rtps::IProxyQueryable /** * Force the sending of our local DPD to all remote RTPSParticipants and multicast Locators. - * @param writer RTPSWriter to use for sending the announcement * @param history history where the change should be added * @param new_change If true a new change (with new seqNum) is created and sent;If false the last change is re-sent * @param dispose sets change kind to NOT_ALIVE_DISPOSED_UNREGISTERED * @param wparams allows to identify the change */ void announceParticipantState( - RTPSWriter& writer, WriterHistory& history, bool new_change, bool dispose = false, diff --git a/src/cpp/rtps/builtin/discovery/participant/PDPClient.cpp b/src/cpp/rtps/builtin/discovery/participant/PDPClient.cpp index cfcf1873718..90e10d43fe7 100644 --- a/src/cpp/rtps/builtin/discovery/participant/PDPClient.cpp +++ b/src/cpp/rtps/builtin/discovery/participant/PDPClient.cpp @@ -739,13 +739,12 @@ void PDPClient::announceParticipantState( // In order to avoid that we send the message directly as in the standard stateless PDP CacheChange_t* change = nullptr; + change = history.create_change( + mp_builtin->m_att.writerPayloadSize, + NOT_ALIVE_DISPOSED_UNREGISTERED, + getLocalParticipantProxyData()->m_key); - if ((change = writer.new_change( - [this]() -> uint32_t - { - return mp_builtin->m_att.writerPayloadSize; - }, - NOT_ALIVE_DISPOSED_UNREGISTERED, getLocalParticipantProxyData()->m_key))) + if (nullptr != change) { // update the sequence number change->sequenceNumber = history.next_sequence_number(); @@ -788,11 +787,11 @@ void PDPClient::announceParticipantState( } // free change - writer.release_change(change); + history.release_change(change); } else { - PDP::announceParticipantState(writer, history, new_change, dispose, wp); + PDP::announceParticipantState(history, new_change, dispose, wp); if (!new_change) { diff --git a/src/cpp/rtps/builtin/discovery/participant/PDPServer.cpp b/src/cpp/rtps/builtin/discovery/participant/PDPServer.cpp index ab3fe13bbb2..f8048d43096 100644 --- a/src/cpp/rtps/builtin/discovery/participant/PDPServer.cpp +++ b/src/cpp/rtps/builtin/discovery/participant/PDPServer.cpp @@ -850,13 +850,7 @@ void PDPServer::announceParticipantState( getMutex()->unlock(); uint32_t cdr_size = proxy_data_copy.get_serialized_size(true); - change = writer.new_change( - [cdr_size]() -> uint32_t - { - return cdr_size; - }, - ALIVE, proxy_data_copy.m_key); - + change = history.create_change(cdr_size, ALIVE, proxy_data_copy.m_key); if (change != nullptr) { CDRMessage_t aux_msg(change->serializedPayload); @@ -915,7 +909,7 @@ void PDPServer::announceParticipantState( // Already there, dispose EPROSIMA_LOG_ERROR(RTPS_PDP_SERVER, "DiscoveryDatabase already initialized with local DATA(p) on creation"); - writer.release_change(change); + history.release_change(change); } } // Doesn't make sense to send the DATA directly if it hasn't been introduced in the history yet (missing @@ -963,14 +957,8 @@ void PDPServer::announceParticipantState( // Unlock PDP mutex since it's no longer needed. getMutex()->unlock(); - change = writer.new_change( - [cdr_size]() -> uint32_t - { - return cdr_size; - }, - NOT_ALIVE_DISPOSED_UNREGISTERED, key); - // Generate the Data(Up) + change = history.create_change(cdr_size, NOT_ALIVE_DISPOSED_UNREGISTERED, key); if (nullptr != change) { // Assign identity @@ -987,7 +975,7 @@ void PDPServer::announceParticipantState( { // Dispose if already there // It may happen if the participant is not removed fast enough - writer.release_change(change); + history.release_change(change); return; } } @@ -1396,7 +1384,7 @@ void PDPServer::process_changes_release_( // Normally Data(Up) will not be in history except in Own Server destruction if (!remove_change_from_writer_history(endpoints->writer.writer_, endpoints->writer.history_.get(), ch)) { - endpoints->writer.writer_->release_change(ch); + endpoints->writer.history_->release_change(ch); } } else @@ -2053,7 +2041,7 @@ void PDPServer::release_change_from_writer( eprosima::fastdds::rtps::CacheChange_t* change) { auto endpoints = static_cast(builtin_endpoints_.get()); - endpoints->writer.writer_->release_change(change); + endpoints->writer.history_->release_change(change); } } // namespace rtps diff --git a/src/cpp/rtps/builtin/discovery/participant/PDPSimple.cpp b/src/cpp/rtps/builtin/discovery/participant/PDPSimple.cpp index d7d1c08395c..706c9042b40 100644 --- a/src/cpp/rtps/builtin/discovery/participant/PDPSimple.cpp +++ b/src/cpp/rtps/builtin/discovery/participant/PDPSimple.cpp @@ -277,16 +277,14 @@ void PDPSimple::announceParticipantState( auto secure = dynamic_cast(builtin_endpoints_.get()); assert(nullptr != secure); - RTPSWriter& writer = *(secure->secure_writer.writer_); WriterHistory& history = *(secure->secure_writer.history_); - PDP::announceParticipantState(writer, history, new_change, dispose, wp); + PDP::announceParticipantState(history, new_change, dispose, wp); } #endif // HAVE_SECURITY auto endpoints = dynamic_cast(builtin_endpoints_.get()); - RTPSWriter& writer = *(endpoints->writer.writer_); WriterHistory& history = *(endpoints->writer.history_); - PDP::announceParticipantState(writer, history, new_change, dispose, wp); + PDP::announceParticipantState(history, new_change, dispose, wp); if (!(dispose || new_change)) { @@ -380,7 +378,7 @@ bool PDPSimple::create_dcps_participant_endpoints() PoolConfig writer_pool_cfg = PoolConfig::from_history_attributes(hatt); writer.payload_pool_ = TopicPayloadPoolRegistry::get(topic_name, writer_pool_cfg); writer.payload_pool_->reserve_history(writer_pool_cfg, false); - writer.history_.reset(new WriterHistory(hatt)); + writer.history_.reset(new WriterHistory(hatt, writer.payload_pool_)); WriterAttributes watt = create_builtin_writer_attributes(); watt.endpoint.reliabilityKind = BEST_EFFORT; @@ -399,7 +397,7 @@ bool PDPSimple::create_dcps_participant_endpoints() } RTPSWriter* rtps_writer = nullptr; - if (mp_RTPSParticipant->createWriter(&rtps_writer, watt, writer.payload_pool_, writer.history_.get(), + if (mp_RTPSParticipant->createWriter(&rtps_writer, watt, writer.history_.get(), nullptr, writer_entity_id, true)) { writer.writer_ = dynamic_cast(rtps_writer); @@ -513,10 +511,10 @@ bool PDPSimple::create_dcps_participant_secure_endpoints() PoolConfig writer_pool_cfg = PoolConfig::from_history_attributes(hatt); writer.payload_pool_ = TopicPayloadPoolRegistry::get(topic_name, writer_pool_cfg); writer.payload_pool_->reserve_history(writer_pool_cfg, false); - writer.history_.reset(new WriterHistory(hatt)); + writer.history_.reset(new WriterHistory(hatt, writer.payload_pool_)); RTPSWriter* rtps_writer = nullptr; - if (mp_RTPSParticipant->createWriter(&rtps_writer, watt, writer.payload_pool_, writer.history_.get(), + if (mp_RTPSParticipant->createWriter(&rtps_writer, watt, writer.history_.get(), nullptr, writer_entity_id, true)) { writer.writer_ = dynamic_cast(rtps_writer); diff --git a/src/cpp/rtps/builtin/liveliness/WLP.cpp b/src/cpp/rtps/builtin/liveliness/WLP.cpp index 972b988fbe6..2068d7c7808 100644 --- a/src/cpp/rtps/builtin/liveliness/WLP.cpp +++ b/src/cpp/rtps/builtin/liveliness/WLP.cpp @@ -241,11 +241,10 @@ bool WLP::createEndpoints() // Built-in writer history HistoryAttributes hatt; set_builtin_writer_history_attributes(hatt, false); - mp_builtinWriterHistory = new WriterHistory(hatt); - PoolConfig writer_pool_cfg = PoolConfig::from_history_attributes(hatt); payload_pool_ = TopicPayloadPoolRegistry::get("DCPSParticipantMessage", writer_pool_cfg); payload_pool_->reserve_history(writer_pool_cfg, false); + mp_builtinWriterHistory = new WriterHistory(hatt, payload_pool_); // Built-in writer WriterAttributes watt; @@ -262,7 +261,6 @@ bool WLP::createEndpoints() if (mp_participant->createWriter( &wout, watt, - payload_pool_, mp_builtinWriterHistory, nullptr, c_EntityId_WriterLiveliness, @@ -342,11 +340,10 @@ bool WLP::createSecureEndpoints() //CREATE WRITER HistoryAttributes hatt; set_builtin_writer_history_attributes(hatt, true); - mp_builtinWriterSecureHistory = new WriterHistory(hatt); - PoolConfig writer_pool_cfg = PoolConfig::from_history_attributes(hatt); secure_payload_pool_ = TopicPayloadPoolRegistry::get("DCPSParticipantMessageSecure", writer_pool_cfg); secure_payload_pool_->reserve_history(writer_pool_cfg, false); + mp_builtinWriterSecureHistory = new WriterHistory(hatt, secure_payload_pool_); WriterAttributes watt; watt.endpoint.unicastLocatorList = mp_builtinProtocols->m_metatrafficUnicastLocatorList; @@ -379,7 +376,7 @@ bool WLP::createSecureEndpoints() } RTPSWriter* wout; - if (mp_participant->createWriter(&wout, watt, secure_payload_pool_, mp_builtinWriterSecureHistory, nullptr, + if (mp_participant->createWriter(&wout, watt, mp_builtinWriterSecureHistory, nullptr, c_EntityId_WriterLivelinessSecure, true)) { mp_builtinWriterSecure = dynamic_cast(wout); @@ -899,17 +896,11 @@ bool WLP::send_liveliness_message( { StatefulWriter* writer = builtin_writer(); WriterHistory* history = builtin_writer_history(); + std::shared_ptr pool = builtin_writer_pool(); std::lock_guard wguard(writer->getMutex()); - CacheChange_t* change = writer->new_change( - []() -> uint32_t - { - return WLP::builtin_participant_data_max_size; - }, - ALIVE, - instance); - + CacheChange_t* change = history->create_change(WLP::builtin_participant_data_max_size, ALIVE, instance); if (change != nullptr) { change->serializedPayload.encapsulation = (uint16_t)DEFAULT_ENCAPSULATION; @@ -971,6 +962,18 @@ WriterHistory* WLP::builtin_writer_history() return ret_val; } +std::shared_ptr WLP::builtin_writer_pool() +{ +#if HAVE_SECURITY + if (mp_participant->security_attributes().is_liveliness_protected) + { + return secure_payload_pool_; + } +#endif // if HAVE_SECURITY + + return payload_pool_; +} + bool WLP::assert_liveliness( GUID_t writer, dds::LivelinessQosPolicyKind kind, diff --git a/src/cpp/rtps/builtin/liveliness/WLP.hpp b/src/cpp/rtps/builtin/liveliness/WLP.hpp index 58e68634413..6ee7976e94e 100644 --- a/src/cpp/rtps/builtin/liveliness/WLP.hpp +++ b/src/cpp/rtps/builtin/liveliness/WLP.hpp @@ -169,6 +169,12 @@ class WLP */ WriterHistory* builtin_writer_history(); + /** + * Get the liveliness builtin writer's payload pool + * @return payload pool + */ + std::shared_ptr builtin_writer_pool(); + #if HAVE_SECURITY bool pairing_remote_reader_with_local_writer_after_security( const GUID_t& local_writer, diff --git a/src/cpp/rtps/history/ReaderHistory.cpp b/src/cpp/rtps/history/ReaderHistory.cpp index 43f05f13392..e8470707baf 100644 --- a/src/cpp/rtps/history/ReaderHistory.cpp +++ b/src/cpp/rtps/history/ReaderHistory.cpp @@ -256,13 +256,6 @@ bool ReaderHistory::get_min_change_from( return ret; } -bool ReaderHistory::do_reserve_cache( - CacheChange_t** change, - uint32_t size) -{ - return BaseReader::downcast(mp_reader)->reserve_cache(size, *change); -} - void ReaderHistory::do_release_cache( CacheChange_t* ch) { diff --git a/src/cpp/rtps/history/WriterHistory.cpp b/src/cpp/rtps/history/WriterHistory.cpp index 363e5083f95..9df5f827992 100644 --- a/src/cpp/rtps/history/WriterHistory.cpp +++ b/src/cpp/rtps/history/WriterHistory.cpp @@ -18,31 +18,107 @@ #include +#include +#include +#include +#include #include +#include +#include #include -#include +#include +#include +#include +#include +#include +#include +#include +#include +#include #include -#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include #include namespace eprosima { namespace fastdds { namespace rtps { +static CacheChange_t* initialize_change( + CacheChange_t* reserved_change, + ChangeKind_t change_kind, + InstanceHandle_t handle, + RTPSWriter* writer) +{ + reserved_change->kind = change_kind; + if ((WITH_KEY == writer->getAttributes().topicKind) && !handle.isDefined()) + { + EPROSIMA_LOG_WARNING(RTPS_WRITER, "Changes in KEYED Writers need a valid instanceHandle"); + } + reserved_change->instanceHandle = handle; + reserved_change->writerGUID = writer->getGuid(); + reserved_change->writer_info.previous = nullptr; + reserved_change->writer_info.next = nullptr; + reserved_change->writer_info.num_sent_submessages = 0; + reserved_change->vendor_id = c_VendorId_eProsima; + return reserved_change; +} + WriteParams WriteParams::WRITE_PARAM_DEFAULT; WriterHistory::WriterHistory( const HistoryAttributes& att) : History(att) - , mp_writer(nullptr) { + PoolConfig cfg = PoolConfig::from_history_attributes(att); + payload_pool_ = BasicPayloadPool::get(cfg, change_pool_); +} + +WriterHistory::WriterHistory( + const HistoryAttributes& att, + const std::shared_ptr& payload_pool) + : History(att) + , change_pool_(std::make_shared(PoolConfig::from_history_attributes(att))) + , payload_pool_(payload_pool) +{ +} +WriterHistory::WriterHistory( + const HistoryAttributes& att, + const std::shared_ptr& payload_pool, + const std::shared_ptr& change_pool) + : History(att) + , change_pool_(change_pool) + , payload_pool_(payload_pool) +{ } WriterHistory::~WriterHistory() { - // TODO Auto-generated destructor stub + // As releasing the change pool will delete the cache changes it owns, + // the payload pool may be called to release their payloads, so we should + // ensure that the payload pool is destroyed after the change pool. + change_pool_.reset(); + payload_pool_.reset(); +} + +const std::shared_ptr& WriterHistory::get_payload_pool() const +{ + return payload_pool_; +} + +const std::shared_ptr& WriterHistory::get_change_pool() const +{ + return change_pool_; } bool WriterHistory::add_change( @@ -207,7 +283,7 @@ History::iterator WriterHistory::remove_change_nts( // Release from pools if ( release ) { - mp_writer->release_change(change); + release_change(change); } return ret_val; @@ -238,7 +314,7 @@ bool WriterHistory::remove_change( if (nullptr != p ) { - mp_writer->release_change(p); + release_change(p); return true; } @@ -313,22 +389,70 @@ bool WriterHistory::remove_min_change( //TODO Hacer metodos de remove_all_changes. y hacer los metodos correspondientes en los writers y publishers. -bool WriterHistory::do_reserve_cache( - CacheChange_t** change, - uint32_t size) +CacheChange_t* WriterHistory::create_change( + ChangeKind_t changeKind, + InstanceHandle_t handle) { - *change = mp_writer->new_change( - [size]() - { - return size; - }, ALIVE); - return *change != nullptr; + EPROSIMA_LOG_INFO(RTPS_WRITER, "Creating new change"); + + std::lock_guard guard(*mp_mutex); + CacheChange_t* reserved_change = nullptr; + if (!change_pool_->reserve_cache(reserved_change)) + { + EPROSIMA_LOG_WARNING(RTPS_WRITER, "Problem reserving cache from pool"); + return nullptr; + } + + return initialize_change(reserved_change, changeKind, handle, mp_writer); +} + +CacheChange_t* WriterHistory::create_change( + uint32_t payload_size, + ChangeKind_t changeKind, + InstanceHandle_t handle) +{ + EPROSIMA_LOG_INFO(RTPS_WRITER, "Creating new change"); + + std::lock_guard guard(*mp_mutex); + CacheChange_t* reserved_change = nullptr; + if (!change_pool_->reserve_cache(reserved_change)) + { + EPROSIMA_LOG_WARNING(RTPS_WRITER, "Problem reserving cache from pool"); + return nullptr; + } + + if (!payload_pool_->get_payload(payload_size, reserved_change->serializedPayload)) + { + change_pool_->release_cache(reserved_change); + EPROSIMA_LOG_WARNING(RTPS_WRITER, "Problem reserving payload from pool"); + return nullptr; + } + + return initialize_change(reserved_change, changeKind, handle, mp_writer); +} + +bool WriterHistory::release_change( + CacheChange_t* ch) +{ + // Asserting preconditions + assert(mp_writer != nullptr); + assert(ch != nullptr); + assert(ch->writerGUID == mp_writer->getGuid()); + + std::lock_guard guard(*mp_mutex); + + IPayloadPool* pool = ch->serializedPayload.payload_owner; + if (pool) + { + pool->release_payload(ch->serializedPayload); + } + return change_pool_->release_cache(ch); } void WriterHistory::do_release_cache( CacheChange_t* ch) { - mp_writer->release_change(ch); + release_change(ch); } void WriterHistory::set_fragments( diff --git a/src/cpp/rtps/messages/RTPSMessageGroup.cpp b/src/cpp/rtps/messages/RTPSMessageGroup.cpp index 348a4844e9c..6db3b348bea 100644 --- a/src/cpp/rtps/messages/RTPSMessageGroup.cpp +++ b/src/cpp/rtps/messages/RTPSMessageGroup.cpp @@ -575,13 +575,13 @@ bool RTPSMessageGroup::add_data( InlineQosWriter* inline_qos; inline_qos = (change.inline_qos.length > 0 && nullptr != change.inline_qos.data) ? &qos_writer : nullptr; - bool copy_data = false; + bool copy_data = (nullptr == change.serializedPayload.payload_owner); #if HAVE_SECURITY uint32_t from_buffer_position = submessage_msg_->pos; bool protect_payload = endpoint_->getAttributes().security_attributes().is_payload_protected; bool protect_submessage = endpoint_->getAttributes().security_attributes().is_submessage_protected; bool protect_rtps = participant_->security_attributes().is_rtps_protected; - copy_data = protect_payload || protect_submessage || protect_rtps; + copy_data = copy_data || protect_payload || protect_submessage || protect_rtps; #endif // if HAVE_SECURITY const EntityId_t& readerId = get_entity_id(sender_->remote_guids()); @@ -694,13 +694,13 @@ bool RTPSMessageGroup::add_data_frag( InlineQosWriter* inline_qos; inline_qos = (change.inline_qos.length > 0 && nullptr != change.inline_qos.data) ? &qos_writer : nullptr; - bool copy_data = false; + bool copy_data = (nullptr == change.serializedPayload.payload_owner); #if HAVE_SECURITY uint32_t from_buffer_position = submessage_msg_->pos; bool protect_payload = endpoint_->getAttributes().security_attributes().is_payload_protected; bool protect_submessage = endpoint_->getAttributes().security_attributes().is_submessage_protected; bool protect_rtps = participant_->security_attributes().is_rtps_protected; - copy_data = protect_payload || protect_submessage || protect_rtps; + copy_data = copy_data || protect_payload || protect_submessage || protect_rtps; #endif // if HAVE_SECURITY const EntityId_t& readerId = get_entity_id(sender_->remote_guids()); diff --git a/src/cpp/rtps/participant/RTPSParticipantImpl.cpp b/src/cpp/rtps/participant/RTPSParticipantImpl.cpp index 3f4716b0838..0b29d33ac7e 100644 --- a/src/cpp/rtps/participant/RTPSParticipantImpl.cpp +++ b/src/cpp/rtps/participant/RTPSParticipantImpl.cpp @@ -26,6 +26,7 @@ #include #include +#include #include #include #include @@ -50,6 +51,7 @@ #include #include #include +#include #include #include #include @@ -60,8 +62,8 @@ #include #include #include -#include #include +#include #include #include #include @@ -948,12 +950,6 @@ bool RTPSParticipantImpl::create_writer( return false; } - if (!SWriter->is_pool_initialized()) - { - delete(SWriter); - return false; - } - // Use participant's external locators if writer has none // WARNING: call before createAndAssociateReceiverswithEndpoint, as the latter intentionally clears external // locators list when using unique_flows feature @@ -1181,135 +1177,77 @@ bool RTPSParticipantImpl::createWriter( const EntityId_t& entityId, bool isBuiltin) { - auto callback = [hist, listen, this] - (const GUID_t& guid, WriterAttributes& param, fastdds::rtps::FlowController* flow_controller, - IPersistenceService* persistence, bool is_reliable) -> RTPSWriter* - { - if (is_reliable) - { - if (persistence != nullptr) - { - return new StatefulPersistentWriter(this, guid, param, flow_controller, - hist, listen, persistence); - } - else - { - return new StatefulWriter(this, guid, param, flow_controller, - hist, listen); - } - } - else - { - if (persistence != nullptr) - { - return new StatelessPersistentWriter(this, guid, param, flow_controller, - hist, listen, persistence); - } - else - { - return new StatelessWriter(this, guid, param, flow_controller, - hist, listen); - } - } - }; - return create_writer(WriterOut, param, entityId, isBuiltin, callback); -} + *WriterOut = nullptr; -bool RTPSParticipantImpl::createWriter( - RTPSWriter** WriterOut, - WriterAttributes& param, - const std::shared_ptr& payload_pool, - WriterHistory* hist, - WriterListener* listen, - const EntityId_t& entityId, - bool isBuiltin) -{ - if (!payload_pool) + if (param.endpoint.data_sharing_configuration().kind() != dds::DataSharingKind::OFF) { - EPROSIMA_LOG_ERROR(RTPS_PARTICIPANT, "Trying to create writer with null payload pool"); + EPROSIMA_LOG_ERROR(RTPS_PARTICIPANT, "Data sharing needs a DataSharing payload pool"); return false; } - auto callback = [hist, listen, &payload_pool, this] - (const GUID_t& guid, WriterAttributes& param, fastdds::rtps::FlowController* flow_controller, - IPersistenceService* persistence, bool is_reliable) -> RTPSWriter* - { - if (is_reliable) - { - if (persistence != nullptr) - { - return new StatefulPersistentWriter(this, guid, param, payload_pool, flow_controller, - hist, listen, persistence); - } - else - { - return new StatefulWriter(this, guid, param, payload_pool, flow_controller, - hist, listen); - } - } - else - { - if (persistence != nullptr) - { - return new StatelessPersistentWriter(this, guid, param, payload_pool, flow_controller, - hist, listen, persistence); - } - else - { - return new StatelessWriter(this, guid, param, payload_pool, flow_controller, - hist, listen); - } - } - }; - return create_writer(WriterOut, param, entityId, isBuiltin, callback); + return create_writer(WriterOut, param, hist, listen, entityId, isBuiltin); } bool RTPSParticipantImpl::create_writer( RTPSWriter** WriterOut, WriterAttributes& watt, - const std::shared_ptr& payload_pool, - const std::shared_ptr& change_pool, WriterHistory* hist, WriterListener* listen, const EntityId_t& entityId, bool isBuiltin) { - if (!payload_pool) + *WriterOut = nullptr; + + if (hist == nullptr) + { + EPROSIMA_LOG_ERROR(RTPS_PARTICIPANT, "Need a WriterHistory to create an RTPSWriter"); + return false; + } + + if (!hist->get_payload_pool()) { - EPROSIMA_LOG_ERROR(RTPS_PARTICIPANT, "Trying to create writer with null payload pool"); + EPROSIMA_LOG_ERROR(RTPS_PARTICIPANT, "WriterHistory needs a payload pool to create an RTPSWriter"); return false; } - auto callback = [hist, listen, &payload_pool, &change_pool, this] + if (!hist->get_change_pool()) + { + EPROSIMA_LOG_ERROR(RTPS_PARTICIPANT, "WriterHistory needs a change pool to create an RTPSWriter"); + return false; + } + + auto callback = [hist, listen, this] (const GUID_t& guid, WriterAttributes& watt, fastdds::rtps::FlowController* flow_controller, IPersistenceService* persistence, bool is_reliable) -> RTPSWriter* { + RTPSWriter* writer = nullptr; + if (is_reliable) { if (persistence != nullptr) { - return new StatefulPersistentWriter(this, guid, watt, payload_pool, change_pool, - flow_controller, hist, listen, persistence); + writer = new StatefulPersistentWriter(this, guid, watt, + flow_controller, hist, listen, persistence); } else { - return new StatefulWriter(this, guid, watt, payload_pool, change_pool, - flow_controller, hist, listen); + writer = new StatefulWriter(this, guid, watt, flow_controller, hist, listen); } } else { if (persistence != nullptr) { - return new StatelessPersistentWriter(this, guid, watt, payload_pool, change_pool, - flow_controller, hist, listen, persistence); + writer = new StatelessPersistentWriter(this, guid, watt, + flow_controller, hist, listen, persistence); } else { - return new StatelessWriter(this, guid, watt, payload_pool, change_pool, - flow_controller, hist, listen); + writer = new StatelessWriter(this, guid, watt, flow_controller, hist, listen); } } + + return writer; }; return create_writer(WriterOut, watt, entityId, isBuiltin, callback); } @@ -3005,13 +2943,12 @@ const fastdds::statistics::rtps::IStatusObserver* RTPSParticipantImpl::create_mo status_queryable, [&](RTPSWriter** WriterOut, WriterAttributes& param, - const std::shared_ptr& payload_pool, WriterHistory* hist, WriterListener* listen, const EntityId_t& entityId, bool isBuiltin) -> bool { - return this->createWriter(WriterOut, param, payload_pool, hist, listen, entityId, isBuiltin); + return this->createWriter(WriterOut, param, hist, listen, entityId, isBuiltin); }, [&](RTPSWriter* w, const fastdds::TopicAttributes& topicAtt, diff --git a/src/cpp/rtps/participant/RTPSParticipantImpl.h b/src/cpp/rtps/participant/RTPSParticipantImpl.h index a547da45750..e9a9ea3acfe 100644 --- a/src/cpp/rtps/participant/RTPSParticipantImpl.h +++ b/src/cpp/rtps/participant/RTPSParticipantImpl.h @@ -830,45 +830,23 @@ class RTPSParticipantImpl /** * Create a Writer in this RTPSParticipant with a custom payload pool. - * @param Writer Pointer to pointer of the Writer, used as output. Only valid if return==true. - * @param param WriterAttributes to define the Writer. - * @param payload_pool Shared pointer to the IPayloadPool - * @param hist Pointer to the WriterHistory. - * @param listen Pointer to the WriterListener. - * @param entityId EntityId assigned to the Writer. - * @param isBuiltin Bool value indicating if the Writer is builtin (Discovery or Liveliness protocol) or is created for the end user. - * @return True if the Writer was correctly created. - */ - bool createWriter( - RTPSWriter** Writer, - WriterAttributes& param, - const std::shared_ptr& payload_pool, - WriterHistory* hist, - WriterListener* listen, - const EntityId_t& entityId = c_EntityId_Unknown, - bool isBuiltin = false); - - /** - * Create a Writer in this RTPSParticipant with a custom payload pool. - * @param Writer Pointer to pointer of the Writer, used as output. Only valid if return==true. - * @param watt WriterAttributes to define the Writer. - * @param payload_pool Shared pointer to the IPayloadPool - * @param change_pool Shared pointer to the IChangePool - * @param hist Pointer to the WriterHistory. - * @param listen Pointer to the WriterListener. - * @param entityId EntityId assigned to the Writer. - * @param isBuiltin Bool value indicating if the Writer is builtin (Discovery or Liveliness protocol) or is created for the end user. + * + * @param Writer Pointer to pointer of the Writer, used as output. Only valid if return==true. + * @param watt WriterAttributes to define the Writer. + * @param hist Pointer to the WriterHistory. + * @param listen Pointer to the WriterListener. + * @param entityId EntityId assigned to the Writer. + * @param isBuiltin Bool value indicating if the Writer is builtin (Discovery or Liveliness protocol) or is created for the end user. + * * @return True if the Writer was correctly created. */ bool create_writer( RTPSWriter** Writer, WriterAttributes& watt, - const std::shared_ptr& payload_pool, - const std::shared_ptr& change_pool, WriterHistory* hist, WriterListener* listen, - const EntityId_t& entityId = c_EntityId_Unknown, - bool isBuiltin = false); + const EntityId_t& entityId, + bool isBuiltin); /** * Create a Reader in this RTPSParticipant. diff --git a/src/cpp/rtps/persistence/PersistenceService.h b/src/cpp/rtps/persistence/PersistenceService.h index dddca8a2a97..fe05dc5dc66 100644 --- a/src/cpp/rtps/persistence/PersistenceService.h +++ b/src/cpp/rtps/persistence/PersistenceService.h @@ -51,20 +51,18 @@ class IPersistenceService /** * Get all data stored for a writer. - * @param persistence_guid GUID of the writer used to store samples. - * @param writer_guid GUID of the writer to load. - * @param changes History of the writer to load. - * @param change_pool Pool where new changes should be obtained from. - * @param payload_pool Pool where payloads should be obtained from. - * @param next_sequence Sequence that should be applied to the next created sample. + * + * @param [in] persistence_guid GUID of the writer used to store samples. + * @param [in] writer_guid GUID of the writer to load. + * @param [in,out] history History of the writer to load. + * @param [out] next_sequence Sequence that should be applied to the next created sample. + * * @return True if operation was successful. */ virtual bool load_writer_from_storage( const std::string& persistence_guid, const GUID_t& writer_guid, WriterHistory* history, - const std::shared_ptr& change_pool, - const std::shared_ptr& payload_pool, SequenceNumber_t& next_sequence) = 0; /** diff --git a/src/cpp/rtps/persistence/SQLite3PersistenceService.cpp b/src/cpp/rtps/persistence/SQLite3PersistenceService.cpp index 1e4e1009581..dc832029e14 100644 --- a/src/cpp/rtps/persistence/SQLite3PersistenceService.cpp +++ b/src/cpp/rtps/persistence/SQLite3PersistenceService.cpp @@ -248,19 +248,18 @@ SQLite3PersistenceService::~SQLite3PersistenceService() /** * Get all data stored for a writer. - * @param persistence_guid GUID of persistence service that holds the data. - * @param writer_guid GUID of the writer to load. - * @param changes History of CacheChanges of the writer. It will be filled. - * @param pool Pool of CacheChanges from which new ones are reserved to add to the history. - * @param next_sequence Buffer to fill with the last sequence number on the history. + * + * @param [in] persistence_guid GUID of the writer used to store samples. + * @param [in] writer_guid GUID of the writer to load. + * @param [in,out] history History of the writer to load. + * @param [out] next_sequence Sequence that should be applied to the next created sample. + * * @return True if operation was successful. */ bool SQLite3PersistenceService::load_writer_from_storage( const std::string& persistence_guid, const GUID_t& writer_guid, WriterHistory* history, - const std::shared_ptr& change_pool, - const std::shared_ptr& payload_pool, SequenceNumber_t& next_sequence) { EPROSIMA_LOG_INFO(RTPS_PERSISTENCE, "Loading writer " << writer_guid); @@ -278,7 +277,8 @@ bool SQLite3PersistenceService::load_writer_from_storage( CacheChange_t* change = nullptr; int size = sqlite3_column_bytes(load_writer_stmt_, 2); - if (!change_pool->reserve_cache(change)) + change = history->create_change(size, ALIVE); + if (nullptr == change) { continue; } @@ -286,11 +286,6 @@ bool SQLite3PersistenceService::load_writer_from_storage( SampleIdentity identity; identity.writer_guid(writer_guid); identity.sequence_number(sn); - if (!payload_pool->get_payload(size, change->serializedPayload)) - { - change_pool->release_cache(change); - continue; - } int instance_size = sqlite3_column_bytes(load_writer_stmt_, 1); instance_size = (instance_size > 16) ? 16 : instance_size; diff --git a/src/cpp/rtps/persistence/SQLite3PersistenceService.h b/src/cpp/rtps/persistence/SQLite3PersistenceService.h index 0fc476e67c1..be617c5810d 100644 --- a/src/cpp/rtps/persistence/SQLite3PersistenceService.h +++ b/src/cpp/rtps/persistence/SQLite3PersistenceService.h @@ -47,17 +47,10 @@ class SQLite3PersistenceService : public IPersistenceService sqlite3* db); virtual ~SQLite3PersistenceService() override; - /** - * Get all data stored for a writer. - * @param writer_guid GUID of the writer to load. - * @return True if operation was successful. - */ bool load_writer_from_storage( const std::string& persistence_guid, const GUID_t& writer_guid, WriterHistory* history, - const std::shared_ptr& change_pool, - const std::shared_ptr& payload_pool, SequenceNumber_t& next_sequence) final; /** diff --git a/src/cpp/rtps/reader/BaseReader.cpp b/src/cpp/rtps/reader/BaseReader.cpp index 05578e04113..6587a8ac05a 100644 --- a/src/cpp/rtps/reader/BaseReader.cpp +++ b/src/cpp/rtps/reader/BaseReader.cpp @@ -114,6 +114,12 @@ BaseReader::~BaseReader() } delete history_state_; + + // As releasing the change pool will delete the cache changes it owns, + // the payload pool may be called to release their payloads, so we should + // ensure that the payload pool is destroyed after the change pool. + change_pool_.reset(); + payload_pool_.reset(); } ReaderListener* BaseReader::get_listener() const diff --git a/src/cpp/rtps/reader/BaseReader.hpp b/src/cpp/rtps/reader/BaseReader.hpp index a542b421059..378b9fd8afe 100644 --- a/src/cpp/rtps/reader/BaseReader.hpp +++ b/src/cpp/rtps/reader/BaseReader.hpp @@ -401,6 +401,12 @@ class BaseReader bool is_datasharing_compatible_with( const fastdds::rtps::WriterProxyData& wdata); + /// Pool of serialized payloads. + std::shared_ptr payload_pool_; + + /// Pool of cache changes. + std::shared_ptr change_pool_; + /// Pointer to the listener associated with this reader. fastdds::rtps::ReaderListener* listener_; /// Whether the reader accepts messages from unmatched writers. diff --git a/src/cpp/rtps/security/SecurityManager.cpp b/src/cpp/rtps/security/SecurityManager.cpp index 757fbecf1bb..fa9926db210 100644 --- a/src/cpp/rtps/security/SecurityManager.cpp +++ b/src/cpp/rtps/security/SecurityManager.cpp @@ -76,6 +76,15 @@ inline bool usleep_bool() return true; } +static CacheChange_t* create_change_for_message( + const ParticipantGenericMessage& message, + WriterHistory* history) +{ + uint32_t cdr_size = static_cast(ParticipantGenericMessageHelper::serialized_size(message)); + cdr_size += 4; // Encapsulation + return history->create_change(cdr_size, ALIVE, c_InstanceHandle_Unknown); +} + SecurityManager::SecurityManager( RTPSParticipantImpl* participant, ISecurityPluginFactory& plugin_factory) @@ -899,12 +908,9 @@ bool SecurityManager::on_process_handshake( ParticipantGenericMessage message = generate_authentication_message(std::move(message_identity), participant_data.m_guid, *handshake_message); - CacheChange_t* change = participant_stateless_message_writer_->new_change([&message]() -> uint32_t - { - return static_cast(ParticipantGenericMessageHelper::serialized_size(message) - + 4 /*encapsulation*/); - } - , ALIVE, c_InstanceHandle_Unknown); + CacheChange_t* change = create_change_for_message( + message, + participant_stateless_message_writer_history_); if (change != nullptr) { @@ -939,12 +945,13 @@ bool SecurityManager::on_process_handshake( else { EPROSIMA_LOG_ERROR(SECURITY, "WriterHistory cannot add the CacheChange_t"); + participant_stateless_message_writer_history_->release_change(change); } } else { - //TODO (Ricardo) Return change. EPROSIMA_LOG_ERROR(SECURITY, "Cannot serialize ParticipantGenericMessage"); + participant_stateless_message_writer_history_->release_change(change); } } else @@ -1096,7 +1103,9 @@ void SecurityManager::delete_participant_stateless_message_pool() bool SecurityManager::create_participant_stateless_message_writer() { - participant_stateless_message_writer_history_ = new WriterHistory(participant_stateless_message_writer_hattr_); + participant_stateless_message_writer_history_ = new WriterHistory( + participant_stateless_message_writer_hattr_, + participant_stateless_message_pool_); const RTPSParticipantAttributes& pattr = participant_->getRTPSParticipantAttributes(); @@ -1109,7 +1118,7 @@ bool SecurityManager::create_participant_stateless_message_writer() watt.matched_readers_allocation = pattr.allocation.participants; RTPSWriter* wout = nullptr; - if (participant_->createWriter(&wout, watt, participant_stateless_message_pool_, + if (participant_->createWriter(&wout, watt, participant_stateless_message_writer_history_, nullptr, participant_stateless_message_writer_entity_id, true)) { @@ -1245,7 +1254,7 @@ void SecurityManager::delete_participant_volatile_message_secure_pool() bool SecurityManager::create_participant_volatile_message_secure_writer() { participant_volatile_message_secure_writer_history_ = - new WriterHistory(participant_volatile_message_secure_hattr_); + new WriterHistory(participant_volatile_message_secure_hattr_, participant_volatile_message_secure_pool_); const RTPSParticipantAttributes& pattr = participant_->getRTPSParticipantAttributes(); @@ -1264,7 +1273,7 @@ bool SecurityManager::create_participant_volatile_message_secure_writer() watt.matched_readers_allocation = pattr.allocation.participants; RTPSWriter* wout = nullptr; - if (participant_->createWriter(&wout, watt, participant_volatile_message_secure_pool_, + if (participant_->createWriter(&wout, watt, participant_volatile_message_secure_writer_history_, this, participant_volatile_message_secure_writer_entity_id, true)) { @@ -2212,13 +2221,9 @@ void SecurityManager::exchange_participant_crypto( ParticipantGenericMessage message = generate_participant_crypto_token_message(remote_participant_guid, local_participant_crypto_tokens); - CacheChange_t* change = participant_volatile_message_secure_writer_->new_change( - [&message]() -> uint32_t - { - return static_cast(ParticipantGenericMessageHelper::serialized_size(message) - + 4 /*encapsulation*/); - } - , ALIVE, c_InstanceHandle_Unknown); + CacheChange_t* change = create_change_for_message( + message, + participant_volatile_message_secure_writer_history_); if (change != nullptr) { @@ -2243,13 +2248,13 @@ void SecurityManager::exchange_participant_crypto( // Send if (!participant_volatile_message_secure_writer_history_->add_change(change)) { - participant_volatile_message_secure_writer_->release_change(change); + participant_volatile_message_secure_writer_history_->release_change(change); EPROSIMA_LOG_ERROR(SECURITY, "WriterHistory cannot add the CacheChange_t"); } } else { - participant_volatile_message_secure_writer_->release_change(change); + participant_volatile_message_secure_writer_history_->release_change(change); EPROSIMA_LOG_ERROR(SECURITY, "Cannot serialize ParticipantGenericMessage"); } } @@ -3031,14 +3036,9 @@ bool SecurityManager::discovered_reader( std::make_tuple(remote_reader_data, remote_reader_handle)); lock.unlock(); - CacheChange_t* change = participant_volatile_message_secure_writer_->new_change( - [&message]() -> uint32_t - { - return static_cast( - ParticipantGenericMessageHelper::serialized_size(message) - + 4 /*encapsulation*/); - } - , ALIVE, c_InstanceHandle_Unknown); + CacheChange_t* change = create_change_for_message( + message, + participant_volatile_message_secure_writer_history_); if (change != nullptr) { @@ -3069,13 +3069,13 @@ bool SecurityManager::discovered_reader( } else { - participant_volatile_message_secure_writer_->release_change(change); + participant_volatile_message_secure_writer_history_->release_change(change); EPROSIMA_LOG_ERROR(SECURITY, "WriterHistory cannot add the CacheChange_t"); } } else { - participant_volatile_message_secure_writer_->release_change(change); + participant_volatile_message_secure_writer_history_->release_change(change); EPROSIMA_LOG_ERROR(SECURITY, "Cannot serialize ParticipantGenericMessage"); } } @@ -3395,14 +3395,9 @@ bool SecurityManager::discovered_writer( std::make_tuple(remote_writer_data, remote_writer_handle)); lock.unlock(); - CacheChange_t* change = participant_volatile_message_secure_writer_->new_change( - [&message]() -> uint32_t - { - return static_cast( - ParticipantGenericMessageHelper::serialized_size(message) - + 4 /*encapsulation*/); - } - , ALIVE, c_InstanceHandle_Unknown); + CacheChange_t* change = create_change_for_message( + message, + participant_volatile_message_secure_writer_history_); if (change != nullptr) { @@ -3433,13 +3428,13 @@ bool SecurityManager::discovered_writer( } else { - participant_volatile_message_secure_writer_->release_change(change); + participant_volatile_message_secure_writer_history_->release_change(change); EPROSIMA_LOG_ERROR(SECURITY, "WriterHistory cannot add the CacheChange_t"); } } else { - participant_volatile_message_secure_writer_->release_change(change); + participant_volatile_message_secure_writer_history_->release_change(change); EPROSIMA_LOG_ERROR(SECURITY, "Cannot serialize ParticipantGenericMessage"); } } diff --git a/src/cpp/rtps/writer/PersistentWriter.cpp b/src/cpp/rtps/writer/PersistentWriter.cpp index 30c2ce79279..aaf5c9b5f5e 100644 --- a/src/cpp/rtps/writer/PersistentWriter.cpp +++ b/src/cpp/rtps/writer/PersistentWriter.cpp @@ -32,8 +32,6 @@ namespace rtps { PersistentWriter::PersistentWriter( const GUID_t& guid, const WriterAttributes& att, - const std::shared_ptr& payload_pool, - const std::shared_ptr& change_pool, WriterHistory* hist, IPersistenceService* persistence) : persistence_(persistence) @@ -45,8 +43,7 @@ PersistentWriter::PersistentWriter( ss << p_guid; persistence_guid_ = ss.str(); - persistence_->load_writer_from_storage(persistence_guid_, guid, hist, - change_pool, payload_pool, hist->m_lastCacheChangeSeqNum); + persistence_->load_writer_from_storage(persistence_guid_, guid, hist, hist->m_lastCacheChangeSeqNum); // Update history state after loading from DB hist->m_isHistoryFull = @@ -54,9 +51,9 @@ PersistentWriter::PersistentWriter( static_cast(hist->m_changes.size()) == hist->m_att.maximumReservedCaches; // Prepare the changes for datasharing if compatible - if (att.endpoint.data_sharing_configuration().kind() != dds::OFF) + if (att.endpoint.data_sharing_configuration().kind() != dds::DataSharingKind::OFF) { - auto pool = std::dynamic_pointer_cast(payload_pool); + auto pool = std::dynamic_pointer_cast(hist->get_payload_pool()); assert(pool != nullptr); for (auto change : hist->m_changes) { diff --git a/src/cpp/rtps/writer/PersistentWriter.hpp b/src/cpp/rtps/writer/PersistentWriter.hpp index f993113707f..6ffe5ec7dbc 100644 --- a/src/cpp/rtps/writer/PersistentWriter.hpp +++ b/src/cpp/rtps/writer/PersistentWriter.hpp @@ -43,8 +43,6 @@ class PersistentWriter PersistentWriter( const GUID_t& guid, const WriterAttributes& att, - const std::shared_ptr& payload_pool, - const std::shared_ptr& change_pool, WriterHistory* hist, IPersistenceService* persistence); diff --git a/src/cpp/rtps/writer/RTPSWriter.cpp b/src/cpp/rtps/writer/RTPSWriter.cpp index f3feeb74289..c46536f33d8 100644 --- a/src/cpp/rtps/writer/RTPSWriter.cpp +++ b/src/cpp/rtps/writer/RTPSWriter.cpp @@ -19,6 +19,7 @@ #include +#include #include #include @@ -56,53 +57,10 @@ RTPSWriter::RTPSWriter( , liveliness_lease_duration_(att.liveliness_lease_duration) , liveliness_announcement_period_(att.liveliness_announcement_period) { - PoolConfig cfg = PoolConfig::from_history_attributes(hist->m_att); - std::shared_ptr change_pool; - std::shared_ptr payload_pool; - payload_pool = BasicPayloadPool::get(cfg, change_pool); - - init(payload_pool, change_pool, att); -} - -RTPSWriter::RTPSWriter( - RTPSParticipantImpl* impl, - const GUID_t& guid, - const WriterAttributes& att, - const std::shared_ptr& payload_pool, - fastdds::rtps::FlowController* flow_controller, - WriterHistory* hist, - WriterListener* listen) - : RTPSWriter( - impl, guid, att, payload_pool, - std::make_shared(PoolConfig::from_history_attributes(hist->m_att)), - flow_controller, hist, listen) -{ -} - -RTPSWriter::RTPSWriter( - RTPSParticipantImpl* impl, - const GUID_t& guid, - const WriterAttributes& att, - const std::shared_ptr& payload_pool, - const std::shared_ptr& change_pool, - fastdds::rtps::FlowController* flow_controller, - WriterHistory* hist, - WriterListener* listen) - : Endpoint(impl, guid, att.endpoint) - , flow_controller_(flow_controller) - , mp_history(hist) - , mp_listener(listen) - , is_async_(att.mode == SYNCHRONOUS_WRITER ? false : true) - , liveliness_kind_(att.liveliness_kind) - , liveliness_lease_duration_(att.liveliness_lease_duration) - , liveliness_announcement_period_(att.liveliness_announcement_period) -{ - init(payload_pool, change_pool, att); + init(att); } void RTPSWriter::init( - const std::shared_ptr& payload_pool, - const std::shared_ptr& change_pool, const WriterAttributes& att) { { @@ -121,8 +79,6 @@ void RTPSWriter::init( } } - payload_pool_ = payload_pool; - change_pool_ = change_pool; fixed_payload_size_ = 0; if (mp_history->m_att.memoryPolicy == PREALLOCATED_MEMORY_MODE) { @@ -131,7 +87,7 @@ void RTPSWriter::init( if (att.endpoint.data_sharing_configuration().kind() != dds::OFF) { - std::shared_ptr pool = std::dynamic_pointer_cast(payload_pool); + std::shared_ptr pool = std::dynamic_pointer_cast(mp_history->get_payload_pool()); if (!pool || !pool->init_shared_memory(this, att.endpoint.data_sharing_configuration().shm_directory())) { EPROSIMA_LOG_ERROR(RTPS_WRITER, "Could not initialize DataSharing writer pool"); @@ -169,7 +125,7 @@ void RTPSWriter::deinit() for (auto it = mp_history->changesBegin(); it != mp_history->changesEnd(); ++it) { - release_change(*it); + mp_history->release_change(*it); } mp_history->m_changes.clear(); @@ -177,88 +133,6 @@ void RTPSWriter::deinit() flow_controller_->unregister_writer(this); } -CacheChange_t* RTPSWriter::new_change( - const std::function& dataCdrSerializedSize, - ChangeKind_t changeKind, - InstanceHandle_t handle) -{ - EPROSIMA_LOG_INFO(RTPS_WRITER, "Creating new change"); - - std::lock_guard guard(mp_mutex); - CacheChange_t* reserved_change = nullptr; - if (!change_pool_->reserve_cache(reserved_change)) - { - EPROSIMA_LOG_WARNING(RTPS_WRITER, "Problem reserving cache from pool"); - return nullptr; - } - - uint32_t payload_size = fixed_payload_size_ ? fixed_payload_size_ : dataCdrSerializedSize(); - if (!payload_pool_->get_payload(payload_size, reserved_change->serializedPayload)) - { - change_pool_->release_cache(reserved_change); - EPROSIMA_LOG_WARNING(RTPS_WRITER, "Problem reserving payload from pool"); - return nullptr; - } - - reserved_change->kind = changeKind; - if (m_att.topicKind == WITH_KEY && !handle.isDefined()) - { - EPROSIMA_LOG_WARNING(RTPS_WRITER, "Changes in KEYED Writers need a valid instanceHandle"); - } - reserved_change->instanceHandle = handle; - reserved_change->writerGUID = m_guid; - reserved_change->writer_info.previous = nullptr; - reserved_change->writer_info.next = nullptr; - reserved_change->writer_info.num_sent_submessages = 0; - reserved_change->vendor_id = c_VendorId_eProsima; - return reserved_change; -} - -CacheChange_t* RTPSWriter::new_change( - ChangeKind_t changeKind, - InstanceHandle_t handle) -{ - EPROSIMA_LOG_INFO(RTPS_WRITER, "Creating new change"); - - std::lock_guard guard(mp_mutex); - CacheChange_t* reserved_change = nullptr; - if (!change_pool_->reserve_cache(reserved_change)) - { - EPROSIMA_LOG_WARNING(RTPS_WRITER, "Problem reserving cache from pool"); - return nullptr; - } - - reserved_change->kind = changeKind; - if (m_att.topicKind == WITH_KEY && !handle.isDefined()) - { - EPROSIMA_LOG_WARNING(RTPS_WRITER, "Changes in KEYED Writers need a valid instanceHandle"); - } - reserved_change->instanceHandle = handle; - reserved_change->writerGUID = m_guid; - reserved_change->writer_info.previous = nullptr; - reserved_change->writer_info.next = nullptr; - reserved_change->writer_info.num_sent_submessages = 0; - reserved_change->vendor_id = c_VendorId_eProsima; - return reserved_change; -} - -bool RTPSWriter::release_change( - CacheChange_t* change) -{ - // Asserting preconditions - assert(change != nullptr); - assert(change->writerGUID == m_guid); - - std::lock_guard guard(mp_mutex); - - IPayloadPool* pool = change->serializedPayload.payload_owner; - if (pool) - { - pool->release_payload(change->serializedPayload); - } - return change_pool_->release_cache(change); -} - SequenceNumber_t RTPSWriter::get_seq_num_min() { CacheChange_t* change; @@ -290,26 +164,6 @@ uint32_t RTPSWriter::getTypeMaxSerialized() return mp_history->getTypeMaxSerialized(); } -bool RTPSWriter::remove_older_changes( - unsigned int max) -{ - EPROSIMA_LOG_INFO(RTPS_WRITER, "Starting process clean_history for writer " << getGuid()); - std::lock_guard guard(mp_mutex); - bool limit = (max != 0); - - bool remove_ret = mp_history->remove_min_change(); - bool at_least_one = remove_ret; - unsigned int count = 1; - - while (remove_ret && (!limit || count < max)) - { - remove_ret = mp_history->remove_min_change(); - ++count; - } - - return at_least_one; -} - constexpr uint32_t info_dst_message_length = 16; constexpr uint32_t info_ts_message_length = 12; constexpr uint32_t data_frag_submessage_header_length = 36; @@ -437,17 +291,6 @@ bool RTPSWriter::is_datasharing_compatible_with( return false; } -bool RTPSWriter::is_pool_initialized() const -{ - if (is_datasharing_compatible()) - { - auto pool = std::dynamic_pointer_cast(payload_pool_); - assert (pool != nullptr); - return pool->is_initialized(); - } - return true; -} - bool RTPSWriter::send_nts( const std::vector& buffers, const uint32_t& total_bytes, diff --git a/src/cpp/rtps/writer/StatefulPersistentWriter.cpp b/src/cpp/rtps/writer/StatefulPersistentWriter.cpp index 30527a9fb0f..0a2e97722a1 100644 --- a/src/cpp/rtps/writer/StatefulPersistentWriter.cpp +++ b/src/cpp/rtps/writer/StatefulPersistentWriter.cpp @@ -37,40 +37,11 @@ StatefulPersistentWriter::StatefulPersistentWriter( WriterListener* listen, IPersistenceService* persistence) : StatefulWriter(pimpl, guid, att, flow_controller, hist, listen) - , PersistentWriter(guid, att, payload_pool_, change_pool_, hist, persistence) + , PersistentWriter(guid, att, hist, persistence) { rebuild_status_after_load(); } -StatefulPersistentWriter::StatefulPersistentWriter( - RTPSParticipantImpl* pimpl, - const GUID_t& guid, - const WriterAttributes& att, - const std::shared_ptr& payload_pool, - fastdds::rtps::FlowController* flow_controller, - WriterHistory* hist, - WriterListener* listen, - IPersistenceService* persistence) - : StatefulWriter(pimpl, guid, att, payload_pool, flow_controller, hist, listen) - , PersistentWriter(guid, att, payload_pool_, change_pool_, hist, persistence) -{ -} - -StatefulPersistentWriter::StatefulPersistentWriter( - RTPSParticipantImpl* pimpl, - const GUID_t& guid, - const WriterAttributes& att, - const std::shared_ptr& payload_pool, - const std::shared_ptr& change_pool, - fastdds::rtps::FlowController* flow_controller, - WriterHistory* hist, - WriterListener* listen, - IPersistenceService* persistence) - : StatefulWriter(pimpl, guid, att, payload_pool, change_pool, flow_controller, hist, listen) - , PersistentWriter(guid, att, payload_pool_, change_pool_, hist, persistence) -{ -} - StatefulPersistentWriter::~StatefulPersistentWriter() { deinit(); diff --git a/src/cpp/rtps/writer/StatefulPersistentWriter.hpp b/src/cpp/rtps/writer/StatefulPersistentWriter.hpp index f888287662c..0273e697fb5 100644 --- a/src/cpp/rtps/writer/StatefulPersistentWriter.hpp +++ b/src/cpp/rtps/writer/StatefulPersistentWriter.hpp @@ -46,27 +46,6 @@ class StatefulPersistentWriter : public StatefulWriter, private PersistentWriter WriterListener* listen = nullptr, IPersistenceService* persistence = nullptr); - StatefulPersistentWriter( - RTPSParticipantImpl*, - const GUID_t& guid, - const WriterAttributes& att, - const std::shared_ptr& payload_pool, - fastdds::rtps::FlowController* flow_controller, - WriterHistory* hist, - WriterListener* listen = nullptr, - IPersistenceService* persistence = nullptr); - - StatefulPersistentWriter( - RTPSParticipantImpl*, - const GUID_t& guid, - const WriterAttributes& att, - const std::shared_ptr& payload_pool, - const std::shared_ptr& change_pool, - fastdds::rtps::FlowController* flow_controller, - WriterHistory* hist, - WriterListener* listen = nullptr, - IPersistenceService* persistence = nullptr); - void print_inconsistent_acknack( const GUID_t& writer_guid, const GUID_t& reader_guid, diff --git a/src/cpp/rtps/writer/StatefulWriter.cpp b/src/cpp/rtps/writer/StatefulWriter.cpp index 857ba17cbd7..aa674715998 100644 --- a/src/cpp/rtps/writer/StatefulWriter.cpp +++ b/src/cpp/rtps/writer/StatefulWriter.cpp @@ -203,77 +203,6 @@ StatefulWriter::StatefulWriter( init(pimpl, att); } -StatefulWriter::StatefulWriter( - RTPSParticipantImpl* pimpl, - const GUID_t& guid, - const WriterAttributes& att, - const std::shared_ptr& payload_pool, - fastdds::rtps::FlowController* flow_controller, - WriterHistory* history, - WriterListener* listener) - : RTPSWriter(pimpl, guid, att, payload_pool, flow_controller, history, listener) - , periodic_hb_event_(nullptr) - , nack_response_event_(nullptr) - , ack_event_(nullptr) - , m_heartbeatCount(0) - , m_times(att.times) - , matched_remote_readers_(att.matched_readers_allocation) - , matched_readers_pool_(att.matched_readers_allocation) - , next_all_acked_notify_sequence_(0, 1) - , all_acked_(false) - , may_remove_change_cond_() - , may_remove_change_(0) - , disable_heartbeat_piggyback_(att.disable_heartbeat_piggyback) - , disable_positive_acks_(att.disable_positive_acks) - , keep_duration_us_(att.keep_duration.to_ns() * 1e-3) - , last_sequence_number_() - , biggest_removed_sequence_number_() - , sendBufferSize_(pimpl->get_min_network_send_buffer_size()) - , currentUsageSendBufferSize_(static_cast(pimpl->get_min_network_send_buffer_size())) - , matched_local_readers_(att.matched_readers_allocation) - , matched_datasharing_readers_(att.matched_readers_allocation) - , locator_selector_general_(*this, att.matched_readers_allocation) - , locator_selector_async_(*this, att.matched_readers_allocation) -{ - init(pimpl, att); -} - -StatefulWriter::StatefulWriter( - RTPSParticipantImpl* pimpl, - const GUID_t& guid, - const WriterAttributes& att, - const std::shared_ptr& payload_pool, - const std::shared_ptr& change_pool, - fastdds::rtps::FlowController* flow_controller, - WriterHistory* hist, - WriterListener* listen) - : RTPSWriter(pimpl, guid, att, payload_pool, change_pool, flow_controller, hist, listen) - , periodic_hb_event_(nullptr) - , nack_response_event_(nullptr) - , ack_event_(nullptr) - , m_heartbeatCount(0) - , m_times(att.times) - , matched_remote_readers_(att.matched_readers_allocation) - , matched_readers_pool_(att.matched_readers_allocation) - , next_all_acked_notify_sequence_(0, 1) - , all_acked_(false) - , may_remove_change_cond_() - , may_remove_change_(0) - , disable_heartbeat_piggyback_(att.disable_heartbeat_piggyback) - , disable_positive_acks_(att.disable_positive_acks) - , keep_duration_us_(att.keep_duration.to_ns() * 1e-3) - , last_sequence_number_() - , biggest_removed_sequence_number_() - , sendBufferSize_(pimpl->get_min_network_send_buffer_size()) - , currentUsageSendBufferSize_(static_cast(pimpl->get_min_network_send_buffer_size())) - , matched_local_readers_(att.matched_readers_allocation) - , matched_datasharing_readers_(att.matched_readers_allocation) - , locator_selector_general_(*this, att.matched_readers_allocation) - , locator_selector_async_(*this, att.matched_readers_allocation) -{ - init(pimpl, att); -} - void StatefulWriter::init( RTPSParticipantImpl* pimpl, const WriterAttributes& att) @@ -384,7 +313,7 @@ StatefulWriter::~StatefulWriter() void StatefulWriter::prepare_datasharing_delivery( CacheChange_t* change) { - auto pool = std::dynamic_pointer_cast(payload_pool_); + auto pool = std::dynamic_pointer_cast(mp_history->get_payload_pool()); assert (pool != nullptr); pool->add_to_shared_history(change); @@ -566,7 +495,7 @@ bool StatefulWriter::change_removed_by_history( // remove from datasharing pool history if (is_datasharing_compatible()) { - auto pool = std::dynamic_pointer_cast(payload_pool_); + auto pool = std::dynamic_pointer_cast(mp_history->get_payload_pool()); assert (pool != nullptr); pool->remove_from_shared_history(a_change); @@ -1786,9 +1715,9 @@ bool StatefulWriter::send_periodic_heartbeat( for (ReaderProxy* reader : matched_datasharing_readers_) { - std::shared_ptr p = std::dynamic_pointer_cast(payload_pool_); - assert(p); - p->assert_liveliness(); + auto pool = std::dynamic_pointer_cast(mp_history->get_payload_pool()); + assert(pool); + pool->assert_liveliness(); reader->datasharing_notify(); unacked_changes = true; } diff --git a/src/cpp/rtps/writer/StatefulWriter.hpp b/src/cpp/rtps/writer/StatefulWriter.hpp index 576227441c5..1286cb359a4 100644 --- a/src/cpp/rtps/writer/StatefulWriter.hpp +++ b/src/cpp/rtps/writer/StatefulWriter.hpp @@ -63,25 +63,6 @@ class StatefulWriter : public RTPSWriter WriterHistory* hist, WriterListener* listen = nullptr); - StatefulWriter( - RTPSParticipantImpl* impl, - const GUID_t& guid, - const WriterAttributes& att, - const std::shared_ptr& payload_pool, - fastdds::rtps::FlowController* flow_controller, - WriterHistory* hist, - WriterListener* listen = nullptr); - - StatefulWriter( - RTPSParticipantImpl* impl, - const GUID_t& guid, - const WriterAttributes& att, - const std::shared_ptr& payload_pool, - const std::shared_ptr& change_pool, - fastdds::rtps::FlowController* flow_controller, - WriterHistory* hist, - WriterListener* listen = nullptr); - void rebuild_status_after_load(); virtual void print_inconsistent_acknack( diff --git a/src/cpp/rtps/writer/StatelessPersistentWriter.cpp b/src/cpp/rtps/writer/StatelessPersistentWriter.cpp index 34f483068f3..7aa760635e6 100644 --- a/src/cpp/rtps/writer/StatelessPersistentWriter.cpp +++ b/src/cpp/rtps/writer/StatelessPersistentWriter.cpp @@ -36,36 +36,7 @@ StatelessPersistentWriter::StatelessPersistentWriter( WriterListener* listen, IPersistenceService* persistence) : StatelessWriter(pimpl, guid, att, flow_controller, hist, listen) - , PersistentWriter(guid, att, payload_pool_, change_pool_, hist, persistence) -{ -} - -StatelessPersistentWriter::StatelessPersistentWriter( - RTPSParticipantImpl* pimpl, - const GUID_t& guid, - const WriterAttributes& att, - const std::shared_ptr& payload_pool, - fastdds::rtps::FlowController* flow_controller, - WriterHistory* hist, - WriterListener* listen, - IPersistenceService* persistence) - : StatelessWriter(pimpl, guid, att, payload_pool, flow_controller, hist, listen) - , PersistentWriter(guid, att, payload_pool_, change_pool_, hist, persistence) -{ -} - -StatelessPersistentWriter::StatelessPersistentWriter( - RTPSParticipantImpl* pimpl, - const GUID_t& guid, - const WriterAttributes& att, - const std::shared_ptr& payload_pool, - const std::shared_ptr& change_pool, - fastdds::rtps::FlowController* flow_controller, - WriterHistory* hist, - WriterListener* listen, - IPersistenceService* persistence) - : StatelessWriter(pimpl, guid, att, payload_pool, change_pool, flow_controller, hist, listen) - , PersistentWriter(guid, att, payload_pool_, change_pool_, hist, persistence) + , PersistentWriter(guid, att, hist, persistence) { } diff --git a/src/cpp/rtps/writer/StatelessPersistentWriter.hpp b/src/cpp/rtps/writer/StatelessPersistentWriter.hpp index ce7dad2eee8..ad883a38cde 100644 --- a/src/cpp/rtps/writer/StatelessPersistentWriter.hpp +++ b/src/cpp/rtps/writer/StatelessPersistentWriter.hpp @@ -46,27 +46,6 @@ class StatelessPersistentWriter : public StatelessWriter, private PersistentWrit WriterListener* listen = nullptr, IPersistenceService* persistence = nullptr); - StatelessPersistentWriter( - RTPSParticipantImpl*, - const GUID_t& guid, - const WriterAttributes& att, - const std::shared_ptr& payload_pool, - fastdds::rtps::FlowController* flow_controller, - WriterHistory* hist, - WriterListener* listen = nullptr, - IPersistenceService* persistence = nullptr); - - StatelessPersistentWriter( - RTPSParticipantImpl*, - const GUID_t& guid, - const WriterAttributes& att, - const std::shared_ptr& payload_pool, - const std::shared_ptr& change_pool, - fastdds::rtps::FlowController* flow_controller, - WriterHistory* hist, - WriterListener* listen = nullptr, - IPersistenceService* persistence = nullptr); - public: virtual ~StatelessPersistentWriter(); diff --git a/src/cpp/rtps/writer/StatelessWriter.cpp b/src/cpp/rtps/writer/StatelessWriter.cpp index 254a6a5a9fd..2410cc1bc2c 100644 --- a/src/cpp/rtps/writer/StatelessWriter.cpp +++ b/src/cpp/rtps/writer/StatelessWriter.cpp @@ -163,43 +163,6 @@ StatelessWriter::StatelessWriter( init(impl, attributes); } -StatelessWriter::StatelessWriter( - RTPSParticipantImpl* impl, - const GUID_t& guid, - const WriterAttributes& attributes, - const std::shared_ptr& payload_pool, - fastdds::rtps::FlowController* flow_controller, - WriterHistory* history, - WriterListener* listener) - : RTPSWriter(impl, guid, attributes, payload_pool, flow_controller, history, listener) - , matched_remote_readers_(attributes.matched_readers_allocation) - , matched_local_readers_(attributes.matched_readers_allocation) - , matched_datasharing_readers_(attributes.matched_readers_allocation) - , matched_readers_pool_(attributes.matched_readers_allocation) - , locator_selector_(*this, attributes.matched_readers_allocation) -{ - init(impl, attributes); -} - -StatelessWriter::StatelessWriter( - RTPSParticipantImpl* participant, - const GUID_t& guid, - const WriterAttributes& attributes, - const std::shared_ptr& payload_pool, - const std::shared_ptr& change_pool, - fastdds::rtps::FlowController* flow_controller, - WriterHistory* history, - WriterListener* listener) - : RTPSWriter(participant, guid, attributes, payload_pool, change_pool, flow_controller, history, listener) - , matched_remote_readers_(attributes.matched_readers_allocation) - , matched_local_readers_(attributes.matched_readers_allocation) - , matched_datasharing_readers_(attributes.matched_readers_allocation) - , matched_readers_pool_(attributes.matched_readers_allocation) - , locator_selector_(*this, attributes.matched_readers_allocation) -{ - init(participant, attributes); -} - void StatelessWriter::init( RTPSParticipantImpl* participant, const WriterAttributes& attributes) @@ -289,7 +252,7 @@ void StatelessWriter::update_reader_info( bool StatelessWriter::datasharing_delivery( CacheChange_t* change) { - auto pool = std::dynamic_pointer_cast(payload_pool_); + auto pool = std::dynamic_pointer_cast(mp_history->get_payload_pool()); assert(pool != nullptr); pool->add_to_shared_history(change); @@ -376,7 +339,7 @@ bool StatelessWriter::change_removed_by_history( // remove from datasharing pool history if (is_datasharing_compatible()) { - auto pool = std::dynamic_pointer_cast(payload_pool_); + auto pool = std::dynamic_pointer_cast(mp_history->get_payload_pool()); assert (pool != nullptr); pool->remove_from_shared_history(change); diff --git a/src/cpp/rtps/writer/StatelessWriter.hpp b/src/cpp/rtps/writer/StatelessWriter.hpp index d887cf1843a..ccfebeb19b7 100644 --- a/src/cpp/rtps/writer/StatelessWriter.hpp +++ b/src/cpp/rtps/writer/StatelessWriter.hpp @@ -60,25 +60,6 @@ class StatelessWriter : public RTPSWriter WriterHistory* history, WriterListener* listener = nullptr); - StatelessWriter( - RTPSParticipantImpl* impl, - const GUID_t& guid, - const WriterAttributes& att, - const std::shared_ptr& payload_pool, - fastdds::rtps::FlowController* flow_controller, - WriterHistory* hist, - WriterListener* listen = nullptr); - - StatelessWriter( - RTPSParticipantImpl* impl, - const GUID_t& guid, - const WriterAttributes& att, - const std::shared_ptr& payload_pool, - const std::shared_ptr& change_pool, - fastdds::rtps::FlowController* flow_controller, - WriterHistory* hist, - WriterListener* listen = nullptr); - public: virtual ~StatelessWriter(); diff --git a/src/cpp/statistics/rtps/monitor-service/MonitorService.cpp b/src/cpp/statistics/rtps/monitor-service/MonitorService.cpp index b6289a14489..d2342534215 100644 --- a/src/cpp/statistics/rtps/monitor-service/MonitorService.cpp +++ b/src/cpp/statistics/rtps/monitor-service/MonitorService.cpp @@ -21,6 +21,7 @@ #include #include +#include #include #include #include @@ -375,10 +376,18 @@ bool MonitorService::add_change( InstanceHandle_t handle; type_.getKey(&status_data, &handle, false); - CacheChange_t* change = status_writer_->new_change( - type_.getSerializedSizeProvider(&status_data), + CacheChange_t* change = status_writer_history_->create_change( (disposed ? fastdds::rtps::NOT_ALIVE_DISPOSED_UNREGISTERED : fastdds::rtps::ALIVE), handle); + if (nullptr != change) + { + uint32_t cdr_size = type_.getSerializedSizeProvider(&status_data)(); + if (!status_writer_payload_pool_->get_payload(cdr_size, change->serializedPayload)) + { + status_writer_history_->release_change(change); + change = nullptr; + } + } if (nullptr != change) { @@ -387,7 +396,7 @@ bool MonitorService::add_change( if (!type_.serialize(&status_data, &change->serializedPayload)) { EPROSIMA_LOG_ERROR(MONITOR_SERVICE, "Serialization failed"); - status_writer_->release_change(change); + status_writer_history_->release_change(change); return false; } @@ -444,22 +453,24 @@ bool MonitorService::create_endpoint() tatt.resourceLimitsQos.max_instances = 0; tatt.resourceLimitsQos.max_samples_per_instance = 1; - status_writer_history_.reset(new eprosima::fastdds::dds::DataWriterHistory(tatt, type_.m_typeSize, - MemoryManagementPolicy_t::PREALLOCATED_WITH_REALLOC_MEMORY_MODE, - []( - const InstanceHandle_t& ) -> void - { - })); - PoolConfig writer_pool_cfg = PoolConfig::from_history_attributes(hatt); status_writer_payload_pool_ = TopicPayloadPoolRegistry::get(MONITOR_SERVICE_TOPIC, writer_pool_cfg); status_writer_payload_pool_->reserve_history(writer_pool_cfg, false); + status_writer_history_.reset(new eprosima::fastdds::dds::DataWriterHistory( + status_writer_payload_pool_, + std::make_shared(writer_pool_cfg), + tatt, type_.m_typeSize, + MemoryManagementPolicy_t::PREALLOCATED_WITH_REALLOC_MEMORY_MODE, + []( + const InstanceHandle_t& ) -> void + { + })); + listener_ = new MonitorServiceListener(this); created = endpoint_creator_(&tmp_writer, watts, - status_writer_payload_pool_, status_writer_history_.get(), listener_, monitor_service_status_writer, diff --git a/src/cpp/statistics/rtps/monitor-service/MonitorService.hpp b/src/cpp/statistics/rtps/monitor-service/MonitorService.hpp index 84e025bfb1e..88546865d96 100644 --- a/src/cpp/statistics/rtps/monitor-service/MonitorService.hpp +++ b/src/cpp/statistics/rtps/monitor-service/MonitorService.hpp @@ -74,7 +74,6 @@ class MonitorService using endpoint_creator_t = std::function&, fastdds::rtps::WriterHistory*, fastdds::rtps::WriterListener*, const fastdds::rtps::EntityId_t&, diff --git a/test/blackbox/common/RTPSAsSocketWriter.hpp b/test/blackbox/common/RTPSAsSocketWriter.hpp index 4e699237274..670dda360ae 100644 --- a/test/blackbox/common/RTPSAsSocketWriter.hpp +++ b/test/blackbox/common/RTPSAsSocketWriter.hpp @@ -136,20 +136,12 @@ class RTPSAsSocketWriter : public eprosima::fastdds::rtps::WriterListener while (it != msgs.end()) { - eprosima::fastdds::rtps::CacheChange_t* ch = writer_->new_change([&]() -> uint32_t - { - size_t current_alignment = 4 + magicword_.size() + 1; -#if FASTCDR_VERSION_MAJOR == 1 - return (uint32_t)(current_alignment + type::getCdrSerializedSize(*it, - current_alignment)); -#else - eprosima::fastcdr::CdrSizeCalculator calculator(eprosima::fastdds::rtps:: - DEFAULT_XCDR_VERSION); - return (uint32_t)(current_alignment + - calculator.calculate_serialized_size(*it, current_alignment)); -#endif // FASTCDR_VERSION_MAJOR == 1 - } - , eprosima::fastdds::rtps::ALIVE); + size_t current_alignment = 4 + magicword_.size() + 1; + eprosima::fastcdr::CdrSizeCalculator calculator(eprosima::fastdds::rtps::DEFAULT_XCDR_VERSION); + uint32_t cdr_size = static_cast( + current_alignment + calculator.calculate_serialized_size(*it, current_alignment)); + eprosima::fastdds::rtps::CacheChange_t* ch = history_->create_change( + cdr_size, eprosima::fastdds::rtps::ALIVE); eprosima::fastcdr::FastBuffer buffer((char*)ch->serializedPayload.data, ch->serializedPayload.max_size); eprosima::fastcdr::Cdr cdr(buffer, eprosima::fastcdr::Cdr::DEFAULT_ENDIAN, diff --git a/test/blackbox/common/RTPSWithRegistrationWriter.hpp b/test/blackbox/common/RTPSWithRegistrationWriter.hpp index 6949a0c33ed..7b54d8793c3 100644 --- a/test/blackbox/common/RTPSWithRegistrationWriter.hpp +++ b/test/blackbox/common/RTPSWithRegistrationWriter.hpp @@ -162,20 +162,19 @@ class RTPSWithRegistrationWriter //Create writerhistory hattr_.payloadMaxSize = type_.m_typeSize; - history_ = new eprosima::fastdds::rtps::WriterHistory(hattr_); - - //Create writer if (has_payload_pool_) { - writer_ = eprosima::fastdds::rtps::RTPSDomain::createRTPSWriter( - participant_, writer_attr_, payload_pool_, history_, &listener_); + history_ = new eprosima::fastdds::rtps::WriterHistory(hattr_, payload_pool_); } else { - writer_ = eprosima::fastdds::rtps::RTPSDomain::createRTPSWriter( - participant_, writer_attr_, history_, &listener_); + history_ = new eprosima::fastdds::rtps::WriterHistory(hattr_); } + //Create writer + writer_ = eprosima::fastdds::rtps::RTPSDomain::createRTPSWriter( + participant_, writer_attr_, history_, &listener_); + if (writer_ == nullptr) { return; @@ -226,7 +225,12 @@ class RTPSWithRegistrationWriter while (it != msgs.end()) { - eprosima::fastdds::rtps::CacheChange_t* ch = writer_->new_change(*it, eprosima::fastdds::rtps::ALIVE); + eprosima::fastcdr::CdrSizeCalculator calculator(eprosima::fastdds::rtps::DEFAULT_XCDR_VERSION); + size_t current_alignment{ 0 }; + uint32_t cdr_size = static_cast( + calculator.calculate_serialized_size(*it, current_alignment)); + eprosima::fastdds::rtps::CacheChange_t* ch = history_->create_change( + cdr_size, eprosima::fastdds::rtps::ALIVE); eprosima::fastcdr::FastBuffer buffer((char*)ch->serializedPayload.data, ch->serializedPayload.max_size); eprosima::fastcdr::Cdr cdr(buffer, eprosima::fastcdr::Cdr::DEFAULT_ENDIAN, @@ -235,11 +239,7 @@ class RTPSWithRegistrationWriter cdr.serialize_encapsulation(); cdr << *it; -#if FASTCDR_VERSION_MAJOR == 1 - ch->serializedPayload.length = static_cast(cdr.getSerializedDataLength()); -#else ch->serializedPayload.length = static_cast(cdr.get_serialized_data_length()); -#endif // FASTCDR_VERSION_MAJOR == 1 if (ch->serializedPayload.length > 65000u) { ch->setFragmentSize(65000u); @@ -253,7 +253,11 @@ class RTPSWithRegistrationWriter eprosima::fastdds::rtps::CacheChange_t* send_sample( type& msg) { - eprosima::fastdds::rtps::CacheChange_t* ch = writer_->new_change(msg, eprosima::fastdds::rtps::ALIVE); + eprosima::fastcdr::CdrSizeCalculator calculator(eprosima::fastdds::rtps::DEFAULT_XCDR_VERSION); + size_t current_alignment{ 0 }; + uint32_t cdr_size = static_cast(calculator.calculate_serialized_size(msg, current_alignment)); + eprosima::fastdds::rtps::CacheChange_t* ch = history_->create_change( + cdr_size, eprosima::fastdds::rtps::ALIVE); eprosima::fastcdr::FastBuffer buffer((char*)ch->serializedPayload.data, ch->serializedPayload.max_size); eprosima::fastcdr::Cdr cdr(buffer, eprosima::fastcdr::Cdr::DEFAULT_ENDIAN, @@ -262,11 +266,7 @@ class RTPSWithRegistrationWriter cdr.serialize_encapsulation(); cdr << msg; -#if FASTCDR_VERSION_MAJOR == 1 - ch->serializedPayload.length = static_cast(cdr.getSerializedDataLength()); -#else ch->serializedPayload.length = static_cast(cdr.get_serialized_data_length()); -#endif // FASTCDR_VERSION_MAJOR == 1 if (ch->serializedPayload.length > 65000u) { ch->setFragmentSize(65000u); diff --git a/test/mock/dds/DataWriterHistory/fastdds/publisher/DataWriterHistory.hpp b/test/mock/dds/DataWriterHistory/fastdds/publisher/DataWriterHistory.hpp index e9a21cb3a5c..da2e45fd677 100644 --- a/test/mock/dds/DataWriterHistory/fastdds/publisher/DataWriterHistory.hpp +++ b/test/mock/dds/DataWriterHistory/fastdds/publisher/DataWriterHistory.hpp @@ -40,39 +40,41 @@ namespace dds { using namespace eprosima::fastdds::rtps; -static HistoryAttributes to_history_attributes( - const TopicAttributes& topic_att, - uint32_t payloadMaxSize, - MemoryManagementPolicy_t mempolicy) +class DataWriterHistory : public WriterHistory { - auto initial_samples = topic_att.resourceLimitsQos.allocated_samples; - auto max_samples = topic_att.resourceLimitsQos.max_samples; - auto extra_samples = topic_att.resourceLimitsQos.extra_samples; +public: - if (topic_att.historyQos.kind != KEEP_ALL_HISTORY_QOS) + static HistoryAttributes to_history_attributes( + const TopicAttributes& topic_att, + uint32_t payloadMaxSize, + MemoryManagementPolicy_t mempolicy) { - max_samples = topic_att.historyQos.depth; - if (topic_att.getTopicKind() != NO_KEY) + auto initial_samples = topic_att.resourceLimitsQos.allocated_samples; + auto max_samples = topic_att.resourceLimitsQos.max_samples; + auto extra_samples = topic_att.resourceLimitsQos.extra_samples; + + if (topic_att.historyQos.kind != KEEP_ALL_HISTORY_QOS) { - max_samples *= topic_att.resourceLimitsQos.max_instances; + max_samples = topic_att.historyQos.depth; + if (topic_att.getTopicKind() != NO_KEY) + { + max_samples *= topic_att.resourceLimitsQos.max_instances; + } + + initial_samples = std::min(initial_samples, max_samples); } - initial_samples = std::min(initial_samples, max_samples); + return HistoryAttributes(mempolicy, payloadMaxSize, initial_samples, max_samples, extra_samples); } - return HistoryAttributes(mempolicy, payloadMaxSize, initial_samples, max_samples, extra_samples); -} - -class DataWriterHistory : public WriterHistory -{ -public: - DataWriterHistory( + const std::shared_ptr& payload_pool, + const std::shared_ptr& change_pool, const TopicAttributes& topic_att, uint32_t payloadMaxSize, MemoryManagementPolicy_t mempolicy, - std::function unack_sample_remove_functor) - : WriterHistory(to_history_attributes(topic_att, payloadMaxSize, mempolicy)) + std::function unack_sample_remove_functor) + : WriterHistory(to_history_attributes(topic_att, payloadMaxSize, mempolicy), payload_pool, change_pool) , history_qos_(topic_att.historyQos) , resource_limited_qos_(topic_att.resourceLimitsQos) , topic_att_(topic_att) diff --git a/test/mock/rtps/DataSharingPayloadPool/rtps/DataSharing/DataSharingPayloadPool.hpp b/test/mock/rtps/DataSharingPayloadPool/rtps/DataSharing/DataSharingPayloadPool.hpp index 5bc5a792115..3c2597a7ad8 100644 --- a/test/mock/rtps/DataSharingPayloadPool/rtps/DataSharing/DataSharingPayloadPool.hpp +++ b/test/mock/rtps/DataSharingPayloadPool/rtps/DataSharing/DataSharingPayloadPool.hpp @@ -21,6 +21,7 @@ #include #include +#include #include namespace eprosima { @@ -76,6 +77,15 @@ class DataSharingPayloadPool : public IPayloadPool return true; } + virtual bool init_shared_memory( + const RTPSWriter* /*writer*/, + const std::string& /*shared_dir*/) + { + // Default implementation is NOP + // will be overriden by children if needed + return false; + } + static std::shared_ptr get_reader_pool( const PoolConfig& /*config*/, bool /*is_volatile*/) diff --git a/test/mock/rtps/DataSharingPayloadPool/rtps/DataSharing/WriterPool.hpp b/test/mock/rtps/DataSharingPayloadPool/rtps/DataSharing/WriterPool.hpp new file mode 100644 index 00000000000..43b6839f2d4 --- /dev/null +++ b/test/mock/rtps/DataSharingPayloadPool/rtps/DataSharing/WriterPool.hpp @@ -0,0 +1,61 @@ +// Copyright 2020 Proyectos y Sistemas de Mantenimiento SL (eProsima). +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +/** + * @file WriterPool.hpp + */ + +#ifndef RTPS_DATASHARING_WRITERPOOL_HPP +#define RTPS_DATASHARING_WRITERPOOL_HPP + +#include + +#include + +#include + + +namespace eprosima { +namespace fastdds { +namespace rtps { + +class WriterPool : public DataSharingPayloadPool +{ +public: + + bool init_shared_memory( + const RTPSWriter* writer, + const std::string& /*shared_dir*/) override + { + writer_guid_ = writer->getGuid(); + return true; + } + + bool is_initialized() const + { + return writer_guid_ != GUID_t::unknown(); + } + + void add_to_shared_history( + const CacheChange_t* /*cache_change*/) + { + } + +}; + +} // namespace rtps +} // namespace fastdds +} // namespace eprosima + +#endif // RTPS_DATASHARING_WRITERPOOL_HPP diff --git a/test/mock/rtps/RTPSDomain/fastdds/rtps/RTPSDomain.hpp b/test/mock/rtps/RTPSDomain/fastdds/rtps/RTPSDomain.hpp index 38f03ddbfa5..a53bb8626e9 100644 --- a/test/mock/rtps/RTPSDomain/fastdds/rtps/RTPSDomain.hpp +++ b/test/mock/rtps/RTPSDomain/fastdds/rtps/RTPSDomain.hpp @@ -93,18 +93,6 @@ class RTPSDomain RTPSParticipant*, const EntityId_t&, WriterAttributes&, - const std::shared_ptr&, - WriterHistory*, - WriterListener* listen = nullptr) - { - writer_->set_listener(listen); - return writer_; - } - - static RTPSWriter* createRTPSWriter( - RTPSParticipant*, - WriterAttributes&, - const std::shared_ptr&, WriterHistory*, WriterListener* listen = nullptr) { diff --git a/test/mock/rtps/RTPSDomainImpl/rtps/RTPSDomainImpl.hpp b/test/mock/rtps/RTPSDomainImpl/rtps/RTPSDomainImpl.hpp index db1bc03dd84..96339cfe9b4 100644 --- a/test/mock/rtps/RTPSDomainImpl/rtps/RTPSDomainImpl.hpp +++ b/test/mock/rtps/RTPSDomainImpl/rtps/RTPSDomainImpl.hpp @@ -89,13 +89,10 @@ class RTPSDomainImpl RTPSParticipant* p, const EntityId_t& entity_id, WriterAttributes& watt, - const std::shared_ptr& payload_pool, - const std::shared_ptr& change_pool, WriterHistory* hist, - WriterListener* listen = nullptr) + WriterListener* listen) { - static_cast(change_pool); - return RTPSDomain::createRTPSWriter(p, entity_id, watt, payload_pool, hist, listen); + return RTPSDomain::createRTPSWriter(p, entity_id, watt, hist, listen); } static RTPSParticipant* clientServerEnvironmentCreationOverride( diff --git a/test/mock/rtps/RTPSParticipantImpl/rtps/participant/RTPSParticipantImpl.h b/test/mock/rtps/RTPSParticipantImpl/rtps/participant/RTPSParticipantImpl.h index dd8843720fc..8634ad0bac1 100644 --- a/test/mock/rtps/RTPSParticipantImpl/rtps/participant/RTPSParticipantImpl.h +++ b/test/mock/rtps/RTPSParticipantImpl/rtps/participant/RTPSParticipantImpl.h @@ -386,15 +386,11 @@ class RTPSParticipantImpl bool create_writer( RTPSWriter**, WriterAttributes&, - const std::shared_ptr&, - const std::shared_ptr&, WriterHistory*, WriterListener*, - const EntityId_t& entityId = c_EntityId_Unknown, - bool isBuiltin = false) + const EntityId_t&, + bool) { - static_cast(entityId); - static_cast(isBuiltin); return true; } diff --git a/test/mock/rtps/RTPSWriter/fastdds/rtps/writer/RTPSWriter.hpp b/test/mock/rtps/RTPSWriter/fastdds/rtps/writer/RTPSWriter.hpp index d5c2f05f4ea..59386997055 100644 --- a/test/mock/rtps/RTPSWriter/fastdds/rtps/writer/RTPSWriter.hpp +++ b/test/mock/rtps/RTPSWriter/fastdds/rtps/writer/RTPSWriter.hpp @@ -117,21 +117,6 @@ class RTPSWriter : public Endpoint #endif // FASTDDS_STATISTICS // *INDENT-OFF* Uncrustify makes a mess with MOCK_METHOD macros - MOCK_METHOD3(new_change, CacheChange_t* ( - const std::function&, - ChangeKind_t, - InstanceHandle_t)); - - MOCK_METHOD2(new_change, CacheChange_t* ( - ChangeKind_t, - InstanceHandle_t)); - - MOCK_METHOD2(new_change, CacheChange_t* ( - const std::function&, - ChangeKind_t)); - - MOCK_METHOD1(release_change, void(CacheChange_t*)); - MOCK_METHOD1(set_separate_sending, void(bool)); MOCK_METHOD0(getRTPSParticipant, RTPSParticipantImpl* ()); diff --git a/test/mock/rtps/WriterHistory/fastdds/rtps/history/WriterHistory.hpp b/test/mock/rtps/WriterHistory/fastdds/rtps/history/WriterHistory.hpp index 99ac2e726ab..c40bca9f8b0 100644 --- a/test/mock/rtps/WriterHistory/fastdds/rtps/history/WriterHistory.hpp +++ b/test/mock/rtps/WriterHistory/fastdds/rtps/history/WriterHistory.hpp @@ -32,6 +32,8 @@ namespace eprosima { namespace fastdds { namespace rtps { +class IChangePool; +class IPayloadPool; class ReaderProxy; class RTPSWriter; @@ -45,6 +47,21 @@ class WriterHistory { } + WriterHistory( + const HistoryAttributes& /*att*/, + const std::shared_ptr&) + : samples_number_(0) + { + } + + WriterHistory( + const HistoryAttributes& /*att*/, + const std::shared_ptr&, + const std::shared_ptr&) + : samples_number_(0) + { + } + WriterHistory() : samples_number_(0) { @@ -55,10 +72,28 @@ class WriterHistory using iterator = std::vector::iterator; // *INDENT-OFF* Uncrustify makes a mess with MOCK_METHOD macros + MOCK_METHOD2(create_change, CacheChange_t* ( + ChangeKind_t, + InstanceHandle_t)); + + CacheChange_t* create_change( + uint32_t size, + ChangeKind_t kind) + { + return create_change(size, kind, InstanceHandle_t{}); + } + + MOCK_METHOD3(create_change, CacheChange_t* ( + uint32_t, + ChangeKind_t, + InstanceHandle_t handle)); + + MOCK_METHOD1(release_change, bool(CacheChange_t*)); + MOCK_METHOD1(add_change_mock, bool(CacheChange_t*)); - // *INDENT-ON* MOCK_METHOD2(add_change, bool(CacheChange_t * a_change, WriteParams & wparams)); + // *INDENT-ON* bool add_change( CacheChange_t* change) diff --git a/test/unittest/dds/publisher/DataWriterTests.cpp b/test/unittest/dds/publisher/DataWriterTests.cpp index f9619a23b42..39fa2015087 100644 --- a/test/unittest/dds/publisher/DataWriterTests.cpp +++ b/test/unittest/dds/publisher/DataWriterTests.cpp @@ -1600,7 +1600,7 @@ class DataWriterImplTest : public DataWriterImpl DataWriterHistory* get_history() { - return &history_; + return history_.get(); } }; diff --git a/test/unittest/rtps/DataSharing/SHMSegmentTests.cpp b/test/unittest/rtps/DataSharing/SHMSegmentTests.cpp index baf42b1ffec..1aed5c7089f 100644 --- a/test/unittest/rtps/DataSharing/SHMSegmentTests.cpp +++ b/test/unittest/rtps/DataSharing/SHMSegmentTests.cpp @@ -48,7 +48,7 @@ TEST(SHMSegmentTests, Writer) HistoryAttributes history_attr; history_attr.payloadMaxSize = 255; history_attr.maximumReservedCaches = 50; - WriterHistory* history = new WriterHistory(history_attr); + WriterAttributes writer_attr; dds::DataSharingQosPolicy dsq; // We select a folder to force the use of SharedFileSegment @@ -58,8 +58,9 @@ TEST(SHMSegmentTests, Writer) std::shared_ptr payload_pool(new WriterPool(history_attr.payloadMaxSize, history_attr.maximumReservedCaches)); + WriterHistory* history = new WriterHistory(history_attr, payload_pool); RTPSWriter* writer = nullptr; - EXPECT_NO_THROW(writer = RTPSDomain::createRTPSWriter(participant, writer_attr, payload_pool, history)); + EXPECT_NO_THROW(writer = RTPSDomain::createRTPSWriter(participant, writer_attr, history)); // RTPSWriter creation failed, as expected. EXPECT_EQ(writer, nullptr); diff --git a/test/unittest/rtps/persistence/PersistenceTests.cpp b/test/unittest/rtps/persistence/PersistenceTests.cpp index 1b468d3b988..0cc57edb3a2 100644 --- a/test/unittest/rtps/persistence/PersistenceTests.cpp +++ b/test/unittest/rtps/persistence/PersistenceTests.cpp @@ -256,6 +256,8 @@ class PersistenceTest : public ::testing::TestWithParam */ TEST_F(PersistenceTest, Writer) { + using testing::_; + const std::string persist_guid("TEST_WRITER"); PropertyPolicy policy; @@ -280,9 +282,18 @@ TEST_F(PersistenceTest, Writer) change.writerGUID = guid; change.serializedPayload.length = 0; + auto create_change = [&pool](uint32_t, ChangeKind_t, InstanceHandle_t) + { + CacheChange_t* ch = nullptr; + return pool->reserve_cache(ch) ? ch : nullptr; + }; + EXPECT_CALL(history, create_change(_, _, _)) + .Times(testing::AnyNumber()) + .WillRepeatedly(testing::Invoke(create_change)); + // Initial load should return empty vector history.m_changes.clear(); - ASSERT_TRUE(service->load_writer_from_storage(persist_guid, guid, &history, pool, payload_pool_, max_seq)); + ASSERT_TRUE(service->load_writer_from_storage(persist_guid, guid, &history, max_seq)); ASSERT_EQ(history.m_changes.size(), 0u); // Add two changes @@ -299,7 +310,7 @@ TEST_F(PersistenceTest, Writer) // Loading should return two changes (seqs = 1, 2) history.m_changes.clear(); - ASSERT_TRUE(service->load_writer_from_storage(persist_guid, guid, &history, pool, payload_pool_, max_seq)); + ASSERT_TRUE(service->load_writer_from_storage(persist_guid, guid, &history, max_seq)); ASSERT_EQ(history.m_changes.size(), 2u); ASSERT_EQ(max_seq, SequenceNumber_t(0, 2u)); uint32_t i = 0; @@ -316,7 +327,7 @@ TEST_F(PersistenceTest, Writer) // Loading should return one change (seq = 2) history.m_changes.clear(); - ASSERT_TRUE(service->load_writer_from_storage(persist_guid, guid, &history, pool, payload_pool_, max_seq)); + ASSERT_TRUE(service->load_writer_from_storage(persist_guid, guid, &history, max_seq)); ASSERT_EQ(history.m_changes.size(), 1u); ASSERT_EQ((*history.m_changes.begin())->sequenceNumber, SequenceNumber_t(0, 2)); ASSERT_EQ(max_seq, SequenceNumber_t(0, 2u)); @@ -325,12 +336,11 @@ TEST_F(PersistenceTest, Writer) history.m_changes.clear(); change.sequenceNumber.low = 2; ASSERT_TRUE(service->remove_writer_change_from_storage(persist_guid, change)); - ASSERT_TRUE(service->load_writer_from_storage(persist_guid, guid, &history, pool, payload_pool_, max_seq)); + ASSERT_TRUE(service->load_writer_from_storage(persist_guid, guid, &history, max_seq)); ASSERT_EQ(history.m_changes.size(), 0u); ASSERT_EQ(max_seq, SequenceNumber_t(0, 2u)); } - /*! * @fn TEST_F(PersistenceTest, SchemaVersionMismatch) * @brief This test checks that an error is issued if the database has an old schema. @@ -359,6 +369,8 @@ TEST_F(PersistenceTest, SchemaVersionMismatch) */ TEST_P(PersistenceTest, SchemaVersionUpdate) { + using testing::_; + auto from = GetParam(); ASSERT_LT(from, 3); @@ -393,9 +405,18 @@ TEST_P(PersistenceTest, SchemaVersionUpdate) change.writerGUID = guid; change.serializedPayload.length = 0; + auto create_change = [&pool](uint32_t, ChangeKind_t, InstanceHandle_t) + { + CacheChange_t* ch = nullptr; + return pool->reserve_cache(ch) ? ch : nullptr; + }; + EXPECT_CALL(history, create_change(_, _, _)) + .Times(testing::AnyNumber()) + .WillRepeatedly(testing::Invoke(create_change)); + // Load data history.m_changes.clear(); - ASSERT_TRUE(service->load_writer_from_storage(persist_guid, guid, &history, pool, payload_pool_, last_seq_number)); + ASSERT_TRUE(service->load_writer_from_storage(persist_guid, guid, &history, last_seq_number)); ASSERT_EQ(history.m_changes.size(), 2u); ASSERT_EQ(last_seq_number, SequenceNumber_t(0, 2u)); uint32_t i = 0; diff --git a/test/unittest/rtps/security/SecurityHandshakeProcessTests.cpp b/test/unittest/rtps/security/SecurityHandshakeProcessTests.cpp index 08d79c5381a..809d0c4d3a3 100644 --- a/test/unittest/rtps/security/SecurityHandshakeProcessTests.cpp +++ b/test/unittest/rtps/security/SecurityHandshakeProcessTests.cpp @@ -46,7 +46,7 @@ TEST_F(SecurityTest, discovered_participant_begin_handshake_request_fail_and_the Ref(remote_identity_handle), _, _)).Times(1). WillOnce(DoAll(SetArgPointee<0>(&handshake_handle), SetArgPointee<1>(&handshake_message), Return(ValidationResult_t::VALIDATION_PENDING_HANDSHAKE_MESSAGE))); - EXPECT_CALL(*stateless_writer_, new_change(_, _, _)).Times(1). + EXPECT_CALL(*stateless_writer_->history_, create_change(_, _, _)).Times(1). WillOnce(Return(change)); EXPECT_CALL(*stateless_writer_->history_, add_change_mock(change)).Times(1). WillOnce(Return(true)); @@ -398,7 +398,7 @@ TEST_F(SecurityTest, discovered_participant_process_message_new_change_fail) WillRepeatedly(Return(true)); EXPECT_CALL(*auth_plugin_, return_identity_handle(&remote_identity_handle, _)).Times(1). WillRepeatedly(Return(true)); - EXPECT_CALL(*stateless_writer_, new_change(_, _, _)).Times(1). + EXPECT_CALL(*stateless_writer_->history_, create_change(_, _, _)).Times(1). WillOnce(Return(nullptr)); EXPECT_CALL(*auth_plugin_, return_handshake_handle(&handshake_handle, _)).Times(1). WillOnce(Return(true)); @@ -463,7 +463,7 @@ TEST_F(SecurityTest, discovered_participant_process_message_add_change_fail) WillRepeatedly(Return(true)); EXPECT_CALL(*auth_plugin_, return_identity_handle(&remote_identity_handle, _)).Times(1). WillRepeatedly(Return(true)); - EXPECT_CALL(*stateless_writer_, new_change(_, _, _)).Times(1). + EXPECT_CALL(*stateless_writer_->history_, create_change(_, _, _)).Times(1). WillOnce(Return(change2)); EXPECT_CALL(*stateless_writer_->history_, add_change_mock(change2)).Times(1). WillOnce(Return(false)); @@ -576,7 +576,7 @@ TEST_F(SecurityTest, discovered_participant_process_message_pending_handshake_re WillRepeatedly(Return(true)); EXPECT_CALL(*auth_plugin_, return_identity_handle(&remote_identity_handle, _)).Times(1). WillRepeatedly(Return(true)); - EXPECT_CALL(*stateless_writer_, new_change(_, _, _)).Times(1). + EXPECT_CALL(*stateless_writer_->history_, create_change(_, _, _)).Times(1). WillOnce(Return(change2)); EXPECT_CALL(*stateless_writer_->history_, add_change_mock(change2)).Times(1). WillOnce(Return(true)); @@ -787,7 +787,7 @@ TEST_F(SecurityTest, discovered_participant_process_message_process_handshake_re WillRepeatedly(Return(true)); EXPECT_CALL(*auth_plugin_, return_identity_handle(&remote_identity_handle_, _)).Times(1). WillRepeatedly(Return(true)); - EXPECT_CALL(*stateless_writer_, new_change(_, _, _)).Times(1). + EXPECT_CALL(*stateless_writer_->history_, create_change(_, _, _)).Times(1). WillOnce(Return(nullptr)); EXPECT_CALL(*auth_plugin_, return_handshake_handle(&handshake_handle_, _)).Times(1). WillOnce(Return(true)); @@ -842,7 +842,7 @@ TEST_F(SecurityTest, discovered_participant_process_message_process_handshake_re WillRepeatedly(Return(true)); EXPECT_CALL(*auth_plugin_, return_identity_handle(&remote_identity_handle_, _)).Times(1). WillRepeatedly(Return(true)); - EXPECT_CALL(*stateless_writer_, new_change(_, _, _)).Times(1). + EXPECT_CALL(*stateless_writer_->history_, create_change(_, _, _)).Times(1). WillOnce(Return(change2)); EXPECT_CALL(*stateless_writer_->history_, add_change_mock(change2)).Times(1). WillOnce(Return(false)); diff --git a/test/unittest/rtps/security/SecurityTests.cpp b/test/unittest/rtps/security/SecurityTests.cpp index dca3c2789f9..29cd35a8e47 100644 --- a/test/unittest/rtps/security/SecurityTests.cpp +++ b/test/unittest/rtps/security/SecurityTests.cpp @@ -86,7 +86,7 @@ void SecurityTest::request_process_ok( Ref(remote_identity_handle_), _, _)).Times(1). WillOnce(DoAll(SetArgPointee<0>(&handshake_handle_), SetArgPointee<1>(&handshake_message), Return(ValidationResult_t::VALIDATION_PENDING_HANDSHAKE_MESSAGE))); - EXPECT_CALL(*stateless_writer_, new_change(_, _, _)).Times(1). + EXPECT_CALL(*stateless_writer_->history_, create_change(_, _, _)).Times(1). WillOnce(Return(change)); EXPECT_CALL(*stateless_writer_->history_, add_change_mock(change)).Times(1). WillOnce(Return(true)); @@ -149,7 +149,7 @@ void SecurityTest::reply_process_ok( Ref(local_identity_handle_), _, _)).Times(1). WillOnce(DoAll(SetArgPointee<0>(&handshake_handle_), SetArgPointee<1>(&handshake_message), Return(ValidationResult_t::VALIDATION_PENDING_HANDSHAKE_MESSAGE))); - EXPECT_CALL(*stateless_writer_, new_change(_, _, _)).Times(1). + EXPECT_CALL(*stateless_writer_->history_, create_change(_, _, _)).Times(1). WillOnce(Return(change2)); EXPECT_CALL(*stateless_writer_->history_, add_change_mock(change2)).Times(1). WillOnce(Return(true)); @@ -217,7 +217,7 @@ void SecurityTest::final_message_process_ok( EXPECT_CALL(*auth_plugin_, process_handshake_rvr(_, _, Ref(handshake_handle_), _)).Times(1). WillOnce(DoAll(SetArgPointee<0>(&handshake_message), Return(ValidationResult_t::VALIDATION_OK_WITH_FINAL_MESSAGE))); - EXPECT_CALL(*stateless_writer_, new_change(_, _, _)).Times(1). + EXPECT_CALL(*stateless_writer_->history_, create_change(_, _, _)).Times(1). WillOnce(Return(change2)); EXPECT_CALL(*stateless_writer_->history_, add_change_mock(change2)).Times(1). WillOnce(Return(true)); @@ -267,10 +267,10 @@ void SecurityTest::expect_kx_exchange( CacheChange_t& kx_change_to_add, CacheChange_t* kx_change_to_remove) { - EXPECT_CALL(*volatile_writer_, new_change(_, _, _)).Times(1).WillOnce( - DoAll(Invoke([&kx_change_to_add](const std::function& f, ChangeKind_t, InstanceHandle_t) + EXPECT_CALL(*volatile_writer_->history_, create_change(_, _, _)).Times(1).WillOnce( + DoAll(Invoke([&kx_change_to_add](uint32_t cdr_size, ChangeKind_t, InstanceHandle_t) { - kx_change_to_add.serializedPayload.reserve(f()); + kx_change_to_add.serializedPayload.reserve(cdr_size); }), Return(&kx_change_to_add))); EXPECT_CALL(*volatile_writer_->history_, add_change_mock(&kx_change_to_add)).Times(1). diff --git a/test/unittest/rtps/security/SecurityValidationRemoteTests.cpp b/test/unittest/rtps/security/SecurityValidationRemoteTests.cpp index 9ea44500190..3e18f207001 100644 --- a/test/unittest/rtps/security/SecurityValidationRemoteTests.cpp +++ b/test/unittest/rtps/security/SecurityValidationRemoteTests.cpp @@ -185,7 +185,7 @@ TEST_F(SecurityTest, discovered_participant_validation_remote_identity_new_chang Ref(remote_identity_handle), _, _)).Times(1). WillOnce(DoAll(SetArgPointee<0>(&handshake_handle), SetArgPointee<1>(&handshake_message), Return(ValidationResult_t::VALIDATION_PENDING_HANDSHAKE_MESSAGE))); - EXPECT_CALL(*stateless_writer_, new_change(_, _, _)).Times(1). + EXPECT_CALL(*stateless_writer_->history_, create_change(_, _, _)).Times(1). WillOnce(Return(nullptr)); EXPECT_CALL(*auth_plugin_, return_identity_handle(&local_identity_handle_, _)).Times(1). WillRepeatedly(Return(true)); @@ -220,7 +220,7 @@ TEST_F(SecurityTest, discovered_participant_validation_remote_identity_add_chang Ref(remote_identity_handle), _, _)).Times(1). WillOnce(DoAll(SetArgPointee<0>(&handshake_handle), SetArgPointee<1>(&handshake_message), Return(ValidationResult_t::VALIDATION_PENDING_HANDSHAKE_MESSAGE))); - EXPECT_CALL(*stateless_writer_, new_change(_, _, _)).Times(1). + EXPECT_CALL(*stateless_writer_->history_, create_change(_, _, _)).Times(1). WillOnce(Return(change)); EXPECT_CALL(*stateless_writer_->history_, add_change_mock(change)).Times(1). WillOnce(Return(false)); @@ -299,7 +299,7 @@ TEST_F(SecurityTest, discovered_participant_validation_remote_identity_pending_h Ref(remote_identity_handle), _, _)).Times(1). WillOnce(DoAll(SetArgPointee<0>(&handshake_handle), SetArgPointee<1>(&handshake_message), Return(ValidationResult_t::VALIDATION_OK_WITH_FINAL_MESSAGE))); - EXPECT_CALL(*stateless_writer_, new_change(_, _, _)).Times(1). + EXPECT_CALL(*stateless_writer_->history_, create_change(_, _, _)).Times(1). WillOnce(Return(change)); EXPECT_CALL(*stateless_writer_->history_, add_change_mock(change)).Times(1). WillOnce(Return(true)); @@ -361,7 +361,7 @@ TEST_F(SecurityTest, discovered_participant_ok) Ref(remote_identity_handle), _, _)).Times(1). WillOnce(DoAll(SetArgPointee<0>(&handshake_handle), SetArgPointee<1>(&handshake_message), Return(ValidationResult_t::VALIDATION_PENDING_HANDSHAKE_MESSAGE))); - EXPECT_CALL(*stateless_writer_, new_change(_, _, _)).Times(1). + EXPECT_CALL(*stateless_writer_->history_, create_change(_, _, _)).Times(1). WillOnce(Return(change)); EXPECT_CALL(*stateless_writer_->history_, add_change_mock(change)).Times(1). WillOnce(Return(true)); @@ -412,7 +412,7 @@ TEST_F(SecurityTest, discovered_participant_validate_remote_fail_and_then_ok) Ref(remote_identity_handle), _, _)).Times(1). WillOnce(DoAll(SetArgPointee<0>(&handshake_handle), SetArgPointee<1>(&handshake_message), Return(ValidationResult_t::VALIDATION_PENDING_HANDSHAKE_MESSAGE))); - EXPECT_CALL(*stateless_writer_, new_change(_, _, _)).Times(1). + EXPECT_CALL(*stateless_writer_->history_, create_change(_, _, _)).Times(1). WillOnce(Return(change)); EXPECT_CALL(*stateless_writer_->history_, add_change_mock(change)).Times(1). WillOnce(Return(true)); diff --git a/test/unittest/rtps/writer/RTPSWriterTests.cpp b/test/unittest/rtps/writer/RTPSWriterTests.cpp index 507b94786e4..f0f610d5d5b 100644 --- a/test/unittest/rtps/writer/RTPSWriterTests.cpp +++ b/test/unittest/rtps/writer/RTPSWriterTests.cpp @@ -108,19 +108,21 @@ void pool_initialization_test ( ASSERT_NE(participant, nullptr); + std::shared_ptr pool; + HistoryAttributes h_attr; h_attr.memoryPolicy = policy; h_attr.initialReservedCaches = initial_reserved_caches; h_attr.payloadMaxSize = TestDataType::data_size; - WriterHistory* history = new WriterHistory(h_attr); - - std::shared_ptr pool; + WriterHistory* history = new WriterHistory(h_attr, pool); WriterAttributes w_attr; - RTPSWriter* writer = RTPSDomain::createRTPSWriter(participant, w_attr, pool, history); + RTPSWriter* writer = RTPSDomain::createRTPSWriter(participant, w_attr, history); EXPECT_EQ(nullptr, writer); + delete history; pool = std::make_shared(); + history = new WriterHistory(h_attr, pool); // Creating the Writer initializes the PayloadPool with the initial reserved size EXPECT_CALL(*pool, get_payload_delegate(TestDataType::data_size, _)) @@ -128,7 +130,7 @@ void pool_initialization_test ( EXPECT_CALL(*pool, release_payload_delegate(_)) .Times(0); - writer = RTPSDomain::createRTPSWriter(participant, w_attr, pool, history); + writer = RTPSDomain::createRTPSWriter(participant, w_attr, history); Mock::VerifyAndClearExpectations(pool.get()); @@ -138,7 +140,10 @@ void pool_initialization_test ( .WillOnce(Return(true)); TestDataType data; - CacheChange_t* ch = writer->new_change(data, ALIVE); + eprosima::fastcdr::CdrSizeCalculator calculator(eprosima::fastdds::rtps::DEFAULT_XCDR_VERSION); + size_t current_alignment{ 0 }; + uint32_t payload_size = (uint32_t)calculator.calculate_serialized_size(data, current_alignment); + CacheChange_t* ch = history->create_change(payload_size, ALIVE); ASSERT_NE(ch, nullptr); // Changes released to the writer have the payload returned to the pool @@ -146,7 +151,7 @@ void pool_initialization_test ( .Times(1) .WillOnce(Return(true)); - writer->release_change(ch); + history->release_change(ch); RTPSDomain::removeRTPSWriter(writer); RTPSDomain::removeRTPSParticipant(participant); diff --git a/test/unittest/statistics/dds/StatisticsDomainParticipantStatusQueryableTests/mock/fastdds/publisher/DataWriterImpl.hpp b/test/unittest/statistics/dds/StatisticsDomainParticipantStatusQueryableTests/mock/fastdds/publisher/DataWriterImpl.hpp index 44d66d69e63..3e53c3d244a 100644 --- a/test/unittest/statistics/dds/StatisticsDomainParticipantStatusQueryableTests/mock/fastdds/publisher/DataWriterImpl.hpp +++ b/test/unittest/statistics/dds/StatisticsDomainParticipantStatusQueryableTests/mock/fastdds/publisher/DataWriterImpl.hpp @@ -83,12 +83,7 @@ class DataWriterImpl : protected rtps::IReaderDataFilter #endif // FASTDDS_STATISTICS DataWriterImpl() - : history_(atts_, - 500, - fastdds::rtps::PREALLOCATED_WITH_REALLOC_MEMORY_MODE, - [](const InstanceHandle_t&) - { - }) + : history_() { gen_guid(); } @@ -100,12 +95,7 @@ class DataWriterImpl : protected rtps::IReaderDataFilter const DataWriterQos&, DataWriterListener* listener = nullptr, std::shared_ptr payload_pool = nullptr) - : history_(atts_, - 500, - fastdds::rtps::PREALLOCATED_WITH_REALLOC_MEMORY_MODE, - [](const InstanceHandle_t&) - { - }) + : history_() { gen_guid(); static_cast(listener); @@ -119,12 +109,7 @@ class DataWriterImpl : protected rtps::IReaderDataFilter const DataWriterQos&, const fastdds::rtps::EntityId_t&, DataWriterListener* listener = nullptr) - : history_(atts_, - 500, - fastdds::rtps::PREALLOCATED_WITH_REALLOC_MEMORY_MODE, - [](const InstanceHandle_t&) - { - }) + : history_() { gen_guid(); static_cast(listener); @@ -139,6 +124,18 @@ class DataWriterImpl : protected rtps::IReaderDataFilter virtual ReturnCode_t enable() { + std::shared_ptr payload_pool; + std::shared_ptr change_pool; + + history_.reset(new DataWriterHistory( + payload_pool, + change_pool, + atts_, + 500, + fastdds::rtps::PREALLOCATED_WITH_REALLOC_MEMORY_MODE, + [](const InstanceHandle_t&) + { + })); return RETCODE_OK; } @@ -430,7 +427,7 @@ class DataWriterImpl : protected rtps::IReaderDataFilter LivelinessLostStatus liveliness_lost_status_; OfferedIncompatibleQosStatus offered_incompatible_qos_status_; std::chrono::duration> lifespan_duration_us_; - DataWriterHistory history_; + std::unique_ptr history_; fastdds::TopicAttributes atts_; }; diff --git a/test/unittest/statistics/rtps/CMakeLists.txt b/test/unittest/statistics/rtps/CMakeLists.txt index f0a109ee321..de69247ee3e 100644 --- a/test/unittest/statistics/rtps/CMakeLists.txt +++ b/test/unittest/statistics/rtps/CMakeLists.txt @@ -76,6 +76,7 @@ set(STATISTICS_RTPS_MONITORSERVICETESTS_SOURCE ${PROJECT_SOURCE_DIR}/src/cpp/rtps/common/Time_t.cpp ${PROJECT_SOURCE_DIR}/src/cpp/rtps/common/Time_t.cpp ${PROJECT_SOURCE_DIR}/src/cpp/rtps/flowcontrol/FlowControllerConsts.cpp + ${PROJECT_SOURCE_DIR}/src/cpp/rtps/history/CacheChangePool.cpp ${PROJECT_SOURCE_DIR}/src/cpp/rtps/history/TopicPayloadPool.cpp ${PROJECT_SOURCE_DIR}/src/cpp/rtps/history/TopicPayloadPoolRegistry.cpp ${PROJECT_SOURCE_DIR}/src/cpp/rtps/messages/CDRMessage.cpp diff --git a/test/unittest/statistics/rtps/MonitorServiceTests.cpp b/test/unittest/statistics/rtps/MonitorServiceTests.cpp index fb1d34a6749..f495e610c4c 100644 --- a/test/unittest/statistics/rtps/MonitorServiceTests.cpp +++ b/test/unittest/statistics/rtps/MonitorServiceTests.cpp @@ -69,7 +69,6 @@ class MonitorServiceTests : public ::testing::Test mock_status_q_, [&](fastdds::rtps::RTPSWriter**, fastdds::rtps::WriterAttributes&, - const std::shared_ptr&, fastdds::rtps::WriterHistory*, fastdds::rtps::WriterListener*, const fastdds::rtps::EntityId_t&, diff --git a/test/unittest/statistics/rtps/RTPSStatisticsTests.cpp b/test/unittest/statistics/rtps/RTPSStatisticsTests.cpp index d9b12b2c539..0250acf8524 100644 --- a/test/unittest/statistics/rtps/RTPSStatisticsTests.cpp +++ b/test/unittest/statistics/rtps/RTPSStatisticsTests.cpp @@ -409,13 +409,7 @@ class RTPSStatisticsTestsImpl ASSERT_NE(nullptr, writer_); - auto writer_change = writer_->new_change( - [length]() -> uint32_t - { - return length; - }, - ALIVE); - + auto writer_change = writer_history_->create_change(length, ALIVE); ASSERT_NE(nullptr, writer_change); std::string str("https://github.com/eProsima/Fast-DDS.git"); @@ -434,13 +428,7 @@ class RTPSStatisticsTestsImpl ASSERT_NE(nullptr, writer_); - auto writer_change = writer_->new_change( - [length]() -> uint32_t - { - return length; - }, - ALIVE); - + auto writer_change = writer_history_->create_change(length, ALIVE); ASSERT_NE(nullptr, writer_change); { diff --git a/test/unittest/statistics/rtps/mock/Publisher/fastdds/publisher/DataWriterHistory.hpp b/test/unittest/statistics/rtps/mock/Publisher/fastdds/publisher/DataWriterHistory.hpp index edbef5a1108..ee5799a6411 100644 --- a/test/unittest/statistics/rtps/mock/Publisher/fastdds/publisher/DataWriterHistory.hpp +++ b/test/unittest/statistics/rtps/mock/Publisher/fastdds/publisher/DataWriterHistory.hpp @@ -24,6 +24,8 @@ #include #include #include +#include +#include namespace eprosima { namespace fastdds { @@ -59,11 +61,13 @@ class DataWriterHistory : public fastdds::rtps::WriterHistory public: DataWriterHistory( - const fastdds::TopicAttributes& topic_att, + const std::shared_ptr& payload_pool, + const std::shared_ptr& change_pool, + const TopicAttributes& topic_att, uint32_t payloadMaxSize, - fastdds::rtps::MemoryManagementPolicy_t mempolicy, - std::function) - : WriterHistory(to_history_attributes(topic_att, payloadMaxSize, mempolicy)) + rtps::MemoryManagementPolicy_t mempolicy, + std::function) + : WriterHistory(to_history_attributes(topic_att, payloadMaxSize, mempolicy), payload_pool, change_pool) { } diff --git a/test/unittest/statistics/rtps/mock/StatisticsBase/statistics/rtps/monitor-service/MonitorService.hpp b/test/unittest/statistics/rtps/mock/StatisticsBase/statistics/rtps/monitor-service/MonitorService.hpp index 5e992d3eb75..10d47e833c6 100644 --- a/test/unittest/statistics/rtps/mock/StatisticsBase/statistics/rtps/monitor-service/MonitorService.hpp +++ b/test/unittest/statistics/rtps/mock/StatisticsBase/statistics/rtps/monitor-service/MonitorService.hpp @@ -54,7 +54,6 @@ class MonitorService using endpoint_creator_t = std::function&, fastdds::rtps::WriterHistory*, fastdds::rtps::WriterListener*, const fastdds::rtps::EntityId_t&, diff --git a/versions.md b/versions.md index e9d3f86a282..7057c00d780 100644 --- a/versions.md +++ b/versions.md @@ -6,10 +6,13 @@ Forthcoming * Remove API marked as deprecated. * Removed deprecated FastRTPS API tests. * Removed no longer supported `FASTRTPS_API_TESTS` CMake options. -* RTPS layer APIs refactor (RTPSReader, ReaderListener, ReaderAttributes): - * Several methods that were meant for internal use have been removed from public API - * All public methods now have `snake_case` names - * All public attributes now have `snake_case` names +* RTPS layer APIs refactor: + * RTPSReader, ReaderListener, ReaderAttributes: + * Several methods that were meant for internal use have been removed from public API + * All public methods now have `snake_case` names + * All public attributes now have `snake_case` names + * RTPSWriter, WriterHistory: + * The responsibility of managing both the `CacheChange` and `Payload` pools has been moved to the `WriterHistory`. * Public API that is no longer public: * XML Parser API no longer public. * ParticipantAttributes