From 5693cee2f91e72ac2cbbb958cbb7cd6b58a479f3 Mon Sep 17 00:00:00 2001
From: Prabir Pradhan <prabirmsp@google.com>
Date: Fri, 31 Dec 2021 06:51:15 -0800
Subject: Allow its WindowInfosListener to outlive PointerController

A strong pointer to PointerController::DisplayInfoListener is given away
when the listener is registered, so PC cannot guarantee that the
listener is destroyed when it is destroyed. This means the listener can
outlive PC, so there is a race condition between PC's destruction and an
update to the listener.

While it could be argued that it is the caller's responsibility to
ensure that the listener is not updated after it is unregistered, there
is no way to guarantee that using strong pointers. Here, we can be
defensive and protect against this case.

Bug: 212672261
Test: atest libinputservice_test
Change-Id: I358a725980cc8c7d6ad0483a9b2a8b8715a03424
---
 libs/input/PointerController.h | 40 +++++++++++++++++++++++++++++++---------
 1 file changed, 31 insertions(+), 9 deletions(-)

(limited to 'libs/input/PointerController.h')

diff --git a/libs/input/PointerController.h b/libs/input/PointerController.h
index 796077f6c38c..2e6e851ee15a 100644
--- a/libs/input/PointerController.h
+++ b/libs/input/PointerController.h
@@ -72,13 +72,31 @@ public:
     void reloadPointerResources();
     void onDisplayViewportsUpdated(std::vector<DisplayViewport>& viewports);
 
-    void onDisplayInfosChanged(const std::vector<gui::DisplayInfo>& displayInfos);
+    void onDisplayInfosChangedLocked(const std::vector<gui::DisplayInfo>& displayInfos)
+            REQUIRES(getLock());
+
+protected:
+    using WindowListenerConsumer =
+            std::function<void(const sp<android::gui::WindowInfosListener>&)>;
+
+    // Constructor used to test WindowInfosListener registration.
+    PointerController(const sp<PointerControllerPolicyInterface>& policy, const sp<Looper>& looper,
+                      const sp<SpriteController>& spriteController,
+                      WindowListenerConsumer registerListener,
+                      WindowListenerConsumer unregisterListener);
 
 private:
+    PointerController(const sp<PointerControllerPolicyInterface>& policy, const sp<Looper>& looper,
+                      const sp<SpriteController>& spriteController);
+
     friend PointerControllerContext::LooperCallback;
     friend PointerControllerContext::MessageHandler;
 
-    mutable std::mutex mLock;
+    // PointerController's DisplayInfoListener can outlive the PointerController because when the
+    // listener is registered, a strong pointer to the listener (which can extend its lifecycle)
+    // is given away. To avoid the small overhead of using two separate locks in these two objects,
+    // we use the DisplayInfoListener's lock in PointerController.
+    std::mutex& getLock() const;
 
     PointerControllerContext mContext;
 
@@ -89,24 +107,28 @@ private:
 
         std::vector<gui::DisplayInfo> mDisplayInfos;
         std::unordered_map<int32_t /* displayId */, TouchSpotController> spotControllers;
-    } mLocked GUARDED_BY(mLock);
+    } mLocked GUARDED_BY(getLock());
 
     class DisplayInfoListener : public gui::WindowInfosListener {
     public:
-        explicit DisplayInfoListener(PointerController& pc) : mPointerController(pc){};
+        explicit DisplayInfoListener(PointerController* pc) : mPointerController(pc){};
         void onWindowInfosChanged(const std::vector<android::gui::WindowInfo>&,
                                   const std::vector<android::gui::DisplayInfo>&) override;
+        void onPointerControllerDestroyed();
+
+        // This lock is also used by PointerController. See PointerController::getLock().
+        std::mutex mLock;
 
     private:
-        PointerController& mPointerController;
+        PointerController* mPointerController GUARDED_BY(mLock);
     };
+
     sp<DisplayInfoListener> mDisplayInfoListener;
+    const WindowListenerConsumer mUnregisterWindowInfosListener;
 
-    const ui::Transform& getTransformForDisplayLocked(int displayId) const REQUIRES(mLock);
+    const ui::Transform& getTransformForDisplayLocked(int displayId) const REQUIRES(getLock());
 
-    PointerController(const sp<PointerControllerPolicyInterface>& policy, const sp<Looper>& looper,
-                      const sp<SpriteController>& spriteController);
-    void clearSpotsLocked();
+    void clearSpotsLocked() REQUIRES(getLock());
 };
 
 } // namespace android
-- 
cgit v1.2.3-59-g8ed1b