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

Commit bad8a9ea authored by Treehugger Robot's avatar Treehugger Robot Committed by Android (Google) Code Review
Browse files

Merge "Remove callback at the end of consumer destructor" into main

parents 2eb60a80 362fa227

include/input/InputConsumerNoResampling.h

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



    private:

        std::function<int(int events)> mCallback;

        const std::function<int(int events)> mCallback;

    };

    sp<LooperEventCallback> mCallback;

    /**

libs/input/InputConsumerNoResampling.cpp

+10 −7
Original line number Diff line number Diff line
@@ -193,13 +193,6 @@ InputConsumerNoResampling::InputConsumerNoResampling( 


InputConsumerNoResampling::~InputConsumerNoResampling() {

    ensureCalledOnLooperThread(__func__);

    while (!mOutboundQueue.empty()) {

        processOutboundEvents();

        // This is our last chance to ack the events. If we don't ack them here, we will get an ANR,

        // so keep trying to send the events as long as they are present in the queue.

    }



    setFdEvents(0);

    // If there are any remaining unread batches, send an ack for them and don't deliver

    // them to callbacks.

    for (auto& [_, batches] : mBatches) {
@@ -208,6 +201,12 @@ InputConsumerNoResampling::~InputConsumerNoResampling() { 
            batches.pop();

        }

    }



    while (!mOutboundQueue.empty()) {

        processOutboundEvents();

        // This is our last chance to ack the events. If we don't ack them here, we will get an ANR,

        // so keep trying to send the events as long as they are present in the queue.

    }

    // However, it is still up to the app to finish any events that have already been delivered

    // to the callbacks. If we wanted to change that behaviour and auto-finish all unfinished events

    // that were already sent to callbacks, we could potentially loop through "mConsumeTimes"
@@ -216,6 +215,10 @@ InputConsumerNoResampling::~InputConsumerNoResampling() { 
    const size_t unfinishedEvents = mConsumeTimes.size();

    LOG_IF(INFO, unfinishedEvents != 0)

            << getName() << " has " << unfinishedEvents << " unfinished event(s)";

    // Remove the fd from epoll, so that Looper does not call 'handleReceiveCallback' anymore.

    // This must be done at the end of the destructor; otherwise, some of the other functions may

    // call 'setFdEvents' as a side-effect, thus adding the fd back to the epoll set of the looper.

    setFdEvents(0);

}



int InputConsumerNoResampling::handleReceiveCallback(int events) {

libs/input/tests/InputConsumer_test.cpp

+33 −3
Original line number Diff line number Diff line
@@ -70,11 +70,20 @@ protected: 
                                           []() { return std::make_unique<LegacyResampler>(); });

    }



    void invokeLooperCallback() const {

    bool invokeLooperCallback() const {

        sp<LooperCallback> callback;

        ASSERT_TRUE(mLooper->getFdStateDebug(mClientTestChannel->getFd(), /*ident=*/nullptr,

                                             /*events=*/nullptr, &callback, /*data=*/nullptr));

        const bool found =

                mLooper->getFdStateDebug(mClientTestChannel->getFd(), /*ident=*/nullptr,

                                         /*events=*/nullptr, &callback, /*data=*/nullptr);

        if (!found) {

            return false;

        }

        if (callback == nullptr) {

            LOG(FATAL) << "Looper has the fd of interest, but the callback is null!";

            return false;

        }

        callback->handleEvent(mClientTestChannel->getFd(), ALOOPER_EVENT_INPUT, /*data=*/nullptr);

        return true;

    }



    void assertOnBatchedInputEventPendingWasCalled() {
@@ -270,6 +279,27 @@ TEST_F(InputConsumerTest, UnhandledEventsNotFinishedInDestructor) { 
    mClientTestChannel->assertNoSentMessages();

}



/**

 * Check what happens when looper invokes callback after consumer has been destroyed.

 * This reproduces a crash where the LooperEventCallback was added back to the Looper during

 * destructor, thus allowing the looper callback to be invoked onto a null consumer object.

 */

TEST_F(InputConsumerTest, LooperCallbackInvokedAfterConsumerDestroyed) {

    mClientTestChannel->enqueueMessage(

            InputMessageBuilder{InputMessage::Type::MOTION, /*seq=*/0}.action(ACTION_DOWN).build());

    mClientTestChannel->enqueueMessage(

            InputMessageBuilder{InputMessage::Type::MOTION, /*seq=*/1}.action(ACTION_MOVE).build());

    ASSERT_TRUE(invokeLooperCallback());

    assertOnBatchedInputEventPendingWasCalled();

    assertReceivedMotionEvent(WithMotionAction(ACTION_DOWN));

    mClientTestChannel->assertFinishMessage(/*seq=*/0, /*handled=*/true);



    // Now, destroy the consumer and invoke the looper callback again after it's been destroyed.

    mConsumer.reset();

    mClientTestChannel->assertFinishMessage(/*seq=*/1, /*handled=*/false);

    ASSERT_FALSE(invokeLooperCallback());

}



/**

 * Send an event to the InputConsumer, but do not invoke "consumeBatchedInputEvents", thus leaving

 * the input event unconsumed by the callbacks. Ensure that no crash occurs when the consumer is