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

Commit 1686374d authored by Dominik Laskowski's avatar Dominik Laskowski
Browse files

SF: Commit initial DisplayState under lock 

The main thread must `applyTransactions` under `mStateLock` during
`onInitializeDisplays`, since a Binder thread could be concurrently
writing to `mCurrentState.displays` while creating or destroying a
virtual display.

Fixes: 244460929
Test: Boot (sporadic crash with no repro steps)
Change-Id: I4f8bac4af7635b9b93c7b79826285a26a109cf9e
parent 14956dc4

services/surfaceflinger/SurfaceFlinger.cpp

+11 −10
Original line number Diff line number Diff line
@@ -4224,7 +4224,7 @@ bool SurfaceFlinger::flushTransactionQueues(VsyncId vsyncId) { 


bool SurfaceFlinger::applyTransactions(std::vector<TransactionState>& transactions,

                                       VsyncId vsyncId) {

    Mutex::Autolock _l(mStateLock);

    Mutex::Autolock lock(mStateLock);

    return applyTransactionsLocked(transactions, vsyncId);

}
@@ -4505,7 +4505,7 @@ bool SurfaceFlinger::applyTransactionState(const FrameTimelineInfo& frameTimelin 


bool SurfaceFlinger::applyAndCommitDisplayTransactionStates(

        std::vector<TransactionState>& transactions) {

    Mutex::Autolock _l(mStateLock);

    Mutex::Autolock lock(mStateLock);

    bool needsTraversal = false;

    uint32_t transactionFlags = 0;

    for (auto& transaction : transactions) {
@@ -5246,7 +5246,7 @@ void SurfaceFlinger::onHandleDestroyed(BBinder* handle, sp<Layer>& layer, uint32 
}



void SurfaceFlinger::onInitializeDisplays() {

    const auto display = getDefaultDisplayDeviceLocked();

    const auto display = FTL_FAKE_GUARD(mStateLock, getDefaultDisplayDeviceLocked());

    if (!display) return;



    const sp<IBinder> token = display->getDisplayToken().promote();
@@ -5254,13 +5254,13 @@ void SurfaceFlinger::onInitializeDisplays() { 


    TransactionState state;

    state.inputWindowCommands = mInputWindowCommands;

    nsecs_t now = systemTime();

    const nsecs_t now = systemTime();

    state.desiredPresentTime = now;

    state.postTime = now;

    state.permissions = layer_state_t::ACCESS_SURFACE_FLINGER;

    state.originPid = mPid;

    state.originUid = static_cast<int>(getuid());

    uint64_t transactionId = (((uint64_t)mPid) << 32) | mUniqueTransactionId++;

    const uint64_t transactionId = (static_cast<uint64_t>(mPid) << 32) | mUniqueTransactionId++;

    state.id = transactionId;



    // reset screen orientation and use primary layer stack
@@ -5280,21 +5280,22 @@ void SurfaceFlinger::onInitializeDisplays() { 
    std::vector<TransactionState> transactions;

    transactions.emplace_back(state);



    // It should be on the main thread, apply it directly.

    if (mLegacyFrontEndEnabled) {

        applyTransactionsLocked(transactions, /*vsyncId=*/{0});

        applyTransactions(transactions, VsyncId{0});

    } else {

        applyAndCommitDisplayTransactionStates(transactions);

    }



    {

        ftl::FakeGuard guard(mStateLock);

        setPowerModeInternal(display, hal::PowerMode::ON);

    }

}



void SurfaceFlinger::initializeDisplays() {

    // Async since we may be called from the main thread.

    static_cast<void>(mScheduler->schedule(

            [this]() FTL_FAKE_GUARD(mStateLock)

                    FTL_FAKE_GUARD(kMainThreadContext) { onInitializeDisplays(); }));

            [this]() FTL_FAKE_GUARD(kMainThreadContext) { onInitializeDisplays(); }));

}



void SurfaceFlinger::setPowerModeInternal(const sp<DisplayDevice>& display, hal::PowerMode mode) {

services/surfaceflinger/SurfaceFlinger.h

+1 −1
Original line number Diff line number Diff line
@@ -857,7 +857,7 @@ private: 


    // Called during boot, and restart after system_server death.

    void initializeDisplays();

    void onInitializeDisplays() REQUIRES(mStateLock, kMainThreadContext);

    void onInitializeDisplays() REQUIRES(kMainThreadContext);



    sp<const DisplayDevice> getDisplayDeviceLocked(const wp<IBinder>& displayToken) const

            REQUIRES(mStateLock) {