diff --git a/CHANGELOG.md b/CHANGELOG.md index ff5ec2a77f..7c294d31b2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,10 @@ All notable changes to this project are documented in this file. ### Changed - 🤖 [ergoCubSN001] Add logging of the wrist and fix the name of the waist imu (https://github.com/ami-iit/bipedal-locomotion-framework/pull/810) + ### Fixed +- Fix the barrier logic for threads synchronization (https://github.com/ami-iit/bipedal-locomotion-framework/pull/811) + ### Removed ## [0.18.0] - 2024-01-23 diff --git a/src/System/include/BipedalLocomotion/System/AdvanceableRunner.h b/src/System/include/BipedalLocomotion/System/AdvanceableRunner.h index 02664a9458..dd768fa284 100644 --- a/src/System/include/BipedalLocomotion/System/AdvanceableRunner.h +++ b/src/System/include/BipedalLocomotion/System/AdvanceableRunner.h @@ -117,7 +117,7 @@ template class AdvanceableRunner * @return a thread of containing the running process. If the runner was not correctly * initialized the thread is invalid. */ - std::thread run(std::optional> barrier = {}); + std::thread run(std::shared_ptr barrier = nullptr); /** * Stop the thread @@ -227,8 +227,7 @@ bool AdvanceableRunner<_Advanceable>::setOutputResource( } template -std::thread -AdvanceableRunner<_Advanceable>::run(std::optional> barrier) +std::thread AdvanceableRunner<_Advanceable>::run(std::shared_ptr barrier) { constexpr auto logPrefix = "[AdvanceableRunner::run]"; @@ -273,8 +272,18 @@ AdvanceableRunner<_Advanceable>::run(std::optional bool { + auto function = [&](std::shared_ptr barrier) -> bool { constexpr auto logPrefix = "[AdvanceableRunner::run]"; + + // synchronize the threads + if (!(barrier == nullptr)) + { + log()->debug("{} - {} This thread is waiting for the other threads.", + logPrefix, + m_info.name); + barrier->wait(); + } + auto time = BipedalLocomotion::clock().now(); auto oldTime = time; auto wakeUpTime = time; @@ -363,16 +372,7 @@ AdvanceableRunner<_Advanceable>::run(std::optionalm_advanceable->close(); }; - // if the barrier is passed the run method, synchronization is performed - if (barrier.has_value()) - { - log()->debug("{} - {} This thread is waiting for the other threads.", - logPrefix, - m_info.name); - barrier.value().get().wait(); - } - - return std::thread(function); + return std::thread(function, barrier); } template void AdvanceableRunner<_Advanceable>::stop() diff --git a/src/System/include/BipedalLocomotion/System/Barrier.h b/src/System/include/BipedalLocomotion/System/Barrier.h index a2576e1970..2b73dc7615 100644 --- a/src/System/include/BipedalLocomotion/System/Barrier.h +++ b/src/System/include/BipedalLocomotion/System/Barrier.h @@ -24,10 +24,9 @@ class Barrier { public: /** - * Constructor. - * @param counter initial value of the expected counter + * Calls the constructor. It creates a new Barrier with the given counter. */ - explicit Barrier(const std::size_t counter); + static std::shared_ptr create(const std::size_t counter); /** * Blocks this thread at the phase synchronization point until its phase completion step is run @@ -40,6 +39,12 @@ class Barrier std::size_t m_initialCount; std::size_t m_count; std::size_t m_generation; + + /** + * Constructor. + * @param counter initial value of the expected counter + */ + explicit Barrier(const std::size_t counter); }; } // namespace System } // namespace BipedalLocomotion diff --git a/src/System/src/Barrier.cpp b/src/System/src/Barrier.cpp index b51901c240..661d71b5d7 100644 --- a/src/System/src/Barrier.cpp +++ b/src/System/src/Barrier.cpp @@ -5,9 +5,11 @@ * distributed under the terms of the BSD-3-Clause license. */ +#include #include #include +#include using namespace BipedalLocomotion::System; @@ -18,11 +20,18 @@ Barrier::Barrier(const std::size_t counter) { } +std::shared_ptr Barrier::create(const std::size_t counter) +{ + return std::shared_ptr(new Barrier(counter)); +} + void Barrier::wait() { + constexpr auto logPrefix = "[Barrier::wait]"; + std::unique_lock lock{m_mutex}; const auto tempGeneration = m_generation; - if ((--m_count) == 1) + if ((--m_count) == 0) { // all threads reached the barrier, so we can consider them synchronized m_generation++; @@ -31,6 +40,7 @@ void Barrier::wait() m_count = m_initialCount; // notify the other threads + log()->debug("{} All threads reached the barrier.", logPrefix); m_cond.notify_all(); } else { diff --git a/src/System/tests/AdvanceableRunnerTest.cpp b/src/System/tests/AdvanceableRunnerTest.cpp index 9ce4658934..6504cd6cd1 100644 --- a/src/System/tests/AdvanceableRunnerTest.cpp +++ b/src/System/tests/AdvanceableRunnerTest.cpp @@ -15,6 +15,7 @@ #include #include #include +#include using namespace BipedalLocomotion::System; using namespace BipedalLocomotion::ParametersHandler; @@ -120,7 +121,7 @@ TEST_CASE("Test Block") SECTION("With synchronization") { constexpr std::size_t numberOfRunners = 2; - Barrier barrier(numberOfRunners); + auto barrier = Barrier::create(numberOfRunners); // run the block auto thread0 = runner0.run(barrier);