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

Commit a3d73dab authored by Jim Shargo's avatar Jim Shargo
Browse files

SurfaceComposer: avoid data races by holding lock more 

There's a data race between SurfaceComposerClient::dispose and
SurfaceComposerClient::getClient which I think is the likely culprit
behind the linked bug's weird refcounting error.

Bug: 408094875
Flag: EXEMPT small bug
Test: presubmit
Change-Id: Id705453200344bb4dc1c88f07dd03504f74cf1b1
parent 7918b0ed

libs/gui/SurfaceComposerClient.cpp

+15 −1
Original line number Diff line number Diff line
@@ -34,6 +34,7 @@ 
#include <gui/TraceUtils.h>

#include <utils/Errors.h>

#include <utils/Log.h>

#include <utils/Mutex.h>

#include <utils/SortedVector.h>

#include <utils/String8.h>

#include <utils/threads.h>
@@ -2399,6 +2400,7 @@ SurfaceComposerClient::SurfaceComposerClient(const sp<ISurfaceComposerClient>& c 


void SurfaceComposerClient::onFirstRef() {

    sp<gui::ISurfaceComposer> sf(ComposerServiceAIDL::getComposerService());

    Mutex::Autolock _lm(mLock);

    if (sf != nullptr && mStatus == NO_INIT) {

        sp<ISurfaceComposerClient> conn;

        binder::Status status = sf->createConnection(&conn);
@@ -2414,10 +2416,12 @@ SurfaceComposerClient::~SurfaceComposerClient() { 
}



status_t SurfaceComposerClient::initCheck() const {

    Mutex::Autolock _lm(mLock);

    return mStatus;

}



sp<IBinder> SurfaceComposerClient::connection() const {

    Mutex::Autolock _lm(mLock);

    return IInterface::asBinder(mClient);

}
@@ -2466,6 +2470,7 @@ status_t SurfaceComposerClient::createSurfaceChecked(const String8& name, uint32 
                                                     const sp<IBinder>& parentHandle,

                                                     LayerMetadata metadata,

                                                     uint32_t* outTransformHint) {

    Mutex::Autolock _lm(mLock);

    status_t err = mStatus;



    if (mStatus == NO_ERROR) {
@@ -2491,6 +2496,8 @@ sp<SurfaceControl> SurfaceComposerClient::mirrorSurface(SurfaceControl* mirrorFr 
        return nullptr;

    }



    Mutex::Autolock _lm(mLock);



    sp<IBinder> mirrorFromHandle = mirrorFromSurface->getHandle();

    gui::CreateSurfaceResult result;

    const binder::Status status = mClient->mirrorSurface(mirrorFromHandle, &result);
@@ -2503,6 +2510,8 @@ sp<SurfaceControl> SurfaceComposerClient::mirrorSurface(SurfaceControl* mirrorFr 
}



sp<SurfaceControl> SurfaceComposerClient::mirrorDisplay(DisplayId displayId) {

    Mutex::Autolock _lm(mLock);



    gui::CreateSurfaceResult result;

    const binder::Status status = mClient->mirrorDisplay(displayId.value, &result);

    const status_t err = statusTFromBinderStatus(status);
@@ -2514,6 +2523,8 @@ sp<SurfaceControl> SurfaceComposerClient::mirrorDisplay(DisplayId displayId) { 
}



status_t SurfaceComposerClient::clearLayerFrameStats(const sp<IBinder>& token) const {

    Mutex::Autolock _lm(mLock);



    if (mStatus != NO_ERROR) {

        return mStatus;

    }
@@ -2523,9 +2534,12 @@ status_t SurfaceComposerClient::clearLayerFrameStats(const sp<IBinder>& token) c 


status_t SurfaceComposerClient::getLayerFrameStats(const sp<IBinder>& token,

    FrameStats* outStats) const {

    Mutex::Autolock _lm(mLock);



    if (mStatus != NO_ERROR) {

        return mStatus;

    }



    gui::FrameStats stats;

    const binder::Status status = mClient->getLayerFrameStats(token, &stats);

    if (status.isOk()) {

libs/gui/include/gui/SurfaceComposerClient.h

+7 −3
Original line number Diff line number Diff line
@@ -28,6 +28,7 @@ 


#include <utils/Errors.h>

#include <utils/RefBase.h>

#include <utils/Mutex.h>

#include <utils/Singleton.h>

#include <utils/SortedVector.h>

#include <utils/threads.h>
@@ -825,7 +826,10 @@ public: 
    static void setDisplayProjection(const sp<IBinder>& token, ui::Rotation orientation,

                                     const Rect& layerStackRect, const Rect& displayRect);



    inline sp<ISurfaceComposerClient> getClient() { return mClient; }

    inline sp<ISurfaceComposerClient> getClient() {

      Mutex::Autolock _lm(mLock);

      return mClient;

    }



    static status_t getDisplayedContentSamplingAttributes(const sp<IBinder>& display,

                                                          ui::PixelFormat* outFormat,
@@ -872,8 +876,8 @@ private: 
    virtual void onFirstRef();



    mutable     Mutex                       mLock;

                status_t                    mStatus;

                sp<ISurfaceComposerClient>  mClient;

                status_t                    mStatus GUARDED_BY(mLock);

                sp<ISurfaceComposerClient>  mClient GUARDED_BY(mLock);

};



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