Snap for 9127121 from 293041cac77386d5a782de721a1ee63a34556965 to tm-qpr1-releaseandroid-13.0.0_r24android-13.0.0_r23android-13.0.0_r22android-13.0.0_r21android-13.0.0_r20android-13.0.0_r19android-13.0.0_r18android-13.0.0_r17android-13.0.0_r16android13-qpr1-s5-releaseandroid13-qpr1-s4-releaseandroid13-qpr1-s3-releaseandroid13-qpr1-s2-releaseandroid13-qpr1-s1-release

Change-Id: I7327923aa6952c485af578121a177262f3281432

10 files changed, 76 insertions, 7 deletions

diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp
index aba81f60d8..a51bbb1553 100644
--- a/libs/gui/BLASTBufferQueue.cpp
+++ b/libs/gui/BLASTBufferQueue.cpp
@@ -307,7 +307,6 @@ void BLASTBufferQueue::transactionCommittedCallback(nsecs_t /*latchTime*/,
             BQA_LOGE("No matching SurfaceControls found: mSurfaceControlsWithPendingCallback was "
                      "empty.");
         }
-
         decStrong((void*)transactionCommittedCallbackThunk);
     }
 }
@@ -350,6 +349,20 @@ void BLASTBufferQueue::transactionCallback(nsecs_t /*latchTime*/, const sp<Fence
                                                     stat.latchTime,
                                                     stat.frameEventStats.dequeueReadyTime);
                 }
+                auto currFrameNumber = stat.frameEventStats.frameNumber;
+                std::vector<ReleaseCallbackId> staleReleases;
+                for (const auto& [key, value]: mSubmitted) {
+                    if (currFrameNumber > key.framenumber) {
+                        staleReleases.push_back(key);
+                    }
+                }
+                for (const auto& staleRelease : staleReleases) {
+                    BQA_LOGE("Faking releaseBufferCallback from transactionCompleteCallback");
+                    BBQ_TRACE("FakeReleaseCallback");
+                    releaseBufferCallbackLocked(staleRelease,
+                        stat.previousReleaseFence ? stat.previousReleaseFence : Fence::NO_FENCE,
+                        stat.currentMaxAcquiredBufferCount);
+                }
             } else {
                 BQA_LOGE("Failed to find matching SurfaceControl in transactionCallback");
             }
