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

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

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
parent e41dbe6a

services/surfaceflinger/SurfaceFlinger.cpp

+12 −8
Original line number Diff line number Diff line
@@ -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);

}



// ---------------------------------------------------------------------------

services/surfaceflinger/SurfaceFlinger.h

+1 −1
Original line number Diff line number Diff line
@@ -396,7 +396,7 @@ private: 
    status_t onLayerDestroyed(const wp<Layer>& layer);



    // remove a layer from SurfaceFlinger immediately

    status_t removeLayer(const sp<Layer>& layer);

    status_t removeLayer(const sp<Layer>& layer, bool topLevelOnly = false);



    // add a layer to SurfaceFlinger

    status_t addClientLayer(const sp<Client>& client,

services/surfaceflinger/SurfaceFlinger_hwc1.cpp

+13 −7
Original line number Diff line number Diff line
@@ -2339,12 +2339,20 @@ 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);

    } else {

        index = mCurrentState.layersSortedByZ.remove(layer);

    }



    // 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
@@ -2769,11 +2777,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);

}



// ---------------------------------------------------------------------------