Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit fae51c43 authored by Chia-I Wu's avatar Chia-I Wu
Browse files

surfaceflinger: fix a potential child layer leak 

We should not remove a child layer from its already removed parent.
Call p->removeChild only after we've checked that the ancestor is
alive.

Apply e6b63e1a and this fix to
SurfaceFlinger_hwc1.cpp as well.

Bug: 37121786
Test: manual stress test
Change-Id: I7b811450a998acc4ad9690bd4eda058ce6588e14
parent 515dc9c5

services/surfaceflinger/SurfaceFlinger.cpp

+2 −2
Original line number Original line Diff line number Diff line
@@ -2689,8 +2689,6 @@ status_t SurfaceFlinger::removeLayer(const sp<Layer>& layer, bool topLevelOnly) 
            return NO_ERROR;

            return NO_ERROR;

        }

        }





        index = p->removeChild(layer);



        sp<Layer> ancestor = p;

        sp<Layer> ancestor = p;

        while (ancestor->getParent() != nullptr) {

        while (ancestor->getParent() != nullptr) {

            ancestor = ancestor->getParent();

            ancestor = ancestor->getParent();
@@ -2699,6 +2697,8 @@ status_t SurfaceFlinger::removeLayer(const sp<Layer>& layer, bool topLevelOnly) 
            ALOGE("removeLayer called with a layer whose parent has been removed");

            ALOGE("removeLayer called with a layer whose parent has been removed");

            return NAME_NOT_FOUND;

            return NAME_NOT_FOUND;

        }

        }



        index = p->removeChild(layer);

    } else {

    } else {

        index = mCurrentState.layersSortedByZ.remove(layer);

        index = mCurrentState.layersSortedByZ.remove(layer);

    }

    }

services/surfaceflinger/SurfaceFlinger_hwc1.cpp

+15 −1
Original line number Original line Diff line number Diff line
@@ -2326,8 +2326,13 @@ status_t SurfaceFlinger::addClientLayer(const sp<Client>& client, 
        if (parent == nullptr) {

        if (parent == nullptr) {

            mCurrentState.layersSortedByZ.add(lbc);

            mCurrentState.layersSortedByZ.add(lbc);

        } else {

        } else {

            if (mCurrentState.layersSortedByZ.indexOf(parent) < 0) {

                ALOGE("addClientLayer called with a removed parent");

                return NAME_NOT_FOUND;

            }

            parent->addChild(lbc);

            parent->addChild(lbc);

        }

        }



        mGraphicBufferProducerList.add(IInterface::asBinder(gbc));

        mGraphicBufferProducerList.add(IInterface::asBinder(gbc));

        mLayersAdded = true;

        mLayersAdded = true;

        mNumLayers++;

        mNumLayers++;
@@ -2349,6 +2354,15 @@ status_t SurfaceFlinger::removeLayer(const sp<Layer>& layer, bool topLevelOnly) 
            return NO_ERROR;

            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);

        index = p->removeChild(layer);

    } else {

    } else {

        index = mCurrentState.layersSortedByZ.remove(layer);

        index = mCurrentState.layersSortedByZ.remove(layer);
@@ -2370,7 +2384,7 @@ status_t SurfaceFlinger::removeLayer(const sp<Layer>& layer, bool topLevelOnly) 




    mLayersPendingRemoval.add(layer);

    mLayersPendingRemoval.add(layer);

    mLayersRemoved = true;

    mLayersRemoved = true;

    mNumLayers--;

    mNumLayers -= 1 + layer->getChildrenCount();

    setTransactionFlags(eTransactionNeeded);

    setTransactionFlags(eTransactionNeeded);

    return NO_ERROR;

    return NO_ERROR;

}

}