Fix racy WorkerThread test

WorkDelegate::DoWork() can be called more times than the value set by
WorkDelegate::SetDesiredWorkCount(). The main test thread may not be
able to “squeeze” its call to WorkerThread::Stop() in after its
WorkDelegate::WaitForWorkCount() returns. If the worker thread cannot be
stopped in time, one or more additional iterations of
WorkDelegate::DoWork() can run. WorkDelegate::DoWork() should take care
to not increment work_count_ beyond the desired value.

Bug: crashpad:169
Test: crashpad_util_test WorkerThread.*
Change-Id: I9e261a2a8a57420e12c0f1c9abd0ee6304dacd53
Reviewed-on: https://chromium-review.googlesource.com/456821
Reviewed-by: Robert Sesek <rsesek@chromium.org>
This commit is contained in:
Mark Mentovai 2017-03-19 15:30:19 -04:00
parent 14138936b5
commit bc5b7b06db

View File

@ -30,8 +30,11 @@ class WorkDelegate : public WorkerThread::Delegate {
~WorkDelegate() {}
void DoWork(const WorkerThread* thread) override {
if (++work_count_ == waiting_for_count_)
semaphore_.Signal();
if (work_count_ < waiting_for_count_) {
if (++work_count_ == waiting_for_count_) {
semaphore_.Signal();
}
}
}
void SetDesiredWorkCount(int times) {
@ -59,6 +62,7 @@ TEST(WorkerThread, DoWork) {
WorkerThread thread(0.05, &delegate);
uint64_t start = ClockMonotonicNanoseconds();
delegate.SetDesiredWorkCount(2);
thread.Start(0);
EXPECT_TRUE(thread.is_running());
@ -103,12 +107,12 @@ TEST(WorkerThread, DoWorkNow) {
WorkDelegate delegate;
WorkerThread thread(100, &delegate);
uint64_t start = ClockMonotonicNanoseconds();
delegate.SetDesiredWorkCount(1);
thread.Start(0);
EXPECT_TRUE(thread.is_running());
uint64_t start = ClockMonotonicNanoseconds();
delegate.WaitForWorkCount();
EXPECT_EQ(1, delegate.work_count());