@@ -390,7 +403,14 @@ void BLASTBufferQueue::releaseBufferCallback(
         const ReleaseCallbackId& id, const sp<Fence>& releaseFence,
         std::optional<uint32_t> currentMaxAcquiredBufferCount) {
     BBQ_TRACE();
+
     std::unique_lock _lock{mMutex};
+    releaseBufferCallbackLocked(id, releaseFence, currentMaxAcquiredBufferCount);
+}
+
+void BLASTBufferQueue::releaseBufferCallbackLocked(const ReleaseCallbackId& id,
+        const sp<Fence>& releaseFence, std::optional<uint32_t> currentMaxAcquiredBufferCount) {
+    ATRACE_CALL();
     BQA_LOGV("releaseBufferCallback %s", id.to_string().c_str());
 
     // Calculate how many buffers we need to hold before we release them back
@@ -408,7 +428,11 @@ void BLASTBufferQueue::releaseBufferCallback(
 
     const auto numPendingBuffersToHold =
             isEGL ? std::max(0u, mMaxAcquiredBuffers - mCurrentMaxAcquiredBufferCount) : 0;
-    mPendingRelease.emplace_back(ReleasedBuffer{id, releaseFence});
+
+    auto rb = ReleasedBuffer{id, releaseFence};
+    if (std::find(mPendingRelease.begin(), mPendingRelease.end(), rb) == mPendingRelease.end()) {
+        mPendingRelease.emplace_back(rb);
+    }
 
     // Release all buffers that are beyond the ones that we need to hold
     while (mPendingRelease.size() > numPendingBuffersToHold) {
diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp
index 9358e29030..0f5192d41c 100644
--- a/libs/gui/SurfaceComposerClient.cpp
+++ b/libs/gui/SurfaceComposerClient.cpp
@@ -352,7 +352,8 @@ void TransactionCompletedListener::onTransactionCompleted(ListenerStats listener
                                       transactionStats.latchTime, surfaceStats.acquireTimeOrFence,
                                       transactionStats.presentFence,
                                       surfaceStats.previousReleaseFence, surfaceStats.transformHint,
-                                      surfaceStats.eventStats);
+                                      surfaceStats.eventStats,
+                                      surfaceStats.currentMaxAcquiredBufferCount);
             }
 
             callbackFunction(transactionStats.latchTime, transactionStats.presentFence,
@@ -377,7 +378,8 @@ void TransactionCompletedListener::onTransactionCompleted(ListenerStats listener
                                       transactionStats.latchTime, surfaceStats.acquireTimeOrFence,
                                       transactionStats.presentFence,
                                       surfaceStats.previousReleaseFence, surfaceStats.transformHint,
-                                      surfaceStats.eventStats);
+                                      surfaceStats.eventStats,
+                                      surfaceStats.currentMaxAcquiredBufferCount);
                 if (callbacksMap[callbackId].surfaceControls[surfaceStats.surfaceControl]) {
                     callbacksMap[callbackId]
                             .surfaceControls[surfaceStats.surfaceControl]
diff --git a/libs/gui/include/gui/BLASTBufferQueue.h b/libs/gui/include/gui/BLASTBufferQueue.h
index 9d287910a5..f5898d20f1 100644
--- a/libs/gui/include/gui/BLASTBufferQueue.h
+++ b/libs/gui/include/gui/BLASTBufferQueue.h
@@ -95,9 +95,12 @@ public:
                                      const std::vector<SurfaceControlStats>& stats);
     void releaseBufferCallback(const ReleaseCallbackId& id, const sp<Fence>& releaseFence,
                                std::optional<uint32_t> currentMaxAcquiredBufferCount);
+    void releaseBufferCallbackLocked(const ReleaseCallbackId& id, const sp<Fence>& releaseFence,
+                               std::optional<uint32_t> currentMaxAcquiredBufferCount);
     void syncNextTransaction(std::function<void(SurfaceComposerClient::Transaction*)> callback,
                              bool acquireSingleBuffer = true);
     void stopContinuousSyncTransaction();
+
     void mergeWithNextTransaction(SurfaceComposerClient::Transaction* t, uint64_t frameNumber);
     void applyPendingTransactions(uint64_t frameNumber);
     SurfaceComposerClient::Transaction* gatherPendingTransactions(uint64_t frameNumber);
@@ -177,6 +180,12 @@ private:
     struct ReleasedBuffer {
         ReleaseCallbackId callbackId;
         sp<Fence> releaseFence;
+        bool operator==(const ReleasedBuffer& rhs) const {
+            // Only compare Id so if we somehow got two callbacks
+            // with different fences we don't decrement mNumAcquired
+            // too far.
+            return rhs.callbackId == callbackId;
+        }
     };
     std::deque<ReleasedBuffer> mPendingRelease GUARDED_BY(mMutex);
 
diff --git a/libs/gui/include/gui/SurfaceComposerClient.h b/libs/gui/include/gui/SurfaceComposerClient.h
index b598b43403..9033e17c53 100644
--- a/libs/gui/include/gui/SurfaceComposerClient.h
+++ b/libs/gui/include/gui/SurfaceComposerClient.h
@@ -65,14 +65,16 @@ struct SurfaceControlStats {
     SurfaceControlStats(const sp<SurfaceControl>& sc, nsecs_t latchTime,
                         std::variant<nsecs_t, sp<Fence>> acquireTimeOrFence,
                         const sp<Fence>& presentFence, const sp<Fence>& prevReleaseFence,
-                        uint32_t hint, FrameEventHistoryStats eventStats)
+                        uint32_t hint, FrameEventHistoryStats eventStats,
+                        uint32_t currentMaxAcquiredBufferCount)
           : surfaceControl(sc),
             latchTime(latchTime),
             acquireTimeOrFence(std::move(acquireTimeOrFence)),
             presentFence(presentFence),
             previousReleaseFence(prevReleaseFence),
             transformHint(hint),
-            frameEventStats(eventStats) {}
+            frameEventStats(eventStats),
+            currentMaxAcquiredBufferCount(currentMaxAcquiredBufferCount) {}
 
     sp<SurfaceControl> surfaceControl;
     nsecs_t latchTime = -1;
@@ -81,6 +83,7 @@ struct SurfaceControlStats {
     sp<Fence> previousReleaseFence;
     uint32_t transformHint = 0;
     FrameEventHistoryStats frameEventStats;
+    uint32_t currentMaxAcquiredBufferCount = 0;
 };
 
 using TransactionCompletedCallbackTakesContext =
diff --git a/libs/gui/include/gui/test/CallbackUtils.h b/libs/gui/include/gui/test/CallbackUtils.h
index 62d1496ccd..08785b49c1 100644
--- a/libs/gui/include/gui/test/CallbackUtils.h
+++ b/libs/gui/include/gui/test/CallbackUtils.h
@@ -135,7 +135,8 @@ private:
         void verifySurfaceControlStats(const SurfaceControlStats& surfaceControlStats,
                                        nsecs_t latchTime) const {
             const auto& [surfaceControl, latch, acquireTimeOrFence, presentFence,
-                         previousReleaseFence, transformHint, frameEvents] = surfaceControlStats;
+                         previousReleaseFence, transformHint, frameEvents, ignore] =
+                surfaceControlStats;
 
             ASSERT_TRUE(std::holds_alternative<nsecs_t>(acquireTimeOrFence));
             ASSERT_EQ(std::get<nsecs_t>(acquireTimeOrFence) > 0,
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 {