Revert "SurfaceFlinger: Share ownership of layers between State and Handle."
This reverts commit f478f9f70b544d9f93680d6734a1af45bbd7f509.
Reason for revert: Causes presubmit issues.
Change-Id: Ibe872baed863a8ee9b9da5722e1671d087ec3a9f
Bug: 118257858
Bug: 62536731
Bug: 111373437
Bug: 111297488
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index badfc6b..f0723e8 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -2878,14 +2878,7 @@
for (const auto& l : mLayersPendingRemoval) {
recordBufferingStats(l->getName().string(),
l->getOccupancyHistory(true));
-
- // We need to release the HWC layers when the Layer is removed
- // from the current state otherwise the HWC layer just continues
- // showing at it's last configured state until we eventually
- // abandon the buffer queue.
- if (l->isRemovedFromCurrentState()) {
- l->destroyAllHwcLayers();
- }
+ l->onRemoved();
}
mLayersPendingRemoval.clear();
}
@@ -3318,6 +3311,10 @@
if (parent == nullptr) {
mCurrentState.layersSortedByZ.add(lbc);
} else {
+ if (parent->isPendingRemoval()) {
+ ALOGE("addClientLayer called with a removed parent");
+ return NAME_NOT_FOUND;
+ }
parent->addChild(lbc);
}
@@ -3344,22 +3341,52 @@
return removeLayerLocked(mStateLock, layer, topLevelOnly);
}
-status_t SurfaceFlinger::removeLayerLocked(const Mutex& lock, const sp<Layer>& layer,
+status_t SurfaceFlinger::removeLayerLocked(const Mutex&, const sp<Layer>& layer,
bool topLevelOnly) {
+ if (layer->isPendingRemoval()) {
+ return NO_ERROR;
+ }
+
const auto& p = layer->getParent();
ssize_t index;
if (p != nullptr) {
if (topLevelOnly) {
return NO_ERROR;
}
+
+ sp<Layer> ancestor = p;
+ while (ancestor->getParent() != nullptr) {
+ ancestor = ancestor->getParent();
+ }
+ if (mCurrentState.layersSortedByZ.indexOf(ancestor) < 0) {
+ ALOGE("removeLayer called with a layer whose parent has been removed");
+ return NAME_NOT_FOUND;
+ }
+
index = p->removeChild(layer);
} else {
index = mCurrentState.layersSortedByZ.remove(layer);
}
- layer->onRemovedFromCurrentState();
+ // 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
+ // so we will succeed in promoting it, but it's already been removed
+ // from mCurrentState. As long as we can find it in mDrawingState we have no problem
+ // otherwise something has gone wrong and we are leaking the layer.
+ if (index < 0 && mDrawingState.layersSortedByZ.indexOf(layer) < 0) {
+ ALOGE("Failed to find layer (%s) in layer parent (%s).",
+ layer->getName().string(),
+ (p != nullptr) ? p->getName().string() : "no-parent");
+ return BAD_VALUE;
+ } else if (index < 0) {
+ return NO_ERROR;
+ }
- markLayerPendingRemovalLocked(lock, layer);
+ layer->onRemovedFromCurrentState();
+ mLayersPendingRemoval.add(layer);
+ mLayersRemoved = true;
+ mNumLayers -= 1 + layer->getChildrenCount();
+ setTransactionFlags(eTransactionNeeded);
return NO_ERROR;
}
@@ -3560,6 +3587,11 @@
return 0;
}
+ if (layer->isPendingRemoval()) {
+ ALOGW("Attempting to set client state on removed layer: %s", layer->getName().string());
+ return 0;
+ }
+
uint32_t flags = 0;
const uint32_t what = s.what;
@@ -3763,6 +3795,11 @@
return;
}
+ if (layer->isPendingRemoval()) {
+ ALOGW("Attempting to destroy on removed layer: %s", layer->getName().string());
+ return;
+ }
+
if (state.what & layer_state_t::eDestroySurface) {
removeLayerLocked(mStateLock, layer);
}
@@ -3935,16 +3972,17 @@
return err;
}
-void SurfaceFlinger::markLayerPendingRemovalLocked(const Mutex&, const sp<Layer>& layer) {
- mLayersPendingRemoval.add(layer);
- mLayersRemoved = true;
- setTransactionFlags(eTransactionNeeded);
-}
-
-void SurfaceFlinger::onHandleDestroyed(const sp<Layer>& layer)
+status_t SurfaceFlinger::onLayerDestroyed(const wp<Layer>& layer)
{
- Mutex::Autolock lock(mStateLock);
- markLayerPendingRemovalLocked(mStateLock, layer);
+ // called by ~LayerCleaner() when all references to the IBinder (handle)
+ // are gone
+ sp<Layer> l = layer.promote();
+ if (l == nullptr) {
+ // The layer has already been removed, carry on
+ return NO_ERROR;
+ }
+ // If we have a parent, then we can continue to live as long as it does.
+ return removeLayer(l, true);
}
// ---------------------------------------------------------------------------
@@ -5337,7 +5375,7 @@
auto layerHandle = reinterpret_cast<Layer::Handle*>(layerHandleBinder.get());
auto parent = layerHandle->owner.promote();
- if (parent == nullptr || parent->isRemovedFromCurrentState()) {
+ if (parent == nullptr || parent->isPendingRemoval()) {
ALOGE("captureLayers called with a removed parent");
return NAME_NOT_FOUND;
}