From c35b42ced268ef7670489acb739a21c00d7d985b Mon Sep 17 00:00:00 2001 From: Dirk Thomas Date: Thu, 6 Aug 2020 14:20:40 -0700 Subject: [PATCH] [backport foxy] Fix trying to get topic data that was already removed (#421) * Fix trying to get topic data that was already removed. (#417) * Fix trying to get topic data that was already removed. Signed-off-by: Chen.Lihui * Revert "Fix trying to get topic data that was already removed." This reverts commit 08632e8ab7b8bd4c021f4d12c247e03e64e0c945. Signed-off-by: Chen.Lihui * Updated based on suggestions Signed-off-by: Chen.Lihui Signed-off-by: Miguel Company Signed-off-by: Dirk Thomas * revert rename of function Signed-off-by: Dirk Thomas Co-authored-by: Chen Lihui --- .../custom_subscriber_info.hpp | 33 +++++++------------ 1 file changed, 11 insertions(+), 22 deletions(-) diff --git a/rmw_fastrtps_shared_cpp/include/rmw_fastrtps_shared_cpp/custom_subscriber_info.hpp b/rmw_fastrtps_shared_cpp/include/rmw_fastrtps_shared_cpp/custom_subscriber_info.hpp index e46bf9e81..b2416ddc3 100644 --- a/rmw_fastrtps_shared_cpp/include/rmw_fastrtps_shared_cpp/custom_subscriber_info.hpp +++ b/rmw_fastrtps_shared_cpp/include/rmw_fastrtps_shared_cpp/custom_subscriber_info.hpp @@ -67,34 +67,23 @@ class SubListener : public EventListenerInterface, public eprosima::fastrtps::Su // SubscriberListener implementation void onSubscriptionMatched( - eprosima::fastrtps::Subscriber * /*sub*/, eprosima::fastrtps::rtps::MatchingInfo & info) final + eprosima::fastrtps::Subscriber * sub, eprosima::fastrtps::rtps::MatchingInfo & info) final { - std::lock_guard lock(internalMutex_); - if (eprosima::fastrtps::rtps::MATCHED_MATCHING == info.status) { - publishers_.insert(info.remoteEndpointGuid); - } else if (eprosima::fastrtps::rtps::REMOVED_MATCHING == info.status) { - publishers_.erase(info.remoteEndpointGuid); + { + std::lock_guard lock(internalMutex_); + if (eprosima::fastrtps::rtps::MATCHED_MATCHING == info.status) { + publishers_.insert(info.remoteEndpointGuid); + } else if (eprosima::fastrtps::rtps::REMOVED_MATCHING == info.status) { + publishers_.erase(info.remoteEndpointGuid); + } } + data_taken(sub); } void onNewDataMessage(eprosima::fastrtps::Subscriber * sub) final { - // Make sure to call into Fast-RTPS before taking the lock to avoid an - // ABBA deadlock between internalMutex_ and mutexes inside of Fast-RTPS. -#if FASTRTPS_VERSION_MAJOR == 1 && FASTRTPS_VERSION_MINOR < 9 - uint64_t unread_count = sub->getUnreadCount(); -#else - uint64_t unread_count = sub->get_unread_count(); -#endif - - std::lock_guard lock(internalMutex_); - - // the change to liveliness_lost_count_ needs to be mutually exclusive with - // rmw_wait() which checks hasEvent() and decides if wait() needs to be called - ConditionalScopedLock clock(conditionMutex_, conditionVariable_); - - data_.store(unread_count, std::memory_order_relaxed); + data_taken(sub); } RMW_FASTRTPS_SHARED_CPP_PUBLIC @@ -153,7 +142,7 @@ class SubListener : public EventListenerInterface, public eprosima::fastrtps::Su #endif std::lock_guard lock(internalMutex_); - ConditionalScopedLock clock(conditionMutex_); + ConditionalScopedLock clock(conditionMutex_, conditionVariable_); data_.store(unread_count, std::memory_order_relaxed); }