From 25d232e4a417dd03ea2be69fb1ffad0d36f165f0 Mon Sep 17 00:00:00 2001 From: jdymitarai Date: Thu, 8 Oct 2026 10:42:28 +0900 Subject: [PATCH 1/2] fix(mutex): hold lock during notify_all in Barrier::wait to prevent teardown race (fixes #1672) --- src/mutex.h | 7 ++----- test/benchmark_gtest.cc | 46 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 5 deletions(-) diff --git a/src/mutex.h b/src/mutex.h index b6b4b71ab0..a1e6f2acf5 100644 --- a/src/mutex.h +++ b/src/mutex.h @@ -117,11 +117,8 @@ class Barrier { // Called by each thread bool wait() EXCLUDES(lock_) { - bool last_thread = false; - { - MutexLock ml(lock_); - last_thread = createBarrier(ml); - } + MutexLock ml(lock_); + bool last_thread = createBarrier(ml); if (last_thread) phase_condition_.notify_all(); return last_thread; } diff --git a/test/benchmark_gtest.cc b/test/benchmark_gtest.cc index bd6304ade2..bcc986cbad 100644 --- a/test/benchmark_gtest.cc +++ b/test/benchmark_gtest.cc @@ -1,8 +1,10 @@ #include #include +#include #include #include "../src/benchmark_register.h" +#include "../src/mutex.h" #include "benchmark/benchmark.h" #include "gmock/gmock.h" #include "gtest/gtest.h" @@ -250,6 +252,50 @@ TEST(TimingTest, PauseAndResumeInLoopReportSaneTime) { EXPECT_LT(runs[0].cpu_accumulated_time, 1.0); } +TEST(BarrierTest, SingleThreadedWaitAndTeardown) { + for (int i = 0; i < 100; ++i) { + auto barrier = std::make_unique(1); + EXPECT_TRUE(barrier->wait()); + } +} + +TEST(BarrierTest, MultiThreadedWaitAndTeardown) { + constexpr int kNumThreads = 8; + for (int i = 0; i < 100; ++i) { + auto barrier = std::make_unique(kNumThreads); + std::vector threads; + threads.reserve(kNumThreads); + for (int t = 0; t < kNumThreads; ++t) { + threads.emplace_back([&barrier]() { + barrier->wait(); + }); + } + for (auto& thread : threads) { + thread.join(); + } + } +} + +TEST(BarrierTest, MultiThreadedRemoveThreadAndTeardown) { + constexpr int kNumThreads = 4; + for (int i = 0; i < 100; ++i) { + auto barrier = std::make_unique(kNumThreads); + std::vector threads; + threads.reserve(kNumThreads); + for (int t = 0; t < kNumThreads - 1; ++t) { + threads.emplace_back([&barrier]() { + barrier->wait(); + }); + } + threads.emplace_back([&barrier]() { + barrier->removeThread(); + }); + for (auto& thread : threads) { + thread.join(); + } + } +} + } // namespace } // namespace internal } // namespace benchmark From 33141b3f0a27aa94522bcca756b021157d93afd5 Mon Sep 17 00:00:00 2001 From: jdymitarai Date: Fri, 9 Oct 2026 22:09:53 +0900 Subject: [PATCH 2/2] style: fix clang-format formatting in BarrierTest --- test/benchmark_gtest.cc | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/test/benchmark_gtest.cc b/test/benchmark_gtest.cc index bcc986cbad..d32ff4dd9e 100644 --- a/test/benchmark_gtest.cc +++ b/test/benchmark_gtest.cc @@ -266,9 +266,7 @@ TEST(BarrierTest, MultiThreadedWaitAndTeardown) { std::vector threads; threads.reserve(kNumThreads); for (int t = 0; t < kNumThreads; ++t) { - threads.emplace_back([&barrier]() { - barrier->wait(); - }); + threads.emplace_back([&barrier]() { barrier->wait(); }); } for (auto& thread : threads) { thread.join(); @@ -283,13 +281,9 @@ TEST(BarrierTest, MultiThreadedRemoveThreadAndTeardown) { std::vector threads; threads.reserve(kNumThreads); for (int t = 0; t < kNumThreads - 1; ++t) { - threads.emplace_back([&barrier]() { - barrier->wait(); - }); + threads.emplace_back([&barrier]() { barrier->wait(); }); } - threads.emplace_back([&barrier]() { - barrier->removeThread(); - }); + threads.emplace_back([&barrier]() { barrier->removeThread(); }); for (auto& thread : threads) { thread.join(); }