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

Commit 6681c023 authored by Patrick Williams's avatar Patrick Williams
Browse files

Skip window infos updates if no listeners 

This change fixes a bug where WindowInfosListenerInvoker's active
message count is incremented for messages when there are no listeners.
In this case, onWindowInfosReported is never called, leading to
out-of-sync window infos once listeners are added back. This scenario
may occur if system server dies.

Bug: 279792237
Test: WindowInfosListenerInvokerTest, manual tested by killing system server
Change-Id: If62f7cc56b48570f633e8e640006f020b053ea6f
parent 5423b303

services/surfaceflinger/SurfaceFlinger.cpp

+15 −29
Original line number Diff line number Diff line
@@ -2529,7 +2529,7 @@ bool SurfaceFlinger::commit(TimePoint frameTime, VsyncId vsyncId, TimePoint expe 
    }



    updateCursorAsync();

    updateInputFlinger(vsyncId);

    updateInputFlinger(vsyncId, frameTime);



    if (mLayerTracingEnabled && !mLayerTracing.flagIsSet(LayerTracing::TRACE_COMPOSITION)) {

        // This will block and tracing should only be enabled for debugging.
@@ -3723,7 +3723,7 @@ void SurfaceFlinger::commitTransactionsLocked(uint32_t transactionFlags) { 
    doCommitTransactions();

}



void SurfaceFlinger::updateInputFlinger(VsyncId vsyncId) {

void SurfaceFlinger::updateInputFlinger(VsyncId vsyncId, TimePoint frameTime) {

    if (!mInputFlinger || (!mUpdateInputInfo && mInputWindowCommands.empty())) {

        return;

    }
@@ -3735,8 +3735,6 @@ void SurfaceFlinger::updateInputFlinger(VsyncId vsyncId) { 
    if (mUpdateInputInfo) {

        mUpdateInputInfo = false;

        updateWindowInfo = true;

        mLastInputFlingerUpdateVsyncId = vsyncId;

        mLastInputFlingerUpdateTimestamp = systemTime();

        buildWindowInfos(windowInfos, displayInfos);

    }
@@ -3758,17 +3756,18 @@ void SurfaceFlinger::updateInputFlinger(VsyncId vsyncId) { 
                                                      inputWindowCommands =

                                                              std::move(mInputWindowCommands),

                                                      inputFlinger = mInputFlinger, this,

                                                      visibleWindowsChanged]() {

                                                      visibleWindowsChanged, vsyncId, frameTime]() {

        ATRACE_NAME("BackgroundExecutor::updateInputFlinger");

        if (updateWindowInfo) {

            mWindowInfosListenerInvoker

                    ->windowInfosChanged(std::move(windowInfos), std::move(displayInfos),

                    ->windowInfosChanged(gui::WindowInfosUpdate{std::move(windowInfos),

                                                                std::move(displayInfos),

                                                                ftl::to_underlying(vsyncId),

                                                                frameTime.ns()},

                                         std::move(

                                                 inputWindowCommands.windowInfosReportedListeners),

                                         /* forceImmediateCall= */ visibleWindowsChanged ||

                                                 !inputWindowCommands.focusRequests.empty(),

                                         mLastInputFlingerUpdateVsyncId,

                                         mLastInputFlingerUpdateTimestamp);

                                                 !inputWindowCommands.focusRequests.empty());

        } else {

            // If there are listeners but no changes to input windows, call the listeners

            // immediately.
@@ -6143,27 +6142,14 @@ void SurfaceFlinger::dumpAllLocked(const DumpArgs& args, const std::string& comp 
    result.append("\n");



    result.append("Window Infos:\n");

    StringAppendF(&result, "  input flinger update vsync id: %" PRId64 "\n",

                  ftl::to_underlying(mLastInputFlingerUpdateVsyncId));

    StringAppendF(&result, "  input flinger update timestamp (ns): %" PRId64 "\n",

                  mLastInputFlingerUpdateTimestamp);

    auto windowInfosDebug = mWindowInfosListenerInvoker->getDebugInfo();

    StringAppendF(&result, "  max send vsync id: %" PRId64 "\n",

                  ftl::to_underlying(windowInfosDebug.maxSendDelayVsyncId));

    StringAppendF(&result, "  max send delay (ns): %" PRId64 " ns\n",

                  windowInfosDebug.maxSendDelayDuration);

    StringAppendF(&result, "  unsent messages: %" PRIu32 "\n",

                  windowInfosDebug.pendingMessageCount);

    result.append("\n");



    if (VsyncId unsentVsyncId = mWindowInfosListenerInvoker->getUnsentMessageVsyncId();

        unsentVsyncId != VsyncId()) {

        StringAppendF(&result, "  unsent input flinger update vsync id: %" PRId64 "\n",

                      ftl::to_underlying(unsentVsyncId));

        StringAppendF(&result, "  unsent input flinger update timestamp (ns): %" PRId64 "\n",

                      mWindowInfosListenerInvoker->getUnsentMessageTimestamp());

        result.append("\n");

    }



    if (uint32_t pendingMessages = mWindowInfosListenerInvoker->getPendingMessageCount();

        pendingMessages != 0) {

        StringAppendF(&result, "  pending input flinger calls: %" PRIu32 "\n",

                      mWindowInfosListenerInvoker->getPendingMessageCount());

        result.append("\n");

    }

}



mat4 SurfaceFlinger::calculateColorMatrix(float saturation) {

services/surfaceflinger/SurfaceFlinger.h

+1 −4
Original line number Diff line number Diff line
@@ -718,7 +718,7 @@ private: 
    void updateLayerHistory(const frontend::LayerSnapshot& snapshot);

    frontend::Update flushLifecycleUpdates() REQUIRES(kMainThreadContext);



    void updateInputFlinger(VsyncId);

    void updateInputFlinger(VsyncId vsyncId, TimePoint frameTime);

    void persistDisplayBrightness(bool needsComposite) REQUIRES(kMainThreadContext);

    void buildWindowInfos(std::vector<gui::WindowInfo>& outWindowInfos,

                          std::vector<gui::DisplayInfo>& outDisplayInfos);
@@ -1250,9 +1250,6 @@ private: 


    VsyncId mLastCommittedVsyncId;



    VsyncId mLastInputFlingerUpdateVsyncId;

    nsecs_t mLastInputFlingerUpdateTimestamp;



    // If blurs should be enabled on this device.

    bool mSupportsBlur = false;

    std::atomic<uint32_t> mFrameMissedCount = 0;

services/surfaceflinger/WindowInfosListenerInvoker.cpp

+84 −64
Original line number Diff line number Diff line
@@ -16,8 +16,11 @@ 


#include <ftl/small_vector.h>

#include <gui/ISurfaceComposer.h>

#include <gui/TraceUtils.h>

#include <gui/WindowInfosUpdate.h>

#include <scheduler/Time.h>



#include "BackgroundExecutor.h"

#include "WindowInfosListenerInvoker.h"



namespace android {
@@ -26,7 +29,7 @@ using gui::DisplayInfo; 
using gui::IWindowInfosListener;

using gui::WindowInfo;



using WindowInfosListenerVector = ftl::SmallVector<const sp<IWindowInfosListener>, 3>;

using WindowInfosListenerVector = ftl::SmallVector<const sp<gui::IWindowInfosListener>, 3>;



struct WindowInfosReportedListenerInvoker : gui::BnWindowInfosReportedListener,

                                            IBinder::DeathRecipient {
@@ -86,29 +89,60 @@ void WindowInfosListenerInvoker::binderDied(const wp<IBinder>& who) { 
}



void WindowInfosListenerInvoker::windowInfosChanged(

        std::vector<WindowInfo> windowInfos, std::vector<DisplayInfo> displayInfos,

        WindowInfosReportedListenerSet reportedListeners, bool forceImmediateCall, VsyncId vsyncId,

        nsecs_t timestamp) {

    reportedListeners.insert(sp<WindowInfosListenerInvoker>::fromExisting(this));

    auto callListeners = [this, windowInfos = std::move(windowInfos),

                          displayInfos = std::move(displayInfos), vsyncId,

                          timestamp](WindowInfosReportedListenerSet reportedListeners) mutable {

        WindowInfosListenerVector windowInfosListeners;

        gui::WindowInfosUpdate update, WindowInfosReportedListenerSet reportedListeners,

        bool forceImmediateCall) {

    WindowInfosListenerVector listeners;

    {

            std::scoped_lock lock(mListenersMutex);

        std::scoped_lock lock{mMessagesMutex};



        if (!mDelayInfo) {

            mDelayInfo = DelayInfo{

                    .vsyncId = update.vsyncId,

                    .frameTime = update.timestamp,

            };

        }



        // If there are unacked messages and this isn't a forced call, then return immediately.

        // If a forced window infos change doesn't happen first, the update will be sent after

        // the WindowInfosReportedListeners are called. If a forced window infos change happens or

        // if there are subsequent delayed messages before this update is sent, then this message

        // will be dropped and the listeners will only be called with the latest info. This is done

        // to reduce the amount of binder memory used.

        if (mActiveMessageCount > 0 && !forceImmediateCall) {

            mDelayedUpdate = std::move(update);

            mReportedListeners.merge(reportedListeners);

            return;

        }



        if (mDelayedUpdate) {

            mDelayedUpdate.reset();

        }



        {

            std::scoped_lock lock{mListenersMutex};

            for (const auto& [_, listener] : mWindowInfosListeners) {

                windowInfosListeners.push_back(listener);

                listeners.push_back(listener);

            }

        }

        if (CC_UNLIKELY(listeners.empty())) {

            mReportedListeners.merge(reportedListeners);

            mDelayInfo.reset();

            return;

        }



        auto reportedInvoker =

                sp<WindowInfosReportedListenerInvoker>::make(windowInfosListeners,

                                                             std::move(reportedListeners));

        reportedListeners.insert(sp<WindowInfosListenerInvoker>::fromExisting(this));

        reportedListeners.merge(mReportedListeners);

        mReportedListeners.clear();



        gui::WindowInfosUpdate update(std::move(windowInfos), std::move(displayInfos),

                                      ftl::to_underlying(vsyncId), timestamp);

        mActiveMessageCount++;

        updateMaxSendDelay();

        mDelayInfo.reset();

    }



        for (const auto& listener : windowInfosListeners) {

    auto reportedInvoker =

            sp<WindowInfosReportedListenerInvoker>::make(listeners, std::move(reportedListeners));



    for (const auto& listener : listeners) {

        sp<IBinder> asBinder = IInterface::asBinder(listener);



        // linkToDeath is used here to ensure that the windowInfosReportedListeners
@@ -121,55 +155,41 @@ void WindowInfosListenerInvoker::windowInfosChanged( 
            reportedInvoker->onWindowInfosReported();

        }

    }

    };



    {

        std::scoped_lock lock(mMessagesMutex);

        // If there are unacked messages and this isn't a forced call, then return immediately.

        // If a forced window infos change doesn't happen first, the update will be sent after

        // the WindowInfosReportedListeners are called. If a forced window infos change happens or

        // if there are subsequent delayed messages before this update is sent, then this message

        // will be dropped and the listeners will only be called with the latest info. This is done

        // to reduce the amount of binder memory used.

        if (mActiveMessageCount > 0 && !forceImmediateCall) {

            mWindowInfosChangedDelayed = std::move(callListeners);

            mUnsentVsyncId = vsyncId;

            mUnsentTimestamp = timestamp;

            mReportedListenersDelayed.merge(reportedListeners);

            return;

        }



        mWindowInfosChangedDelayed = nullptr;

        mUnsentVsyncId = VsyncId();

        mUnsentTimestamp = -1;

        reportedListeners.merge(mReportedListenersDelayed);

        mActiveMessageCount++;

    }

    callListeners(std::move(reportedListeners));

}



binder::Status WindowInfosListenerInvoker::onWindowInfosReported() {

    std::function<void(WindowInfosReportedListenerSet)> callListeners;

    WindowInfosReportedListenerSet reportedListeners;



    BackgroundExecutor::getInstance().sendCallbacks({[this]() {

        gui::WindowInfosUpdate update;

        {

            std::scoped_lock lock{mMessagesMutex};

            mActiveMessageCount--;

        if (!mWindowInfosChangedDelayed || mActiveMessageCount > 0) {

            if (!mDelayedUpdate || mActiveMessageCount > 0) {

                return;

            }

            update = std::move(*mDelayedUpdate);

            mDelayedUpdate.reset();

        }

        windowInfosChanged(std::move(update), {}, false);

    }});

    return binder::Status::ok();

}



        mActiveMessageCount++;

        callListeners = std::move(mWindowInfosChangedDelayed);

        mWindowInfosChangedDelayed = nullptr;

        mUnsentVsyncId = VsyncId();

        mUnsentTimestamp = -1;

        reportedListeners = std::move(mReportedListenersDelayed);

        mReportedListenersDelayed.clear();

WindowInfosListenerInvoker::DebugInfo WindowInfosListenerInvoker::getDebugInfo() {

    std::scoped_lock lock{mMessagesMutex};

    updateMaxSendDelay();

    mDebugInfo.pendingMessageCount = mActiveMessageCount;

    return mDebugInfo;

}



    callListeners(std::move(reportedListeners));

    return binder::Status::ok();

void WindowInfosListenerInvoker::updateMaxSendDelay() {

    if (!mDelayInfo) {

        return;

    }

    nsecs_t delay = TimePoint::now().ns() - mDelayInfo->frameTime;

    if (delay > mDebugInfo.maxSendDelayDuration) {

        mDebugInfo.maxSendDelayDuration = delay;

        mDebugInfo.maxSendDelayVsyncId = VsyncId{mDelayInfo->vsyncId};

    }

}



} // namespace android

services/surfaceflinger/WindowInfosListenerInvoker.h

+19 −21
Original line number Diff line number Diff line
@@ -16,6 +16,7 @@ 


#pragma once



#include <optional>

#include <unordered_set>



#include <android/gui/BnWindowInfosReportedListener.h>
@@ -40,26 +41,18 @@ public: 
    void addWindowInfosListener(sp<gui::IWindowInfosListener>);

    void removeWindowInfosListener(const sp<gui::IWindowInfosListener>& windowInfosListener);



    void windowInfosChanged(std::vector<gui::WindowInfo>, std::vector<gui::DisplayInfo>,

    void windowInfosChanged(gui::WindowInfosUpdate update,

                            WindowInfosReportedListenerSet windowInfosReportedListeners,

                            bool forceImmediateCall, VsyncId vsyncId, nsecs_t timestamp);

                            bool forceImmediateCall);



    binder::Status onWindowInfosReported() override;



    VsyncId getUnsentMessageVsyncId() {

        std::scoped_lock lock(mMessagesMutex);

        return mUnsentVsyncId;

    }



    nsecs_t getUnsentMessageTimestamp() {

        std::scoped_lock lock(mMessagesMutex);

        return mUnsentTimestamp;

    }



    uint32_t getPendingMessageCount() {

        std::scoped_lock lock(mMessagesMutex);

        return mActiveMessageCount;

    }

    struct DebugInfo {

        VsyncId maxSendDelayVsyncId;

        nsecs_t maxSendDelayDuration;

        uint32_t pendingMessageCount;

    };

    DebugInfo getDebugInfo();



protected:

    void binderDied(const wp<IBinder>& who) override;
@@ -73,11 +66,16 @@ private: 


    std::mutex mMessagesMutex;

    uint32_t mActiveMessageCount GUARDED_BY(mMessagesMutex) = 0;

    std::function<void(WindowInfosReportedListenerSet)> mWindowInfosChangedDelayed

            GUARDED_BY(mMessagesMutex);

    VsyncId mUnsentVsyncId GUARDED_BY(mMessagesMutex);

    nsecs_t mUnsentTimestamp GUARDED_BY(mMessagesMutex) = -1;

    WindowInfosReportedListenerSet mReportedListenersDelayed;

    std::optional<gui::WindowInfosUpdate> mDelayedUpdate GUARDED_BY(mMessagesMutex);

    WindowInfosReportedListenerSet mReportedListeners;



    DebugInfo mDebugInfo GUARDED_BY(mMessagesMutex);

    struct DelayInfo {

        int64_t vsyncId;

        nsecs_t frameTime;

    };

    std::optional<DelayInfo> mDelayInfo GUARDED_BY(mMessagesMutex);

    void updateMaxSendDelay() REQUIRES(mMessagesMutex);

};



} // namespace android

services/surfaceflinger/fuzzer/surfaceflinger_fuzzers_utils.h

+1 −1
Original line number Diff line number Diff line
@@ -590,7 +590,7 @@ public: 
        mFlinger->binderDied(display);

        mFlinger->onFirstRef();



        mFlinger->updateInputFlinger(VsyncId{0});

        mFlinger->updateInputFlinger(VsyncId{}, TimePoint{});

        mFlinger->updateCursorAsync();



        mutableScheduler().setVsyncConfig({.sfOffset = mFdp.ConsumeIntegral<nsecs_t>(),