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

Commit 7918c825 authored by Arpit Singh's avatar Arpit Singh
Browse files

Refactor PointerChoreographer lock 

Pointer choreographer and WindowInfoListener use separate locks which
is problematic. As WindowInfosUpdate callback is triggered from the
display thread, a deadlock might occur due to race between display and
reader thread. Both trying to acquire both the locks.

This becomes much more likely when we have multiple displays connected.

Test: Verify pointer indicators are hidden on screenshots of secure windows
Test: atest inputflinger_tests
Bug: 367660694
Flag: EXEMPT refactor

Change-Id: I7417c0071322a093643734432f8f0236bd89a317
parent a4bec9be

services/inputflinger/PointerChoreographer.cpp

+49 −43
Original line number Diff line number Diff line
@@ -139,23 +139,24 @@ PointerChoreographer::PointerChoreographer( 
        mShowTouchesEnabled(false),

        mStylusPointerIconEnabled(false),

        mCurrentFocusedDisplay(ui::LogicalDisplayId::DEFAULT),

        mIsWindowInfoListenerRegistered(false),

        mWindowInfoListener(sp<PointerChoreographerDisplayInfoListener>::make(this)),

        mRegisterListener(registerListener),

        mUnregisterListener(unregisterListener) {}



PointerChoreographer::~PointerChoreographer() {

    std::scoped_lock _l(mLock);

    if (mWindowInfoListener == nullptr) {

        return;

    if (mIsWindowInfoListenerRegistered) {

        mUnregisterListener(mWindowInfoListener);

        mIsWindowInfoListenerRegistered = false;

    }

    mWindowInfoListener->onPointerChoreographerDestroyed();

    mUnregisterListener(mWindowInfoListener);

}



void PointerChoreographer::notifyInputDevicesChanged(const NotifyInputDevicesChangedArgs& args) {

    PointerDisplayChange pointerDisplayChange;



    { // acquire lock

        std::scoped_lock _l(mLock);

        std::scoped_lock _l(getLock());



        mInputDeviceInfos = args.inputDeviceInfos;

        pointerDisplayChange = updatePointerControllersLocked();
@@ -191,7 +192,7 @@ void PointerChoreographer::fadeMouseCursorOnKeyPress(const android::NotifyKeyArg 
        return;

    }



    std::scoped_lock _l(mLock);

    std::scoped_lock _l(getLock());

    ui::LogicalDisplayId targetDisplay = args.displayId;

    if (targetDisplay == ui::LogicalDisplayId::INVALID) {

        targetDisplay = mCurrentFocusedDisplay;
@@ -204,7 +205,7 @@ void PointerChoreographer::fadeMouseCursorOnKeyPress(const android::NotifyKeyArg 
}



NotifyMotionArgs PointerChoreographer::processMotion(const NotifyMotionArgs& args) {

    std::scoped_lock _l(mLock);

    std::scoped_lock _l(getLock());



    if (isFromMouse(args)) {

        return processMouseEventLocked(args);
@@ -433,7 +434,7 @@ void PointerChoreographer::notifyDeviceReset(const NotifyDeviceResetArgs& args) 
}



void PointerChoreographer::processDeviceReset(const NotifyDeviceResetArgs& args) {

    std::scoped_lock _l(mLock);

    std::scoped_lock _l(getLock());

    mTouchPointersByDevice.erase(args.deviceId);

    mStylusPointersByDevice.erase(args.deviceId);

    mDrawingTabletPointersByDevice.erase(args.deviceId);
@@ -447,17 +448,22 @@ void PointerChoreographer::onControllerAddedOrRemovedLocked() { 
    bool requireListener = !mTouchPointersByDevice.empty() || !mMousePointersByDisplay.empty() ||

            !mDrawingTabletPointersByDevice.empty() || !mStylusPointersByDevice.empty();



    if (requireListener && mWindowInfoListener == nullptr) {

        mWindowInfoListener = sp<PointerChoreographerDisplayInfoListener>::make(this);

        mWindowInfoListener->setInitialDisplayInfos(mRegisterListener(mWindowInfoListener));

        onPrivacySensitiveDisplaysChangedLocked(mWindowInfoListener->getPrivacySensitiveDisplays());

    } else if (!requireListener && mWindowInfoListener != nullptr) {

    // PointerChoreographer uses Listener's lock which is already held by caller

    base::ScopedLockAssertion assumeLocked(mWindowInfoListener->mLock);



    if (requireListener && !mIsWindowInfoListenerRegistered) {

        mIsWindowInfoListenerRegistered = true;

        mWindowInfoListener->setInitialDisplayInfosLocked(mRegisterListener(mWindowInfoListener));

        onPrivacySensitiveDisplaysChangedLocked(

                mWindowInfoListener->getPrivacySensitiveDisplaysLocked());

    } else if (!requireListener && mIsWindowInfoListenerRegistered) {

        mIsWindowInfoListenerRegistered = false;

        mUnregisterListener(mWindowInfoListener);

        mWindowInfoListener = nullptr;

    } else if (requireListener && mWindowInfoListener != nullptr) {

    } else if (requireListener) {

        // controller may have been added to an existing privacy sensitive display, we need to

        // update all controllers again

        onPrivacySensitiveDisplaysChangedLocked(mWindowInfoListener->getPrivacySensitiveDisplays());

        onPrivacySensitiveDisplaysChangedLocked(

                mWindowInfoListener->getPrivacySensitiveDisplaysLocked());

    }

}
@@ -494,7 +500,7 @@ void PointerChoreographer::onPrivacySensitiveDisplaysChangedLocked( 
void PointerChoreographer::notifyPointerCaptureChanged(

        const NotifyPointerCaptureChangedArgs& args) {

    if (args.request.isEnable()) {

        std::scoped_lock _l(mLock);

        std::scoped_lock _l(getLock());

        for (const auto& [_, mousePointerController] : mMousePointersByDisplay) {

            mousePointerController->fade(PointerControllerInterface::Transition::IMMEDIATE);

        }
@@ -502,14 +508,8 @@ void PointerChoreographer::notifyPointerCaptureChanged( 
    mNextListener.notify(args);

}



void PointerChoreographer::onPrivacySensitiveDisplaysChanged(

        const std::unordered_set<ui::LogicalDisplayId>& privacySensitiveDisplays) {

    std::scoped_lock _l(mLock);

    onPrivacySensitiveDisplaysChangedLocked(privacySensitiveDisplays);

}



void PointerChoreographer::dump(std::string& dump) {

    std::scoped_lock _l(mLock);

    std::scoped_lock _l(getLock());



    dump += "PointerChoreographer:\n";

    dump += StringPrintf(INDENT "Show Touches Enabled: %s\n",
@@ -579,6 +579,10 @@ bool PointerChoreographer::canUnfadeOnDisplay(ui::LogicalDisplayId displayId) { 
    return mDisplaysWithPointersHidden.find(displayId) == mDisplaysWithPointersHidden.end();

}



std::mutex& PointerChoreographer::getLock() const {

    return mWindowInfoListener->mLock;

}



PointerChoreographer::PointerDisplayChange PointerChoreographer::updatePointerControllersLocked() {

    std::set<ui::LogicalDisplayId /*displayId*/> mouseDisplaysToKeep;

    std::set<DeviceId> touchDevicesToKeep;
@@ -641,7 +645,7 @@ PointerChoreographer::PointerDisplayChange PointerChoreographer::updatePointerCo 
    std::erase_if(mDrawingTabletPointersByDevice, [&drawingTabletDevicesToKeep](const auto& pair) {

        return drawingTabletDevicesToKeep.find(pair.first) == drawingTabletDevicesToKeep.end();

    });

    std::erase_if(mMouseDevices, [&](DeviceId id) REQUIRES(mLock) {

    std::erase_if(mMouseDevices, [&](DeviceId id) REQUIRES(getLock()) {

        return std::find_if(mInputDeviceInfos.begin(), mInputDeviceInfos.end(),

                            [id](const auto& info) { return info.getId() == id; }) ==

                mInputDeviceInfos.end();
@@ -677,7 +681,7 @@ void PointerChoreographer::setDefaultMouseDisplayId(ui::LogicalDisplayId display 
    PointerDisplayChange pointerDisplayChange;



    { // acquire lock

        std::scoped_lock _l(mLock);

        std::scoped_lock _l(getLock());



        mDefaultMouseDisplayId = displayId;

        pointerDisplayChange = updatePointerControllersLocked();
@@ -690,7 +694,7 @@ void PointerChoreographer::setDisplayViewports(const std::vector<DisplayViewport 
    PointerDisplayChange pointerDisplayChange;



    { // acquire lock

        std::scoped_lock _l(mLock);

        std::scoped_lock _l(getLock());

        for (const auto& viewport : viewports) {

            const ui::LogicalDisplayId displayId = viewport.displayId;

            if (const auto it = mMousePointersByDisplay.find(displayId);
@@ -719,7 +723,7 @@ void PointerChoreographer::setDisplayViewports(const std::vector<DisplayViewport 


std::optional<DisplayViewport> PointerChoreographer::getViewportForPointerDevice(

        ui::LogicalDisplayId associatedDisplayId) {

    std::scoped_lock _l(mLock);

    std::scoped_lock _l(getLock());

    const ui::LogicalDisplayId resolvedDisplayId = getTargetMouseDisplayLocked(associatedDisplayId);

    if (const auto viewport = findViewportByIdLocked(resolvedDisplayId); viewport) {

        return *viewport;
@@ -728,7 +732,7 @@ std::optional<DisplayViewport> PointerChoreographer::getViewportForPointerDevice 
}



FloatPoint PointerChoreographer::getMouseCursorPosition(ui::LogicalDisplayId displayId) {

    std::scoped_lock _l(mLock);

    std::scoped_lock _l(getLock());

    const ui::LogicalDisplayId resolvedDisplayId = getTargetMouseDisplayLocked(displayId);

    if (auto it = mMousePointersByDisplay.find(resolvedDisplayId);

        it != mMousePointersByDisplay.end()) {
@@ -741,7 +745,7 @@ void PointerChoreographer::setShowTouchesEnabled(bool enabled) { 
    PointerDisplayChange pointerDisplayChange;



    { // acquire lock

        std::scoped_lock _l(mLock);

        std::scoped_lock _l(getLock());

        if (mShowTouchesEnabled == enabled) {

            return;

        }
@@ -756,7 +760,7 @@ void PointerChoreographer::setStylusPointerIconEnabled(bool enabled) { 
    PointerDisplayChange pointerDisplayChange;



    { // acquire lock

        std::scoped_lock _l(mLock);

        std::scoped_lock _l(getLock());

        if (mStylusPointerIconEnabled == enabled) {

            return;

        }
@@ -770,7 +774,7 @@ void PointerChoreographer::setStylusPointerIconEnabled(bool enabled) { 
bool PointerChoreographer::setPointerIcon(

        std::variant<std::unique_ptr<SpriteIcon>, PointerIconStyle> icon,

        ui::LogicalDisplayId displayId, DeviceId deviceId) {

    std::scoped_lock _l(mLock);

    std::scoped_lock _l(getLock());

    if (deviceId < 0) {

        LOG(WARNING) << "Invalid device id " << deviceId << ". Cannot set pointer icon.";

        return false;
@@ -821,7 +825,7 @@ bool PointerChoreographer::setPointerIcon( 
}



void PointerChoreographer::setPointerIconVisibility(ui::LogicalDisplayId displayId, bool visible) {

    std::scoped_lock lock(mLock);

    std::scoped_lock lock(getLock());

    if (visible) {

        mDisplaysWithPointersHidden.erase(displayId);

        // We do not unfade the icons here, because we don't know when the last event happened.
@@ -843,14 +847,14 @@ void PointerChoreographer::setPointerIconVisibility(ui::LogicalDisplayId display 
}



void PointerChoreographer::setFocusedDisplay(ui::LogicalDisplayId displayId) {

    std::scoped_lock lock(mLock);

    std::scoped_lock lock(getLock());

    mCurrentFocusedDisplay = displayId;

}



PointerChoreographer::ControllerConstructor PointerChoreographer::getMouseControllerConstructor(

        ui::LogicalDisplayId displayId) {

    std::function<std::shared_ptr<PointerControllerInterface>()> ctor =

            [this, displayId]() REQUIRES(mLock) {

            [this, displayId]() REQUIRES(getLock()) {

                auto pc = mPolicy.createPointerController(

                        PointerControllerInterface::ControllerType::MOUSE);

                if (const auto viewport = findViewportByIdLocked(displayId); viewport) {
@@ -864,7 +868,7 @@ PointerChoreographer::ControllerConstructor PointerChoreographer::getMouseContro 
PointerChoreographer::ControllerConstructor PointerChoreographer::getStylusControllerConstructor(

        ui::LogicalDisplayId displayId) {

    std::function<std::shared_ptr<PointerControllerInterface>()> ctor =

            [this, displayId]() REQUIRES(mLock) {

            [this, displayId]() REQUIRES(getLock()) {

                auto pc = mPolicy.createPointerController(

                        PointerControllerInterface::ControllerType::STYLUS);

                if (const auto viewport = findViewportByIdLocked(displayId); viewport) {
@@ -875,9 +879,11 @@ PointerChoreographer::ControllerConstructor PointerChoreographer::getStylusContr 
    return ConstructorDelegate(std::move(ctor));

}



// --- PointerChoreographer::PointerChoreographerDisplayInfoListener ---



void PointerChoreographer::PointerChoreographerDisplayInfoListener::onWindowInfosChanged(

        const gui::WindowInfosUpdate& windowInfosUpdate) {

    std::scoped_lock _l(mListenerLock);

    std::scoped_lock _l(mLock);

    if (mPointerChoreographer == nullptr) {

        return;

    }
@@ -885,25 +891,25 @@ void PointerChoreographer::PointerChoreographerDisplayInfoListener::onWindowInfo 
            getPrivacySensitiveDisplaysFromWindowInfos(windowInfosUpdate.windowInfos);

    if (newPrivacySensitiveDisplays != mPrivacySensitiveDisplays) {

        mPrivacySensitiveDisplays = std::move(newPrivacySensitiveDisplays);

        mPointerChoreographer->onPrivacySensitiveDisplaysChanged(mPrivacySensitiveDisplays);

        // PointerChoreographer uses Listener's lock.

        base::ScopedLockAssertion assumeLocked(mPointerChoreographer->getLock());

        mPointerChoreographer->onPrivacySensitiveDisplaysChangedLocked(mPrivacySensitiveDisplays);

    }

}



void PointerChoreographer::PointerChoreographerDisplayInfoListener::setInitialDisplayInfos(

void PointerChoreographer::PointerChoreographerDisplayInfoListener::setInitialDisplayInfosLocked(

        const std::vector<gui::WindowInfo>& windowInfos) {

    std::scoped_lock _l(mListenerLock);

    mPrivacySensitiveDisplays = getPrivacySensitiveDisplaysFromWindowInfos(windowInfos);

}



std::unordered_set<ui::LogicalDisplayId /*displayId*/>

PointerChoreographer::PointerChoreographerDisplayInfoListener::getPrivacySensitiveDisplays() {

    std::scoped_lock _l(mListenerLock);

PointerChoreographer::PointerChoreographerDisplayInfoListener::getPrivacySensitiveDisplaysLocked() {

    return mPrivacySensitiveDisplays;

}



void PointerChoreographer::PointerChoreographerDisplayInfoListener::

        onPointerChoreographerDestroyed() {

    std::scoped_lock _l(mListenerLock);

    std::scoped_lock _l(mLock);

    mPointerChoreographer = nullptr;

}

services/inputflinger/PointerChoreographer.h

+52 −40
Original line number Diff line number Diff line
@@ -118,31 +118,38 @@ public: 
private:

    using PointerDisplayChange = std::optional<

            std::tuple<ui::LogicalDisplayId /*displayId*/, FloatPoint /*cursorPosition*/>>;

    [[nodiscard]] PointerDisplayChange updatePointerControllersLocked() REQUIRES(mLock);

    [[nodiscard]] PointerDisplayChange calculatePointerDisplayChangeToNotify() REQUIRES(mLock);



    // PointerChoreographer's DisplayInfoListener can outlive the PointerChoreographer because when

    // the listener is registered and called from display thread, a strong pointer to the listener

    // (which can extend its lifecycle) is given away.

    // If we use two locks it can also cause deadlocks due to race in acquiring them between the

    // display and reader thread.

    // To avoid these problems we use DisplayInfoListener's lock in PointerChoreographer.

    std::mutex& getLock() const;



    [[nodiscard]] PointerDisplayChange updatePointerControllersLocked() REQUIRES(getLock());

    [[nodiscard]] PointerDisplayChange calculatePointerDisplayChangeToNotify() REQUIRES(getLock());

    const DisplayViewport* findViewportByIdLocked(ui::LogicalDisplayId displayId) const

            REQUIRES(mLock);

            REQUIRES(getLock());

    ui::LogicalDisplayId getTargetMouseDisplayLocked(ui::LogicalDisplayId associatedDisplayId) const

            REQUIRES(mLock);

            REQUIRES(getLock());

    std::pair<ui::LogicalDisplayId /*displayId*/, PointerControllerInterface&>

    ensureMouseControllerLocked(ui::LogicalDisplayId associatedDisplayId) REQUIRES(mLock);

    InputDeviceInfo* findInputDeviceLocked(DeviceId deviceId) REQUIRES(mLock);

    bool canUnfadeOnDisplay(ui::LogicalDisplayId displayId) REQUIRES(mLock);

    ensureMouseControllerLocked(ui::LogicalDisplayId associatedDisplayId) REQUIRES(getLock());

    InputDeviceInfo* findInputDeviceLocked(DeviceId deviceId) REQUIRES(getLock());

    bool canUnfadeOnDisplay(ui::LogicalDisplayId displayId) REQUIRES(getLock());



    void fadeMouseCursorOnKeyPress(const NotifyKeyArgs& args);

    NotifyMotionArgs processMotion(const NotifyMotionArgs& args);

    NotifyMotionArgs processMouseEventLocked(const NotifyMotionArgs& args) REQUIRES(mLock);

    NotifyMotionArgs processTouchpadEventLocked(const NotifyMotionArgs& args) REQUIRES(mLock);

    void processDrawingTabletEventLocked(const NotifyMotionArgs& args) REQUIRES(mLock);

    void processTouchscreenAndStylusEventLocked(const NotifyMotionArgs& args) REQUIRES(mLock);

    void processStylusHoverEventLocked(const NotifyMotionArgs& args) REQUIRES(mLock);

    NotifyMotionArgs processMouseEventLocked(const NotifyMotionArgs& args) REQUIRES(getLock());

    NotifyMotionArgs processTouchpadEventLocked(const NotifyMotionArgs& args) REQUIRES(getLock());

    void processDrawingTabletEventLocked(const NotifyMotionArgs& args) REQUIRES(getLock());

    void processTouchscreenAndStylusEventLocked(const NotifyMotionArgs& args) REQUIRES(getLock());

    void processStylusHoverEventLocked(const NotifyMotionArgs& args) REQUIRES(getLock());

    void processDeviceReset(const NotifyDeviceResetArgs& args);

    void onControllerAddedOrRemovedLocked() REQUIRES(mLock);

    void onControllerAddedOrRemovedLocked() REQUIRES(getLock());

    void onPrivacySensitiveDisplaysChangedLocked(

            const std::unordered_set<ui::LogicalDisplayId>& privacySensitiveDisplays)

            REQUIRES(mLock);

    void onPrivacySensitiveDisplaysChanged(

            const std::unordered_set<ui::LogicalDisplayId>& privacySensitiveDisplays);

            REQUIRES(getLock());



    /* This listener keeps tracks of visible privacy sensitive displays and updates the

     * choreographer if there are any changes.
@@ -156,49 +163,50 @@ private: 
        explicit PointerChoreographerDisplayInfoListener(PointerChoreographer* pc)

              : mPointerChoreographer(pc){};

        void onWindowInfosChanged(const gui::WindowInfosUpdate&) override;

        void setInitialDisplayInfos(const std::vector<gui::WindowInfo>& windowInfos);

        std::unordered_set<ui::LogicalDisplayId /*displayId*/> getPrivacySensitiveDisplays();

        void setInitialDisplayInfosLocked(const std::vector<gui::WindowInfo>& windowInfos)

                REQUIRES(mLock);

        std::unordered_set<ui::LogicalDisplayId> getPrivacySensitiveDisplaysLocked()

                REQUIRES(mLock);

        void onPointerChoreographerDestroyed();



        // This lock is also used by PointerChoreographer. See PointerChoreographer::getLock().

        std::mutex mLock;



    private:

        std::mutex mListenerLock;

        PointerChoreographer* mPointerChoreographer GUARDED_BY(mListenerLock);

        PointerChoreographer* mPointerChoreographer GUARDED_BY(mLock);

        std::unordered_set<ui::LogicalDisplayId /*displayId*/> mPrivacySensitiveDisplays

                GUARDED_BY(mListenerLock);

                GUARDED_BY(mLock);

    };

    sp<PointerChoreographerDisplayInfoListener> mWindowInfoListener GUARDED_BY(mLock);



    using ControllerConstructor =

            ConstructorDelegate<std::function<std::shared_ptr<PointerControllerInterface>()>>;

    ControllerConstructor mTouchControllerConstructor GUARDED_BY(mLock);

    ControllerConstructor mTouchControllerConstructor GUARDED_BY(getLock());

    ControllerConstructor getMouseControllerConstructor(ui::LogicalDisplayId displayId)

            REQUIRES(mLock);

            REQUIRES(getLock());

    ControllerConstructor getStylusControllerConstructor(ui::LogicalDisplayId displayId)

            REQUIRES(mLock);



    std::mutex mLock;

            REQUIRES(getLock());



    InputListenerInterface& mNextListener;

    PointerChoreographerPolicyInterface& mPolicy;



    std::map<ui::LogicalDisplayId, std::shared_ptr<PointerControllerInterface>>

            mMousePointersByDisplay GUARDED_BY(mLock);

            mMousePointersByDisplay GUARDED_BY(getLock());

    std::map<DeviceId, std::shared_ptr<PointerControllerInterface>> mTouchPointersByDevice

            GUARDED_BY(mLock);

            GUARDED_BY(getLock());

    std::map<DeviceId, std::shared_ptr<PointerControllerInterface>> mStylusPointersByDevice

            GUARDED_BY(mLock);

            GUARDED_BY(getLock());

    std::map<DeviceId, std::shared_ptr<PointerControllerInterface>> mDrawingTabletPointersByDevice

            GUARDED_BY(mLock);



    ui::LogicalDisplayId mDefaultMouseDisplayId GUARDED_BY(mLock);

    ui::LogicalDisplayId mNotifiedPointerDisplayId GUARDED_BY(mLock);

    std::vector<InputDeviceInfo> mInputDeviceInfos GUARDED_BY(mLock);

    std::set<DeviceId> mMouseDevices GUARDED_BY(mLock);

    std::vector<DisplayViewport> mViewports GUARDED_BY(mLock);

    bool mShowTouchesEnabled GUARDED_BY(mLock);

    bool mStylusPointerIconEnabled GUARDED_BY(mLock);

            GUARDED_BY(getLock());



    ui::LogicalDisplayId mDefaultMouseDisplayId GUARDED_BY(getLock());

    ui::LogicalDisplayId mNotifiedPointerDisplayId GUARDED_BY(getLock());

    std::vector<InputDeviceInfo> mInputDeviceInfos GUARDED_BY(getLock());

    std::set<DeviceId> mMouseDevices GUARDED_BY(getLock());

    std::vector<DisplayViewport> mViewports GUARDED_BY(getLock());

    bool mShowTouchesEnabled GUARDED_BY(getLock());

    bool mStylusPointerIconEnabled GUARDED_BY(getLock());

    std::set<ui::LogicalDisplayId /*displayId*/> mDisplaysWithPointersHidden;

    ui::LogicalDisplayId mCurrentFocusedDisplay GUARDED_BY(mLock);

    ui::LogicalDisplayId mCurrentFocusedDisplay GUARDED_BY(getLock());



protected:

    using WindowListenerRegisterConsumer = std::function<std::vector<gui::WindowInfo>(
@@ -211,6 +219,10 @@ protected: 
                                  const WindowListenerUnregisterConsumer& unregisterListener);



private:

    // WindowInfoListener object should always exist while PointerChoreographer exists, because we

    // need to use the lock from it. But we don't always need to register the listener.

    bool mIsWindowInfoListenerRegistered GUARDED_BY(getLock());

    const sp<PointerChoreographerDisplayInfoListener> mWindowInfoListener;

    const WindowListenerRegisterConsumer mRegisterListener;

    const WindowListenerUnregisterConsumer mUnregisterListener;

};