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

Commit 9f330c54 authored by Siarhei Vishniakou's avatar Siarhei Vishniakou
Browse files

Add lock to protect UnwantedInteractionBlocker

The call to 'dump' may come from any thread, and therefore could cause a
crash. Add a lock to protect this input stage.

To run the test:
adb shell -t "/data/nativetest64/inputflinger_tests/inputflinger_tests --gtest_filter='*Dump*' --gtest_repeat=100000 --gtest_break_on_failure"

Before this patch, the test failed after ~5K - ~13K iterations (took
10-20 seconds to crash).

Bug: 232645962
Test: m inputflinger_tests && adb sync data && <run the test>
Change-Id: I2a199690450bc5bb4a8576aa59075e99d37a531b
parent 639db652
Loading
Loading
Loading
Loading
+9 −0
Original line number Diff line number Diff line
@@ -58,4 +58,13 @@ std::string dumpMap(const std::map<K, V>& map, std::string (*keyToString)(const

const char* toString(bool value);

/**
 * Add "prefix" to the beginning of each line in the provided string
 * "str".
 * The string 'str' is typically multi-line.
 * The most common use case for this function is to add some padding
 * when dumping state.
 */
std::string addLinePrefix(std::string str, const std::string& prefix);

} // namespace android
 No newline at end of file
+17 −0
Original line number Diff line number Diff line
@@ -17,6 +17,7 @@
#define LOG_TAG "PrintTools"

#include <input/PrintTools.h>
#include <sstream>

namespace android {

@@ -24,4 +25,20 @@ const char* toString(bool value) {
    return value ? "true" : "false";
}

std::string addLinePrefix(std::string str, const std::string& prefix) {
    std::stringstream ss;
    bool newLineStarted = true;
    for (const auto& ch : str) {
        if (newLineStarted) {
            ss << prefix;
            newLineStarted = false;
        }
        if (ch == '\n') {
            newLineStarted = true;
        }
        ss << ch;
    }
    return ss.str();
}

} // namespace android
+39 −23
Original line number Diff line number Diff line
@@ -367,7 +367,7 @@ void MotionClassifier::dump(std::string& dump) {

// --- InputClassifier ---

InputClassifier::InputClassifier(InputListenerInterface& listener) : mListener(listener) {}
InputClassifier::InputClassifier(InputListenerInterface& listener) : mQueuedListener(listener) {}

void InputClassifier::onBinderDied(void* cookie) {
    InputClassifier* classifier = static_cast<InputClassifier*>(cookie);
@@ -417,55 +417,67 @@ void InputClassifier::setMotionClassifierEnabled(bool enabled) {

void InputClassifier::notifyConfigurationChanged(const NotifyConfigurationChangedArgs* args) {
    // pass through
    mListener.notifyConfigurationChanged(args);
    mQueuedListener.notifyConfigurationChanged(args);
    mQueuedListener.flush();
}

void InputClassifier::notifyKey(const NotifyKeyArgs* args) {
    // pass through
    mListener.notifyKey(args);
    mQueuedListener.notifyKey(args);
    mQueuedListener.flush();
}

void InputClassifier::notifyMotion(const NotifyMotionArgs* args) {
    { // acquire lock
        std::scoped_lock lock(mLock);
        // MotionClassifier is only used for touch events, for now
        const bool sendToMotionClassifier = mMotionClassifier && isTouchEvent(*args);
        if (!sendToMotionClassifier) {
        mListener.notifyMotion(args);
        return;
    }

            mQueuedListener.notifyMotion(args);
        } else {
            NotifyMotionArgs newArgs(*args);
            newArgs.classification = mMotionClassifier->classify(newArgs);
    mListener.notifyMotion(&newArgs);
            mQueuedListener.notifyMotion(&newArgs);
        }
    } // release lock
    mQueuedListener.flush();
}

void InputClassifier::notifySensor(const NotifySensorArgs* args) {
    // pass through
    mListener.notifySensor(args);
    mQueuedListener.notifySensor(args);
    mQueuedListener.flush();
}

void InputClassifier::notifyVibratorState(const NotifyVibratorStateArgs* args) {
    // pass through
    mListener.notifyVibratorState(args);
    mQueuedListener.notifyVibratorState(args);
    mQueuedListener.flush();
}

void InputClassifier::notifySwitch(const NotifySwitchArgs* args) {
    // pass through
    mListener.notifySwitch(args);
    mQueuedListener.notifySwitch(args);
    mQueuedListener.flush();
}

void InputClassifier::notifyDeviceReset(const NotifyDeviceResetArgs* args) {
    { // acquire lock
        std::scoped_lock lock(mLock);
        if (mMotionClassifier) {
            mMotionClassifier->reset(*args);
        }
    } // release lock

    // continue to next stage
    mListener.notifyDeviceReset(args);
    mQueuedListener.notifyDeviceReset(args);
    mQueuedListener.flush();
}

void InputClassifier::notifyPointerCaptureChanged(const NotifyPointerCaptureChangedArgs* args) {
    // pass through
    mListener.notifyPointerCaptureChanged(args);
    mQueuedListener.notifyPointerCaptureChanged(args);
    mQueuedListener.flush();
}

void InputClassifier::setMotionClassifierLocked(
@@ -490,6 +502,10 @@ void InputClassifier::dump(std::string& dump) {
    dump += "\n";
}

void InputClassifier::monitor() {
    std::scoped_lock lock(mLock);
}

InputClassifier::~InputClassifier() {
}

+5 −1
Original line number Diff line number Diff line
@@ -96,6 +96,9 @@ public:
     */
    virtual void dump(std::string& dump) = 0;

    /* Called by the heatbeat to ensures that the classifier has not deadlocked. */
    virtual void monitor() = 0;

    InputClassifierInterface() { }
    virtual ~InputClassifierInterface() { }
};
@@ -247,6 +250,7 @@ public:
    void notifyPointerCaptureChanged(const NotifyPointerCaptureChangedArgs* args) override;

    void dump(std::string& dump) override;
    void monitor() override;

    ~InputClassifier();

@@ -257,7 +261,7 @@ private:
    // Protect access to mMotionClassifier, since it may become null via a hidl callback
    std::mutex mLock;
    // The next stage to pass input events to
    InputListenerInterface& mListener;
    QueuedInputListener mQueuedListener;

    std::unique_ptr<MotionClassifierInterface> mMotionClassifier GUARDED_BY(mLock);
    std::future<void> mInitializeMotionClassifier GUARDED_BY(mLock);
+10 −3
Original line number Diff line number Diff line
@@ -62,8 +62,8 @@ InputManager::InputManager(
        const sp<InputDispatcherPolicyInterface>& dispatcherPolicy) {
    mDispatcher = createInputDispatcher(dispatcherPolicy);
    mClassifier = std::make_unique<InputClassifier>(*mDispatcher);
    mUnwantedInteractionBlocker = std::make_unique<UnwantedInteractionBlocker>(*mClassifier);
    mReader = createInputReader(readerPolicy, *mUnwantedInteractionBlocker);
    mBlocker = std::make_unique<UnwantedInteractionBlocker>(*mClassifier);
    mReader = createInputReader(readerPolicy, *mBlocker);
}

InputManager::~InputManager() {
@@ -111,7 +111,7 @@ InputReaderInterface& InputManager::getReader() {
}

UnwantedInteractionBlockerInterface& InputManager::getUnwantedInteractionBlocker() {
    return *mUnwantedInteractionBlocker;
    return *mBlocker;
}

InputClassifierInterface& InputManager::getClassifier() {
@@ -122,6 +122,13 @@ InputDispatcherInterface& InputManager::getDispatcher() {
    return *mDispatcher;
}

void InputManager::monitor() {
    mReader->monitor();
    mBlocker->monitor();
    mClassifier->monitor();
    mDispatcher->monitor();
}

// Used by tests only.
binder::Status InputManager::createInputChannel(const std::string& name, InputChannel* outChannel) {
    IPCThreadState* ipc = IPCThreadState::self();
Loading