Fix crash from asynchronous GPU metrics
Making the scope more accurate that only acquire the lock when trying
to access frame info in FrameInfoVisualizer, then make it irrelevant
to the real draw operation.
Bug: 317995179
Test: 1.going to developer options
2. swapping the "profile hwui" option from "none" to "bars"
and back a couple times, no crash
Change-Id: I069a28a7e847c0c3fca94fd9c43e95382f501b80
Merged-In: I069a28a7e847c0c3fca94fd9c43e95382f501b80
diff --git a/libs/hwui/renderthread/CanvasContext.cpp b/libs/hwui/renderthread/CanvasContext.cpp
index 618c896..f690783 100644
--- a/libs/hwui/renderthread/CanvasContext.cpp
+++ b/libs/hwui/renderthread/CanvasContext.cpp
@@ -592,14 +592,10 @@
{
// 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()), mBufferParams);
+ mLightInfo, mRenderNodes, &(profiler()), mBufferParams,
+ profilerLock());
}
uint64_t frameCompleteNr = getFrameNumber();
@@ -729,7 +725,7 @@
mCurrentFrameInfo->markFrameCompleted();
mCurrentFrameInfo->set(FrameInfoIndex::GpuCompleted)
= mCurrentFrameInfo->get(FrameInfoIndex::FrameCompleted);
- std::scoped_lock lock(mFrameMetricsReporterMutex);
+ std::scoped_lock lock(mFrameInfoMutex);
mJankTracker.finishFrame(*mCurrentFrameInfo, mFrameMetricsReporter, frameCompleteNr,
mSurfaceControlGenerationId);
}
@@ -758,7 +754,7 @@
void CanvasContext::reportMetricsWithPresentTime() {
{ // acquire lock
- std::scoped_lock lock(mFrameMetricsReporterMutex);
+ std::scoped_lock lock(mFrameInfoMutex);
if (mFrameMetricsReporter == nullptr) {
return;
}
@@ -793,7 +789,7 @@
forthBehind->set(FrameInfoIndex::DisplayPresentTime) = presentTime;
{ // acquire lock
- std::scoped_lock lock(mFrameMetricsReporterMutex);
+ std::scoped_lock lock(mFrameInfoMutex);
if (mFrameMetricsReporter != nullptr) {
mFrameMetricsReporter->reportFrameMetrics(forthBehind->data(), true /*hasPresentTime*/,
frameNumber, surfaceControlId);
@@ -802,7 +798,7 @@
}
void CanvasContext::addFrameMetricsObserver(FrameMetricsObserver* observer) {
- std::scoped_lock lock(mFrameMetricsReporterMutex);
+ std::scoped_lock lock(mFrameInfoMutex);
if (mFrameMetricsReporter.get() == nullptr) {
mFrameMetricsReporter.reset(new FrameMetricsReporter());
}
@@ -816,7 +812,7 @@
}
void CanvasContext::removeFrameMetricsObserver(FrameMetricsObserver* observer) {
- std::scoped_lock lock(mFrameMetricsReporterMutex);
+ std::scoped_lock lock(mFrameInfoMutex);
if (mFrameMetricsReporter.get() != nullptr) {
mFrameMetricsReporter->removeObserver(observer);
if (!mFrameMetricsReporter->hasObservers()) {
@@ -853,7 +849,7 @@
FrameInfo* frameInfo = instance->getFrameInfoFromLast4(frameNumber, surfaceControlId);
if (frameInfo != nullptr) {
- std::scoped_lock lock(instance->mFrameMetricsReporterMutex);
+ std::scoped_lock lock(instance->mFrameInfoMutex);
frameInfo->set(FrameInfoIndex::FrameCompleted) = std::max(gpuCompleteTime,
frameInfo->get(FrameInfoIndex::SwapBuffersCompleted));
frameInfo->set(FrameInfoIndex::GpuCompleted) = std::max(
diff --git a/libs/hwui/renderthread/CanvasContext.h b/libs/hwui/renderthread/CanvasContext.h
index 32ac5af..3978fbc 100644
--- a/libs/hwui/renderthread/CanvasContext.h
+++ b/libs/hwui/renderthread/CanvasContext.h
@@ -161,6 +161,7 @@
void notifyFramePending();
FrameInfoVisualizer& profiler() { return mProfiler; }
+ std::mutex& profilerLock() { return mFrameInfoMutex; }
void dumpFrames(int fd);
void resetFrameStats();
@@ -340,8 +341,8 @@
JankTracker mJankTracker;
FrameInfoVisualizer mProfiler;
std::unique_ptr<FrameMetricsReporter> mFrameMetricsReporter
- GUARDED_BY(mFrameMetricsReporterMutex);
- std::mutex mFrameMetricsReporterMutex;
+ GUARDED_BY(mFrameInfoMutex);
+ std::mutex mFrameInfoMutex;
std::set<RenderNode*> mPrefetchedLayers;
diff --git a/libs/hwui/renderthread/IRenderPipeline.h b/libs/hwui/renderthread/IRenderPipeline.h
index 6c2cb9d..023c29a 100644
--- a/libs/hwui/renderthread/IRenderPipeline.h
+++ b/libs/hwui/renderthread/IRenderPipeline.h
@@ -67,7 +67,8 @@
const Rect& contentDrawBounds, bool opaque, const LightInfo& lightInfo,
const std::vector<sp<RenderNode>>& renderNodes,
FrameInfoVisualizer* profiler,
- const HardwareBufferRenderParams& bufferParams) = 0;
+ const HardwareBufferRenderParams& bufferParams,
+ std::mutex& profilerLock) = 0;
virtual bool swapBuffers(const Frame& frame, bool drew, const SkRect& screenDirty,
FrameInfo* currentFrameInfo, bool* requireSwap) = 0;
virtual DeferredLayerUpdater* createTextureLayer() = 0;