diff --git a/.github/workflows/cmake_ubuntu_sanitizers.yml b/.github/workflows/cmake_ubuntu_sanitizers.yml index 337bc79bb..15f931166 100644 --- a/.github/workflows/cmake_ubuntu_sanitizers.yml +++ b/.github/workflows/cmake_ubuntu_sanitizers.yml @@ -63,7 +63,7 @@ jobs: GTEST_COLOR: "On" ASAN_OPTIONS: "color=always" UBSAN_OPTIONS: "halt_on_error=1:print_stacktrace=1:color=always" - TSAN_OPTIONS: "color=always" + TSAN_OPTIONS: "suppressions=../../../tests/tsan_suppressions.txt:color=always" # There is a known issue with TSAN on recent kernel versions. Without the vm.mmap_rnd_bits=28 # workaround all binaries with TSan enabled crash with "FATAL: ThreadSanitizer: unexpected memory mapping" run: sudo sysctl vm.mmap_rnd_bits=28 && ctest --test-dir build/${{env.BUILD_TYPE}} --output-on-failure diff --git a/src/loggers/groot2_publisher.cpp b/src/loggers/groot2_publisher.cpp index 1a9240191..22219b732 100644 --- a/src/loggers/groot2_publisher.cpp +++ b/src/loggers/groot2_publisher.cpp @@ -81,7 +81,7 @@ struct Groot2Publisher::PImpl std::string tree_xml; - std::atomic_bool active_server = false; + std::atomic_bool active_server = true; std::thread server_thread; std::mutex status_mutex; @@ -98,7 +98,8 @@ struct Groot2Publisher::PImpl std::unordered_map pre_hooks; std::unordered_map post_hooks; - std::chrono::system_clock::time_point last_heartbeat; + std::mutex last_heartbeat_mutex; + std::chrono::steady_clock::time_point last_heartbeat; std::chrono::milliseconds max_heartbeat_delay = std::chrono::milliseconds(5000); std::atomic_bool recording = false; @@ -241,7 +242,6 @@ void Groot2Publisher::serverLoop() { auto const serialized_uuid = CreateRandomUUID(); - _p->active_server = true; auto& socket = _p->server; auto sendErrorReply = [&socket](const std::string& msg) { @@ -252,7 +252,10 @@ void Groot2Publisher::serverLoop() }; // initialize _p->last_heartbeat - _p->last_heartbeat = std::chrono::system_clock::now(); + { + const std::unique_lock lk(_p->last_heartbeat_mutex); + _p->last_heartbeat = std::chrono::steady_clock::now(); + } while(_p->active_server) { @@ -261,8 +264,12 @@ void Groot2Publisher::serverLoop() { continue; } + // this heartbeat will help establishing if Groot is connected or not - _p->last_heartbeat = std::chrono::system_clock::now(); + { + const std::unique_lock lk(_p->last_heartbeat_mutex); + _p->last_heartbeat = std::chrono::steady_clock::now(); + } std::string const request_str = requestMsg[0].to_string(); if(request_str.size() != Monitor::RequestHeader::size()) @@ -512,10 +519,13 @@ void Groot2Publisher::heartbeatLoop() { std::this_thread::sleep_for(std::chrono::milliseconds(10)); - auto now = std::chrono::system_clock::now(); + auto now = std::chrono::steady_clock::now(); const bool prev_heartbeat = has_heartbeat; - has_heartbeat = (now - _p->last_heartbeat < _p->max_heartbeat_delay); + { + const std::unique_lock lk(_p->last_heartbeat_mutex); + has_heartbeat = (now - _p->last_heartbeat < _p->max_heartbeat_delay); + } // if we loose or gain heartbeat, disable/enable all breakpoints if(has_heartbeat != prev_heartbeat) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 6367ef4c3..f598badcf 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -12,6 +12,7 @@ set(BT_TESTS gtest_enums.cpp gtest_factory.cpp gtest_fallback.cpp + gtest_groot2_publisher.cpp gtest_parallel.cpp gtest_preconditions.cpp gtest_ports.cpp diff --git a/tests/gtest_groot2_publisher.cpp b/tests/gtest_groot2_publisher.cpp new file mode 100644 index 000000000..41a3d1432 --- /dev/null +++ b/tests/gtest_groot2_publisher.cpp @@ -0,0 +1,48 @@ +#include +#include +#include + +#include +#include +#include + +using namespace std::chrono_literals; + +namespace +{ +static const char* xml_text = R"( + + + + + +)"; + +void throwRuntimeError() +{ + BT::BehaviorTreeFactory factory; + factory.registerSimpleAction("ThrowRuntimeError", [](BT::TreeNode&) -> BT::NodeStatus { + throw BT::RuntimeError("Oops!"); + }); + + auto tree = factory.createTreeFromText(xml_text); + BT::Groot2Publisher publisher(tree); + EXPECT_THROW(tree.tickExactlyOnce(), BT::RuntimeError); +} +} // namespace + +TEST(Groot2PublisherTest, EnsureNoInfiniteLoopOnThrow) +{ + EXPECT_EXIT( + { + auto fut = std::async(std::launch::async, throwRuntimeError); + auto status = fut.wait_for(1s); // shouldn't run for more than 1 second + if(status != std::future_status::ready) + { + std::cerr << "Test took too long. Possible infinite loop.\n"; + std::exit(EXIT_FAILURE); + } + std::exit(EXIT_SUCCESS); + }, + ::testing::ExitedWithCode(EXIT_SUCCESS), ".*"); +} diff --git a/tests/tsan_suppressions.txt b/tests/tsan_suppressions.txt new file mode 100644 index 000000000..e045f985d --- /dev/null +++ b/tests/tsan_suppressions.txt @@ -0,0 +1,4 @@ +# ThreadSanitizer suppressions file for behaviortree_cpp_test + +# ZeroMQ false positives +race:zmq::epoll_t::add_fd