Release RefreshRateOveray's SurfaceControl on the main thread
SurfaceControl assumes that its destruction happens without
SurfaceFlinger::mStateLock held, otherwise it causes a deadlock.
Bug: 237485868
Test: Enable refresh rate overlay and change resolution
Change-Id: I152586aaaf414ed0b90a6e6a8d78e867146ab96c
Merged-In: I152586aaaf414ed0b90a6e6a8d78e867146ab96c
diff --git a/services/surfaceflinger/RefreshRateOverlay.cpp b/services/surfaceflinger/RefreshRateOverlay.cpp
index d4435c2..a9180d4 100644
--- a/services/surfaceflinger/RefreshRateOverlay.cpp
+++ b/services/surfaceflinger/RefreshRateOverlay.cpp
@@ -16,9 +16,10 @@
#include <algorithm>
-#include "RefreshRateOverlay.h"
+#include "BackgroundExecutor.h"
#include "Client.h"
#include "Layer.h"
+#include "RefreshRateOverlay.h"
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wconversion"
@@ -56,6 +57,14 @@
} // namespace
+SurfaceControlHolder::~SurfaceControlHolder() {
+ // Hand the sp<SurfaceControl> to the helper thread to release the last
+ // reference. This makes sure that the SurfaceControl is destructed without
+ // SurfaceFlinger::mStateLock held.
+ BackgroundExecutor::getInstance().sendCallbacks(
+ {[sc = std::move(mSurfaceControl)]() mutable { sc.clear(); }});
+}
+
void RefreshRateOverlay::SevenSegmentDrawer::drawSegment(Segment segment, int left, SkColor color,
SkCanvas& canvas) {
const SkRect rect = [&]() {
@@ -210,21 +219,27 @@
return buffers;
}
+std::unique_ptr<SurfaceControlHolder> createSurfaceControlHolder() {
+ sp<SurfaceControl> surfaceControl =
+ SurfaceComposerClient::getDefault()
+ ->createSurface(String8("RefreshRateOverlay"), kBufferWidth, kBufferHeight,
+ PIXEL_FORMAT_RGBA_8888,
+ ISurfaceComposerClient::eFXSurfaceBufferState);
+ return std::make_unique<SurfaceControlHolder>(std::move(surfaceControl));
+}
+
RefreshRateOverlay::RefreshRateOverlay(FpsRange fpsRange, bool showSpinner)
: mFpsRange(fpsRange),
mShowSpinner(showSpinner),
- mSurfaceControl(SurfaceComposerClient::getDefault()
- ->createSurface(String8("RefreshRateOverlay"), kBufferWidth,
- kBufferHeight, PIXEL_FORMAT_RGBA_8888,
- ISurfaceComposerClient::eFXSurfaceBufferState)) {
+ mSurfaceControl(createSurfaceControlHolder()) {
if (!mSurfaceControl) {
ALOGE("%s: Failed to create buffer state layer", __func__);
return;
}
- createTransaction(mSurfaceControl)
- .setLayer(mSurfaceControl, INT32_MAX - 2)
- .setTrustedOverlay(mSurfaceControl, true)
+ createTransaction(mSurfaceControl->get())
+ .setLayer(mSurfaceControl->get(), INT32_MAX - 2)
+ .setTrustedOverlay(mSurfaceControl->get(), true)
.apply();
}
@@ -233,7 +248,7 @@
if (!mSurfaceControl) return kNoBuffers;
const auto transformHint =
- static_cast<ui::Transform::RotationFlags>(mSurfaceControl->getTransformHint());
+ static_cast<ui::Transform::RotationFlags>(mSurfaceControl->get()->getTransformHint());
// Tell SurfaceFlinger about the pre-rotation on the buffer.
const auto transform = [&] {
@@ -247,7 +262,9 @@
}
}();
- createTransaction(mSurfaceControl).setTransform(mSurfaceControl, transform).apply();
+ createTransaction(mSurfaceControl->get())
+ .setTransform(mSurfaceControl->get(), transform)
+ .apply();
BufferCache::const_iterator it = mBufferCache.find({fps.getIntValue(), transformHint});
if (it == mBufferCache.end()) {
@@ -289,21 +306,21 @@
Rect frame((3 * width) >> 4, height >> 5);
frame.offsetBy(width >> 5, height >> 4);
- createTransaction(mSurfaceControl)
- .setMatrix(mSurfaceControl, frame.getWidth() / static_cast<float>(kBufferWidth), 0, 0,
- frame.getHeight() / static_cast<float>(kBufferHeight))
- .setPosition(mSurfaceControl, frame.left, frame.top)
+ createTransaction(mSurfaceControl->get())
+ .setMatrix(mSurfaceControl->get(), frame.getWidth() / static_cast<float>(kBufferWidth),
+ 0, 0, frame.getHeight() / static_cast<float>(kBufferHeight))
+ .setPosition(mSurfaceControl->get(), frame.left, frame.top)
.apply();
}
void RefreshRateOverlay::setLayerStack(ui::LayerStack stack) {
- createTransaction(mSurfaceControl).setLayerStack(mSurfaceControl, stack).apply();
+ createTransaction(mSurfaceControl->get()).setLayerStack(mSurfaceControl->get(), stack).apply();
}
void RefreshRateOverlay::changeRefreshRate(Fps fps) {
mCurrentFps = fps;
const auto buffer = getOrCreateBuffers(fps)[mFrame];
- createTransaction(mSurfaceControl).setBuffer(mSurfaceControl, buffer).apply();
+ createTransaction(mSurfaceControl->get()).setBuffer(mSurfaceControl->get(), buffer).apply();
}
void RefreshRateOverlay::animate() {
@@ -312,7 +329,7 @@
const auto& buffers = getOrCreateBuffers(*mCurrentFps);
mFrame = (mFrame + 1) % buffers.size();
const auto buffer = buffers[mFrame];
- createTransaction(mSurfaceControl).setBuffer(mSurfaceControl, buffer).apply();
+ createTransaction(mSurfaceControl->get()).setBuffer(mSurfaceControl->get(), buffer).apply();
}
} // namespace android
diff --git a/services/surfaceflinger/RefreshRateOverlay.h b/services/surfaceflinger/RefreshRateOverlay.h
index a465a36..a2966e6 100644
--- a/services/surfaceflinger/RefreshRateOverlay.h
+++ b/services/surfaceflinger/RefreshRateOverlay.h
@@ -33,6 +33,20 @@
class GraphicBuffer;
class SurfaceControl;
+class SurfaceFlinger;
+
+// Helper class to delete the SurfaceControl on a helper thread as
+// SurfaceControl assumes its destruction happens without SurfaceFlinger::mStateLock held.
+class SurfaceControlHolder {
+public:
+ explicit SurfaceControlHolder(sp<SurfaceControl> sc) : mSurfaceControl(std::move(sc)){};
+ ~SurfaceControlHolder();
+
+ const sp<SurfaceControl>& get() const { return mSurfaceControl; }
+
+private:
+ sp<SurfaceControl> mSurfaceControl;
+};
class RefreshRateOverlay {
public:
@@ -75,7 +89,7 @@
const FpsRange mFpsRange; // For color interpolation.
const bool mShowSpinner;
- const sp<SurfaceControl> mSurfaceControl;
+ const std::unique_ptr<SurfaceControlHolder> mSurfaceControl;
};
} // namespace android
