fix: report threads as 1 in State during single-threaded manager passes (fixes #1849)

src/benchmark_api_internal.cc

@@ -93,25 +93,28 @@ State BenchmarkInstance::Run(
     IterationCount iters, int thread_id, internal::ThreadTimer* timer,
     internal::ThreadManager* manager,
     internal::PerfCountersMeasurement* perf_counters_measurement,
-    ProfilerManager* profiler_manager) const {
-  State st(name_.function_name, iters, args_, thread_id, threads_, timer,
+    ProfilerManager* profiler_manager, int thread_count) const {
+  const int n_threads = thread_count > 0 ? thread_count : threads_;
+  State st(name_.function_name, iters, args_, thread_id, n_threads, timer,
            manager, perf_counters_measurement, profiler_manager);
   benchmark_.Run(st);
   return st;
 }
 
-void BenchmarkInstance::Setup() const {
+void BenchmarkInstance::Setup(int thread_count) const {
   if (setup_ != nullptr) {
-    State st(name_.function_name, /*iters*/ 1, args_, /*thread_id*/ 0, threads_,
-             nullptr, nullptr, nullptr, nullptr);
+    const int n_threads = thread_count > 0 ? thread_count : threads_;
+    State st(name_.function_name, /*iters*/ 1, args_, /*thread_id*/ 0,
+             n_threads, nullptr, nullptr, nullptr, nullptr);
     setup_(st);
   }
 }
 
-void BenchmarkInstance::Teardown() const {
+void BenchmarkInstance::Teardown(int thread_count) const {
   if (teardown_ != nullptr) {
-    State st(name_.function_name, /*iters*/ 1, args_, /*thread_id*/ 0, threads_,
-             nullptr, nullptr, nullptr, nullptr);
+    const int n_threads = thread_count > 0 ? thread_count : threads_;
+    State st(name_.function_name, /*iters*/ 1, args_, /*thread_id*/ 0,
+             n_threads, nullptr, nullptr, nullptr, nullptr);
     teardown_(st);
   }
 }

src/benchmark_api_internal.h

@@ -41,16 +41,16 @@ class BenchmarkInstance {
   double min_warmup_time() const { return min_warmup_time_; }
   IterationCount iterations() const { return iterations_; }
   int threads() const { return threads_; }
-  void Setup() const;
-  void Teardown() const;
+  void Setup(int thread_count = -1) const;
+  void Teardown(int thread_count = -1) const;
   const auto& GetUserThreadRunnerFactory() const {
     return benchmark_.threadrunner_;
   }
 
   State Run(IterationCount iters, int thread_id, internal::ThreadTimer* timer,
             internal::ThreadManager* manager,
             internal::PerfCountersMeasurement* perf_counters_measurement,
-            ProfilerManager* profiler_manager) const;
+            ProfilerManager* profiler_manager, int thread_count = -1) const;
 
  private:
   BenchmarkName name_;

src/benchmark_runner.cc

@@ -144,14 +144,14 @@ BenchmarkReporter::Run CreateRunReport(
 void RunInThread(const BenchmarkInstance* b, IterationCount iters,
                  int thread_id, ThreadManager* manager,
                  PerfCountersMeasurement* perf_counters_measurement,
-                 ProfilerManager* profiler_manager_) {
+                 ProfilerManager* profiler_manager_, int thread_count = -1) {
   internal::ThreadTimer timer(
       b->measure_process_cpu_time()
           ? internal::ThreadTimer::CreateProcessCpuTime()
           : internal::ThreadTimer::Create());
 
   State st = b->Run(iters, thread_id, &timer, manager,
-                    perf_counters_measurement, profiler_manager_);
+                    perf_counters_measurement, profiler_manager_, thread_count);
   if (!(st.skipped() || st.iterations() >= st.max_iterations)) {
     st.SkipWithError(
         "The benchmark didn't run, nor was it explicitly skipped. Please call "
@@ -464,28 +464,30 @@ MemoryManager::Result BenchmarkRunner::RunMemoryManager(
     IterationCount memory_iterations) {
   std::unique_ptr<internal::ThreadManager> manager;
   manager.reset(new internal::ThreadManager(1));
-  b.Setup();
+  b.Setup(/*thread_count=*/1);
   memory_manager->Start();
   RunInThread(&b, memory_iterations, 0, manager.get(),
               perf_counters_measurement_ptr,
-              /*profiler_manager=*/nullptr);
+              /*profiler_manager=*/nullptr,
+              /*thread_count=*/1);
   MemoryManager::Result memory_result;
   memory_manager->Stop(memory_result);
   manager.reset();
-  b.Teardown();
+  b.Teardown(/*thread_count=*/1);
   memory_result.memory_iterations = memory_iterations;
   return memory_result;
 }
 
 void BenchmarkRunner::RunProfilerManager(IterationCount profile_iterations) {
   std::unique_ptr<internal::ThreadManager> manager;
   manager.reset(new internal::ThreadManager(1));
-  b.Setup();
+  b.Setup(/*thread_count=*/1);
   RunInThread(&b, profile_iterations, 0, manager.get(),
               /*perf_counters_measurement_ptr=*/nullptr,
-              /*profiler_manager=*/profiler_manager);
+              /*profiler_manager=*/profiler_manager,
+              /*thread_count=*/1);
   manager.reset();
-  b.Teardown();
+  b.Teardown(/*thread_count=*/1);
 }
 
 void BenchmarkRunner::DoOneRepetition() {

test/memory_manager_ordering_gtest.cc

@@ -1,6 +1,6 @@
-// Setup()/Teardown() must run outside the MemoryManager Start()/Stop()
-// window, as they already do outside the timed region. See #2149.
-
+#include <chrono>
+#include <condition_variable>
+#include <mutex>
 #include <vector>
 
 #include "benchmark/benchmark.h"
@@ -48,6 +48,58 @@ void BM_ordering(State& state) {
 }
 BENCHMARK(BM_ordering)->Iterations(1)->Setup(DoSetup)->Teardown(DoTeardown);
 
+// Regression test for #1849: multithreaded benchmarks with memory manager
+// run single-threaded during the memory manager pass, and state.threads() must
+// report 1 (not the benchmark's configured thread count) to prevent hangs on
+// barrier synchronization.
+bool memory_pass_threads_is_one = false;
+bool benchmark_pass_threads_is_four = false;
+
+class MultithreadedOrderingFixture : public Fixture {
+ public:
+  void SetUp(const State& state) override { Sync(state.threads()); }
+
+  void TearDown(const State& state) override { Sync(state.threads()); }
+
+ private:
+  void Sync(int thread_count) {
+    std::unique_lock<std::mutex> lock(mutex_);
+    int gen = generation_;
+    if (++arrived_ == thread_count) {
+      arrived_ = 0;
+      ++generation_;
+      cv_.notify_all();
+    } else {
+      ASSERT_TRUE(cv_.wait_for(lock, std::chrono::seconds(5), [&] {
+        return generation_ != gen;
+      })) << "Timed out waiting for all threads; state.threads() was wrong";
+    }
+  }
+
+  std::mutex mutex_;
+  std::condition_variable cv_;
+  int arrived_ = 0;
+  int generation_ = 0;
+};
+
+BENCHMARK_DEFINE_F(MultithreadedOrderingFixture, BM_MultithreadedSync)
+(State& state) {
+  if (in_measurement_window) {
+    if (state.threads() == 1) {
+      memory_pass_threads_is_one = true;
+    }
+  } else {
+    if (state.threads() == 4) {
+      benchmark_pass_threads_is_four = true;
+    }
+  }
+  for (auto _ : state) {
+  }
+}
+BENCHMARK_REGISTER_F(MultithreadedOrderingFixture, BM_MultithreadedSync)
+    ->Iterations(1)
+    ->Threads(4);
+
 // Swallows reporter output.
 class NullReporter : public BenchmarkReporter {
  public:
@@ -68,6 +120,10 @@ TEST(MemoryManagerOrdering, SetupTeardownRunOutsideMeasurementWindow) {
   EXPECT_TRUE(memory_manager_ran) << "MemoryManager measurement pass never ran";
   EXPECT_TRUE(setup_ran) << "Setup callback never ran";
   EXPECT_TRUE(teardown_ran) << "Teardown callback never ran";
+  EXPECT_TRUE(memory_pass_threads_is_one)
+      << "state.threads() was not 1 during MemoryManager measurement pass";
+  EXPECT_TRUE(benchmark_pass_threads_is_four)
+      << "state.threads() was not 4 during normal iterations";
   EXPECT_FALSE(in_measurement_window);
 }