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

Commit ac2b152a authored by Leon Scroggins's avatar Leon Scroggins Committed by Automerger Merge Worker
Browse files

Merge "Avoid deadlock in MessageQueue::onNewVsyncSchedule" into udc-dev am: 8c5541a6 

Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/native/+/22476128



Change-Id: I0c624af79bb024494cfc3c467ec476c259022d0e
Signed-off-by: default avatarAutomerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
parents 68aafab3 8c5541a6

services/surfaceflinger/Scheduler/MessageQueue.cpp

+30 −7
Original line number Original line Diff line number Diff line
@@ -78,20 +78,42 @@ void MessageQueue::vsyncCallback(nsecs_t vsyncTime, nsecs_t targetWakeupTime, ns 
void MessageQueue::initVsync(std::shared_ptr<scheduler::VSyncDispatch> dispatch,

void MessageQueue::initVsync(std::shared_ptr<scheduler::VSyncDispatch> dispatch,

                             frametimeline::TokenManager& tokenManager,

                             frametimeline::TokenManager& tokenManager,

                             std::chrono::nanoseconds workDuration) {

                             std::chrono::nanoseconds workDuration) {

    std::unique_ptr<scheduler::VSyncCallbackRegistration> oldRegistration;

    {

        std::lock_guard lock(mVsync.mutex);

        std::lock_guard lock(mVsync.mutex);

        mVsync.workDuration = workDuration;

        mVsync.workDuration = workDuration;

        mVsync.tokenManager = &tokenManager;

        mVsync.tokenManager = &tokenManager;

    onNewVsyncScheduleLocked(std::move(dispatch));

        oldRegistration = onNewVsyncScheduleLocked(std::move(dispatch));

    }



    // See comments in onNewVsyncSchedule. Today, oldRegistration should be

    // empty, but nothing prevents us from calling initVsync multiple times, so

    // go ahead and destruct it outside the lock for safety.

    oldRegistration.reset();

}

}





void MessageQueue::onNewVsyncSchedule(std::shared_ptr<scheduler::VSyncDispatch> dispatch) {

void MessageQueue::onNewVsyncSchedule(std::shared_ptr<scheduler::VSyncDispatch> dispatch) {

    std::unique_ptr<scheduler::VSyncCallbackRegistration> oldRegistration;

    {

        std::lock_guard lock(mVsync.mutex);

        std::lock_guard lock(mVsync.mutex);

    onNewVsyncScheduleLocked(std::move(dispatch));

        oldRegistration = onNewVsyncScheduleLocked(std::move(dispatch));

    }



    // The old registration needs to be deleted after releasing mVsync.mutex to

    // avoid deadlock. This is because the callback may be running on the timer

    // thread. In that case, timerCallback sets

    // VSyncDispatchTimerQueueEntry::mRunning to true, then attempts to lock

    // mVsync.mutex. But if it's already locked, the VSyncCallbackRegistration's

    // destructor has to wait until VSyncDispatchTimerQueueEntry::mRunning is

    // set back to false, but it won't be until mVsync.mutex is released.

    oldRegistration.reset();

}

}





void MessageQueue::onNewVsyncScheduleLocked(std::shared_ptr<scheduler::VSyncDispatch> dispatch) {

std::unique_ptr<scheduler::VSyncCallbackRegistration> MessageQueue::onNewVsyncScheduleLocked(

        std::shared_ptr<scheduler::VSyncDispatch> dispatch) {

    const bool reschedule = mVsync.registration &&

    const bool reschedule = mVsync.registration &&

            mVsync.registration->cancel() == scheduler::CancelResult::Cancelled;

            mVsync.registration->cancel() == scheduler::CancelResult::Cancelled;

    auto oldRegistration = std::move(mVsync.registration);

    mVsync.registration = std::make_unique<

    mVsync.registration = std::make_unique<

            scheduler::VSyncCallbackRegistration>(std::move(dispatch),

            scheduler::VSyncCallbackRegistration>(std::move(dispatch),

                                                  std::bind(&MessageQueue::vsyncCallback, this,

                                                  std::bind(&MessageQueue::vsyncCallback, this,
@@ -105,6 +127,7 @@ void MessageQueue::onNewVsyncScheduleLocked(std::shared_ptr<scheduler::VSyncDisp 
                                               .readyDuration = 0,

                                               .readyDuration = 0,

                                               .earliestVsync = mVsync.lastCallbackTime.ns()});

                                               .earliestVsync = mVsync.lastCallbackTime.ns()});

    }

    }

    return oldRegistration;

}

}





void MessageQueue::destroyVsync() {

void MessageQueue::destroyVsync() {

services/surfaceflinger/Scheduler/MessageQueue.h

+5 −2
Original line number Original line Diff line number Diff line
@@ -117,9 +117,9 @@ private: 




    struct Vsync {

    struct Vsync {

        frametimeline::TokenManager* tokenManager = nullptr;

        frametimeline::TokenManager* tokenManager = nullptr;

        std::unique_ptr<scheduler::VSyncCallbackRegistration> registration;





        mutable std::mutex mutex;

        mutable std::mutex mutex;

        std::unique_ptr<scheduler::VSyncCallbackRegistration> registration GUARDED_BY(mutex);

        TracedOrdinal<std::chrono::nanoseconds> workDuration

        TracedOrdinal<std::chrono::nanoseconds> workDuration

                GUARDED_BY(mutex) = {"VsyncWorkDuration-sf", std::chrono::nanoseconds(0)};

                GUARDED_BY(mutex) = {"VsyncWorkDuration-sf", std::chrono::nanoseconds(0)};

        TimePoint lastCallbackTime GUARDED_BY(mutex);

        TimePoint lastCallbackTime GUARDED_BY(mutex);
@@ -129,7 +129,10 @@ private: 




    Vsync mVsync;

    Vsync mVsync;





    void onNewVsyncScheduleLocked(std::shared_ptr<scheduler::VSyncDispatch>) REQUIRES(mVsync.mutex);

    // Returns the old registration so it can be destructed outside the lock to

    // avoid deadlock.

    std::unique_ptr<scheduler::VSyncCallbackRegistration> onNewVsyncScheduleLocked(

            std::shared_ptr<scheduler::VSyncDispatch>) REQUIRES(mVsync.mutex);





public:

public:

    explicit MessageQueue(ICompositor&);

    explicit MessageQueue(ICompositor&);