From c0783e7217e05cd27dd732da0427b87798e382b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ricardo=20Gonz=C3=A1lez?= Date: Mon, 5 Jul 2021 07:34:33 +0200 Subject: [PATCH] Fix deadlock with security and timers. [11923] (#2034) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Refs #11680. Fix deadlock with security and timers. Signed-off-by: Ricardo González Moreno * Apply suggestions from code review Co-authored-by: Miguel Company * Refs 11923. Linter fix Signed-off-by: Miguel Company Co-authored-by: Miguel Company --- src/cpp/rtps/security/SecurityManager.cpp | 24 +++++++++++++++++++---- src/cpp/rtps/security/SecurityManager.h | 2 +- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/src/cpp/rtps/security/SecurityManager.cpp b/src/cpp/rtps/security/SecurityManager.cpp index a9998b456ab..c1bab35c052 100644 --- a/src/cpp/rtps/security/SecurityManager.cpp +++ b/src/cpp/rtps/security/SecurityManager.cpp @@ -455,6 +455,10 @@ void SecurityManager::destroy() SecurityException exception; + // AuthUniquePtr must be removed after unlock SecurityManager's mutex. + // This is to avoid a deadlock with TimedEvents. + std::vector auths_to_remove; + for (auto& dp_it : discovered_participants_) { ParticipantCryptoHandle* participant_crypto_handle = dp_it.second.get_participant_crypto(); @@ -475,7 +479,9 @@ void SecurityManager::destroy() authentication_plugin_->return_sharedsecret_handle(shared_secret_handle, exception); } - remove_discovered_participant_info(dp_it.second.get_auth()); + DiscoveredParticipantInfo::AuthUniquePtr remote_participant_info = dp_it.second.get_auth(); + remove_discovered_participant_info(remote_participant_info); + auths_to_remove.push_back(std::move(remote_participant_info)); } discovered_participants_.clear(); @@ -500,6 +506,8 @@ void SecurityManager::destroy() mutex_.unlock(); + auths_to_remove.clear(); + delete_entities(); if (crypto_plugin_ != nullptr) @@ -528,7 +536,7 @@ void SecurityManager::destroy() } void SecurityManager::remove_discovered_participant_info( - DiscoveredParticipantInfo::AuthUniquePtr&& auth_ptr) + const DiscoveredParticipantInfo::AuthUniquePtr& auth_ptr) { SecurityException exception; @@ -562,7 +570,11 @@ bool SecurityManager::restore_discovered_participant_info( } else { - remove_discovered_participant_info(std::move(auth_ptr)); + // AuthUniquePtr must be removed after unlock SecurityManager's mutex. + // This is to avoid a deadlock with TimedEvents. + DiscoveredParticipantInfo::AuthUniquePtr remote_participant_info = std::move(auth_ptr); + remove_discovered_participant_info(remote_participant_info); + lock.unlock(); } return returned_value; @@ -736,9 +748,13 @@ void SecurityManager::remove_participant( authentication_plugin_->return_sharedsecret_handle(shared_secret_handle, exception); } - remove_discovered_participant_info(dp_it->second.get_auth()); + // AuthUniquePtr must be removed after unlock SecurityManager's mutex. + // This is to avoid a deadlock with TimedEvents. + DiscoveredParticipantInfo::AuthUniquePtr remote_participant_info = dp_it->second.get_auth(); + remove_discovered_participant_info(remote_participant_info); discovered_participants_.erase(dp_it); + lock.unlock(); } } diff --git a/src/cpp/rtps/security/SecurityManager.h b/src/cpp/rtps/security/SecurityManager.h index 7ffc427b1d4..9ddce64c5db 100644 --- a/src/cpp/rtps/security/SecurityManager.h +++ b/src/cpp/rtps/security/SecurityManager.h @@ -426,7 +426,7 @@ class SecurityManager void cancel_init(); void remove_discovered_participant_info( - DiscoveredParticipantInfo::AuthUniquePtr&& auth_ptr); + const DiscoveredParticipantInfo::AuthUniquePtr& auth_ptr); bool restore_discovered_participant_info( const GUID_t& remote_participant_key,