Fix a race condition in RecurrentTimer.
Previously RecurrentTimer might not exit correctly if the mStopRequested
is set to true after we check for the value in the loop, but before
we start the wait. We will notify before we start waiting, causing the
wait never to return until timeout.
This CL guards updating mStopRequested with lock to make sure that it
must not change between our check and the wait.
After the fix we see no more flaky tests and decreased test execution
time since we will not wait for the next event to come before we end
the timer.
Test: atest android.hardware.automotive.vehicle@2.0-default-impl-unit-tests
Bug: 311757267
Change-Id: Iab1d72b954b4b02aa68e6fbbabcb97b572614d35
diff --git a/automotive/vehicle/2.0/default/common/include/vhal_v2_0/RecurrentTimer.h b/automotive/vehicle/2.0/default/common/include/vhal_v2_0/RecurrentTimer.h
index 0ed8742..0f5987e 100644
--- a/automotive/vehicle/2.0/default/common/include/vhal_v2_0/RecurrentTimer.h
+++ b/automotive/vehicle/2.0/default/common/include/vhal_v2_0/RecurrentTimer.h
@@ -122,21 +122,29 @@
}
std::unique_lock<std::mutex> g(mLock);
+ // mStopRequested might be set to true after we enter the loop. Must check inside
+ // the lock to make sure the value will not change before we start the wait.
+ if (mStopRequested) {
+ return;
+ }
mCond.wait_until(g, nextEventTime); // nextEventTime can be nanoseconds::max()
}
}
void stop() {
- mStopRequested = true;
{
std::lock_guard<std::mutex> g(mLock);
mCookieToEventsMap.clear();
+ // Even though this is atomic, this must be set inside the lock to make sure we will
+ // not change this after we check mStopRequested, but before we start the wait.
+ mStopRequested = true;
}
mCond.notify_one();
if (mTimerThread.joinable()) {
mTimerThread.join();
}
}
+
private:
mutable std::mutex mLock;
std::thread mTimerThread;