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

Commit ed234c62 authored by Patrick Williams's avatar Patrick Williams
Browse files

Ack messages when WindowInfosListeners are removed 

This fix updates WindowInfosListenersInvoker::removeWindowInfosListener to automatically ack any unacked messages associated with the listener being removed. This fixes a race where the invoker may send the listener a message while the client is removing the listener and terminating its process.

Bug: 300644295
Test: WindowInfosListenerInvokerTest
Change-Id: I93a5fbecb03bb8eeea8d8385bdf906051d595ae2
parent 1d6ad144

services/surfaceflinger/WindowInfosListenerInvoker.cpp

+19 −15
Original line number Diff line number Diff line
@@ -56,16 +56,21 @@ void WindowInfosListenerInvoker::removeWindowInfosListener( 
        ATRACE_NAME("WindowInfosListenerInvoker::removeWindowInfosListener");

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

        asBinder->unlinkToDeath(sp<DeathRecipient>::fromExisting(this));

        mWindowInfosListeners.erase(asBinder);

        eraseListenerAndAckMessages(asBinder);

    }});

}



void WindowInfosListenerInvoker::binderDied(const wp<IBinder>& who) {

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

        ATRACE_NAME("WindowInfosListenerInvoker::binderDied");

        auto it = mWindowInfosListeners.find(who);

        eraseListenerAndAckMessages(who);

    }});

}



void WindowInfosListenerInvoker::eraseListenerAndAckMessages(const wp<IBinder>& binder) {

    auto it = mWindowInfosListeners.find(binder);

    int64_t listenerId = it->second.first;

        mWindowInfosListeners.erase(who);

    mWindowInfosListeners.erase(binder);



    std::vector<int64_t> vsyncIds;

    for (auto& [vsyncId, state] : mUnackedState) {
@@ -78,7 +83,6 @@ void WindowInfosListenerInvoker::binderDied(const wp<IBinder>& who) { 
    for (int64_t vsyncId : vsyncIds) {

        ackWindowInfosReceived(vsyncId, listenerId);

    }

    }});

}



void WindowInfosListenerInvoker::windowInfosChanged(

services/surfaceflinger/WindowInfosListenerInvoker.h

+1 −0
Original line number Diff line number Diff line
@@ -67,6 +67,7 @@ private: 


    std::optional<gui::WindowInfosUpdate> mDelayedUpdate;

    WindowInfosReportedListenerSet mReportedListeners;

    void eraseListenerAndAckMessages(const wp<IBinder>&);



    struct UnackedState {

        ftl::SmallVector<int64_t, kStaticCapacity> unackedListenerIds;

services/surfaceflinger/tests/unittests/WindowInfosListenerInvokerTest.cpp

+38 −0
Original line number Diff line number Diff line
@@ -245,4 +245,42 @@ TEST_F(WindowInfosListenerInvokerTest, noListeners) { 
    EXPECT_EQ(callCount, 1);

}



// Test that WindowInfosListenerInvoker#removeWindowInfosListener acks any unacked messages for

// the removed listener.

TEST_F(WindowInfosListenerInvokerTest, removeListenerAcks) {

    // Don't ack in this listener to ensure there's an unacked message when the listener is later

    // removed.

    gui::WindowInfosListenerInfo listenerToBeRemovedInfo;

    auto listenerToBeRemoved = sp<Listener>::make([](const gui::WindowInfosUpdate&) {});

    mInvoker->addWindowInfosListener(listenerToBeRemoved, &listenerToBeRemovedInfo);



    std::mutex mutex;

    std::condition_variable cv;

    int callCount = 0;

    gui::WindowInfosListenerInfo listenerInfo;

    mInvoker->addWindowInfosListener(sp<Listener>::make([&](const gui::WindowInfosUpdate& update) {

                                         std::scoped_lock lock{mutex};

                                         callCount++;

                                         cv.notify_one();

                                         listenerInfo.windowInfosPublisher

                                                 ->ackWindowInfosReceived(update.vsyncId,

                                                                          listenerInfo.listenerId);

                                     }),

                                     &listenerInfo);



    BackgroundExecutor::getInstance().sendCallbacks(

            {[&]() { mInvoker->windowInfosChanged({}, {}, false); }});

    mInvoker->removeWindowInfosListener(listenerToBeRemoved);

    BackgroundExecutor::getInstance().sendCallbacks(

            {[&]() { mInvoker->windowInfosChanged({}, {}, false); }});



    // Verify that the second listener is called twice. If unacked messages aren't removed when the

    // first listener is removed, this will fail.

    {

        std::unique_lock lock{mutex};

        cv.wait(lock, [&]() { return callCount == 2; });

    }

    EXPECT_EQ(callCount, 2);

}



} // namespace android