Snap for 9127121 from 293041cac77386d5a782de721a1ee63a34556965 to tm-qpr1-release
Change-Id: I7327923aa6952c485af578121a177262f3281432
diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp
index aba81f6..a51bbb1 100644
--- a/libs/gui/BLASTBufferQueue.cpp
+++ b/libs/gui/BLASTBufferQueue.cpp
@@ -307,7 +307,6 @@
BQA_LOGE("No matching SurfaceControls found: mSurfaceControlsWithPendingCallback was "
"empty.");
}
-
decStrong((void*)transactionCommittedCallbackThunk);
}
}
@@ -350,6 +349,20 @@
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 @@
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 @@
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 9358e29..0f5192d 100644
--- a/libs/gui/SurfaceComposerClient.cpp
+++ b/libs/gui/SurfaceComposerClient.cpp
@@ -352,7 +352,8 @@
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 @@
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 9d28791..f5898d2 100644
--- a/libs/gui/include/gui/BLASTBufferQueue.h
+++ b/libs/gui/include/gui/BLASTBufferQueue.h
@@ -95,9 +95,12 @@
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 @@
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 b598b43..9033e17 100644
--- a/libs/gui/include/gui/SurfaceComposerClient.h
+++ b/libs/gui/include/gui/SurfaceComposerClient.h
@@ -65,14 +65,16 @@
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 @@
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 62d1496..08785b4 100644
--- a/libs/gui/include/gui/test/CallbackUtils.h
+++ b/libs/gui/include/gui/test/CallbackUtils.h
@@ -135,7 +135,8 @@
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 a011998..989700f 100644
--- a/services/inputflinger/reader/InputDevice.cpp
+++ b/services/inputflinger/reader/InputDevice.cpp
@@ -232,6 +232,10 @@
}
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 a693496..8065f57 100644
--- a/services/inputflinger/reader/controller/PeripheralController.cpp
+++ b/services/inputflinger/reader/controller/PeripheralController.cpp
@@ -524,4 +524,8 @@
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 b1bc8c7..ac951eb 100644
--- a/services/inputflinger/reader/controller/PeripheralController.h
+++ b/services/inputflinger/reader/controller/PeripheralController.h
@@ -31,6 +31,7 @@
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 @@
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 7688a43..306e361 100644
--- a/services/inputflinger/reader/controller/PeripheralControllerInterface.h
+++ b/services/inputflinger/reader/controller/PeripheralControllerInterface.h
@@ -33,6 +33,8 @@
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 e1befed..74d4f3b 100644
--- a/services/inputflinger/tests/InputReader_test.cpp
+++ b/services/inputflinger/tests/InputReader_test.cpp
@@ -2157,6 +2157,8 @@
~FakePeripheralController() override {}
+ int32_t getEventHubId() const { return getDeviceContext().getEventHubId(); }
+
void populateDeviceInfo(InputDeviceInfo* deviceInfo) override {}
void dump(std::string& dump) override {}
@@ -2190,6 +2192,7 @@
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 @@
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 {