Properly protect mFrameMetricsReporter

This field actually requires a special lock, mFrameMetricsReporterMutex.
But there isn't a GUARDED_BY annotation for it. And even if there was,
the compiler feature of -Wthread-safety was not active in this code, so
this error would not have been caught.

To fix this, enable the compiler annotation and add GUARDED_BY
annotation to mFrameMetricsReporter.
And finally, use this lock to properly protect this field.

Bug: 192330836
Test: atest hwui_unit_tests
Change-Id: I76950bfa01bbd7ccdc54c4e8c114430b5aeddf1a

1 files changed, 25 insertions, 15 deletions

diff --git a/libs/hwui/renderthread/CanvasContext.cpp b/libs/hwui/renderthread/CanvasContext.cpp
index 81cee6103d22..adea4184c4fd 100644
--- a/libs/hwui/renderthread/CanvasContext.cpp
+++ b/libs/hwui/renderthread/CanvasContext.cpp
@@ -614,6 +614,7 @@ nsecs_t CanvasContext::draw() {
             mCurrentFrameInfo->markFrameCompleted();
             mCurrentFrameInfo->set(FrameInfoIndex::GpuCompleted)
                     = mCurrentFrameInfo->get(FrameInfoIndex::FrameCompleted);
+            std::scoped_lock lock(mFrameMetricsReporterMutex);
             mJankTracker.finishFrame(*mCurrentFrameInfo, mFrameMetricsReporter);
         }
     }
@@ -638,9 +639,12 @@ void CanvasContext::cleanupResources() {
 }
 
 void CanvasContext::reportMetricsWithPresentTime() {
-    if (mFrameMetricsReporter == nullptr) {
-        return;
-    }
+    {  // acquire lock
+        std::scoped_lock lock(mFrameMetricsReporterMutex);
+        if (mFrameMetricsReporter == nullptr) {
+            return;
+        }
+    }  // release lock
     if (mNativeSurface == nullptr) {
         return;
     }
@@ -665,7 +669,22 @@ void CanvasContext::reportMetricsWithPresentTime() {
             nullptr /*outReleaseTime*/);
 
     forthBehind->set(FrameInfoIndex::DisplayPresentTime) = presentTime;
-    mFrameMetricsReporter->reportFrameMetrics(forthBehind->data(), true /*hasPresentTime*/);
+    {  // acquire lock
+        std::scoped_lock lock(mFrameMetricsReporterMutex);
+        if (mFrameMetricsReporter != nullptr) {
+            mFrameMetricsReporter->reportFrameMetrics(forthBehind->data(), true /*hasPresentTime*/);
+        }
+    }  // release lock
+}
+
+FrameInfo* CanvasContext::getFrameInfoFromLast4(uint64_t frameNumber) {
+    std::scoped_lock lock(mLast4FrameInfosMutex);
+    for (size_t i = 0; i < mLast4FrameInfos.size(); i++) {
+        if (mLast4FrameInfos[i].second == frameNumber) {
+            return mLast4FrameInfos[i].first;
+        }
+    }
+    return nullptr;
 }
 
 void CanvasContext::onSurfaceStatsAvailable(void* context, ASurfaceControl* control,
@@ -679,22 +698,13 @@ void CanvasContext::onSurfaceStatsAvailable(void* context, ASurfaceControl* cont
     nsecs_t gpuCompleteTime = functions.getAcquireTimeFunc(stats);
     uint64_t frameNumber = functions.getFrameNumberFunc(stats);
 
-    FrameInfo* frameInfo = nullptr;
-    {
-        std::lock_guard(instance->mLast4FrameInfosMutex);
-        for (size_t i = 0; i < instance->mLast4FrameInfos.size(); i++) {
-            if (instance->mLast4FrameInfos[i].second == frameNumber) {
-                frameInfo = instance->mLast4FrameInfos[i].first;
-                break;
-            }
-        }
-    }
+    FrameInfo* frameInfo = instance->getFrameInfoFromLast4(frameNumber);
 
     if (frameInfo != nullptr) {
         frameInfo->set(FrameInfoIndex::FrameCompleted) = std::max(gpuCompleteTime,
                 frameInfo->get(FrameInfoIndex::SwapBuffersCompleted));
         frameInfo->set(FrameInfoIndex::GpuCompleted) = gpuCompleteTime;
-        std::lock_guard(instance->mFrameMetricsReporterMutex);
+        std::scoped_lock lock(instance->mFrameMetricsReporterMutex);
         instance->mJankTracker.finishFrame(*frameInfo, instance->mFrameMetricsReporter);
     }
 }