Skip to content

Resolve race on approx_time_ in FastClock. #23

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
May 5, 2014
Merged

Resolve race on approx_time_ in FastClock. #23

merged 1 commit into from
May 5, 2014

Conversation

ckennelly
Copy link
Contributor

InitType should hold a lock before storing to approx_time_, which is later read by the background worker. When the worker is actively running (i.e., not blocked on bg_cond_) it holds bg_mutex_.

InitType is called during benchmark setup only, so any contention induced for the mutex should not have performance/accuracy consequences.

This race was found with thread sanitizer:

WARNING: ThreadSanitizer: data race (pid=8066)
  Read of size 4 at 0x7fffdfe11828 by thread T1 (mutexes: write M11):
    #0 benchmark::State::FastClock::NowMicros() const /home/ckennelly/projects/benchmark/src/benchmark.cc:565 (exe+0x0000000818fb)
    #1 benchmark::State::FastClock::BGThread() /home/ckennelly/projects/benchmark/src/benchmark.cc:615 (exe+0x000000094e35)
    #2 benchmark::State::FastClock::BGThreadWrapper(void*) /home/ckennelly/projects/benchmark/src/benchmark.cc:593 (exe+0x000000094c33)

  Previous write of size 4 at 0x7fffdfe11828 by main thread:
    #0 benchmark::State::FastClock::InitType(benchmark::State::FastClock::Type) /home/ckennelly/projects/benchmark/src/benchmark.cc:579 (exe+0x000000081863)
    #1 benchmark::State::StartRunning() /home/ckennelly/projects/benchmark/src/benchmark.cc:1028 (exe+0x00000007b1f9)
    #2 benchmark::State::KeepRunning() /home/ckennelly/projects/benchmark/src/benchmark.cc:958 (exe+0x000000079af6)
    #3 benchmark::internal::Benchmark::MeasureOverhead() /home/ckennelly/projects/benchmark/src/benchmark.cc:830 (exe+0x000000079703)
    #4 benchmark::Initialize(int*, char const**) /home/ckennelly/projects/benchmark/src/benchmark.cc:1234 (exe+0x00000007dc25)
    #5 main /home/ckennelly/projects/benchmark/test/benchmark_test.cc:147 (exe+0x00000006b549)

  Location is stack of main thread.

  Mutex M11 created at:
    #0 pthread_mutex_init ??:0 (exe+0x00000003bdc5)
    #1 FastClock /home/ckennelly/projects/benchmark/src/benchmark.cc:541 (exe+0x000000094b39)
    #2 FastClock /home/ckennelly/projects/benchmark/src/benchmark.cc:543 (exe+0x000000080dbc)
    #3 benchmark::internal::Benchmark::MeasureOverhead() /home/ckennelly/projects/benchmark/src/benchmark.cc:827 (exe+0x0000000796c1)
    #4 benchmark::Initialize(int*, char const**) /home/ckennelly/projects/benchmark/src/benchmark.cc:1234 (exe+0x00000007dc25)
    #5 main /home/ckennelly/projects/benchmark/test/benchmark_test.cc:147 (exe+0x00000006b549)

  Thread T1 (tid=8068, running) created by main thread at:
    #0 pthread_create ??:0 (exe+0x00000003b812)
    #1 FastClock /home/ckennelly/projects/benchmark/src/benchmark.cc:542 (exe+0x000000094b61)
    #2 FastClock /home/ckennelly/projects/benchmark/src/benchmark.cc:543 (exe+0x000000080dbc)
    #3 benchmark::internal::Benchmark::MeasureOverhead() /home/ckennelly/projects/benchmark/src/benchmark.cc:827 (exe+0x0000000796c1)
    #4 benchmark::Initialize(int*, char const**) /home/ckennelly/projects/benchmark/src/benchmark.cc:1234 (exe+0x00000007dc25)
    #5 main /home/ckennelly/projects/benchmark/test/benchmark_test.cc:147 (exe+0x00000006b549)

SUMMARY: ThreadSanitizer: data race /home/ckennelly/projects/benchmark/src/benchmark.cc:565 benchmark::State::FastClock::NowMicros() const

@ckennelly
Resolve race on approx_time_ in FastClock.
2c9b29c 
InitType should hold a lock before storing to approx_time_, which is later
read by the background worker.  When the worker is actively running (i.e., not
blocked on bg_cond_) it holds bg_mutex_.

InitType is called during benchmark setup only, so any contention induced for
the mutex should not have performance/accuracy consequences.
dmah42 pushed a commit that referenced this pull request May 5, 2014
@dominichamon
Merge pull request #23 from ckennelly/clock_initialization
e5a4319 
Resolve race on approx_time_ in FastClock.
@dmah42 dmah42 merged commit e5a4319 into google:master May 5, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ckennelly @dmah42