SF: Avoid promoting parent layer in binder thread
Some components in SF extend the life cycle of layer objects
causing them to be destroyed without the state lock held. If a binder
thread promotes a layer, there is a chance it will be left holding the
last reference. This will cause the layer to be destroyed in the binder
thread, resulting in invalid accesses and crashes.
Fix this by tracking root layers with a variable to avoid promoting
parent layer in binder thread.
Test: presubmit, manually test refresh rate overlay
Test: go/wm-smoke
Fixes: 186412934
Change-Id: Icd9f4e851bbd92c887e113e52505ce4d8eb3ea0c
diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h
index e726d37..6b56b2d 100644
--- a/services/surfaceflinger/Layer.h
+++ b/services/surfaceflinger/Layer.h
@@ -796,6 +796,11 @@
// for symmetry with Vector::remove
ssize_t removeChild(const sp<Layer>& layer);
sp<Layer> getParent() const { return mCurrentParent.promote(); }
+
+ // Should be called with the surfaceflinger statelock held
+ bool isAtRoot() const { return mIsAtRoot; }
+ void setIsAtRoot(bool isAtRoot) { mIsAtRoot = isAtRoot; }
+
bool hasParent() const { return getParent() != nullptr; }
Rect getScreenBounds(bool reduceTransparentRegion = true) const;
bool setChildLayer(const sp<Layer>& childLayer, int32_t z);
@@ -1103,6 +1108,8 @@
// A list of regions on this layer that should have blurs.
const std::vector<BlurRegion> getBlurRegions() const;
+
+ bool mIsAtRoot = false;
};
std::ostream& operator<<(std::ostream& stream, const Layer::FrameRate& rate);
diff --git a/services/surfaceflinger/RefreshRateOverlay.cpp b/services/surfaceflinger/RefreshRateOverlay.cpp
index 663e62a..27a1c28 100644
--- a/services/surfaceflinger/RefreshRateOverlay.cpp
+++ b/services/surfaceflinger/RefreshRateOverlay.cpp
@@ -197,6 +197,7 @@
Mutex::Autolock _l(mFlinger.mStateLock);
mLayer = mClient->getLayerUser(mIBinder);
mLayer->setFrameRate(Layer::FrameRate(Fps(0.0f), Layer::FrameRateCompatibility::NoVote));
+ mLayer->setIsAtRoot(true);
// setting Layer's Z requires resorting layersSortedByZ
ssize_t idx = mFlinger.mDrawingState.layersSortedByZ.indexOf(mLayer);
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 2650fa0..7270016 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -4181,6 +4181,7 @@
: nullptr;
if (layer->reparent(parentHandle)) {
if (!hadParent) {
+ layer->setIsAtRoot(false);
mCurrentState.layersSortedByZ.remove(layer);
}
flags |= eTransactionNeeded | eTraversalNeeded;
@@ -4447,7 +4448,8 @@
// with the idea that the parent holds a reference and will eventually
// be cleaned up. However no one cleans up the top-level so we do so
// here.
- if (layer->getParent() == nullptr) {
+ if (layer->isAtRoot()) {
+ layer->setIsAtRoot(false);
mCurrentState.layersSortedByZ.remove(layer);
}
markLayerPendingRemovalLocked(layer);
@@ -6924,6 +6926,7 @@
}
if (parent == nullptr && allowAddRoot) {
+ layer->setIsAtRoot(true);
mCurrentState.layersSortedByZ.add(layer);
} else if (parent == nullptr) {
layer->onRemovedFromCurrentState();
