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
diff --git a/libs/hwui/renderthread/CanvasContext.cpp b/libs/hwui/renderthread/CanvasContext.cpp
index 81cee61..adea418 100644
--- a/libs/hwui/renderthread/CanvasContext.cpp
+++ b/libs/hwui/renderthread/CanvasContext.cpp
@@ -614,6 +614,7 @@
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::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 @@
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 @@
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);
}
}
diff --git a/libs/hwui/renderthread/CanvasContext.h b/libs/hwui/renderthread/CanvasContext.h
index 85af3e4..6dbfcc34 100644
--- a/libs/hwui/renderthread/CanvasContext.h
+++ b/libs/hwui/renderthread/CanvasContext.h
@@ -160,6 +160,7 @@
void setContentDrawBounds(const Rect& bounds) { mContentDrawBounds = bounds; }
void addFrameMetricsObserver(FrameMetricsObserver* observer) {
+ std::scoped_lock lock(mFrameMetricsReporterMutex);
if (mFrameMetricsReporter.get() == nullptr) {
mFrameMetricsReporter.reset(new FrameMetricsReporter());
}
@@ -168,10 +169,10 @@
}
void removeFrameMetricsObserver(FrameMetricsObserver* observer) {
+ std::scoped_lock lock(mFrameMetricsReporterMutex);
if (mFrameMetricsReporter.get() != nullptr) {
mFrameMetricsReporter->removeObserver(observer);
if (!mFrameMetricsReporter->hasObservers()) {
- std::lock_guard lock(mFrameMetricsReporterMutex);
mFrameMetricsReporter.reset(nullptr);
}
}
@@ -245,6 +246,8 @@
*/
void reportMetricsWithPresentTime();
+ FrameInfo* getFrameInfoFromLast4(uint64_t frameNumber);
+
// The same type as Frame.mWidth and Frame.mHeight
int32_t mLastFrameWidth = 0;
int32_t mLastFrameHeight = 0;
@@ -305,7 +308,8 @@
std::string mName;
JankTracker mJankTracker;
FrameInfoVisualizer mProfiler;
- std::unique_ptr<FrameMetricsReporter> mFrameMetricsReporter;
+ std::unique_ptr<FrameMetricsReporter> mFrameMetricsReporter
+ GUARDED_BY(mFrameMetricsReporterMutex);
std::mutex mFrameMetricsReporterMutex;
std::set<RenderNode*> mPrefetchedLayers;
