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

Commit 4e21979f authored by Siarhei Vishniakou's avatar Siarhei Vishniakou
Browse files

Check window token when extending timeout 

When the "no focused window" ANR is raised, the policy responds with a
timeout. The timeout indicates whether or not to extend the ANR, and by
how much.

When we process the response from the policy, it is done in the
'commands' portion of the dispatch cycle. At that time, we already have
lost the context about the ANR.

We used to rely on the optional 'mNoFocusedWindowTimeout' being set to
determine whether this is a 'no focused window' ANR. However, there's a
better way to do this, by simply checking the inputs that were first
provided to the policy.

This way, we are free to modify 'mNoFocusedWindowTimeout' in other
portions of the code.

Other changes in this CL:

* In 'onAnrLocked', we are assuming the connection is non-null, so let's
pass it by reference. This will get us closer to the goal of using
std::unique_ptr for the connection.

* In 'InputDispatcher_test', when we had a missing ANR, we would go on
to try to derefence a null (or empty) object. This caused crashes on
hwasan build. Fix this by returning default pair when the error
condition has been detected.

Bug: 164754075
Test: atest inputflinger_tests
Change-Id: I656315eda001f1fff31cd265eadce5397606b855
Merged-In: I656315eda001f1fff31cd265eadce5397606b855
parent 1b7f7628

services/inputflinger/dispatcher/InputDispatcher.cpp

+15 −15
Original line number Diff line number Diff line
@@ -518,7 +518,7 @@ nsecs_t InputDispatcher::processAnrsLocked() { 
    connection->responsive = false;

    // Stop waking up for this unresponsive connection

    mAnrTracker.eraseToken(connection->inputChannel->getConnectionToken());

    onAnrLocked(connection);

    onAnrLocked(*connection);

    return LONG_LONG_MIN;

}
@@ -4542,12 +4542,12 @@ void InputDispatcher::onFocusChangedLocked(const sp<InputWindowHandle>& oldFocus 
    postCommandLocked(std::move(commandEntry));

}



void InputDispatcher::onAnrLocked(const sp<Connection>& connection) {

void InputDispatcher::onAnrLocked(const Connection& connection) {

    // Since we are allowing the policy to extend the timeout, maybe the waitQueue

    // is already healthy again. Don't raise ANR in this situation

    if (connection->waitQueue.empty()) {

    if (connection.waitQueue.empty()) {

        ALOGI("Not raising ANR because the connection %s has recovered",

              connection->inputChannel->getName().c_str());

              connection.inputChannel->getName().c_str());

        return;

    }

    /**
@@ -4558,21 +4558,21 @@ void InputDispatcher::onAnrLocked(const sp<Connection>& connection) { 
     * processes the events linearly. So providing information about the oldest entry seems to be

     * most useful.

     */

    DispatchEntry* oldestEntry = *connection->waitQueue.begin();

    DispatchEntry* oldestEntry = *connection.waitQueue.begin();

    const nsecs_t currentWait = now() - oldestEntry->deliveryTime;

    std::string reason =

            android::base::StringPrintf("%s is not responding. Waited %" PRId64 "ms for %s",

                                        connection->inputChannel->getName().c_str(),

                                        connection.inputChannel->getName().c_str(),

                                        ns2ms(currentWait),

                                        oldestEntry->eventEntry->getDescription().c_str());



    updateLastAnrStateLocked(getWindowHandleLocked(connection->inputChannel->getConnectionToken()),

    updateLastAnrStateLocked(getWindowHandleLocked(connection.inputChannel->getConnectionToken()),

                             reason);



    std::unique_ptr<CommandEntry> commandEntry =

            std::make_unique<CommandEntry>(&InputDispatcher::doNotifyAnrLockedInterruptible);

    commandEntry->inputApplicationHandle = nullptr;

    commandEntry->inputChannel = connection->inputChannel;

    commandEntry->inputChannel = connection.inputChannel;

    commandEntry->reason = std::move(reason);

    postCommandLocked(std::move(commandEntry));

}
@@ -4672,15 +4672,15 @@ void InputDispatcher::doNotifyAnrLockedInterruptible(CommandEntry* commandEntry) 
void InputDispatcher::extendAnrTimeoutsLocked(const sp<InputApplicationHandle>& application,

                                              const sp<IBinder>& connectionToken,

                                              nsecs_t timeoutExtension) {

    sp<Connection> connection = getConnectionLocked(connectionToken);

    if (connection == nullptr) {

        if (mNoFocusedWindowTimeoutTime.has_value() && application != nullptr) {

            // Maybe ANR happened because there's no focused window?

    if (connectionToken == nullptr && application != nullptr) {

        // The ANR happened because there's no focused window

        mNoFocusedWindowTimeoutTime = now() + timeoutExtension;

        mAwaitedFocusedApplication = application;

        } else {

            // It's also possible that the connection already disappeared. No action necessary.

    }



    sp<Connection> connection = getConnectionLocked(connectionToken);

    if (connection == nullptr) {

        // It's possible that the connection already disappeared. No action necessary.

        return;

    }

services/inputflinger/dispatcher/InputDispatcher.h

+1 −1
Original line number Diff line number Diff line
@@ -489,7 +489,7 @@ private: 
            REQUIRES(mLock);

    void onFocusChangedLocked(const sp<InputWindowHandle>& oldFocus,

                              const sp<InputWindowHandle>& newFocus) REQUIRES(mLock);

    void onAnrLocked(const sp<Connection>& connection) REQUIRES(mLock);

    void onAnrLocked(const Connection& connection) REQUIRES(mLock);

    void onAnrLocked(const sp<InputApplicationHandle>& application) REQUIRES(mLock);

    void updateLastAnrStateLocked(const sp<InputWindowHandle>& window, const std::string& reason)

            REQUIRES(mLock);

services/inputflinger/tests/InputDispatcher_test.cpp

+1 −0
Original line number Diff line number Diff line
@@ -150,6 +150,7 @@ public: 
        const std::chrono::duration waited = std::chrono::steady_clock::now() - start;

        if (mAnrApplications.empty() || mAnrWindowTokens.empty()) {

            ADD_FAILURE() << "Did not receive ANR callback";

            return {};

        }

        // Ensure that the ANR didn't get raised too early. We can't be too strict here because

        // the dispatcher started counting before this function was called