Fix crash from asynchronous GPU metrics

Fixes: 241751056
Test: repo steps from bug
Change-Id: I3d1434bc4aa240863611d4f740815391d4067784

2 files changed, 15 insertions, 5 deletions

diff --git a/libs/hwui/FrameInfoVisualizer.cpp b/libs/hwui/FrameInfoVisualizer.cpp
index 3a8e559f6d7e..687e4dd324d3 100644
--- a/libs/hwui/FrameInfoVisualizer.cpp
+++ b/libs/hwui/FrameInfoVisualizer.cpp
@@ -179,7 +179,7 @@ void FrameInfoVisualizer::initializeRects(const int baseline, const int width) {
 void FrameInfoVisualizer::nextBarSegment(FrameInfoIndex start, FrameInfoIndex end) {
     int fast_i = (mNumFastRects - 1) * 4;
     int janky_i = (mNumJankyRects - 1) * 4;
-    ;
+
     for (size_t fi = 0; fi < mFrameSource.size(); fi++) {
         if (mFrameSource[fi][FrameInfoIndex::Flags] & FrameInfoFlags::SkippedFrame) {
             continue;
diff --git a/libs/hwui/renderthread/CanvasContext.cpp b/libs/hwui/renderthread/CanvasContext.cpp
index 976117b9bbd4..75d3ff7753cb 100644
--- a/libs/hwui/renderthread/CanvasContext.cpp
+++ b/libs/hwui/renderthread/CanvasContext.cpp
@@ -512,9 +512,19 @@ nsecs_t CanvasContext::draw() {
 
     ATRACE_FORMAT("Drawing " RECT_STRING, SK_RECT_ARGS(dirty));
 
-    const auto drawResult = mRenderPipeline->draw(frame, windowDirty, dirty, mLightGeometry,
-                                                  &mLayerUpdateQueue, mContentDrawBounds, mOpaque,
-                                                  mLightInfo, mRenderNodes, &(profiler()));
+    IRenderPipeline::DrawResult drawResult;
+    {
+        // FrameInfoVisualizer accesses the frame events, which cannot be mutated mid-draw
+        // or it can lead to memory corruption.
+        // This lock is overly broad, but it's the quickest fix since this mutex is otherwise
+        // not visible to IRenderPipeline much less FrameInfoVisualizer. And since this is
+        // the thread we're primarily concerned about being responsive, this being too broad
+        // shouldn't pose a performance issue.
+        std::scoped_lock lock(mFrameMetricsReporterMutex);
+        drawResult = mRenderPipeline->draw(frame, windowDirty, dirty, mLightGeometry,
+                                           &mLayerUpdateQueue, mContentDrawBounds, mOpaque,
+                                           mLightInfo, mRenderNodes, &(profiler()));
+    }
 
     uint64_t frameCompleteNr = getFrameNumber();
 
@@ -754,11 +764,11 @@ void CanvasContext::onSurfaceStatsAvailable(void* context, int32_t surfaceContro
     FrameInfo* frameInfo = instance->getFrameInfoFromLast4(frameNumber, surfaceControlId);
 
     if (frameInfo != nullptr) {
+        std::scoped_lock lock(instance->mFrameMetricsReporterMutex);
         frameInfo->set(FrameInfoIndex::FrameCompleted) = std::max(gpuCompleteTime,
                 frameInfo->get(FrameInfoIndex::SwapBuffersCompleted));
         frameInfo->set(FrameInfoIndex::GpuCompleted) = std::max(
                 gpuCompleteTime, frameInfo->get(FrameInfoIndex::CommandSubmissionCompleted));
-        std::scoped_lock lock(instance->mFrameMetricsReporterMutex);
         instance->mJankTracker.finishFrame(*frameInfo, instance->mFrameMetricsReporter, frameNumber,
                                            surfaceControlId);
     }