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

Commit b2253605 authored by Leon Scroggins III's avatar Leon Scroggins III
Browse files

Fix for potential deadlock in EventThread::onNewVsyncSchedule 

I haven't seen such a deadlock, but looking over
I86e66df59c64e81c4aa721a25250697f61237488 and the potential deadlock
that inspired it, I think there could be a similar problem in
EventThread. As I understand it, this is partially because of the
fact that EventThread does not unregister the old callback. This is
addressed in I3c1eccb36914f29560600d48bb08b1b8f2fe7c96.

Fix the deadlock with a similar approach as
I86e66df59c64e81c4aa721a25250697f61237488: hold on to the old
registration until after releasing the mutex.

Bug: 279209321
Test: infeasible
Change-Id: If490f88115aa298d31aa9ad392a1f9f9dc987549
parent 6c440aeb

services/surfaceflinger/Scheduler/EventThread.cpp

+13 −3
Original line number Diff line number Diff line
@@ -692,17 +692,27 @@ const char* EventThread::toCString(State state) { 
}



void EventThread::onNewVsyncSchedule(std::shared_ptr<scheduler::VsyncSchedule> schedule) {

    // Hold onto the old registration until after releasing the mutex to avoid deadlock.

    scheduler::VSyncCallbackRegistration oldRegistration =

            onNewVsyncScheduleInternal(std::move(schedule));

}



scheduler::VSyncCallbackRegistration EventThread::onNewVsyncScheduleInternal(

        std::shared_ptr<scheduler::VsyncSchedule> schedule) {

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

    const bool reschedule = mVsyncRegistration.cancel() == scheduler::CancelResult::Cancelled;

    mVsyncSchedule = std::move(schedule);

    mVsyncRegistration =

    auto oldRegistration =

            std::exchange(mVsyncRegistration,

                          scheduler::VSyncCallbackRegistration(mVsyncSchedule->getDispatch(),

                                                 createDispatchCallback(), mThreadName);

                                                               createDispatchCallback(),

                                                               mThreadName));

    if (reschedule) {

        mVsyncRegistration.schedule({.workDuration = mWorkDuration.get().count(),

                                     .readyDuration = mReadyDuration.count(),

                                     .earliestVsync = mLastVsyncCallbackTime.ns()});

    }

    return oldRegistration;

}



scheduler::VSyncDispatch::Callback EventThread::createDispatchCallback() {

services/surfaceflinger/Scheduler/EventThread.h

+6 −1
Original line number Diff line number Diff line
@@ -174,7 +174,7 @@ public: 


    size_t getEventThreadConnectionCount() override;



    void onNewVsyncSchedule(std::shared_ptr<scheduler::VsyncSchedule>) override;

    void onNewVsyncSchedule(std::shared_ptr<scheduler::VsyncSchedule>) override EXCLUDES(mMutex);



private:

    friend EventThreadTest;
@@ -201,6 +201,11 @@ private: 


    scheduler::VSyncDispatch::Callback createDispatchCallback();



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

    // avoid deadlock.

    scheduler::VSyncCallbackRegistration onNewVsyncScheduleInternal(

            std::shared_ptr<scheduler::VsyncSchedule>) EXCLUDES(mMutex);



    const char* const mThreadName;

    TracedOrdinal<int> mVsyncTracer;

    TracedOrdinal<std::chrono::nanoseconds> mWorkDuration GUARDED_BY(mMutex);