From ce4b7bc42c105280d05262a8f7932cfc6ba18c4d Mon Sep 17 00:00:00 2001 From: jparisu Date: Mon, 7 Nov 2022 12:09:16 +0100 Subject: [PATCH 1/8] Add protection to internal DiscoveryDatabase variable Signed-off-by: jparisu --- src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.cpp b/src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.cpp index eb4ad87aa8d..40e6c6bac0f 100644 --- a/src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.cpp +++ b/src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.cpp @@ -2599,6 +2599,8 @@ void DiscoveryDataBase::persistence_enable( bool DiscoveryDataBase::is_participant_local( const eprosima::fastrtps::rtps::GuidPrefix_t& participant_prefix) { + std::unique_lock lock(mutex_); + auto pit = participants_.find(participant_prefix); if (pit != participants_.end()) { From c91dffe049e4fd6e8afa2a268d21253c6200c487 Mon Sep 17 00:00:00 2001 From: jparisu Date: Mon, 7 Nov 2022 13:01:23 +0100 Subject: [PATCH 2/8] Protect DiscoverySharedInfo internal variables Signed-off-by: jparisu --- .../discovery/database/DiscoveryDataBase.cpp | 3 ++- .../database/DiscoveryDataQueueInfo.hpp | 8 +++--- .../DiscoveryParticipantsAckStatus.cpp | 1 + .../DiscoveryParticipantsAckStatus.hpp | 3 +++ .../database/DiscoverySharedInfo.cpp | 16 ++++++++++++ .../database/DiscoverySharedInfo.hpp | 26 ++++++++++++++++--- 6 files changed, 48 insertions(+), 9 deletions(-) diff --git a/src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.cpp b/src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.cpp index 40e6c6bac0f..741d9c499b2 100644 --- a/src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.cpp +++ b/src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.cpp @@ -663,7 +663,8 @@ void DiscoveryDataBase::create_new_participant_from_change_( DiscoveryParticipantInfo part(ch, server_guid_prefix_, change_data); std::pair::iterator, bool> ret = - participants_.insert(std::make_pair(change_guid.guidPrefix, part)); + participants_.insert(std::make_pair(change_guid.guidPrefix, std::move(part))); + // If insert was successful if (ret.second) { diff --git a/src/cpp/rtps/builtin/discovery/database/DiscoveryDataQueueInfo.hpp b/src/cpp/rtps/builtin/discovery/database/DiscoveryDataQueueInfo.hpp index 2ad4d079db6..dbe8b276c42 100644 --- a/src/cpp/rtps/builtin/discovery/database/DiscoveryDataQueueInfo.hpp +++ b/src/cpp/rtps/builtin/discovery/database/DiscoveryDataQueueInfo.hpp @@ -60,7 +60,7 @@ class DiscoveryPDPDataQueueInfo : public DiscoveryDataQueueInfo DiscoveryPDPDataQueueInfo( eprosima::fastrtps::rtps::CacheChange_t* change, - DiscoveryParticipantChangeData participant_change_data) + const DiscoveryParticipantChangeData& participant_change_data) : DiscoveryDataQueueInfo(change) , participant_change_data_(participant_change_data) { @@ -77,7 +77,7 @@ class DiscoveryPDPDataQueueInfo : public DiscoveryDataQueueInfo private: - DiscoveryParticipantChangeData participant_change_data_; + const DiscoveryParticipantChangeData participant_change_data_; }; @@ -87,7 +87,7 @@ class DiscoveryEDPDataQueueInfo : public DiscoveryDataQueueInfo DiscoveryEDPDataQueueInfo( eprosima::fastrtps::rtps::CacheChange_t* change, - eprosima::fastrtps::string_255 topic) + const eprosima::fastrtps::string_255& topic) : DiscoveryDataQueueInfo(change) , topic_(topic) { @@ -104,7 +104,7 @@ class DiscoveryEDPDataQueueInfo : public DiscoveryDataQueueInfo private: - eprosima::fastrtps::string_255 topic_; + const eprosima::fastrtps::string_255 topic_; }; diff --git a/src/cpp/rtps/builtin/discovery/database/DiscoveryParticipantsAckStatus.cpp b/src/cpp/rtps/builtin/discovery/database/DiscoveryParticipantsAckStatus.cpp index 6232854ed7b..852186619b4 100644 --- a/src/cpp/rtps/builtin/discovery/database/DiscoveryParticipantsAckStatus.cpp +++ b/src/cpp/rtps/builtin/discovery/database/DiscoveryParticipantsAckStatus.cpp @@ -36,6 +36,7 @@ void DiscoveryParticipantsAckStatus::add_or_update_participant( const eprosima::fastrtps::rtps::GuidPrefix_t& guid_p, bool status = false) { + std::lock_guard _(mutex_); relevant_participants_map_[guid_p] = status; } diff --git a/src/cpp/rtps/builtin/discovery/database/DiscoveryParticipantsAckStatus.hpp b/src/cpp/rtps/builtin/discovery/database/DiscoveryParticipantsAckStatus.hpp index 31cfd356de9..72e563f93b7 100644 --- a/src/cpp/rtps/builtin/discovery/database/DiscoveryParticipantsAckStatus.hpp +++ b/src/cpp/rtps/builtin/discovery/database/DiscoveryParticipantsAckStatus.hpp @@ -21,6 +21,7 @@ #define _FASTDDS_RTPS_DISCOVERY_PARTICIPANT_ACK_STATUS_H_ #include +#include #include #include @@ -73,6 +74,8 @@ class DiscoveryParticipantsAckStatus private: + mutable std::mutex mutex_; + std::map relevant_participants_map_; }; diff --git a/src/cpp/rtps/builtin/discovery/database/DiscoverySharedInfo.cpp b/src/cpp/rtps/builtin/discovery/database/DiscoverySharedInfo.cpp index 805241a9468..27b78d40e59 100644 --- a/src/cpp/rtps/builtin/discovery/database/DiscoverySharedInfo.cpp +++ b/src/cpp/rtps/builtin/discovery/database/DiscoverySharedInfo.cpp @@ -39,6 +39,22 @@ DiscoverySharedInfo::DiscoverySharedInfo( add_or_update_ack_participant(known_participant, true); } +DiscoverySharedInfo::DiscoverySharedInfo(const DiscoverySharedInfo& other) noexcept + : change_(other.change_) +{ + // Must be done locking other internal values so they do not change during this process + std::lock_guard _(other.mutex_); + relevant_participants_builtin_ack_status_ = other.relevant_participants_builtin_ack_status_; +} + +DiscoverySharedInfo::DiscoverySharedInfo(DiscoverySharedInfo&& other) noexcept + : change_(std::move(other.change_)) + , relevant_participants_builtin_ack_status_(std::move(other.relevant_participants_builtin_ack_status_)) +{ + // It does not require mutex as other is supposed to not be used from other thread after this call. + // Do nothing +} + eprosima::fastrtps::rtps::CacheChange_t* DiscoverySharedInfo::update_and_unmatch( eprosima::fastrtps::rtps::CacheChange_t* change) { diff --git a/src/cpp/rtps/builtin/discovery/database/DiscoverySharedInfo.hpp b/src/cpp/rtps/builtin/discovery/database/DiscoverySharedInfo.hpp index 5d7fb42f6e6..21721d5f3d9 100644 --- a/src/cpp/rtps/builtin/discovery/database/DiscoverySharedInfo.hpp +++ b/src/cpp/rtps/builtin/discovery/database/DiscoverySharedInfo.hpp @@ -20,6 +20,8 @@ #ifndef _FASTDDS_RTPS_DISCOVERY_SHARED_INFO_H_ #define _FASTDDS_RTPS_DISCOVERY_SHARED_INFO_H_ +#include + #include #include #include @@ -50,6 +52,12 @@ class DiscoverySharedInfo { } + /** + * @brief Copy constructors required as mutex make this class not copyable by default. + */ + DiscoverySharedInfo(const DiscoverySharedInfo& other) noexcept; + DiscoverySharedInfo(DiscoverySharedInfo&& other) noexcept; + virtual eprosima::fastrtps::rtps::CacheChange_t* update_and_unmatch( eprosima::fastrtps::rtps::CacheChange_t* change); @@ -60,27 +68,33 @@ class DiscoverySharedInfo const eprosima::fastrtps::rtps::GuidPrefix_t& guid_p, bool status = false) { - EPROSIMA_LOG_INFO(DISCOVERY_DATABASE, "Adding relevant participant " << guid_p << " with status " << status << " to " << - fastrtps::rtps::iHandle2GUID( - change_->instanceHandle)); + EPROSIMA_LOG_INFO( + DISCOVERY_DATABASE, + "Adding relevant participant " << guid_p + << " with status " << status + << " to " << fastrtps::rtps::iHandle2GUID(change_->instanceHandle)); + std::lock_guard _(mutex_); relevant_participants_builtin_ack_status_.add_or_update_participant(guid_p, status); } void remove_participant( const eprosima::fastrtps::rtps::GuidPrefix_t& guid_p) { + std::lock_guard _(mutex_); relevant_participants_builtin_ack_status_.remove_participant(guid_p); } bool is_matched( const eprosima::fastrtps::rtps::GuidPrefix_t& guid_p) const { + std::lock_guard _(mutex_); return relevant_participants_builtin_ack_status_.is_matched(guid_p); } bool is_relevant_participant( const eprosima::fastrtps::rtps::GuidPrefix_t& guid_p) const { + std::lock_guard _(mutex_); return relevant_participants_builtin_ack_status_.is_relevant_participant(guid_p); } @@ -91,21 +105,25 @@ class DiscoverySharedInfo std::vector relevant_participants() const { + std::lock_guard _(mutex_); return relevant_participants_builtin_ack_status_.relevant_participants(); } bool is_acked_by_all() const { + std::lock_guard _(mutex_); return relevant_participants_builtin_ack_status_.is_acked_by_all(); } virtual void to_json( nlohmann::json& j) const; -private: +protected: eprosima::fastrtps::rtps::CacheChange_t* change_; + mutable std::mutex mutex_; + // new class is used in order to could change it in the future for a more efficient implementation eprosima::fastdds::rtps::ddb::DiscoveryParticipantsAckStatus relevant_participants_builtin_ack_status_; From cc7d4bcd13a4ec18327e0a3ac942889634991123 Mon Sep 17 00:00:00 2001 From: jparisu Date: Thu, 10 Nov 2022 07:42:14 +0100 Subject: [PATCH 3/8] Fix non protected method calls in DiscoveryDatabase Signed-off-by: jparisu --- .../discovery/database/DiscoveryDataBase.cpp | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.cpp b/src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.cpp index 741d9c499b2..fe1e450e275 100644 --- a/src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.cpp +++ b/src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.cpp @@ -661,9 +661,11 @@ void DiscoveryDataBase::create_new_participant_from_change_( { fastrtps::rtps::GUID_t change_guid = guid_from_change(ch); - DiscoveryParticipantInfo part(ch, server_guid_prefix_, change_data); std::pair::iterator, bool> ret = - participants_.insert(std::make_pair(change_guid.guidPrefix, std::move(part))); + participants_.insert( + std::make_pair( + change_guid.guidPrefix, + DiscoveryParticipantInfo(ch, server_guid_prefix_, change_data))); // If insert was successful if (ret.second) @@ -1593,6 +1595,8 @@ fastrtps::rtps::CacheChange_t* DiscoveryDataBase::cache_change_own_participant() const std::vector DiscoveryDataBase::direct_clients_and_servers() { + std::unique_lock lock(mutex_); + std::vector direct_clients_and_servers; // Iterate over participants to add the remote ones that are direct clients or servers for (auto participant: participants_) @@ -1612,6 +1616,8 @@ const std::vector DiscoveryDataBase::direct_client bool DiscoveryDataBase::server_acked_by_my_servers() { + std::unique_lock lock(mutex_); + if (servers_.size() == 0) { return true; @@ -1619,8 +1625,8 @@ bool DiscoveryDataBase::server_acked_by_my_servers() // Find the server's participant and check whether all its servers have ACKed the server's DATA(p) auto this_server = participants_.find(server_guid_prefix_); - // check it is always there + assert(this_server != participants_.end()); for (auto prefix : servers_) @@ -1635,6 +1641,8 @@ bool DiscoveryDataBase::server_acked_by_my_servers() std::vector DiscoveryDataBase::ack_pending_servers() { + std::unique_lock lock(mutex_); + std::vector ack_pending_servers; // Find the server's participant and check whether all its servers have ACKed the server's DATA(p) auto this_server = participants_.find(server_guid_prefix_); @@ -1703,6 +1711,8 @@ DiscoveryDataBase::AckedFunctor::~AckedFunctor() void DiscoveryDataBase::AckedFunctor::operator () ( const eprosima::fastrtps::rtps::ReaderProxy* reader_proxy) { + std::unique_lock lock(db_->mutex_); + EPROSIMA_LOG_INFO(DISCOVERY_DATABASE, "functor operator in change: " << change_->instanceHandle); EPROSIMA_LOG_INFO(DISCOVERY_DATABASE, "for reader proxy: " << reader_proxy->guid()); // Check whether the change has been acknowledged by a given reader From 151b95401a46cd364dde6f1b44b13a09adc3e309 Mon Sep 17 00:00:00 2001 From: jparisu Date: Tue, 22 Nov 2022 08:05:58 +0100 Subject: [PATCH 4/8] Remove unneeded mutexes in internal classes Signed-off-by: jparisu --- .../database/DiscoveryParticipantsAckStatus.cpp | 1 - .../database/DiscoveryParticipantsAckStatus.hpp | 11 ++--------- .../discovery/database/DiscoverySharedInfo.cpp | 16 ---------------- .../discovery/database/DiscoverySharedInfo.hpp | 17 ++++------------- 4 files changed, 6 insertions(+), 39 deletions(-) diff --git a/src/cpp/rtps/builtin/discovery/database/DiscoveryParticipantsAckStatus.cpp b/src/cpp/rtps/builtin/discovery/database/DiscoveryParticipantsAckStatus.cpp index 852186619b4..6232854ed7b 100644 --- a/src/cpp/rtps/builtin/discovery/database/DiscoveryParticipantsAckStatus.cpp +++ b/src/cpp/rtps/builtin/discovery/database/DiscoveryParticipantsAckStatus.cpp @@ -36,7 +36,6 @@ void DiscoveryParticipantsAckStatus::add_or_update_participant( const eprosima::fastrtps::rtps::GuidPrefix_t& guid_p, bool status = false) { - std::lock_guard _(mutex_); relevant_participants_map_[guid_p] = status; } diff --git a/src/cpp/rtps/builtin/discovery/database/DiscoveryParticipantsAckStatus.hpp b/src/cpp/rtps/builtin/discovery/database/DiscoveryParticipantsAckStatus.hpp index 72e563f93b7..9eeb99ac0d3 100644 --- a/src/cpp/rtps/builtin/discovery/database/DiscoveryParticipantsAckStatus.hpp +++ b/src/cpp/rtps/builtin/discovery/database/DiscoveryParticipantsAckStatus.hpp @@ -21,7 +21,6 @@ #define _FASTDDS_RTPS_DISCOVERY_PARTICIPANT_ACK_STATUS_H_ #include -#include #include #include @@ -42,13 +41,9 @@ class DiscoveryParticipantsAckStatus public: - DiscoveryParticipantsAckStatus() - { - } + DiscoveryParticipantsAckStatus() = default; - ~DiscoveryParticipantsAckStatus() - { - } + ~DiscoveryParticipantsAckStatus() = default; void add_or_update_participant( const eprosima::fastrtps::rtps::GuidPrefix_t& guid_p, @@ -74,8 +69,6 @@ class DiscoveryParticipantsAckStatus private: - mutable std::mutex mutex_; - std::map relevant_participants_map_; }; diff --git a/src/cpp/rtps/builtin/discovery/database/DiscoverySharedInfo.cpp b/src/cpp/rtps/builtin/discovery/database/DiscoverySharedInfo.cpp index 27b78d40e59..805241a9468 100644 --- a/src/cpp/rtps/builtin/discovery/database/DiscoverySharedInfo.cpp +++ b/src/cpp/rtps/builtin/discovery/database/DiscoverySharedInfo.cpp @@ -39,22 +39,6 @@ DiscoverySharedInfo::DiscoverySharedInfo( add_or_update_ack_participant(known_participant, true); } -DiscoverySharedInfo::DiscoverySharedInfo(const DiscoverySharedInfo& other) noexcept - : change_(other.change_) -{ - // Must be done locking other internal values so they do not change during this process - std::lock_guard _(other.mutex_); - relevant_participants_builtin_ack_status_ = other.relevant_participants_builtin_ack_status_; -} - -DiscoverySharedInfo::DiscoverySharedInfo(DiscoverySharedInfo&& other) noexcept - : change_(std::move(other.change_)) - , relevant_participants_builtin_ack_status_(std::move(other.relevant_participants_builtin_ack_status_)) -{ - // It does not require mutex as other is supposed to not be used from other thread after this call. - // Do nothing -} - eprosima::fastrtps::rtps::CacheChange_t* DiscoverySharedInfo::update_and_unmatch( eprosima::fastrtps::rtps::CacheChange_t* change) { diff --git a/src/cpp/rtps/builtin/discovery/database/DiscoverySharedInfo.hpp b/src/cpp/rtps/builtin/discovery/database/DiscoverySharedInfo.hpp index 21721d5f3d9..b9091c274c1 100644 --- a/src/cpp/rtps/builtin/discovery/database/DiscoverySharedInfo.hpp +++ b/src/cpp/rtps/builtin/discovery/database/DiscoverySharedInfo.hpp @@ -48,15 +48,13 @@ class DiscoverySharedInfo eprosima::fastrtps::rtps::CacheChange_t* change, const eprosima::fastrtps::rtps::GuidPrefix_t& known_participant); - ~DiscoverySharedInfo() - { - } + ~DiscoverySharedInfo() = default; /** * @brief Copy constructors required as mutex make this class not copyable by default. */ - DiscoverySharedInfo(const DiscoverySharedInfo& other) noexcept; - DiscoverySharedInfo(DiscoverySharedInfo&& other) noexcept; + DiscoverySharedInfo(const DiscoverySharedInfo& other) noexcept = default; + DiscoverySharedInfo(DiscoverySharedInfo&& other) noexcept = default; virtual eprosima::fastrtps::rtps::CacheChange_t* update_and_unmatch( eprosima::fastrtps::rtps::CacheChange_t* change); @@ -68,33 +66,30 @@ class DiscoverySharedInfo const eprosima::fastrtps::rtps::GuidPrefix_t& guid_p, bool status = false) { + std::lock_guard _(mutex_); EPROSIMA_LOG_INFO( DISCOVERY_DATABASE, "Adding relevant participant " << guid_p << " with status " << status << " to " << fastrtps::rtps::iHandle2GUID(change_->instanceHandle)); - std::lock_guard _(mutex_); relevant_participants_builtin_ack_status_.add_or_update_participant(guid_p, status); } void remove_participant( const eprosima::fastrtps::rtps::GuidPrefix_t& guid_p) { - std::lock_guard _(mutex_); relevant_participants_builtin_ack_status_.remove_participant(guid_p); } bool is_matched( const eprosima::fastrtps::rtps::GuidPrefix_t& guid_p) const { - std::lock_guard _(mutex_); return relevant_participants_builtin_ack_status_.is_matched(guid_p); } bool is_relevant_participant( const eprosima::fastrtps::rtps::GuidPrefix_t& guid_p) const { - std::lock_guard _(mutex_); return relevant_participants_builtin_ack_status_.is_relevant_participant(guid_p); } @@ -105,13 +100,11 @@ class DiscoverySharedInfo std::vector relevant_participants() const { - std::lock_guard _(mutex_); return relevant_participants_builtin_ack_status_.relevant_participants(); } bool is_acked_by_all() const { - std::lock_guard _(mutex_); return relevant_participants_builtin_ack_status_.is_acked_by_all(); } @@ -122,8 +115,6 @@ class DiscoverySharedInfo eprosima::fastrtps::rtps::CacheChange_t* change_; - mutable std::mutex mutex_; - // new class is used in order to could change it in the future for a more efficient implementation eprosima::fastdds::rtps::ddb::DiscoveryParticipantsAckStatus relevant_participants_builtin_ack_status_; From 345dabff30812ecca9c322f82c6a776166c9313a Mon Sep 17 00:00:00 2001 From: jparisu Date: Tue, 22 Nov 2022 10:06:55 +0100 Subject: [PATCH 5/8] apply suggestions Signed-off-by: jparisu --- .../discovery/database/DiscoveryDataBase.cpp | 50 +++++++++---------- .../database/DiscoverySharedInfo.hpp | 6 --- 2 files changed, 25 insertions(+), 31 deletions(-) diff --git a/src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.cpp b/src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.cpp index fe1e450e275..13cc90c61f4 100644 --- a/src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.cpp +++ b/src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.cpp @@ -79,7 +79,7 @@ std::vector DiscoveryDataBase::clear() } EPROSIMA_LOG_INFO(DISCOVERY_DATABASE, "Clearing DiscoveryDataBase"); - std::unique_lock lock(mutex_); + std::lock_guard guard(mutex_); /* Clear receive queues. Set changes inside to release */ while (!pdp_data_queue_.Empty()) @@ -161,7 +161,7 @@ bool DiscoveryDataBase::pdp_is_relevant( } // Lock(shared mode) mutex locally - std::unique_lock lock(mutex_); + std::lock_guard guard(mutex_); EPROSIMA_LOG_INFO(DISCOVERY_DATABASE, "PDP is " << change.instanceHandle << " relevant to " << reader_guid); @@ -185,7 +185,7 @@ bool DiscoveryDataBase::edp_publications_is_relevant( fastrtps::rtps::GUID_t change_guid = guid_from_change(&change); // Lock(shared mode) mutex locally - std::unique_lock lock(mutex_); + std::lock_guard guard(mutex_); auto itp = participants_.find(change_guid.guidPrefix); if (itp == participants_.end()) @@ -218,7 +218,7 @@ bool DiscoveryDataBase::edp_subscriptions_is_relevant( fastrtps::rtps::GUID_t change_guid = guid_from_change(&change); // Lock(shared mode) mutex locally - std::unique_lock lock(mutex_); + std::lock_guard guard(mutex_); auto itp = participants_.find(change_guid.guidPrefix); if (itp == participants_.end()) @@ -320,7 +320,7 @@ bool DiscoveryDataBase::update( if (is_persistent_ && guid_from_change(change).guidPrefix != server_guid_prefix_) { // Does not allow to the server to erase the ddb before this message has been processed - std::unique_lock lock(data_queues_mutex_); + std::lock_guard guard(data_queues_mutex_); nlohmann::json j; ddb::to_json(j, *change); backup_file_ << j; @@ -352,7 +352,7 @@ bool DiscoveryDataBase::update( if (is_persistent_ && guid_from_change(change).guidPrefix != server_guid_prefix_) { // Does not allow to the server to erase the ddb before this message has been process - std::unique_lock lock(data_queues_mutex_); + std::lock_guard guard(data_queues_mutex_); nlohmann::json j; ddb::to_json(j, *change); backup_file_ << j; @@ -380,14 +380,14 @@ bool DiscoveryDataBase::update( const std::vector DiscoveryDataBase::changes_to_dispose() { // lock(sharing mode) mutex locally - std::unique_lock lock(mutex_); + std::lock_guard guard(mutex_); return disposals_; } void DiscoveryDataBase::clear_changes_to_dispose() { // lock(exclusive mode) mutex locally - std::unique_lock lock(mutex_); + std::lock_guard guard(mutex_); disposals_.clear(); } @@ -396,56 +396,56 @@ void DiscoveryDataBase::clear_changes_to_dispose() const std::vector DiscoveryDataBase::pdp_to_send() { // lock(sharing mode) mutex locally - std::unique_lock lock(mutex_); + std::lock_guard guard(mutex_); return pdp_to_send_; } void DiscoveryDataBase::clear_pdp_to_send() { // lock(exclusive mode) mutex locally - std::unique_lock lock(mutex_); + std::lock_guard guard(mutex_); pdp_to_send_.clear(); } const std::vector DiscoveryDataBase::edp_publications_to_send() { // lock(sharing mode) mutex locally - std::unique_lock lock(mutex_); + std::lock_guard guard(mutex_); return edp_publications_to_send_; } void DiscoveryDataBase::clear_edp_publications_to_send() { // lock(exclusive mode) mutex locally - std::unique_lock lock(mutex_); + std::lock_guard guard(mutex_); edp_publications_to_send_.clear(); } const std::vector DiscoveryDataBase::edp_subscriptions_to_send() { // lock(sharing mode) mutex locally - std::unique_lock lock(mutex_); + std::lock_guard guard(mutex_); return edp_subscriptions_to_send_; } void DiscoveryDataBase::clear_edp_subscriptions_to_send() { // lock(exclusive mode) mutex locally - std::unique_lock lock(mutex_); + std::lock_guard guard(mutex_); edp_subscriptions_to_send_.clear(); } const std::vector DiscoveryDataBase::changes_to_release() { // lock(sharing mode) mutex locally - std::unique_lock lock(mutex_); + std::lock_guard guard(mutex_); return changes_to_release_; } void DiscoveryDataBase::clear_changes_to_release() { // lock(exclusive mode) mutex locally - std::unique_lock lock(mutex_); + std::lock_guard guard(mutex_); changes_to_release_.clear(); } @@ -460,7 +460,7 @@ void DiscoveryDataBase::process_pdp_data_queue() } // Lock(exclusive mode) mutex locally - std::unique_lock lock(mutex_); + std::lock_guard guard(mutex_); // Swap DATA queues pdp_data_queue_.Swap(); @@ -500,7 +500,7 @@ bool DiscoveryDataBase::process_edp_data_queue() bool is_dirty_topic = false; // Lock(exclusive mode) mutex locally - std::unique_lock lock(mutex_); + std::lock_guard guard(mutex_); // Swap DATA queues edp_data_queue_.Swap(); @@ -1349,7 +1349,7 @@ bool DiscoveryDataBase::process_dirty_topics() // EPROSIMA_LOG_INFO(DISCOVERY_DATABASE, "process_dirty_topics start"); // Get shared lock - std::unique_lock lock(mutex_); + std::lock_guard guard(mutex_); // Iterator objects are declared here because they are reused in each iteration of the loops std::map::iterator parts_reader_it; @@ -1492,7 +1492,7 @@ bool DiscoveryDataBase::delete_entity_of_change( } // Lock(exclusive mode) mutex locally - std::unique_lock lock(mutex_); + std::lock_guard guard(mutex_); if (change->kind == fastrtps::rtps::ChangeKind_t::ALIVE) { @@ -1595,7 +1595,7 @@ fastrtps::rtps::CacheChange_t* DiscoveryDataBase::cache_change_own_participant() const std::vector DiscoveryDataBase::direct_clients_and_servers() { - std::unique_lock lock(mutex_); + std::lock_guard guard(mutex_); std::vector direct_clients_and_servers; // Iterate over participants to add the remote ones that are direct clients or servers @@ -1616,7 +1616,7 @@ const std::vector DiscoveryDataBase::direct_client bool DiscoveryDataBase::server_acked_by_my_servers() { - std::unique_lock lock(mutex_); + std::lock_guard guard(mutex_); if (servers_.size() == 0) { @@ -1641,7 +1641,7 @@ bool DiscoveryDataBase::server_acked_by_my_servers() std::vector DiscoveryDataBase::ack_pending_servers() { - std::unique_lock lock(mutex_); + std::lock_guard guard(mutex_); std::vector ack_pending_servers; // Find the server's participant and check whether all its servers have ACKed the server's DATA(p) @@ -1711,7 +1711,7 @@ DiscoveryDataBase::AckedFunctor::~AckedFunctor() void DiscoveryDataBase::AckedFunctor::operator () ( const eprosima::fastrtps::rtps::ReaderProxy* reader_proxy) { - std::unique_lock lock(db_->mutex_); + std::lock_guard guard(db_->mutex_); EPROSIMA_LOG_INFO(DISCOVERY_DATABASE, "functor operator in change: " << change_->instanceHandle); EPROSIMA_LOG_INFO(DISCOVERY_DATABASE, "for reader proxy: " << reader_proxy->guid()); @@ -2610,7 +2610,7 @@ void DiscoveryDataBase::persistence_enable( bool DiscoveryDataBase::is_participant_local( const eprosima::fastrtps::rtps::GuidPrefix_t& participant_prefix) { - std::unique_lock lock(mutex_); + std::lock_guard guard(mutex_); auto pit = participants_.find(participant_prefix); if (pit != participants_.end()) diff --git a/src/cpp/rtps/builtin/discovery/database/DiscoverySharedInfo.hpp b/src/cpp/rtps/builtin/discovery/database/DiscoverySharedInfo.hpp index b9091c274c1..17f4b326bf9 100644 --- a/src/cpp/rtps/builtin/discovery/database/DiscoverySharedInfo.hpp +++ b/src/cpp/rtps/builtin/discovery/database/DiscoverySharedInfo.hpp @@ -50,12 +50,6 @@ class DiscoverySharedInfo ~DiscoverySharedInfo() = default; - /** - * @brief Copy constructors required as mutex make this class not copyable by default. - */ - DiscoverySharedInfo(const DiscoverySharedInfo& other) noexcept = default; - DiscoverySharedInfo(DiscoverySharedInfo&& other) noexcept = default; - virtual eprosima::fastrtps::rtps::CacheChange_t* update_and_unmatch( eprosima::fastrtps::rtps::CacheChange_t* change); From 988698590451cc1d4abbb08fe12cc7eb9df7853f Mon Sep 17 00:00:00 2001 From: jparisu Date: Tue, 22 Nov 2022 10:50:46 +0100 Subject: [PATCH 6/8] uncrustify Signed-off-by: jparisu --- .../rtps/builtin/discovery/database/DiscoveryDataBase.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.cpp b/src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.cpp index 13cc90c61f4..0c7dbba91aa 100644 --- a/src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.cpp +++ b/src/cpp/rtps/builtin/discovery/database/DiscoveryDataBase.cpp @@ -663,9 +663,9 @@ void DiscoveryDataBase::create_new_participant_from_change_( std::pair::iterator, bool> ret = participants_.insert( - std::make_pair( - change_guid.guidPrefix, - DiscoveryParticipantInfo(ch, server_guid_prefix_, change_data))); + std::make_pair( + change_guid.guidPrefix, + DiscoveryParticipantInfo(ch, server_guid_prefix_, change_data))); // If insert was successful if (ret.second) From f0c910e1d0c466e3ad60232c76dbb1cb2b924adc Mon Sep 17 00:00:00 2001 From: jparisu Date: Tue, 22 Nov 2022 10:56:56 +0100 Subject: [PATCH 7/8] remove include mutex Signed-off-by: jparisu --- .../rtps/builtin/discovery/database/DiscoverySharedInfo.hpp | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/cpp/rtps/builtin/discovery/database/DiscoverySharedInfo.hpp b/src/cpp/rtps/builtin/discovery/database/DiscoverySharedInfo.hpp index 17f4b326bf9..73582917481 100644 --- a/src/cpp/rtps/builtin/discovery/database/DiscoverySharedInfo.hpp +++ b/src/cpp/rtps/builtin/discovery/database/DiscoverySharedInfo.hpp @@ -20,8 +20,6 @@ #ifndef _FASTDDS_RTPS_DISCOVERY_SHARED_INFO_H_ #define _FASTDDS_RTPS_DISCOVERY_SHARED_INFO_H_ -#include - #include #include #include @@ -60,7 +58,6 @@ class DiscoverySharedInfo const eprosima::fastrtps::rtps::GuidPrefix_t& guid_p, bool status = false) { - std::lock_guard _(mutex_); EPROSIMA_LOG_INFO( DISCOVERY_DATABASE, "Adding relevant participant " << guid_p From 942596edf5840fba9046a3771b228e3bbeb4daa6 Mon Sep 17 00:00:00 2001 From: jparisu Date: Wed, 7 Dec 2022 12:49:21 +0100 Subject: [PATCH 8/8] uncrustify Signed-off-by: jparisu --- .../rtps/builtin/discovery/database/DiscoverySharedInfo.hpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cpp/rtps/builtin/discovery/database/DiscoverySharedInfo.hpp b/src/cpp/rtps/builtin/discovery/database/DiscoverySharedInfo.hpp index 73582917481..530a814333c 100644 --- a/src/cpp/rtps/builtin/discovery/database/DiscoverySharedInfo.hpp +++ b/src/cpp/rtps/builtin/discovery/database/DiscoverySharedInfo.hpp @@ -61,8 +61,8 @@ class DiscoverySharedInfo EPROSIMA_LOG_INFO( DISCOVERY_DATABASE, "Adding relevant participant " << guid_p - << " with status " << status - << " to " << fastrtps::rtps::iHandle2GUID(change_->instanceHandle)); + << " with status " << status + << " to " << fastrtps::rtps::iHandle2GUID(change_->instanceHandle)); relevant_participants_builtin_ack_status_.add_or_update_participant(guid_p, status); }