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

Commit 6e592063 authored by Chavi Weingarten's avatar Chavi Weingarten
Browse files

Use strong reference to parent Handle when creating layer 

If LayerCreatedState holds a weak reference to the parent handle
the following can occur:
1. handleLayerCreatedLocked is called and promotes the handle to
a strong ref.

2. The client reference to that layer is removed.

3. When handleLayerCreatedLocked returns, the last reference is
dropped. The Handle dtor is called which then calls
onHandleDestroyed. That function tries to acquire the lock, but
it's already held by the caller.

4. This will cause a deadlock.

Instead, hold a strong reference in the LayerCreatedState. The
reference will be held until flushTransactionQueues finishes,
which will then clear the Handle reference. This occurs outside
the lock so there should be no issue

Fixes: 202621651
Test: Hard to reproduce, but no other issues
Change-Id: I0b1ea54b63cbdef1fcf96eedffa4fbfa6959f697
parent 5feba92d

services/surfaceflinger/SurfaceFlinger.cpp

+3 −3
Original line number Diff line number Diff line
@@ -6758,7 +6758,7 @@ void TransactionState::traverseStatesWithBuffers( 
}



void SurfaceFlinger::setLayerCreatedState(const sp<IBinder>& handle, const wp<Layer>& layer,

                                          const wp<IBinder>& parent, const wp<Layer> parentLayer,

                                          const sp<IBinder>& parent, const wp<Layer> parentLayer,

                                          const wp<IBinder>& producer, bool addToRoot) {

    Mutex::Autolock lock(mCreatedLayersLock);

    mCreatedLayers[handle->localBinder()] =
@@ -6802,9 +6802,9 @@ sp<Layer> SurfaceFlinger::handleLayerCreatedLocked(const sp<IBinder>& handle) { 
    sp<Layer> parent;

    bool allowAddRoot = state->addToRoot;

    if (state->initialParent != nullptr) {

        parent = fromHandle(state->initialParent.promote()).promote();

        parent = fromHandle(state->initialParent).promote();

        if (parent == nullptr) {

            ALOGE("Invalid parent %p", state->initialParent.unsafe_get());

            ALOGE("Invalid parent %p", state->initialParent.get());

            allowAddRoot = false;

        }

    } else if (state->initialParentLayer != nullptr) {

services/surfaceflinger/SurfaceFlinger.h

+3 −3
Original line number Diff line number Diff line
@@ -1344,7 +1344,7 @@ private: 
            GUARDED_BY(mStateLock);

    mutable Mutex mCreatedLayersLock;

    struct LayerCreatedState {

        LayerCreatedState(const wp<Layer>& layer, const wp<IBinder>& parent,

        LayerCreatedState(const wp<Layer>& layer, const sp<IBinder>& parent,

                          const wp<Layer> parentLayer, const wp<IBinder>& producer, bool addToRoot)

              : layer(layer),

                initialParent(parent),
@@ -1354,7 +1354,7 @@ private: 
        wp<Layer> layer;

        // Indicates the initial parent of the created layer, only used for creating layer in

        // SurfaceFlinger. If nullptr, it may add the created layer into the current root layers.

        wp<IBinder> initialParent;

        sp<IBinder> initialParent;

        wp<Layer> initialParentLayer;

        // Indicates the initial graphic buffer producer of the created layer, only used for

        // creating layer in SurfaceFlinger.
@@ -1369,7 +1369,7 @@ private: 
    // thread.

    std::unordered_map<BBinder*, std::unique_ptr<LayerCreatedState>> mCreatedLayers;

    void setLayerCreatedState(const sp<IBinder>& handle, const wp<Layer>& layer,

                              const wp<IBinder>& parent, const wp<Layer> parentLayer,

                              const sp<IBinder>& parent, const wp<Layer> parentLayer,

                              const wp<IBinder>& producer, bool addToRoot);

    auto getLayerCreatedState(const sp<IBinder>& handle);

    sp<Layer> handleLayerCreatedLocked(const sp<IBinder>& handle) REQUIRES(mStateLock);