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

1 files changed, 12 insertions, 8 deletions

diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index a49e8f4aa8..6174185969 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -2679,14 +2679,18 @@ status_t SurfaceFlinger::addClientLayer(const sp<Client>& client,
     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 @@ status_t SurfaceFlinger::removeLayer(const sp<Layer>& layer) {
             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 @@ status_t SurfaceFlinger::onLayerDestroyed(const wp<Layer>& layer)
     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);
 }
 
 // ---------------------------------------------------------------------------