Avoid promoting weak reference of Layer on Binder thread.
BufferQueueLayer registers itself to ConsumerBase so it can receive
callbacks about incoming frames. When doing this, ConsumerBase holds a
weak reference to BufferQueueLayer. This would normally be fine except
ConsumerBase promotes the weak pointer on a Binder thread.
This leads to a race where the layer is ready to get destroyed in
SurfaceFlinger, but ConsumerBase promotes the layer. The layer will get
destroyed after the function finishes, but now it gets destroyed off the
main thread. When this happens, a lot of unexpected behavior occurs
since the layer destructor expects the call to only happen on the main
thread and therefore, doesn't hold any locks.
Instead of giving ConsumerBase a weak reference to the layer, we can
give it a reference to a listener. This listener can be managed by
BufferQueueLayer and will only hold a raw reference to the Layer.
ConsumerBase can promote the listener, but it will not be holding a
strong reference to the Layer, allowing the layer to get destroyed on
the main thread.
We hold a lock when accessing the raw reference to ensure it's not being
destroyed. The raw reference is nulled when the layer is getting
destroyed to ensure no more calls are made to the layer after it's
destroyed.
Fixes: 143735125
Fixes: 143200062
Test: go/wm-smoke
Change-Id: I47122b0e4c6d73f23a64027fd3622b2d7fd28871
diff --git a/services/surfaceflinger/BufferQueueLayer.cpp b/services/surfaceflinger/BufferQueueLayer.cpp
index d51d34b..543b0c4 100644
--- a/services/surfaceflinger/BufferQueueLayer.cpp
+++ b/services/surfaceflinger/BufferQueueLayer.cpp
@@ -35,6 +35,7 @@
BufferQueueLayer::BufferQueueLayer(const LayerCreationArgs& args) : BufferLayer(args) {}
BufferQueueLayer::~BufferQueueLayer() {
+ mContentsChangedListener->abandon();
mConsumer->abandon();
}
@@ -480,7 +481,9 @@
mFlinger->getFactory().createBufferLayerConsumer(consumer, mFlinger->getRenderEngine(),
mTextureName, this);
mConsumer->setConsumerUsageBits(getEffectiveUsage(0));
- mConsumer->setContentsChangedListener(this);
+
+ mContentsChangedListener = new ContentsChangedListener(this);
+ mConsumer->setContentsChangedListener(mContentsChangedListener);
mConsumer->setName(String8(mName.data(), mName.size()));
// BufferQueueCore::mMaxDequeuedBufferCount is default to 1
@@ -552,4 +555,57 @@
return layer;
}
+// -----------------------------------------------------------------------
+// Interface implementation for BufferLayerConsumer::ContentsChangedListener
+// -----------------------------------------------------------------------
+
+void BufferQueueLayer::ContentsChangedListener::onFrameAvailable(const BufferItem& item) {
+ Mutex::Autolock lock(mMutex);
+ if (mBufferQueueLayer != nullptr) {
+ mBufferQueueLayer->onFrameAvailable(item);
+ }
+}
+
+void BufferQueueLayer::ContentsChangedListener::onFrameReplaced(const BufferItem& item) {
+ Mutex::Autolock lock(mMutex);
+ if (mBufferQueueLayer != nullptr) {
+ mBufferQueueLayer->onFrameReplaced(item);
+ }
+}
+
+void BufferQueueLayer::ContentsChangedListener::onSidebandStreamChanged() {
+ Mutex::Autolock lock(mMutex);
+ if (mBufferQueueLayer != nullptr) {
+ mBufferQueueLayer->onSidebandStreamChanged();
+ }
+}
+
+void BufferQueueLayer::ContentsChangedListener::onFrameDequeued(const uint64_t bufferId) {
+ Mutex::Autolock lock(mMutex);
+ if (mBufferQueueLayer != nullptr) {
+ mBufferQueueLayer->onFrameDequeued(bufferId);
+ }
+}
+
+void BufferQueueLayer::ContentsChangedListener::onFrameDetached(const uint64_t bufferId) {
+ Mutex::Autolock lock(mMutex);
+ if (mBufferQueueLayer != nullptr) {
+ mBufferQueueLayer->onFrameDetached(bufferId);
+ }
+}
+
+void BufferQueueLayer::ContentsChangedListener::onFrameCancelled(const uint64_t bufferId) {
+ Mutex::Autolock lock(mMutex);
+ if (mBufferQueueLayer != nullptr) {
+ mBufferQueueLayer->onFrameCancelled(bufferId);
+ }
+}
+
+void BufferQueueLayer::ContentsChangedListener::abandon() {
+ Mutex::Autolock lock(mMutex);
+ mBufferQueueLayer = nullptr;
+}
+
+// -----------------------------------------------------------------------
+
} // namespace android