Merge changes I55af2ec2,I4aa5b86c

* changes:
  SurfaceFlinger: add thread name to OneShotTimer
  SurfaceFlinger: fix OneShotTimer timespec overflow

7 files changed, 62 insertions, 26 deletions

diff --git a/services/surfaceflinger/DisplayHardware/PowerAdvisor.cpp b/services/surfaceflinger/DisplayHardware/PowerAdvisor.cpp
index 4b4c050f8e..901e19a6a2 100644
--- a/services/surfaceflinger/DisplayHardware/PowerAdvisor.cpp
+++ b/services/surfaceflinger/DisplayHardware/PowerAdvisor.cpp
@@ -64,7 +64,7 @@ int32_t getUpdateTimeout() {
 PowerAdvisor::PowerAdvisor()
       : mUseUpdateImminentTimer(getUpdateTimeout() > 0),
         mUpdateImminentTimer(
-                OneShotTimer::Interval(getUpdateTimeout()),
+                "UpdateImminentTimer", OneShotTimer::Interval(getUpdateTimeout()),
                 /* resetCallback */ [this] { mSendUpdateImminent.store(false); },
                 /* timeoutCallback */ [this] { mSendUpdateImminent.store(true); }) {
     if (mUseUpdateImminentTimer) {
diff --git a/services/surfaceflinger/RegionSamplingThread.cpp b/services/surfaceflinger/RegionSamplingThread.cpp
index b7b7e4658e..2511eb37b3 100644
--- a/services/surfaceflinger/RegionSamplingThread.cpp
+++ b/services/surfaceflinger/RegionSamplingThread.cpp
@@ -175,6 +175,7 @@ RegionSamplingThread::RegionSamplingThread(SurfaceFlinger& flinger, Scheduler& s
         mScheduler(scheduler),
         mTunables(tunables),
         mIdleTimer(
+                "RegionSamplingIdleTimer",
                 std::chrono::duration_cast<std::chrono::milliseconds>(
                         mTunables.mSamplingTimerTimeout),
                 [] {}, [this] { checkForStaleLuma(); }),
diff --git a/services/surfaceflinger/Scheduler/OneShotTimer.cpp b/services/surfaceflinger/Scheduler/OneShotTimer.cpp
index 27838003c4..ce3b0c6cfa 100644
--- a/services/surfaceflinger/Scheduler/OneShotTimer.cpp
+++ b/services/surfaceflinger/Scheduler/OneShotTimer.cpp
@@ -16,6 +16,8 @@
 
 #include "OneShotTimer.h"
 
+#include <utils/Log.h>
+#include <utils/Timers.h>
 #include <chrono>
 #include <sstream>
 #include <thread>
@@ -29,25 +31,30 @@ constexpr int64_t kNsToSeconds = std::chrono::duration_cast<std::chrono::nanosec
 // (tv_sec) is the whole count of seconds. The second (tv_nsec) is the
 // nanosecond part of the count. This function takes care of translation.
 void calculateTimeoutTime(std::chrono::nanoseconds timestamp, timespec* spec) {
-    clock_gettime(CLOCK_MONOTONIC, spec);
-    spec->tv_sec += static_cast<__kernel_time_t>(timestamp.count() / kNsToSeconds);
-    spec->tv_nsec += timestamp.count() % kNsToSeconds;
+    const nsecs_t timeout = systemTime(CLOCK_MONOTONIC) + timestamp.count();
+    spec->tv_sec = static_cast<__kernel_time_t>(timeout / kNsToSeconds);
+    spec->tv_nsec = timeout % kNsToSeconds;
 }
 } // namespace
 
 namespace android {
 namespace scheduler {
 
-OneShotTimer::OneShotTimer(const Interval& interval, const ResetCallback& resetCallback,
+OneShotTimer::OneShotTimer(std::string name, const Interval& interval,
+                           const ResetCallback& resetCallback,
                            const TimeoutCallback& timeoutCallback)
-      : mInterval(interval), mResetCallback(resetCallback), mTimeoutCallback(timeoutCallback) {}
+      : mName(std::move(name)),
+        mInterval(interval),
+        mResetCallback(resetCallback),
+        mTimeoutCallback(timeoutCallback) {}
 
 OneShotTimer::~OneShotTimer() {
     stop();
 }
 
 void OneShotTimer::start() {
-    sem_init(&mSemaphore, 0, 0);
+    int result = sem_init(&mSemaphore, 0, 0);
+    LOG_ALWAYS_FATAL_IF(result, "sem_init failed");
 
     if (!mThread.joinable()) {
         // Only create thread if it has not been created.
@@ -57,15 +64,21 @@ void OneShotTimer::start() {
 
 void OneShotTimer::stop() {
     mStopTriggered = true;
-    sem_post(&mSemaphore);
+    int result = sem_post(&mSemaphore);
+    LOG_ALWAYS_FATAL_IF(result, "sem_post failed");
 
     if (mThread.joinable()) {
         mThread.join();
-        sem_destroy(&mSemaphore);
+        result = sem_destroy(&mSemaphore);
+        LOG_ALWAYS_FATAL_IF(result, "sem_destroy failed");
     }
 }
 
 void OneShotTimer::loop() {
+    if (pthread_setname_np(pthread_self(), mName.c_str())) {
+        ALOGW("Failed to set thread name on dispatch thread");
+    }
+
     TimerState state = TimerState::RESET;
     while (true) {
         bool triggerReset = false;
@@ -77,7 +90,12 @@ void OneShotTimer::loop() {
         }
 
         if (state == TimerState::IDLE) {
-            sem_wait(&mSemaphore);
+            int result = sem_wait(&mSemaphore);
+            if (result && errno != EINTR) {
+                std::stringstream ss;
+                ss << "sem_wait failed (" << errno << ")";
+                LOG_ALWAYS_FATAL("%s", ss.str().c_str());
+            }
             continue;
         }
 
@@ -101,7 +119,12 @@ void OneShotTimer::loop() {
             // Wait for mInterval time for semaphore signal.
             struct timespec ts;
             calculateTimeoutTime(std::chrono::nanoseconds(mInterval), &ts);
-            sem_clockwait(&mSemaphore, CLOCK_MONOTONIC, &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());
+            }
 
             state = checkForResetAndStop(state);
             if (state == TimerState::RESET) {
@@ -135,7 +158,8 @@ OneShotTimer::TimerState OneShotTimer::checkForResetAndStop(TimerState state) {
 
 void OneShotTimer::reset() {
     mResetTriggered = true;
-    sem_post(&mSemaphore);
+    int result = sem_post(&mSemaphore);
+    LOG_ALWAYS_FATAL_IF(result, "sem_post failed");
 }
 
 std::string OneShotTimer::dump() const {
diff --git a/services/surfaceflinger/Scheduler/OneShotTimer.h b/services/surfaceflinger/Scheduler/OneShotTimer.h
index 8bbd4f5983..3690ce7542 100644
--- a/services/surfaceflinger/Scheduler/OneShotTimer.h
+++ b/services/surfaceflinger/Scheduler/OneShotTimer.h
@@ -36,7 +36,7 @@ public:
     using ResetCallback = std::function<void()>;
     using TimeoutCallback = std::function<void()>;
 
-    OneShotTimer(const Interval& interval, const ResetCallback& resetCallback,
+    OneShotTimer(std::string name, const Interval& interval, const ResetCallback& resetCallback,
                  const TimeoutCallback& timeoutCallback);
     ~OneShotTimer();
 
@@ -81,6 +81,9 @@ private:
     // Semaphore to keep mThread synchronized.
     sem_t mSemaphore;
 
+    // Timer's name.
+    std::string mName;
+
     // Interval after which timer expires.
     const Interval mInterval;
 
diff --git a/services/surfaceflinger/Scheduler/Scheduler.cpp b/services/surfaceflinger/Scheduler/Scheduler.cpp
index a14019eeb5..37066317ce 100644
--- a/services/surfaceflinger/Scheduler/Scheduler.cpp
+++ b/services/surfaceflinger/Scheduler/Scheduler.cpp
@@ -135,7 +135,7 @@ Scheduler::Scheduler(const scheduler::RefreshRateConfigs& configs, ISchedulerCal
         const auto callback = mOptions.supportKernelTimer ? &Scheduler::kernelIdleTimerCallback
                                                           : &Scheduler::idleTimerCallback;
         mIdleTimer.emplace(
-                std::chrono::milliseconds(millis),
+                "IdleTimer", std::chrono::milliseconds(millis),
                 [this, callback] { std::invoke(callback, this, TimerState::Reset); },
                 [this, callback] { std::invoke(callback, this, TimerState::Expired); });
         mIdleTimer->start();
@@ -144,7 +144,7 @@ Scheduler::Scheduler(const scheduler::RefreshRateConfigs& configs, ISchedulerCal
     if (const int64_t millis = set_touch_timer_ms(0); millis > 0) {
         // Touch events are coming to SF every 100ms, so the timer needs to be higher than that
         mTouchTimer.emplace(
-                std::chrono::milliseconds(millis),
+                "TouchTimer", std::chrono::milliseconds(millis),
                 [this] { touchTimerCallback(TimerState::Reset); },
                 [this] { touchTimerCallback(TimerState::Expired); });
         mTouchTimer->start();
@@ -152,7 +152,7 @@ Scheduler::Scheduler(const scheduler::RefreshRateConfigs& configs, ISchedulerCal
 
     if (const int64_t millis = set_display_power_timer_ms(0); millis > 0) {
         mDisplayPowerTimer.emplace(
-                std::chrono::milliseconds(millis),
+                "DisplayPowerTimer", std::chrono::milliseconds(millis),
                 [this] { displayPowerTimerCallback(TimerState::Reset); },
                 [this] { displayPowerTimerCallback(TimerState::Expired); });
         mDisplayPowerTimer->start();
diff --git a/services/surfaceflinger/tests/unittests/OneShotTimerTest.cpp b/services/surfaceflinger/tests/unittests/OneShotTimerTest.cpp
index 0208728026..cfbb3f5e9f 100644
--- a/services/surfaceflinger/tests/unittests/OneShotTimerTest.cpp
+++ b/services/surfaceflinger/tests/unittests/OneShotTimerTest.cpp
@@ -57,11 +57,12 @@ protected:
 namespace {
 TEST_F(OneShotTimerTest, createAndDestroyTest) {
     mIdleTimer = std::make_unique<scheduler::OneShotTimer>(
-            3ms, [] {}, [] {});
+            "TestTimer", 3ms, [] {}, [] {});
 }
 
 TEST_F(OneShotTimerTest, startStopTest) {
-    mIdleTimer = std::make_unique<scheduler::OneShotTimer>(30ms, mResetTimerCallback.getInvocable(),
+    mIdleTimer = std::make_unique<scheduler::OneShotTimer>("TestTimer", 30ms,
+                                                           mResetTimerCallback.getInvocable(),
                                                            mExpiredTimerCallback.getInvocable());
     auto startTime = std::chrono::steady_clock::now();
     mIdleTimer->start();
@@ -81,7 +82,8 @@ TEST_F(OneShotTimerTest, startStopTest) {
 }
 
 TEST_F(OneShotTimerTest, resetTest) {
-    mIdleTimer = std::make_unique<scheduler::OneShotTimer>(20ms, mResetTimerCallback.getInvocable(),
+    mIdleTimer = std::make_unique<scheduler::OneShotTimer>("TestTimer", 20ms,
+                                                           mResetTimerCallback.getInvocable(),
                                                            mExpiredTimerCallback.getInvocable());
     mIdleTimer->start();
     EXPECT_TRUE(mResetTimerCallback.waitForCall().has_value());
@@ -106,7 +108,8 @@ TEST_F(OneShotTimerTest, resetTest) {
 }
 
 TEST_F(OneShotTimerTest, resetBackToBackTest) {
-    mIdleTimer = std::make_unique<scheduler::OneShotTimer>(20ms, mResetTimerCallback.getInvocable(),
+    mIdleTimer = std::make_unique<scheduler::OneShotTimer>("TestTimer", 20ms,
+                                                           mResetTimerCallback.getInvocable(),
                                                            mExpiredTimerCallback.getInvocable());
     mIdleTimer->start();
     EXPECT_TRUE(mResetTimerCallback.waitForCall().has_value());
@@ -137,7 +140,8 @@ TEST_F(OneShotTimerTest, resetBackToBackTest) {
 }
 
 TEST_F(OneShotTimerTest, startNotCalledTest) {
-    mIdleTimer = std::make_unique<scheduler::OneShotTimer>(3ms, mResetTimerCallback.getInvocable(),
+    mIdleTimer = std::make_unique<scheduler::OneShotTimer>("TestTimer", 3ms,
+                                                           mResetTimerCallback.getInvocable(),
                                                            mExpiredTimerCallback.getInvocable());
     // The start hasn't happened, so the callback does not happen.
     EXPECT_FALSE(mExpiredTimerCallback.waitForCall(waitTimeForUnexpected3msCallback).has_value());
@@ -149,7 +153,8 @@ TEST_F(OneShotTimerTest, startNotCalledTest) {
 }
 
 TEST_F(OneShotTimerTest, idleTimerIdlesTest) {
-    mIdleTimer = std::make_unique<scheduler::OneShotTimer>(3ms, mResetTimerCallback.getInvocable(),
+    mIdleTimer = std::make_unique<scheduler::OneShotTimer>("TestTimer", 3ms,
+                                                           mResetTimerCallback.getInvocable(),
                                                            mExpiredTimerCallback.getInvocable());
     mIdleTimer->start();
     EXPECT_TRUE(mResetTimerCallback.waitForCall().has_value());
@@ -169,7 +174,8 @@ TEST_F(OneShotTimerTest, idleTimerIdlesTest) {
 }
 
 TEST_F(OneShotTimerTest, timeoutCallbackExecutionTest) {
-    mIdleTimer = std::make_unique<scheduler::OneShotTimer>(3ms, mResetTimerCallback.getInvocable(),
+    mIdleTimer = std::make_unique<scheduler::OneShotTimer>("TestTimer", 3ms,
+                                                           mResetTimerCallback.getInvocable(),
                                                            mExpiredTimerCallback.getInvocable());
     mIdleTimer->start();
     EXPECT_TRUE(mResetTimerCallback.waitForCall().has_value());
@@ -178,7 +184,8 @@ TEST_F(OneShotTimerTest, timeoutCallbackExecutionTest) {
 }
 
 TEST_F(OneShotTimerTest, noCallbacksAfterStopAndResetTest) {
-    mIdleTimer = std::make_unique<scheduler::OneShotTimer>(3ms, mResetTimerCallback.getInvocable(),
+    mIdleTimer = std::make_unique<scheduler::OneShotTimer>("TestTimer", 3ms,
+                                                           mResetTimerCallback.getInvocable(),
                                                            mExpiredTimerCallback.getInvocable());
     mIdleTimer->start();
     EXPECT_TRUE(mResetTimerCallback.waitForCall().has_value());
@@ -192,7 +199,8 @@ TEST_F(OneShotTimerTest, noCallbacksAfterStopAndResetTest) {
 }
 
 TEST_F(OneShotTimerTest, noCallbacksAfterStopTest) {
-    mIdleTimer = std::make_unique<scheduler::OneShotTimer>(3ms, mResetTimerCallback.getInvocable(),
+    mIdleTimer = std::make_unique<scheduler::OneShotTimer>("TestTimer", 3ms,
+                                                           mResetTimerCallback.getInvocable(),
                                                            mExpiredTimerCallback.getInvocable());
     mIdleTimer->start();
     EXPECT_TRUE(mResetTimerCallback.waitForCall().has_value());
diff --git a/services/surfaceflinger/tests/unittests/TestableScheduler.h b/services/surfaceflinger/tests/unittests/TestableScheduler.h
index a9d9dc08ad..1b6e9ed514 100644
--- a/services/surfaceflinger/tests/unittests/TestableScheduler.h
+++ b/services/surfaceflinger/tests/unittests/TestableScheduler.h
@@ -83,7 +83,7 @@ public:
             mTouchTimer.reset();
         }
         mTouchTimer.emplace(
-                std::chrono::milliseconds(millis),
+                "Testable Touch timer", std::chrono::milliseconds(millis),
                 [this] { touchTimerCallback(TimerState::Reset); },
                 [this] { touchTimerCallback(TimerState::Expired); });
         mTouchTimer->start();