diff options
| -rw-r--r-- | services/surfaceflinger/Scheduler/OneShotTimer.cpp | 23 | ||||
| -rw-r--r-- | services/surfaceflinger/tests/unittests/OneShotTimerTest.cpp | 34 |
2 files changed, 46 insertions, 11 deletions
diff --git a/services/surfaceflinger/Scheduler/OneShotTimer.cpp b/services/surfaceflinger/Scheduler/OneShotTimer.cpp index 9c6e56dfa8..3c8dc64f10 100644 --- a/services/surfaceflinger/Scheduler/OneShotTimer.cpp +++ b/services/surfaceflinger/Scheduler/OneShotTimer.cpp @@ -118,16 +118,17 @@ void OneShotTimer::loop() { auto triggerTime = mClock->now() + mInterval; state = TimerState::WAITING; while (true) { - mWaiting = true; - constexpr auto zero = std::chrono::steady_clock::duration::zero(); - // Wait for mInterval time to check if we need to reset or drop into the idle state. - struct timespec ts; - calculateTimeoutTime(std::chrono::nanoseconds(mInterval), &ts); - int result = sem_clockwait(&mSemaphore, CLOCK_MONOTONIC, &ts); - if (result && errno != ETIMEDOUT && errno != EINTR) { - std::stringstream ss; - ss << "sem_clockwait failed (" << errno << ")"; - LOG_ALWAYS_FATAL("%s", ss.str().c_str()); + // Wait until triggerTime time to check if we need to reset or drop into the idle state. + if (const auto triggerInterval = triggerTime - mClock->now(); triggerInterval > 0ns) { + mWaiting = true; + struct timespec ts; + calculateTimeoutTime(triggerInterval, &ts); + int result = sem_clockwait(&mSemaphore, CLOCK_MONOTONIC, &ts); + if (result && errno != ETIMEDOUT && errno != EINTR) { + std::stringstream ss; + ss << "sem_clockwait failed (" << errno << ")"; + LOG_ALWAYS_FATAL("%s", ss.str().c_str()); + } } mWaiting = false; @@ -136,7 +137,7 @@ void OneShotTimer::loop() { break; } - if (state == TimerState::WAITING && (triggerTime - mClock->now()) <= zero) { + if (state == TimerState::WAITING && (triggerTime - mClock->now()) <= 0ns) { triggerTimeout = true; state = TimerState::IDLE; break; diff --git a/services/surfaceflinger/tests/unittests/OneShotTimerTest.cpp b/services/surfaceflinger/tests/unittests/OneShotTimerTest.cpp index 597e5e71a2..aafc323a9b 100644 --- a/services/surfaceflinger/tests/unittests/OneShotTimerTest.cpp +++ b/services/surfaceflinger/tests/unittests/OneShotTimerTest.cpp @@ -130,6 +130,40 @@ TEST_F(OneShotTimerTest, DISABLED_resetBackToBackTest) { EXPECT_FALSE(mResetTimerCallback.waitForUnexpectedCall().has_value()); } +// TODO(b/186417847) This test is new and passes locally, but may be flaky +TEST_F(OneShotTimerTest, DISABLED_resetBackToBackSlowAdvanceTest) { + fake::FakeClock* clock = new fake::FakeClock(); + mIdleTimer = std::make_unique<scheduler::OneShotTimer>("TestTimer", 1ms, + mResetTimerCallback.getInvocable(), + mExpiredTimerCallback.getInvocable(), + std::unique_ptr<fake::FakeClock>(clock)); + mIdleTimer->start(); + EXPECT_TRUE(mResetTimerCallback.waitForCall().has_value()); + + mIdleTimer->reset(); + EXPECT_FALSE(mResetTimerCallback.waitForUnexpectedCall().has_value()); + EXPECT_FALSE(mExpiredTimerCallback.waitForUnexpectedCall().has_value()); + + clock->advanceTime(200us); + mIdleTimer->reset(); + + // Normally we would check that the timer callbacks weren't invoked here + // after resetting the timer, but we need to precisely control the timing of + // this test, and checking that callbacks weren't invoked requires non-zero + // time. + + clock->advanceTime(1500us); + EXPECT_TRUE(mExpiredTimerCallback.waitForCall(1100us).has_value()); + mIdleTimer->reset(); + EXPECT_TRUE(mResetTimerCallback.waitForCall().has_value()); + + mIdleTimer->stop(); + clock->advanceTime(2ms); + // Final quick check that no more callback were observed. + EXPECT_FALSE(mExpiredTimerCallback.waitForUnexpectedCall().has_value()); + EXPECT_FALSE(mResetTimerCallback.waitForUnexpectedCall().has_value()); +} + TEST_F(OneShotTimerTest, startNotCalledTest) { fake::FakeClock* clock = new fake::FakeClock(); mIdleTimer = std::make_unique<scheduler::OneShotTimer>("TestTimer", 1ms, |