Delete mController when eventHub device is going away

When an event hub device is going away, the controller associated with
it should be deleted.

Before this CL, mController would remain, and its use would result in an
illegal memory access, because it was storing a reference to a deleted
context.

Bug: 245770596
Test: atest inputflinger_tests:InputDeviceTest#DumpDoesNotCrash
Merged-In: I298f43172472f74fa4d5d8e0b7f52bd5c13d14f7
Change-Id: I298f43172472f74fa4d5d8e0b7f52bd5c13d14f7
(cherry picked from commit 30feb8c162aa2e5348bba20e99e8db2a61bac6e7)

5 files changed, 30 insertions, 0 deletions

diff --git a/services/inputflinger/reader/InputDevice.cpp b/services/inputflinger/reader/InputDevice.cpp
index a0119986a6..989700f6cf 100644
--- a/services/inputflinger/reader/InputDevice.cpp
+++ b/services/inputflinger/reader/InputDevice.cpp
@@ -232,6 +232,10 @@ void InputDevice::addEventHubDevice(int32_t eventHubId, bool populateMappers) {
 }
 
 void InputDevice::removeEventHubDevice(int32_t eventHubId) {
+    if (mController != nullptr && mController->getEventHubId() == eventHubId) {
+        // Delete mController, since the corresponding eventhub device is going away
+        mController = nullptr;
+    }
     mDevices.erase(eventHubId);
 }
 
diff --git a/services/inputflinger/reader/controller/PeripheralController.cpp b/services/inputflinger/reader/controller/PeripheralController.cpp
index a6934960c9..8065f57524 100644
--- a/services/inputflinger/reader/controller/PeripheralController.cpp
+++ b/services/inputflinger/reader/controller/PeripheralController.cpp
@@ -524,4 +524,8 @@ std::optional<int32_t> PeripheralController::getLightPlayerId(int32_t lightId) {
     return light->getLightPlayerId();
 }
 
+int32_t PeripheralController::getEventHubId() const {
+    return getDeviceContext().getEventHubId();
+}
+
 } // namespace android
diff --git a/services/inputflinger/reader/controller/PeripheralController.h b/services/inputflinger/reader/controller/PeripheralController.h
index b1bc8c732c..ac951ebe2a 100644
--- a/services/inputflinger/reader/controller/PeripheralController.h
+++ b/services/inputflinger/reader/controller/PeripheralController.h
@@ -31,6 +31,7 @@ public:
     explicit PeripheralController(InputDeviceContext& deviceContext);
     ~PeripheralController() override;
 
+    int32_t getEventHubId() const override;
     void populateDeviceInfo(InputDeviceInfo* deviceInfo) override;
     void dump(std::string& dump) override;
     bool setLightColor(int32_t lightId, int32_t color) override;
@@ -43,6 +44,7 @@ public:
 private:
     inline int32_t getDeviceId() { return mDeviceContext.getId(); }
     inline InputDeviceContext& getDeviceContext() { return mDeviceContext; }
+    inline InputDeviceContext& getDeviceContext() const { return mDeviceContext; }
 
     InputDeviceContext& mDeviceContext;
     void configureLights();
diff --git a/services/inputflinger/reader/controller/PeripheralControllerInterface.h b/services/inputflinger/reader/controller/PeripheralControllerInterface.h
index 7688a431d1..306e36119b 100644
--- a/services/inputflinger/reader/controller/PeripheralControllerInterface.h
+++ b/services/inputflinger/reader/controller/PeripheralControllerInterface.h
@@ -33,6 +33,8 @@ public:
     PeripheralControllerInterface() {}
     virtual ~PeripheralControllerInterface() {}
 
+    virtual int32_t getEventHubId() const = 0;
+
     // Interface methods for Battery
     virtual std::optional<int32_t> getBatteryCapacity(int32_t batteryId) = 0;
     virtual std::optional<int32_t> getBatteryStatus(int32_t batteryId) = 0;
diff --git a/services/inputflinger/tests/InputReader_test.cpp b/services/inputflinger/tests/InputReader_test.cpp
index e1befedcff..74d4f3b99f 100644
--- a/services/inputflinger/tests/InputReader_test.cpp
+++ b/services/inputflinger/tests/InputReader_test.cpp
@@ -2157,6 +2157,8 @@ public:
 
     ~FakePeripheralController() override {}
 
+    int32_t getEventHubId() const { return getDeviceContext().getEventHubId(); }
+
     void populateDeviceInfo(InputDeviceInfo* deviceInfo) override {}
 
     void dump(std::string& dump) override {}
@@ -2190,6 +2192,7 @@ private:
     InputDeviceContext& mDeviceContext;
     inline int32_t getDeviceId() { return mDeviceContext.getId(); }
     inline InputDeviceContext& getDeviceContext() { return mDeviceContext; }
+    inline InputDeviceContext& getDeviceContext() const { return mDeviceContext; }
 };
 
 TEST_F(InputReaderTest, BatteryGetCapacity) {
@@ -2931,6 +2934,21 @@ TEST_F(InputDeviceTest, Configure_UniqueId_CorrectlyMatches) {
     ASSERT_EQ(DISPLAY_UNIQUE_ID, mDevice->getAssociatedDisplayUniqueId());
 }
 
+/**
+ * This test reproduces a crash caused by a dangling reference that remains after device is added
+ * and removed. The reference is accessed in InputDevice::dump(..);
+ */
+TEST_F(InputDeviceTest, DumpDoesNotCrash) {
+    constexpr int32_t TEST_EVENTHUB_ID = 10;
+    mFakeEventHub->addDevice(TEST_EVENTHUB_ID, "Test EventHub device", InputDeviceClass::BATTERY);
+
+    InputDevice device(mReader->getContext(), 1 /*id*/, 2 /*generation*/, {} /*identifier*/);
+    device.addEventHubDevice(TEST_EVENTHUB_ID, true /*populateMappers*/);
+    device.removeEventHubDevice(TEST_EVENTHUB_ID);
+    std::string dumpStr, eventHubDevStr;
+    device.dump(dumpStr, eventHubDevStr);
+}
+
 // --- InputMapperTest ---
 
 class InputMapperTest : public testing::Test {