surfaceflinger: Layer::getParent requires state lock held
We rely on mStateLock to synchronize accesses to
Layer::mCurrentParent.
Bug: 38505866
Test: manual stress test
Change-Id: I5f8ec358ed7e35df28f8c6aec31ae6ee51cb5b93
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index a49e8f4..6174185 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -2679,14 +2679,18 @@
return NO_ERROR;
}
-status_t SurfaceFlinger::removeLayer(const sp<Layer>& layer) {
+status_t SurfaceFlinger::removeLayer(const sp<Layer>& layer, bool topLevelOnly) {
Mutex::Autolock _l(mStateLock);
const auto& p = layer->getParent();
- const ssize_t index = (p != nullptr) ? p->removeChild(layer) :
- mCurrentState.layersSortedByZ.remove(layer);
-
+ ssize_t index;
if (p != nullptr) {
+ if (topLevelOnly) {
+ return NO_ERROR;
+ }
+
+ index = p->removeChild(layer);
+
sp<Layer> ancestor = p;
while (ancestor->getParent() != nullptr) {
ancestor = ancestor->getParent();
@@ -2695,6 +2699,8 @@
ALOGE("removeLayer called with a layer whose parent has been removed");
return NAME_NOT_FOUND;
}
+ } else {
+ index = mCurrentState.layersSortedByZ.remove(layer);
}
// As a matter of normal operation, the LayerCleaner will produce a second
@@ -3125,11 +3131,9 @@
if (l == nullptr) {
// The layer has already been removed, carry on
return NO_ERROR;
- } if (l->getParent() != nullptr) {
- // If we have a parent, then we can continue to live as long as it does.
- return NO_ERROR;
}
- return removeLayer(l);
+ // If we have a parent, then we can continue to live as long as it does.
+ return removeLayer(l, true);
}
// ---------------------------------------------------------------------------
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index c89e26f..9239538 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -396,7 +396,7 @@
status_t onLayerDestroyed(const wp<Layer>& layer);
// remove a layer from SurfaceFlinger immediately
- status_t removeLayer(const sp<Layer>& layer);
+ status_t removeLayer(const sp<Layer>& layer, bool topLevelOnly = false);
// add a layer to SurfaceFlinger
status_t addClientLayer(const sp<Client>& client,
diff --git a/services/surfaceflinger/SurfaceFlinger_hwc1.cpp b/services/surfaceflinger/SurfaceFlinger_hwc1.cpp
index e19e021..3d421d2 100644
--- a/services/surfaceflinger/SurfaceFlinger_hwc1.cpp
+++ b/services/surfaceflinger/SurfaceFlinger_hwc1.cpp
@@ -2339,12 +2339,20 @@
return NO_ERROR;
}
-status_t SurfaceFlinger::removeLayer(const sp<Layer>& layer) {
+status_t SurfaceFlinger::removeLayer(const sp<Layer>& layer, bool topLevelOnly) {
Mutex::Autolock _l(mStateLock);
const auto& p = layer->getParent();
- const ssize_t index = (p != nullptr) ? p->removeChild(layer) :
- mCurrentState.layersSortedByZ.remove(layer);
+ ssize_t index;
+ if (p != nullptr) {
+ if (topLevelOnly) {
+ return NO_ERROR;
+ }
+
+ index = p->removeChild(layer);
+ } else {
+ index = mCurrentState.layersSortedByZ.remove(layer);
+ }
// As a matter of normal operation, the LayerCleaner will produce a second
// attempt to remove the surface. The Layer will be kept alive in mDrawingState
@@ -2769,11 +2777,9 @@
if (l == nullptr) {
// The layer has already been removed, carry on
return NO_ERROR;
- } if (l->getParent() != nullptr) {
- // If we have a parent, then we can continue to live as long as it does.
- return NO_ERROR;
}
- return removeLayer(l);
+ // If we have a parent, then we can continue to live as long as it does.
+ return removeLayer(l, true);
}
// ---------------------------------------------------------------------------