Commit 54e18b89 by Chris Kennelly

Resolve benchmark cleanup race condition in issue #20.

The multithreaded API for benchmarks provides that teardown can happen in thread 0. For this to be safe, all other threads executing the benchmark function need to have exited. Otherwise, thread 0 may begin to teardown shared resources before the other threads have stopped using these resources as they are in their last loop of while (KeepRunning()) { ... }. This change creates a single exit point for KeepRunning() to return false. When running a multithreaded benchmark, thread 0 blocks on KeepRunning() until all other threads have exited. This approach allows for there to be no change to the user-facing API exemplified in the BM_MultiThreaded example.
parent efb9c302
...@@ -665,8 +665,10 @@ struct Benchmark::Instance { ...@@ -665,8 +665,10 @@ struct Benchmark::Instance {
struct State::SharedState { struct State::SharedState {
const internal::Benchmark::Instance* instance; const internal::Benchmark::Instance* instance;
pthread_mutex_t mu; pthread_mutex_t mu;
pthread_cond_t cond;
int starting; // Number of threads that have entered STARTING state int starting; // Number of threads that have entered STARTING state
int stopping; // Number of threads that have entered STOPPING state int stopping; // Number of threads that have entered STOPPING state
int exited; // Number of threads that have complete exited
int threads; // Number of total threads that are running concurrently int threads; // Number of total threads that are running concurrently
ThreadStats stats; ThreadStats stats;
std::vector<BenchmarkReporter::Run> runs; // accumulated runs std::vector<BenchmarkReporter::Run> runs; // accumulated runs
...@@ -676,11 +678,16 @@ struct State::SharedState { ...@@ -676,11 +678,16 @@ struct State::SharedState {
: instance(b), : instance(b),
starting(0), starting(0),
stopping(0), stopping(0),
exited(0),
threads(b == nullptr ? 1 : b->threads) { threads(b == nullptr ? 1 : b->threads) {
pthread_mutex_init(&mu, nullptr); pthread_mutex_init(&mu, nullptr);
pthread_cond_init(&cond, nullptr);
} }
~SharedState() { pthread_mutex_destroy(&mu); } ~SharedState() {
pthread_cond_destroy(&cond);
pthread_mutex_destroy(&mu);
}
DISALLOW_COPY_AND_ASSIGN(SharedState) DISALLOW_COPY_AND_ASSIGN(SharedState)
}; };
...@@ -953,22 +960,42 @@ bool State::KeepRunning() { ...@@ -953,22 +960,42 @@ bool State::KeepRunning() {
return true; return true;
} }
// To block thread 0 until all other threads exit, we have a signal exit
// point for KeepRunning() to return false. The fast path above always
// returns true.
bool ret = false;
switch (state_) { switch (state_) {
case STATE_INITIAL: case STATE_INITIAL:
return StartRunning(); ret = StartRunning();
break;
case STATE_STARTING: case STATE_STARTING:
CHECK(false); CHECK(false);
return true; ret = true;
break;
case STATE_RUNNING: case STATE_RUNNING:
return FinishInterval(); ret = FinishInterval();
break;
case STATE_STOPPING: case STATE_STOPPING:
return MaybeStop(); ret = MaybeStop();
break;
case STATE_STOPPED: case STATE_STOPPED:
CHECK(false); CHECK(false);
return true; ret = true;
break;
} }
CHECK(false);
return false; if (!ret && shared_->threads > 1 && thread_index == 0){
mutex_lock l(&shared_->mu);
// Block until all other threads have exited. We can then safely cleanup
// without other threads continuing to access shared variables inside the
// user-provided run function.
while (shared_->exited < shared_->threads - 1) {
pthread_cond_wait(&shared_->cond, &shared_->mu);
}
}
return ret;
} }
void State::PauseTiming() { start_pause_ = walltime::Now(); } void State::PauseTiming() { start_pause_ = walltime::Now(); }
...@@ -1157,6 +1184,17 @@ void* State::RunWrapper(void* arg) { ...@@ -1157,6 +1184,17 @@ void* State::RunWrapper(void* arg) {
State* that = (State*)arg; State* that = (State*)arg;
CHECK(that != nullptr); CHECK(that != nullptr);
that->Run(); that->Run();
mutex_lock l(&that->shared_->mu);
that->shared_->exited++;
if (that->thread_index > 0 &&
that->shared_->exited == that->shared_->threads - 1) {
// All threads but thread 0 have exited the user-provided run function.
// Thread 0 can now wake up and exit.
pthread_cond_signal(&that->shared_->cond);
}
return nullptr; return nullptr;
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment