Merge "SurfaceFlinger/LayerRenderArea: Hold lock while modifying hierarchy" into tm-dev

2 files changed, 15 insertions, 12 deletions

diff --git a/services/surfaceflinger/LayerRenderArea.cpp b/services/surfaceflinger/LayerRenderArea.cpp
index a1e14559e9..896f25404d 100644
--- a/services/surfaceflinger/LayerRenderArea.cpp
+++ b/services/surfaceflinger/LayerRenderArea.cpp
@@ -26,18 +26,12 @@
 namespace android {
 namespace {
 
-struct ReparentForDrawing {
-    const sp<Layer>& oldParent;
-
-    ReparentForDrawing(const sp<Layer>& oldParent, const sp<Layer>& newParent,
-                       const Rect& drawingBounds)
-          : oldParent(oldParent) {
+void reparentForDrawing(const sp<Layer>& oldParent, const sp<Layer>& newParent,
+                   const Rect& drawingBounds) {
         // Compute and cache the bounds for the new parent layer.
         newParent->computeBounds(drawingBounds.toFloatRect(), ui::Transform(),
-                                 0.f /* shadowRadius */);
+            0.f /* shadowRadius */);
         oldParent->setChildrenDrawingParent(newParent);
-    }
-    ~ReparentForDrawing() { oldParent->setChildrenDrawingParent(oldParent); }
 };
 
 } // namespace
@@ -114,11 +108,19 @@ void LayerRenderArea::render(std::function<void()> drawLayers) {
     } else {
         // In the "childrenOnly" case we reparent the children to a screenshot
         // layer which has no properties set and which does not draw.
+        //  We hold the statelock as the reparent-for-drawing operation modifies the
+        //  hierarchy and there could be readers on Binder threads, like dump.
         sp<ContainerLayer> screenshotParentLayer = mFlinger.getFactory().createContainerLayer(
-                {&mFlinger, nullptr, "Screenshot Parent"s, 0, LayerMetadata()});
-
-        ReparentForDrawing reparent(mLayer, screenshotParentLayer, sourceCrop);
+                  {&mFlinger, nullptr, "Screenshot Parent"s, 0, LayerMetadata()});
+        {
+            Mutex::Autolock _l(mFlinger.mStateLock);
+            reparentForDrawing(mLayer, screenshotParentLayer, sourceCrop);
+        }
         drawLayers();
+        {
+            Mutex::Autolock _l(mFlinger.mStateLock);
+            mLayer->setChildrenDrawingParent(mLayer);
+        }
     }
 }
 
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index 370afc1265..81afa9b0fa 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -375,6 +375,7 @@ private:
     friend class MonitoredProducer;
     friend class RefreshRateOverlay;
     friend class RegionSamplingThread;
+    friend class LayerRenderArea;
     friend class LayerTracing;
 
     // For unit tests