From c632229a5835a0eb38a5a7eec4796fec7b6ba67b Mon Sep 17 00:00:00 2001 From: danipiza Date: Fri, 11 Sep 2026 11:01:00 +0200 Subject: [PATCH 1/6] [FIX] Added wait_for Signed-off-by: danipiza --- cpp_utils/include/cpp_utils/event/impl/SignalManager.ipp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/cpp_utils/include/cpp_utils/event/impl/SignalManager.ipp b/cpp_utils/include/cpp_utils/event/impl/SignalManager.ipp index 224e6442..7ed2892b 100644 --- a/cpp_utils/include/cpp_utils/event/impl/SignalManager.ipp +++ b/cpp_utils/include/cpp_utils/event/impl/SignalManager.ipp @@ -20,6 +20,7 @@ #pragma once #include +#include #include #include @@ -153,11 +154,16 @@ void SignalManager::signal_handler_routine_() noexcept template void SignalManager::signal_handler_thread_routine_() noexcept { + // Maximum time this thread blocks in signal_received_cv_ before re-checking signals_received_ , + // and thus maximum time a signal may take to be handled when its notification is missed. + constexpr std::chrono::milliseconds MAXIMUM_WAIT_TIME(100); + while (!signal_handler_thread_stop_.load()) { std::unique_lock lock(signal_received_cv_mutex_); - signal_received_cv_.wait( + signal_received_cv_.wait_for( lock, + MAXIMUM_WAIT_TIME, [this] { return signals_received_.load() > 0 || From 184577a800e9276c107afb80d3d3e299897505d9 Mon Sep 17 00:00:00 2001 From: danipiza Date: Fri, 11 Sep 2026 11:17:49 +0200 Subject: [PATCH 2/6] Temporal commit - Execute 20 times each test Signed-off-by: danipiza --- .github/workflows/reusable-workflow.yml | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/.github/workflows/reusable-workflow.yml b/.github/workflows/reusable-workflow.yml index 08b18b96..f65fad8e 100644 --- a/.github/workflows/reusable-workflow.yml +++ b/.github/workflows/reusable-workflow.yml @@ -75,14 +75,26 @@ jobs: dependencies_artifact_postfix: ${{ inputs.dependencies_artifact_postfix }} secret_token: ${{ secrets.GITHUB_TOKEN }} - - name: Compile and run tests - id: compile_and_test - uses: eProsima/eProsima-CI/multiplatform/colcon_build_test@v0 + # Build and test are kept as separate steps so that a compilation failure and a test + # failure are immediately distinguishable in the job summary and in the step timings. + - name: Compile + uses: eProsima/eProsima-CI/multiplatform/colcon_build@v0 with: - packages_names: ${{ env.code_packages_names }} + colcon_build_args: '--packages-up-to ${{ env.code_packages_names }}' cmake_args: -DBUILD_TESTS=ON -DCMAKE_BUILD_TYPE=${{ matrix.cmake_build_type }} workspace_dependencies: install - ctest_args: --label-exclude "xfail" + colcon_meta_file: src/.github/workflows/configurations/${{ runner.os }}/colcon.meta + + - name: Run tests + id: test + uses: eProsima/eProsima-CI/multiplatform/colcon_test@v0 + with: + packages_names: ${{ env.code_packages_names }} + workspace_dependencies: install + # TEMPORARY: '--repeat until-fail:20' runs every test up to 20 times and stops at the + # first failure, to check that the SignalManager lost-wakeup fix removed the flakiness + # of SignalEventHandlerTest. Drop this flag before merging, it multiplies the CI time. + ctest_args: --label-exclude "xfail" --repeat until-fail:20 colcon_meta_file: src/.github/workflows/configurations/${{ runner.os }}/colcon.meta test_report_artifact: test_report${{ inputs.dependencies_artifact_postfix }}_${{ inputs.custom_version_build }}_${{ inputs.os }}_${{ matrix.cmake_build_type }} @@ -92,7 +104,7 @@ jobs: if: success() || failure() with: name: "Report: ${{ inputs.os }} | ${{ matrix.cmake_build_type }} " - path: "${{ steps.compile_and_test.outputs.ctest_results_path }}*.xml" + path: "${{ steps.test.outputs.ctest_results_path }}*.xml" working-directory: 'src' path-replace-backslashes: 'true' list-tests: 'failed' From db48068f2952f5fe7a04c51aa19cbf1bd2904d55 Mon Sep 17 00:00:00 2001 From: danipiza Date: Fri, 11 Sep 2026 11:18:14 +0200 Subject: [PATCH 3/6] [STYLE] Applied Uncrustify Signed-off-by: danipiza --- .../cpp_utils/event/impl/SignalManager.ipp | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/cpp_utils/include/cpp_utils/event/impl/SignalManager.ipp b/cpp_utils/include/cpp_utils/event/impl/SignalManager.ipp index 7ed2892b..d505fe6c 100644 --- a/cpp_utils/include/cpp_utils/event/impl/SignalManager.ipp +++ b/cpp_utils/include/cpp_utils/event/impl/SignalManager.ipp @@ -30,16 +30,16 @@ namespace eprosima { namespace utils { namespace event { -template +template std::recursive_mutex SignalManager::instance_mutex_; -template +template std::condition_variable SignalManager::signal_received_cv_; -template +template std::atomic SignalManager::signals_received_(0); -template +template SignalManager& SignalManager::get_instance() noexcept { std::lock_guard lock(instance_mutex_); @@ -48,7 +48,7 @@ SignalManager& SignalManager::get_instance() noexcept return instance_; } -template +template SignalManager::SignalManager() noexcept : signal_handler_thread_stop_(false) , current_last_id_(0) @@ -62,7 +62,7 @@ SignalManager::SignalManager() noexcept &SignalManager::signal_handler_thread_routine_, this); } -template +template SignalManager::~SignalManager() noexcept { { @@ -77,7 +77,7 @@ SignalManager::~SignalManager() noexcept "Destroying SignalManager in signal: " << SigVal << "."); } -template +template UniqueCallbackId SignalManager::register_callback( std::function callback) noexcept { @@ -92,7 +92,7 @@ UniqueCallbackId SignalManager::register_callback( return new_id; } -template +template void SignalManager::unregister_callback( UniqueCallbackId id) { @@ -107,7 +107,7 @@ void SignalManager::unregister_callback( "Erase callback from signal " << SigVal << "."); } -template +template UniqueCallbackId SignalManager::new_unique_id_() noexcept { std::lock_guard lock(last_id_mutex_); @@ -115,7 +115,7 @@ UniqueCallbackId SignalManager::new_unique_id_() noexcept return current_last_id_; } -template +template void SignalManager::signal_handler_function_( int sigval) noexcept { @@ -126,7 +126,7 @@ void SignalManager::signal_handler_function_( signal_received_(); } -template +template void SignalManager::signal_received_() noexcept { // Normally \c signals_received_ should be guarded by \c signal_received_cv_mutex_ in order to prevent @@ -137,7 +137,7 @@ void SignalManager::signal_received_() noexcept signal_received_cv_.notify_one(); } -template +template void SignalManager::signal_handler_routine_() noexcept { std::lock_guard lock(active_callbacks_mutex_); @@ -151,7 +151,7 @@ void SignalManager::signal_handler_routine_() noexcept } } -template +template void SignalManager::signal_handler_thread_routine_() noexcept { // Maximum time this thread blocks in signal_received_cv_ before re-checking signals_received_ , @@ -167,7 +167,7 @@ void SignalManager::signal_handler_thread_routine_() noexcept [this] { return signals_received_.load() > 0 || - signal_handler_thread_stop_.load(); + signal_handler_thread_stop_.load(); }); if (signal_handler_thread_stop_.load()) From 689ad1affd85be2ffb444507483bee0edab306ea Mon Sep 17 00:00:00 2001 From: danipiza Date: Fri, 11 Sep 2026 11:30:53 +0200 Subject: [PATCH 4/6] Temporal commit - Execute 20 times each test TSAN & ASAN Signed-off-by: danipiza --- .github/workflows/reusable-ubuntu-ci.yml | 51 ++++++++++++++++++------ 1 file changed, 39 insertions(+), 12 deletions(-) diff --git a/.github/workflows/reusable-ubuntu-ci.yml b/.github/workflows/reusable-ubuntu-ci.yml index bbcaa90a..ac3548b4 100644 --- a/.github/workflows/reusable-ubuntu-ci.yml +++ b/.github/workflows/reusable-ubuntu-ci.yml @@ -114,15 +114,26 @@ jobs: dependencies_artifact_postfix: ${{ inputs.dependencies_artifact_postfix }} secret_token: ${{ secrets.GITHUB_TOKEN }} - - name: Compile and run tests - id: compile_and_test - uses: eProsima/eProsima-CI/multiplatform/colcon_build_test@main + # Build and test are kept as separate steps so that a compilation failure and a test + # failure are immediately distinguishable in the job summary and in the step timings. + - name: Compile + uses: eProsima/eProsima-CI/multiplatform/colcon_build@main with: - packages_names: ${{ env.cpp_packages_names }} + colcon_build_args: '--packages-up-to ${{ env.cpp_packages_names }}' workspace_dependencies: install cmake_build_type: Debug cmake_args: -DBUILD_TESTS=ON -DASAN_BUILD=ON - ctest_args: --label-exclude "xfail|xtsan" + + - name: Run tests + id: test + uses: eProsima/eProsima-CI/multiplatform/colcon_test@main + with: + packages_names: ${{ env.cpp_packages_names }} + workspace_dependencies: install + # TEMPORARY: '--repeat until-fail:20' runs every test up to 20 times and stops at the + # first failure, to check that the SignalManager lost-wakeup fix removed the flakiness + # of SignalEventHandlerTest. Drop this flag before merging, it multiplies the CI time. + ctest_args: --label-exclude "xfail|xtsan" --repeat until-fail:20 test_report_artifact: test_report_asan${{ inputs.dependencies_artifact_postfix }}_${{ inputs.custom_version_build }} @@ -131,7 +142,7 @@ jobs: if: success() || failure() with: name: "Report: ASAN " - path: "${{ steps.compile_and_test.outputs.ctest_results_path }}*.xml" + path: "${{ steps.test.outputs.ctest_results_path }}*.xml" working-directory: 'src' list-tests: 'failed' list-suites: 'failed' @@ -158,20 +169,36 @@ jobs: dependencies_artifact_postfix: ${{ inputs.dependencies_artifact_postfix }}_tsan secret_token: ${{ secrets.GITHUB_TOKEN }} - - name: Compile and run tests - id: compile_and_test - uses: eProsima/eProsima-CI/multiplatform/colcon_build_test@v0 + # Build and test are kept as separate steps so that a compilation failure and a test + # failure are immediately distinguishable in the job summary and in the step timings. + - name: Compile + uses: eProsima/eProsima-CI/multiplatform/colcon_build@v0 env: # GCC 11.3 (Ubuntu Jammy default) produces several false positives regarding timed synchronization protocols # These issues were fixed in GCC 12 so we upgrade to that version. CC: gcc-12 CXX: g++-12 with: - packages_names: ${{ env.cpp_packages_names }} + colcon_build_args: '--packages-up-to ${{ env.cpp_packages_names }}' workspace_dependencies: install cmake_build_type: Debug cmake_args: -DBUILD_TESTS=ON -DTSAN_BUILD=ON - ctest_args: --label-exclude "xfail|xasan" + + - name: Run tests + id: test + uses: eProsima/eProsima-CI/multiplatform/colcon_test@v0 + env: + # GCC 11.3 (Ubuntu Jammy default) produces several false positives regarding timed synchronization protocols + # These issues were fixed in GCC 12 so we upgrade to that version. + CC: gcc-12 + CXX: g++-12 + with: + packages_names: ${{ env.cpp_packages_names }} + workspace_dependencies: install + # TEMPORARY: '--repeat until-fail:20' runs every test up to 20 times and stops at the + # first failure, to check that the SignalManager lost-wakeup fix removed the flakiness + # of SignalEventHandlerTest. Drop this flag before merging, it multiplies the CI time. + ctest_args: --label-exclude "xfail|xasan" --repeat until-fail:20 test_report_artifact: test_report_tsan${{ inputs.dependencies_artifact_postfix }}_${{ inputs.custom_version_build }} - name: Test Report @@ -179,7 +206,7 @@ jobs: if: success() || failure() with: name: "Report: TSAN " - path: "${{ steps.compile_and_test.outputs.ctest_results_path }}*.xml" + path: "${{ steps.test.outputs.ctest_results_path }}*.xml" working-directory: 'src' list-tests: 'failed' list-suites: 'failed' From 3783af89572478233a420ecdb8906c2f21e8b6c8 Mon Sep 17 00:00:00 2001 From: danipiza Date: Fri, 11 Sep 2026 11:44:12 +0200 Subject: [PATCH 5/6] Temporal commit - Removed Fix Signed-off-by: danipiza --- cpp_utils/include/cpp_utils/event/impl/SignalManager.ipp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/cpp_utils/include/cpp_utils/event/impl/SignalManager.ipp b/cpp_utils/include/cpp_utils/event/impl/SignalManager.ipp index d505fe6c..2e4ef991 100644 --- a/cpp_utils/include/cpp_utils/event/impl/SignalManager.ipp +++ b/cpp_utils/include/cpp_utils/event/impl/SignalManager.ipp @@ -20,7 +20,7 @@ #pragma once #include -#include +//#include #include #include @@ -156,14 +156,13 @@ void SignalManager::signal_handler_thread_routine_() noexcept { // Maximum time this thread blocks in signal_received_cv_ before re-checking signals_received_ , // and thus maximum time a signal may take to be handled when its notification is missed. - constexpr std::chrono::milliseconds MAXIMUM_WAIT_TIME(100); + //constexpr std::chrono::milliseconds MAXIMUM_WAIT_TIME(100); while (!signal_handler_thread_stop_.load()) { std::unique_lock lock(signal_received_cv_mutex_); - signal_received_cv_.wait_for( + signal_received_cv_.wait( lock, - MAXIMUM_WAIT_TIME, [this] { return signals_received_.load() > 0 || From 797fbd90d07638c3a60c58337c154797dd55d4f9 Mon Sep 17 00:00:00 2001 From: danipiza Date: Fri, 11 Sep 2026 12:03:04 +0200 Subject: [PATCH 6/6] Reverted Temporal commits Signed-off-by: danipiza --- .github/workflows/reusable-ubuntu-ci.yml | 51 +++++-------------- .github/workflows/reusable-workflow.yml | 24 +++------ .../cpp_utils/event/impl/SignalManager.ipp | 7 +-- 3 files changed, 22 insertions(+), 60 deletions(-) diff --git a/.github/workflows/reusable-ubuntu-ci.yml b/.github/workflows/reusable-ubuntu-ci.yml index ac3548b4..bbcaa90a 100644 --- a/.github/workflows/reusable-ubuntu-ci.yml +++ b/.github/workflows/reusable-ubuntu-ci.yml @@ -114,26 +114,15 @@ jobs: dependencies_artifact_postfix: ${{ inputs.dependencies_artifact_postfix }} secret_token: ${{ secrets.GITHUB_TOKEN }} - # Build and test are kept as separate steps so that a compilation failure and a test - # failure are immediately distinguishable in the job summary and in the step timings. - - name: Compile - uses: eProsima/eProsima-CI/multiplatform/colcon_build@main + - name: Compile and run tests + id: compile_and_test + uses: eProsima/eProsima-CI/multiplatform/colcon_build_test@main with: - colcon_build_args: '--packages-up-to ${{ env.cpp_packages_names }}' + packages_names: ${{ env.cpp_packages_names }} workspace_dependencies: install cmake_build_type: Debug cmake_args: -DBUILD_TESTS=ON -DASAN_BUILD=ON - - - name: Run tests - id: test - uses: eProsima/eProsima-CI/multiplatform/colcon_test@main - with: - packages_names: ${{ env.cpp_packages_names }} - workspace_dependencies: install - # TEMPORARY: '--repeat until-fail:20' runs every test up to 20 times and stops at the - # first failure, to check that the SignalManager lost-wakeup fix removed the flakiness - # of SignalEventHandlerTest. Drop this flag before merging, it multiplies the CI time. - ctest_args: --label-exclude "xfail|xtsan" --repeat until-fail:20 + ctest_args: --label-exclude "xfail|xtsan" test_report_artifact: test_report_asan${{ inputs.dependencies_artifact_postfix }}_${{ inputs.custom_version_build }} @@ -142,7 +131,7 @@ jobs: if: success() || failure() with: name: "Report: ASAN " - path: "${{ steps.test.outputs.ctest_results_path }}*.xml" + path: "${{ steps.compile_and_test.outputs.ctest_results_path }}*.xml" working-directory: 'src' list-tests: 'failed' list-suites: 'failed' @@ -169,36 +158,20 @@ jobs: dependencies_artifact_postfix: ${{ inputs.dependencies_artifact_postfix }}_tsan secret_token: ${{ secrets.GITHUB_TOKEN }} - # Build and test are kept as separate steps so that a compilation failure and a test - # failure are immediately distinguishable in the job summary and in the step timings. - - name: Compile - uses: eProsima/eProsima-CI/multiplatform/colcon_build@v0 + - name: Compile and run tests + id: compile_and_test + uses: eProsima/eProsima-CI/multiplatform/colcon_build_test@v0 env: # GCC 11.3 (Ubuntu Jammy default) produces several false positives regarding timed synchronization protocols # These issues were fixed in GCC 12 so we upgrade to that version. CC: gcc-12 CXX: g++-12 with: - colcon_build_args: '--packages-up-to ${{ env.cpp_packages_names }}' + packages_names: ${{ env.cpp_packages_names }} workspace_dependencies: install cmake_build_type: Debug cmake_args: -DBUILD_TESTS=ON -DTSAN_BUILD=ON - - - name: Run tests - id: test - uses: eProsima/eProsima-CI/multiplatform/colcon_test@v0 - env: - # GCC 11.3 (Ubuntu Jammy default) produces several false positives regarding timed synchronization protocols - # These issues were fixed in GCC 12 so we upgrade to that version. - CC: gcc-12 - CXX: g++-12 - with: - packages_names: ${{ env.cpp_packages_names }} - workspace_dependencies: install - # TEMPORARY: '--repeat until-fail:20' runs every test up to 20 times and stops at the - # first failure, to check that the SignalManager lost-wakeup fix removed the flakiness - # of SignalEventHandlerTest. Drop this flag before merging, it multiplies the CI time. - ctest_args: --label-exclude "xfail|xasan" --repeat until-fail:20 + ctest_args: --label-exclude "xfail|xasan" test_report_artifact: test_report_tsan${{ inputs.dependencies_artifact_postfix }}_${{ inputs.custom_version_build }} - name: Test Report @@ -206,7 +179,7 @@ jobs: if: success() || failure() with: name: "Report: TSAN " - path: "${{ steps.test.outputs.ctest_results_path }}*.xml" + path: "${{ steps.compile_and_test.outputs.ctest_results_path }}*.xml" working-directory: 'src' list-tests: 'failed' list-suites: 'failed' diff --git a/.github/workflows/reusable-workflow.yml b/.github/workflows/reusable-workflow.yml index f65fad8e..08b18b96 100644 --- a/.github/workflows/reusable-workflow.yml +++ b/.github/workflows/reusable-workflow.yml @@ -75,26 +75,14 @@ jobs: dependencies_artifact_postfix: ${{ inputs.dependencies_artifact_postfix }} secret_token: ${{ secrets.GITHUB_TOKEN }} - # Build and test are kept as separate steps so that a compilation failure and a test - # failure are immediately distinguishable in the job summary and in the step timings. - - name: Compile - uses: eProsima/eProsima-CI/multiplatform/colcon_build@v0 - with: - colcon_build_args: '--packages-up-to ${{ env.code_packages_names }}' - cmake_args: -DBUILD_TESTS=ON -DCMAKE_BUILD_TYPE=${{ matrix.cmake_build_type }} - workspace_dependencies: install - colcon_meta_file: src/.github/workflows/configurations/${{ runner.os }}/colcon.meta - - - name: Run tests - id: test - uses: eProsima/eProsima-CI/multiplatform/colcon_test@v0 + - name: Compile and run tests + id: compile_and_test + uses: eProsima/eProsima-CI/multiplatform/colcon_build_test@v0 with: packages_names: ${{ env.code_packages_names }} + cmake_args: -DBUILD_TESTS=ON -DCMAKE_BUILD_TYPE=${{ matrix.cmake_build_type }} workspace_dependencies: install - # TEMPORARY: '--repeat until-fail:20' runs every test up to 20 times and stops at the - # first failure, to check that the SignalManager lost-wakeup fix removed the flakiness - # of SignalEventHandlerTest. Drop this flag before merging, it multiplies the CI time. - ctest_args: --label-exclude "xfail" --repeat until-fail:20 + ctest_args: --label-exclude "xfail" colcon_meta_file: src/.github/workflows/configurations/${{ runner.os }}/colcon.meta test_report_artifact: test_report${{ inputs.dependencies_artifact_postfix }}_${{ inputs.custom_version_build }}_${{ inputs.os }}_${{ matrix.cmake_build_type }} @@ -104,7 +92,7 @@ jobs: if: success() || failure() with: name: "Report: ${{ inputs.os }} | ${{ matrix.cmake_build_type }} " - path: "${{ steps.test.outputs.ctest_results_path }}*.xml" + path: "${{ steps.compile_and_test.outputs.ctest_results_path }}*.xml" working-directory: 'src' path-replace-backslashes: 'true' list-tests: 'failed' diff --git a/cpp_utils/include/cpp_utils/event/impl/SignalManager.ipp b/cpp_utils/include/cpp_utils/event/impl/SignalManager.ipp index 2e4ef991..d505fe6c 100644 --- a/cpp_utils/include/cpp_utils/event/impl/SignalManager.ipp +++ b/cpp_utils/include/cpp_utils/event/impl/SignalManager.ipp @@ -20,7 +20,7 @@ #pragma once #include -//#include +#include #include #include @@ -156,13 +156,14 @@ void SignalManager::signal_handler_thread_routine_() noexcept { // Maximum time this thread blocks in signal_received_cv_ before re-checking signals_received_ , // and thus maximum time a signal may take to be handled when its notification is missed. - //constexpr std::chrono::milliseconds MAXIMUM_WAIT_TIME(100); + constexpr std::chrono::milliseconds MAXIMUM_WAIT_TIME(100); while (!signal_handler_thread_stop_.load()) { std::unique_lock lock(signal_received_cv_mutex_); - signal_received_cv_.wait( + signal_received_cv_.wait_for( lock, + MAXIMUM_WAIT_TIME, [this] { return signals_received_.load() > 0 ||