Remove root layer when clearing mLayersPendingRemoval
The previous code removed the root layer from layersSortedByZ when the
Layer handle was destroyed. However this causes issues if the Layer is
created and then dropped quickly on the client side. What happened was
the following:
1. Client creates a Layer
2. SF creates the Layer, but doesn't add it to the hierarchy yet
3. Client drops the Layer reference
4. LayerCleaner dtor is called which calls SF#onHandleDestroyed. This
removed the Layer from layersSortedByZ and added it to mLayersPendingRemoval
5. SF main thread runs and adds the Layer to the hierarchy. So for the
case of root, it adds it to layersSortedByZ
6. mLayersPendingRemoval is traversed and cleared, but since layersSortedByZ still
has a reference to the Layer, it's never destroyed.
With this change, the Layer will be created, but then removed from layersSortedByZ
when mLayersPendingRemoval is traversed. That way the Layer reference
can be dropped.
Test: SurfaceFlingerStress#create_and_destroy
Fixes: 229754763
Change-Id: I0537781d6e13fbdeb2502ed6a7e65dbcfd7b28ce
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 1d43c6e..e7b8792 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -3456,6 +3456,15 @@
l->latchAndReleaseBuffer();
}
+ // If a layer has a parent, we allow it to out-live it's handle
+ // 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 (l->isAtRoot()) {
+ l->setIsAtRoot(false);
+ mCurrentState.layersSortedByZ.remove(l);
+ }
+
// If the layer has been removed and has no parent, then it will not be reachable
// when traversing layers on screen. Add the layer to the offscreenLayers set to
// ensure we can copy its current to drawing state.
@@ -4729,14 +4738,6 @@
void SurfaceFlinger::onHandleDestroyed(BBinder* handle, sp<Layer>& layer) {
Mutex::Autolock lock(mStateLock);
- // If a layer has a parent, we allow it to out-live it's handle
- // 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->isAtRoot()) {
- layer->setIsAtRoot(false);
- mCurrentState.layersSortedByZ.remove(layer);
- }
markLayerPendingRemovalLocked(layer);
mBufferCountTracker.remove(handle);
layer.clear();