Metrics Collector: Prevent queuing interactions from ignored devices
We don't collect metrics for injected events. Since the interactions
queue is processed from the InputReader thread, if many interactions
are notified by InputDispatcher when the Reader thread is not active
(e.g. for interactions from injected events), it's possible for the
interactions queue to grow indefinitely.
To mitigate this:
- Do not queue interactions from ignored devices, such as those
from injected events that use VIRTUAL_KEYBOARD_ID. This prevents
the queue from growing in the first place due to injected events,
which do not come from the Reader thread.
- Ensure the interactions queue is bounded so that it cannot grow
indefinitely.
Bug: 287676652
Test: manual
Test: atest inputflinger_tests
Change-Id: I1ce27b1817704dd83307fab1917465dcb85ac31e
diff --git a/services/inputflinger/InputDeviceMetricsCollector.cpp b/services/inputflinger/InputDeviceMetricsCollector.cpp
index 89e37db..2799327 100644
--- a/services/inputflinger/InputDeviceMetricsCollector.cpp
+++ b/services/inputflinger/InputDeviceMetricsCollector.cpp
@@ -39,6 +39,8 @@
*/
const bool DEBUG = __android_log_is_loggable(ANDROID_LOG_DEBUG, LOG_TAG, ANDROID_LOG_INFO);
+constexpr size_t INTERACTIONS_QUEUE_CAPACITY = 500;
+
int32_t linuxBusToInputDeviceBusEnum(int32_t linuxBus) {
// When adding cases to this switch, also add them to the copy of this method in
// TouchpadInputMapper.cpp.
@@ -201,7 +203,10 @@
InputDeviceMetricsCollector::InputDeviceMetricsCollector(InputListenerInterface& listener,
InputDeviceMetricsLogger& logger,
nanoseconds usageSessionTimeout)
- : mNextListener(listener), mLogger(logger), mUsageSessionTimeout(usageSessionTimeout) {}
+ : mNextListener(listener),
+ mLogger(logger),
+ mUsageSessionTimeout(usageSessionTimeout),
+ mInteractionsQueue(INTERACTIONS_QUEUE_CAPACITY) {}
void InputDeviceMetricsCollector::notifyInputDevicesChanged(
const NotifyInputDevicesChangedArgs& args) {
@@ -262,6 +267,9 @@
void InputDeviceMetricsCollector::notifyDeviceInteraction(int32_t deviceId, nsecs_t timestamp,
const std::set<Uid>& uids) {
+ if (isIgnoredInputDeviceId(deviceId)) {
+ return;
+ }
mInteractionsQueue.push(DeviceId{deviceId}, timestamp, uids);
}
diff --git a/services/inputflinger/SyncQueue.h b/services/inputflinger/SyncQueue.h
index 62efd55..84ccace 100644
--- a/services/inputflinger/SyncQueue.h
+++ b/services/inputflinger/SyncQueue.h
@@ -27,6 +27,10 @@
template <class T>
class SyncQueue {
public:
+ SyncQueue() = default;
+
+ SyncQueue(size_t capacity) : mCapacity(capacity) {}
+
/** Retrieve and remove the oldest object. Returns std::nullopt if the queue is empty. */
std::optional<T> pop() {
std::scoped_lock lock(mLock);
@@ -38,14 +42,23 @@
return t;
};
- /** Add a new object to the queue. */
+ /**
+ * Add a new object to the queue.
+ * Return true if an element was successfully added.
+ * Return false if the queue is full.
+ */
template <class... Args>
- void push(Args&&... args) {
+ bool push(Args&&... args) {
std::scoped_lock lock(mLock);
+ if (mCapacity && mQueue.size() == mCapacity) {
+ return false;
+ }
mQueue.emplace_back(args...);
+ return true;
};
private:
+ const std::optional<size_t> mCapacity;
std::mutex mLock;
std::list<T> mQueue GUARDED_BY(mLock);
};
diff --git a/services/inputflinger/tests/InputDeviceMetricsCollector_test.cpp b/services/inputflinger/tests/InputDeviceMetricsCollector_test.cpp
index 0aa5e23..484e7d6 100644
--- a/services/inputflinger/tests/InputDeviceMetricsCollector_test.cpp
+++ b/services/inputflinger/tests/InputDeviceMetricsCollector_test.cpp
@@ -421,6 +421,7 @@
// Device was used.
mMetricsCollector.notifyMotion(generateMotionArgs(ignoredDeviceId));
mTestListener.assertNotifyMotionWasCalled();
+ mMetricsCollector.notifyDeviceInteraction(ignoredDeviceId, TIME.count(), uids({0, 1, 2}));
ASSERT_NO_FATAL_FAILURE(assertUsageNotLogged());
// Device was used again after the usage timeout expired, but we still don't log usage.
diff --git a/services/inputflinger/tests/SyncQueue_test.cpp b/services/inputflinger/tests/SyncQueue_test.cpp
index af2f961..b57ccc2 100644
--- a/services/inputflinger/tests/SyncQueue_test.cpp
+++ b/services/inputflinger/tests/SyncQueue_test.cpp
@@ -50,6 +50,18 @@
}
}
+// Make sure the queue has strict capacity limits.
+TEST(SyncQueueTest, QueueReachesCapacity) {
+ constexpr size_t capacity = 3;
+ SyncQueue<int> queue(capacity);
+
+ // First 3 elements should be added successfully
+ ASSERT_TRUE(queue.push(1));
+ ASSERT_TRUE(queue.push(2));
+ ASSERT_TRUE(queue.push(3));
+ ASSERT_FALSE(queue.push(4)) << "Queue should reach capacity at size " << capacity;
+}
+
TEST(SyncQueueTest, AllowsMultipleThreads) {
SyncQueue<int> queue;