commit 5c18c1cf64640266ee5cbad00ac9b22be8beb6a5
author Kevin DuBois <kevindubois@google.com> Wed May 27 15:50:50 2020 -0700
committer Kevin DuBois <kevindubois@google.com> Fri Jun 05 07:50:52 2020 -0700
tree b1de30c7d243acc62c959c62e24805b756e5c150
parent c43da98656bbf1944991b5d3fa415e2a8f2c966c

SF: avoid updating queue if schedule() is called.

Avoids a rarish race condition where a callback is scheduled in the
interim time between the TimerDispatch starting to run and the callback
for that scheduled callback is invoked. In this condition, the code will
now have the next callback pend until the timer queue processes the when
to wakeup next.

Bug: 154303580
Test: 3 new unit tests
Test: boot to home, check some animations
Test: overnight dogfood with patch.

Change-Id: I0e7e2e3698ed6d1765082db20d9cf25f6e6c2db2

services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp

diff --git a/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp b/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp
index cd15617..abeacfe 100644
--- a/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp
+++ b/services/surfaceflinger/Scheduler/VSyncDispatchTimerQueue.cpp
@@ -87,10 +87,26 @@
     return ScheduleResult::Scheduled;
 }
 
+void VSyncDispatchTimerQueueEntry::addPendingWorkloadUpdate(nsecs_t workDuration,
+                                                            nsecs_t earliestVsync) {
+    mWorkloadUpdateInfo = {.earliestVsync = earliestVsync, .duration = workDuration};
+}
+
+bool VSyncDispatchTimerQueueEntry::hasPendingWorkloadUpdate() const {
+    return mWorkloadUpdateInfo.has_value();
+}
+
 void VSyncDispatchTimerQueueEntry::update(VSyncTracker& tracker, nsecs_t now) {
-    if (!mArmedInfo) {
+    if (!mArmedInfo && !mWorkloadUpdateInfo) {
         return;
     }
+
+    if (mWorkloadUpdateInfo) {
+        mEarliestVsync = mWorkloadUpdateInfo->earliestVsync;
+        mWorkDuration = mWorkloadUpdateInfo->duration;
+        mWorkloadUpdateInfo.reset();
+    }
+
     auto const nextVsyncTime =
             tracker.nextAnticipatedVSyncTimeFrom(std::max(mEarliestVsync, now + mWorkDuration));
     mArmedInfo = {nextVsyncTime - mWorkDuration, nextVsyncTime};
@@ -192,7 +208,7 @@
     std::optional<std::string_view> nextWakeupName;
     for (auto it = mCallbacks.begin(); it != mCallbacks.end(); it++) {
         auto& callback = it->second;
-        if (!callback->wakeupTime()) {
+        if (!callback->wakeupTime() && !callback->hasPendingWorkloadUpdate()) {
             continue;
         }
 
@@ -291,6 +307,15 @@
         }
         auto& callback = it->second;
         auto const now = mTimeKeeper->now();
+
+        /* If the timer thread will run soon, we'll apply this work update via the callback
+         * timer recalculation to avoid cancelling a callback that is about to fire. */
+        auto const rearmImminent = now > mIntendedWakeupTime;
+        if (CC_UNLIKELY(rearmImminent)) {
+            callback->addPendingWorkloadUpdate(workDuration, earliestVsync);
+            return ScheduleResult::Scheduled;
+        }
+
         result = callback->schedule(workDuration, earliestVsync, mTracker, now);
         if (result == ScheduleResult::CannotSchedule) {
             return result;