search

ros2 / rmw_fastrtps

Implementation of the ROS Middleware (rmw) Interface using eProsima's Fast RTPS.
Apache License 2.0
155 stars 117 forks source link

avoid using dds common public mutex directly #725

Closed iuhilnehc-ynos closed 11 months ago

iuhilnehc-ynos commented 11 months ago

to address one warning TSAN about lock-order-inversion mentioned at https://github.com/ros2/ros2/issues/1488#issuecomment-1752278901.

TSAN warning log

```shell WARNING: ThreadSanitizer: lock-order-inversion (potential deadlock) (pid=1926091) Cycle in lock order graph: M254870179752816080 (0x000000000000) => M277106926251347976 (0x000000000000) => M254870179752816080 Mutex M277106926251347976 acquired here while holding mutex M254870179752816080 in main thread: #0 pthread_mutex_lock ../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:4240 (libtsan.so.0+0x53908) #1 __gthread_mutex_lock /usr/include/x86_64-linux-gnu/c++/11/bits/gthr-default.h:749 (librmw_fastrtps_cpp.so+0x2669e) #2 std::mutex::lock() /usr/include/c++/11/bits/std_mutex.h:100 (librmw_fastrtps_cpp.so+0x26726) #3 std::lock_guard::lock_guard(std::mutex&) /usr/include/c++/11/bits/std_mutex.h:229 (librmw_fastrtps_cpp.so+0x283ec) #4 rmw_create_client /home/chenlh/Projects/ROS2/ros2-humble/src/ros2/rmw_fastrtps/rmw_fastrtps_cpp/src/rmw_client.cpp:440 (librmw_fastrtps_cpp.so+0x46b7a) #5 rmw_create_client /home/chenlh/Projects/ROS2/ros2-humble/src/ros2/rmw_implementation/rmw_implementation/src/functions.cpp:484 (librmw_implementation.so+0x7e28) #6 rcl_client_init /home/chenlh/Projects/ROS2/ros2-humble/src/ros2/rcl/rcl/src/rcl/client.c:107 (librcl.so+0x1909d) #7 rcl_action_client_init /home/chenlh/Projects/ROS2/ros2-humble/src/ros2/rcl/rcl_action/src/rcl_action/action_client.c:218 (librcl_action.so+0x6f19) #8 TestActionGraphMultiNodeFixture_action_client_init_maybe_fail_Test::TestBody() /home/chenlh/Projects/ROS2/ros2-humble/src/ros2/rcl/rcl_action/test/rcl_action/test_graph.cpp:626 (test_graph__rmw_fastrtps_cpp+0x86fe6) #9 void testing::internal::HandleSehExceptionsInMethodIfSupported(testing::Test*, void (testing::Test::*)(), char const*) /home/chenlh/Projects/ROS2/ros2-humble/install/gtest_vendor/src/gtest_vendor/./src/gtest.cc:2433 (test_graph__rmw_fastrtps_cpp+0xdbaa7) #10 void testing::internal::HandleExceptionsInMethodIfSupported(testing::Test*, void (testing::Test::*)(), char const*) /home/chenlh/Projects/ROS2/ros2-humble/install/gtest_vendor/src/gtest_vendor/./src/gtest.cc:2469 (test_graph__rmw_fastrtps_cpp+0xd06c0) #11 testing::Test::Run() /home/chenlh/Projects/ROS2/ros2-humble/install/gtest_vendor/src/gtest_vendor/./src/gtest.cc:2508 (test_graph__rmw_fastrtps_cpp+0xa07eb) #12 testing::TestInfo::Run() /home/chenlh/Projects/ROS2/ros2-humble/install/gtest_vendor/src/gtest_vendor/./src/gtest.cc:2684 (test_graph__rmw_fastrtps_cpp+0xa1500) #13 testing::TestSuite::Run() /home/chenlh/Projects/ROS2/ros2-humble/install/gtest_vendor/src/gtest_vendor/./src/gtest.cc:2816 (test_graph__rmw_fastrtps_cpp+0xa1fd2) #14 testing::internal::UnitTestImpl::RunAllTests() /home/chenlh/Projects/ROS2/ros2-humble/install/gtest_vendor/src/gtest_vendor/./src/gtest.cc:5338 (test_graph__rmw_fastrtps_cpp+0xb0868) #15 bool testing::internal::HandleSehExceptionsInMethodIfSupported(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) /home/chenlh/Projects/ROS2/ros2-humble/install/gtest_vendor/src/gtest_vendor/./src/gtest.cc:2433 (test_graph__rmw_fastrtps_cpp+0xdd935) #16 bool testing::internal::HandleExceptionsInMethodIfSupported(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) /home/chenlh/Projects/ROS2/ros2-humble/install/gtest_vendor/src/gtest_vendor/./src/gtest.cc:2469 (test_graph__rmw_fastrtps_cpp+0xd22fe) #17 testing::UnitTest::Run() /home/chenlh/Projects/ROS2/ros2-humble/install/gtest_vendor/src/gtest_vendor/./src/gtest.cc:4925 (test_graph__rmw_fastrtps_cpp+0xae93f) #18 RUN_ALL_TESTS() /home/chenlh/Projects/ROS2/ros2-humble/install/gtest_vendor/src/gtest_vendor/include/gtest/gtest.h:2473 (test_graph__rmw_fastrtps_cpp+0x95e1a) #19 main /home/chenlh/Projects/ROS2/ros2-humble/install/gtest_vendor/src/gtest_vendor/src/gtest_main.cc:45 (test_graph__rmw_fastrtps_cpp+0x95d0e) Hint: use TSAN_OPTIONS=second_deadlock_stack=1 to get more informative warning message Mutex M254870179752816080 acquired here while holding mutex M277106926251347976 in main thread: #0 pthread_mutex_lock ../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:4240 (libtsan.so.0+0x53908) #1 __gthread_mutex_lock /usr/include/x86_64-linux-gnu/c++/11/bits/gthr-default.h:749 (librcl_logging_spdlog.so+0x10f5d) #2 std::mutex::lock() /usr/include/c++/11/bits/std_mutex.h:100 (librcl_logging_spdlog.so+0x11038) #3 std::lock_guard::lock_guard(std::mutex&) /usr/include/c++/11/bits/std_mutex.h:229 (librcl_logging_spdlog.so+0x120d0) #4 rmw_fastrtps_shared_cpp::destroy_subscription(char const*, CustomParticipantInfo*, rmw_subscription_s*, bool) /home/chenlh/Projects/ROS2/ros2-humble/src/ros2/rmw_fastrtps/rmw_fastrtps_shared_cpp/src/subscription.cpp:44 (librmw_fastrtps_shared_cpp.so+0x155af0) #5 rmw_create_subscription /home/chenlh/Projects/ROS2/ros2-humble/src/ros2/rmw_fastrtps/rmw_fastrtps_cpp/src/rmw_subscription.cpp:104 (librmw_fastrtps_cpp.so+0x6627b) #6 rmw_create_subscription /home/chenlh/Projects/ROS2/ros2-humble/src/ros2/rmw_implementation/rmw_implementation/src/functions.cpp:391 (librmw_implementation.so+0x708b) #7 rcl_subscription_init /home/chenlh/Projects/ROS2/ros2-humble/src/ros2/rcl/rcl/src/rcl/subscription.c:101 (librcl.so+0x351bb) #8 rcl_action_client_init /home/chenlh/Projects/ROS2/ros2-humble/src/ros2/rcl/rcl_action/src/rcl_action/action_client.c:222 (librcl_action.so+0x7563) #9 TestActionGraphMultiNodeFixture_action_client_init_maybe_fail_Test::TestBody() /home/chenlh/Projects/ROS2/ros2-humble/src/ros2/rcl/rcl_action/test/rcl_action/test_graph.cpp:626 (test_graph__rmw_fastrtps_cpp+0x86fe6) #10 void testing::internal::HandleSehExceptionsInMethodIfSupported(testing::Test*, void (testing::Test::*)(), char const*) /home/chenlh/Projects/ROS2/ros2-humble/install/gtest_vendor/src/gtest_vendor/./src/gtest.cc:2433 (test_graph__rmw_fastrtps_cpp+0xdbaa7) #11 void testing::internal::HandleExceptionsInMethodIfSupported(testing::Test*, void (testing::Test::*)(), char const*) /home/chenlh/Projects/ROS2/ros2-humble/install/gtest_vendor/src/gtest_vendor/./src/gtest.cc:2469 (test_graph__rmw_fastrtps_cpp+0xd06c0) #12 testing::Test::Run() /home/chenlh/Projects/ROS2/ros2-humble/install/gtest_vendor/src/gtest_vendor/./src/gtest.cc:2508 (test_graph__rmw_fastrtps_cpp+0xa07eb) #13 testing::TestInfo::Run() /home/chenlh/Projects/ROS2/ros2-humble/install/gtest_vendor/src/gtest_vendor/./src/gtest.cc:2684 (test_graph__rmw_fastrtps_cpp+0xa1500) #14 testing::TestSuite::Run() /home/chenlh/Projects/ROS2/ros2-humble/install/gtest_vendor/src/gtest_vendor/./src/gtest.cc:2816 (test_graph__rmw_fastrtps_cpp+0xa1fd2) #15 testing::internal::UnitTestImpl::RunAllTests() /home/chenlh/Projects/ROS2/ros2-humble/install/gtest_vendor/src/gtest_vendor/./src/gtest.cc:5338 (test_graph__rmw_fastrtps_cpp+0xb0868) #16 bool testing::internal::HandleSehExceptionsInMethodIfSupported(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) /home/chenlh/Projects/ROS2/ros2-humble/install/gtest_vendor/src/gtest_vendor/./src/gtest.cc:2433 (test_graph__rmw_fastrtps_cpp+0xdd935) #17 bool testing::internal::HandleExceptionsInMethodIfSupported(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) /home/chenlh/Projects/ROS2/ros2-humble/install/gtest_vendor/src/gtest_vendor/./src/gtest.cc:2469 (test_graph__rmw_fastrtps_cpp+0xd22fe) #18 testing::UnitTest::Run() /home/chenlh/Projects/ROS2/ros2-humble/install/gtest_vendor/src/gtest_vendor/./src/gtest.cc:4925 (test_graph__rmw_fastrtps_cpp+0xae93f) #19 RUN_ALL_TESTS() /home/chenlh/Projects/ROS2/ros2-humble/install/gtest_vendor/src/gtest_vendor/include/gtest/gtest.h:2473 (test_graph__rmw_fastrtps_cpp+0x95e1a) #20 main /home/chenlh/Projects/ROS2/ros2-humble/install/gtest_vendor/src/gtest_vendor/src/gtest_main.cc:45 (test_graph__rmw_fastrtps_cpp+0x95d0e) SUMMARY: ThreadSanitizer: lock-order-inversion (potential deadlock) /usr/include/x86_64-linux-gnu/c++/11/bits/gthr-default.h:749 in __gthread_mutex_lock ```

iuhilnehc-ynos commented 11 months ago

Alternatively, we could consider moving the node_update_context locking into rmw_dds_common::Context, marking it private, and then removing the locking from here. That seems like a better idea anyway.

Thoughts?

Thanks. I think it's a good idea. I'll use this method even though it refactors more code. These repositories (rmw_dds_common, rmw_fastrtps, rmw_cyclonedds, rmw_connextdds) need to be updated.

iuhilnehc-ynos commented 11 months ago

It depends on https://github.com/ros2/rmw_dds_common/pull/73

iuhilnehc-ynos commented 11 months ago

repos: https://gist.githubusercontent.com/iuhilnehc-ynos/a6544c0ac62eba5ef9599facc17dff4b/raw/99fb79fca04ae24be0c9903fa5697cb955719e82/ros2.repos

CI: