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

Commit e0237bbe authored by Chavi Weingarten's avatar Chavi Weingarten
Browse files

Add thread safety check to libgui build 

Fix places that had errors to ensure locks are correctly held in libgui

Test: Builds
Bug: 268091723
Change-Id: I8a94edeb649608ff66d25a482026a81074466305
parent b00dc402

libs/gui/Android.bp

+4 −0
Original line number Diff line number Diff line
@@ -254,6 +254,10 @@ cc_library_shared { 
    lto: {

        thin: true,

    },



    cflags: [

        "-Wthread-safety",

    ],

}



// Used by media codec services exclusively as a static lib for

libs/gui/BLASTBufferQueue.cpp

+30 −28
Original line number Diff line number Diff line
@@ -35,6 +35,7 @@ 
#include <private/gui/ComposerService.h>

#include <private/gui/ComposerServiceAIDL.h>



#include <android-base/thread_annotations.h>

#include <chrono>



using namespace std::chrono_literals;
@@ -63,6 +64,10 @@ namespace android { 
    ATRACE_FORMAT("%s - %s(f:%u,a:%u)" x, __FUNCTION__, mName.c_str(), mNumFrameAvailable, \

                  mNumAcquired, ##__VA_ARGS__)



#define UNIQUE_LOCK_WITH_ASSERTION(mutex) \

    std::unique_lock _lock{mutex};        \

    base::ScopedLockAssertion assumeLocked(mutex);



void BLASTBufferItemConsumer::onDisconnect() {

    Mutex::Autolock lock(mMutex);

    mPreviouslyConnected = mCurrentlyConnected;
@@ -207,7 +212,7 @@ void BLASTBufferQueue::update(const sp<SurfaceControl>& surface, uint32_t width, 
                              int32_t format) {

    LOG_ALWAYS_FATAL_IF(surface == nullptr, "BLASTBufferQueue: mSurfaceControl must not be NULL");



    std::unique_lock _lock{mMutex};

    std::lock_guard _lock{mMutex};

    if (mFormat != format) {

        mFormat = format;

        mBufferItemConsumer->setDefaultBufferFormat(convertBufferFormat(format));
@@ -277,7 +282,7 @@ void BLASTBufferQueue::transactionCommittedCallback(nsecs_t /*latchTime*/, 
                                                    const sp<Fence>& /*presentFence*/,

                                                    const std::vector<SurfaceControlStats>& stats) {

    {

        std::unique_lock _lock{mMutex};

        std::lock_guard _lock{mMutex};

        BBQ_TRACE();

        BQA_LOGV("transactionCommittedCallback");

        if (!mSurfaceControlsWithPendingCallback.empty()) {
@@ -325,7 +330,7 @@ static void transactionCallbackThunk(void* context, nsecs_t latchTime, 
void BLASTBufferQueue::transactionCallback(nsecs_t /*latchTime*/, const sp<Fence>& /*presentFence*/,

                                           const std::vector<SurfaceControlStats>& stats) {

    {

        std::unique_lock _lock{mMutex};

        std::lock_guard _lock{mMutex};

        BBQ_TRACE();

        BQA_LOGV("transactionCallback");
@@ -406,9 +411,8 @@ void BLASTBufferQueue::flushShadowQueue() { 
void BLASTBufferQueue::releaseBufferCallback(

        const ReleaseCallbackId& id, const sp<Fence>& releaseFence,

        std::optional<uint32_t> currentMaxAcquiredBufferCount) {

    std::lock_guard _lock{mMutex};

    BBQ_TRACE();



    std::unique_lock _lock{mMutex};

    releaseBufferCallbackLocked(id, releaseFence, currentMaxAcquiredBufferCount,

                                false /* fakeRelease */);

}
@@ -423,10 +427,8 @@ void BLASTBufferQueue::releaseBufferCallbackLocked( 
    // to the buffer queue. This will prevent higher latency when we are running

    // on a lower refresh rate than the max supported. We only do that for EGL

    // clients as others don't care about latency

    const bool isEGL = [&] {

    const auto it = mSubmitted.find(id);

        return it != mSubmitted.end() && it->second.mApi == NATIVE_WINDOW_API_EGL;

    }();

    const bool isEGL = it != mSubmitted.end() && it->second.mApi == NATIVE_WINDOW_API_EGL;



    if (currentMaxAcquiredBufferCount) {

        mCurrentMaxAcquiredBufferCount = *currentMaxAcquiredBufferCount;
@@ -607,7 +609,7 @@ status_t BLASTBufferQueue::acquireNextBufferLocked( 
    }



    {

        std::unique_lock _lock{mTimestampMutex};

        std::lock_guard _lock{mTimestampMutex};

        auto dequeueTime = mDequeueTimestamps.find(buffer->getId());

        if (dequeueTime != mDequeueTimestamps.end()) {

            Parcel p;
@@ -662,11 +664,11 @@ void BLASTBufferQueue::acquireAndReleaseBuffer() { 
void BLASTBufferQueue::onFrameAvailable(const BufferItem& item) {

    std::function<void(SurfaceComposerClient::Transaction*)> prevCallback = nullptr;

    SurfaceComposerClient::Transaction* prevTransaction = nullptr;

    bool waitForTransactionCallback = !mSyncedFrameNumbers.empty();



    {

        std::unique_lock _lock{mMutex};

        UNIQUE_LOCK_WITH_ASSERTION(mMutex);

        BBQ_TRACE();

        bool waitForTransactionCallback = !mSyncedFrameNumbers.empty();



        const bool syncTransactionSet = mTransactionReadyCallback != nullptr;

        BQA_LOGV("onFrameAvailable-start syncTransactionSet=%s", boolToString(syncTransactionSet));
@@ -745,25 +747,24 @@ void BLASTBufferQueue::onFrameReplaced(const BufferItem& item) { 
}



void BLASTBufferQueue::onFrameDequeued(const uint64_t bufferId) {

    std::unique_lock _lock{mTimestampMutex};

    std::lock_guard _lock{mTimestampMutex};

    mDequeueTimestamps[bufferId] = systemTime();

};



void BLASTBufferQueue::onFrameCancelled(const uint64_t bufferId) {

    std::unique_lock _lock{mTimestampMutex};

    std::lock_guard _lock{mTimestampMutex};

    mDequeueTimestamps.erase(bufferId);

};



void BLASTBufferQueue::syncNextTransaction(

        std::function<void(SurfaceComposerClient::Transaction*)> callback,

        bool acquireSingleBuffer) {

    BBQ_TRACE();



    std::function<void(SurfaceComposerClient::Transaction*)> prevCallback = nullptr;

    SurfaceComposerClient::Transaction* prevTransaction = nullptr;



    {

        std::lock_guard _lock{mMutex};

        BBQ_TRACE();

        // We're about to overwrite the previous call so we should invoke that callback

        // immediately.

        if (mTransactionReadyCallback) {
@@ -829,8 +830,8 @@ bool BLASTBufferQueue::rejectBuffer(const BufferItem& item) { 
class BBQSurface : public Surface {

private:

    std::mutex mMutex;

    sp<BLASTBufferQueue> mBbq;

    bool mDestroyed = false;

    sp<BLASTBufferQueue> mBbq GUARDED_BY(mMutex);

    bool mDestroyed GUARDED_BY(mMutex) = false;



public:

    BBQSurface(const sp<IGraphicBufferProducer>& igbp, bool controlledByApp,
@@ -851,7 +852,7 @@ public: 


    status_t setFrameRate(float frameRate, int8_t compatibility,

                          int8_t changeFrameRateStrategy) override {

        std::unique_lock _lock{mMutex};

        std::lock_guard _lock{mMutex};

        if (mDestroyed) {

            return DEAD_OBJECT;

        }
@@ -864,7 +865,7 @@ public: 


    status_t setFrameTimelineInfo(uint64_t frameNumber,

                                  const FrameTimelineInfo& frameTimelineInfo) override {

        std::unique_lock _lock{mMutex};

        std::lock_guard _lock{mMutex};

        if (mDestroyed) {

            return DEAD_OBJECT;

        }
@@ -874,7 +875,7 @@ public: 
    void destroy() override {

        Surface::destroy();



        std::unique_lock _lock{mMutex};

        std::lock_guard _lock{mMutex};

        mDestroyed = true;

        mBbq = nullptr;

    }
@@ -884,7 +885,7 @@ public: 
// no timing issues.

status_t BLASTBufferQueue::setFrameRate(float frameRate, int8_t compatibility,

                                        bool shouldBeSeamless) {

    std::unique_lock _lock{mMutex};

    std::lock_guard _lock{mMutex};

    SurfaceComposerClient::Transaction t;



    return t.setFrameRate(mSurfaceControl, frameRate, compatibility, shouldBeSeamless).apply();
@@ -894,20 +895,20 @@ status_t BLASTBufferQueue::setFrameTimelineInfo(uint64_t frameNumber, 
                                                const FrameTimelineInfo& frameTimelineInfo) {

    ATRACE_FORMAT("%s(%s) frameNumber: %" PRIu64 " vsyncId: %" PRId64, __func__, mName.c_str(),

                  frameNumber, frameTimelineInfo.vsyncId);

    std::unique_lock _lock{mMutex};

    std::lock_guard _lock{mMutex};

    mPendingFrameTimelines.push({frameNumber, frameTimelineInfo});

    return OK;

}



void BLASTBufferQueue::setSidebandStream(const sp<NativeHandle>& stream) {

    std::unique_lock _lock{mMutex};

    std::lock_guard _lock{mMutex};

    SurfaceComposerClient::Transaction t;



    t.setSidebandStream(mSurfaceControl, stream).apply();

}



sp<Surface> BLASTBufferQueue::getSurface(bool includeSurfaceControlHandle) {

    std::unique_lock _lock{mMutex};

    std::lock_guard _lock{mMutex};

    sp<IBinder> scHandle = nullptr;

    if (includeSurfaceControlHandle && mSurfaceControl) {

        scHandle = mSurfaceControl->getHandle();
@@ -1098,6 +1099,7 @@ PixelFormat BLASTBufferQueue::convertBufferFormat(PixelFormat& format) { 
}



uint32_t BLASTBufferQueue::getLastTransformHint() const {

    std::lock_guard _lock{mMutex};

    if (mSurfaceControl != nullptr) {

        return mSurfaceControl->getTransformHint();

    } else {
@@ -1106,18 +1108,18 @@ uint32_t BLASTBufferQueue::getLastTransformHint() const { 
}



uint64_t BLASTBufferQueue::getLastAcquiredFrameNum() {

    std::unique_lock _lock{mMutex};

    std::lock_guard _lock{mMutex};

    return mLastAcquiredFrameNumber;

}



bool BLASTBufferQueue::isSameSurfaceControl(const sp<SurfaceControl>& surfaceControl) const {

    std::unique_lock _lock{mMutex};

    std::lock_guard _lock{mMutex};

    return SurfaceControl::isSameSurface(mSurfaceControl, surfaceControl);

}



void BLASTBufferQueue::setTransactionHangCallback(

        std::function<void(const std::string&)> callback) {

    std::unique_lock _lock{mMutex};

    std::lock_guard _lock{mMutex};

    mTransactionHangCallback = callback;

}

libs/gui/SurfaceComposerClient.cpp

+3 −0
Original line number Diff line number Diff line
@@ -53,6 +53,7 @@ 
#include <ui/DisplayState.h>

#include <ui/DynamicDisplayInfo.h>



#include <android-base/thread_annotations.h>

#include <private/gui/ComposerService.h>

#include <private/gui/ComposerServiceAIDL.h>
@@ -2988,6 +2989,7 @@ void ReleaseCallbackThread::threadMain() { 
    while (true) {

        {

            std::unique_lock<std::mutex> lock(mMutex);

            base::ScopedLockAssertion assumeLocked(mMutex);

            callbackInfos = std::move(mCallbackInfos);

            mCallbackInfos = {};

        }
@@ -3000,6 +3002,7 @@ void ReleaseCallbackThread::threadMain() { 


        {

            std::unique_lock<std::mutex> lock(mMutex);

            base::ScopedLockAssertion assumeLocked(mMutex);

            if (mCallbackInfos.size() == 0) {

                mReleaseCallbackPending.wait(lock);

            }

libs/gui/include/gui/BLASTBufferQueue.h

+11 −11
Original line number Diff line number Diff line
@@ -44,23 +44,23 @@ public: 
            mCurrentlyConnected(false),

            mPreviouslyConnected(false) {}



    void onDisconnect() override;

    void onDisconnect() override EXCLUDES(mMutex);

    void addAndGetFrameTimestamps(const NewFrameEventsEntry* newTimestamps,

                                  FrameEventHistoryDelta* outDelta) override REQUIRES(mMutex);

                                  FrameEventHistoryDelta* outDelta) override EXCLUDES(mMutex);

    void updateFrameTimestamps(uint64_t frameNumber, nsecs_t refreshStartTime,

                               const sp<Fence>& gpuCompositionDoneFence,

                               const sp<Fence>& presentFence, const sp<Fence>& prevReleaseFence,

                               CompositorTiming compositorTiming, nsecs_t latchTime,

                               nsecs_t dequeueReadyTime) REQUIRES(mMutex);

    void getConnectionEvents(uint64_t frameNumber, bool* needsDisconnect);

                               nsecs_t dequeueReadyTime) EXCLUDES(mMutex);

    void getConnectionEvents(uint64_t frameNumber, bool* needsDisconnect) EXCLUDES(mMutex);



protected:

    void onSidebandStreamChanged() override REQUIRES(mMutex);

    void onSidebandStreamChanged() override EXCLUDES(mMutex);



private:

    const wp<BLASTBufferQueue> mBLASTBufferQueue;



    uint64_t mCurrentFrameNumber = 0;

    uint64_t mCurrentFrameNumber GUARDED_BY(mMutex) = 0;



    Mutex mMutex;

    ConsumerFrameEventHistory mFrameEventHistory GUARDED_BY(mMutex);
@@ -94,7 +94,7 @@ public: 
                               std::optional<uint32_t> currentMaxAcquiredBufferCount);

    void releaseBufferCallbackLocked(const ReleaseCallbackId& id, const sp<Fence>& releaseFence,

                                     std::optional<uint32_t> currentMaxAcquiredBufferCount,

                                     bool fakeRelease);

                                     bool fakeRelease) REQUIRES(mMutex);

    void syncNextTransaction(std::function<void(SurfaceComposerClient::Transaction*)> callback,

                             bool acquireSingleBuffer = true);

    void stopContinuousSyncTransaction();
@@ -150,7 +150,7 @@ private: 
    // mNumAcquired (buffers that queued to SF)  mPendingRelease.size() (buffers that are held by

    // blast). This counter is read by android studio profiler.

    std::string mQueuedBufferTrace;

    sp<SurfaceControl> mSurfaceControl;

    sp<SurfaceControl> mSurfaceControl GUARDED_BY(mMutex);



    mutable std::mutex mMutex;

    std::condition_variable mCallbackCV;
@@ -252,7 +252,7 @@ private: 
    // callback for them.

    std::queue<sp<SurfaceControl>> mSurfaceControlsWithPendingCallback GUARDED_BY(mMutex);



    uint32_t mCurrentMaxAcquiredBufferCount;

    uint32_t mCurrentMaxAcquiredBufferCount GUARDED_BY(mMutex);



    // Flag to determine if syncTransaction should only acquire a single buffer and then clear or

    // continue to acquire buffers until explicitly cleared
@@ -276,8 +276,8 @@ private: 
    // need to set this flag, notably only in the case where we are transitioning from a previous

    // transaction applied by us (one way, may not yet have reached server) and an upcoming

    // transaction that will be applied by some sync consumer.

    bool mAppliedLastTransaction = false;

    uint64_t mLastAppliedFrameNumber = 0;

    bool mAppliedLastTransaction GUARDED_BY(mMutex) = false;

    uint64_t mLastAppliedFrameNumber GUARDED_BY(mMutex) = 0;



    std::function<void(const std::string&)> mTransactionHangCallback;

libs/gui/include/gui/SurfaceComposerClient.h

+1 −1
Original line number Diff line number Diff line
@@ -921,7 +921,7 @@ public: 
    void onTrustedPresentationChanged(int id, bool presentedWithinThresholds) override;



private:

    ReleaseBufferCallback popReleaseBufferCallbackLocked(const ReleaseCallbackId&);

    ReleaseBufferCallback popReleaseBufferCallbackLocked(const ReleaseCallbackId&) REQUIRES(mMutex);

    static sp<TransactionCompletedListener> sInstance;

};