Fully delete duplicate entries inside LatencyTracker
Inside LatencyTracker, we use 2 collections that must always be in sync:
mTimelines and mEventTimes.
However, when duplicate input events are encountered, we do not treat
these collections equally. In one of them, we fully remove all data
associated with the duplicate inputEventId. In the other one, however,
we only remove the entry that matches both the inputEventId and the
eventTime. This means that the two collections will get out of sync,
because the entry with the mismatching eventTime will be kept in
mEventTimes.
To fix this, fully remove all entries with the duplicate inputEventId,
no matter what the eventTime is. Unfortunately, it means that we have to
traverse the entire collection of mEventTimes. However, since this event
should not occur often, it should not impact performance.
This issue was discovered via the use of the newly added fuzzer for
LatencyTracker. The fuzzer would hit this condition in about 2 seconds
after the run has started. This issue could have been identified the
fuzzer was in place from the start.
The removal of api 'reportNow':
The api 'reportNow' was added for convenience of writing tests. However,
this api has reduced the code coverage, since we would now short-circuit
the code around 'trackListener' that was responsible for event
reporting. This is why, when the issue was first being worked on, the
tests for duplicate events did not yield any crashes. To avoid such
problems in the future, fully remove this api and replace with a
'triggerEventReporting' method that's defined in the tests. This way,
the api's that are used by production code and by tests are identical.
Bug: 169866723
Bug: 205511073
Test: m inputflinger_latencytracker_fuzzer && adb sync data && adb shell /data/fuzz/arm64/inputflinger_latencytracker_fuzzer/inputflinger_latencytracker_fuzzer
Test: atest inputflinger_tests:LatencyTrackerTest
Change-Id: I04069cf2c9eb1154e3324b25c8c4bdff32d84d3f
Merged-In: I04069cf2c9eb1154e3324b25c8c4bdff32d84d3f
diff --git a/services/inputflinger/dispatcher/LatencyTracker.cpp b/services/inputflinger/dispatcher/LatencyTracker.cpp
index d634dcd..52f189c 100644
--- a/services/inputflinger/dispatcher/LatencyTracker.cpp
+++ b/services/inputflinger/dispatcher/LatencyTracker.cpp
@@ -50,13 +50,12 @@
* key-value pair. Equivalent to the imaginary std api std::multimap::erase(key, value).
*/
template <typename K, typename V>
-static void eraseByKeyAndValue(std::multimap<K, V>& map, K key, V value) {
- auto iterpair = map.equal_range(key);
-
- for (auto it = iterpair.first; it != iterpair.second; ++it) {
+static void eraseByValue(std::multimap<K, V>& map, const V& value) {
+ for (auto it = map.begin(); it != map.end();) {
if (it->second == value) {
- map.erase(it);
- break;
+ it = map.erase(it);
+ } else {
+ it++;
}
}
}
@@ -76,9 +75,7 @@
// confuse us by reporting the rest of the timeline for one of them. This should happen
// rarely, so we won't lose much data
mTimelines.erase(it);
- // In case we have another input event with a different id and at the same eventTime,
- // only erase this specific inputEventId.
- eraseByKeyAndValue(mEventTimes, eventTime, inputEventId);
+ eraseByValue(mEventTimes, inputEventId);
return;
}
mTimelines.emplace(inputEventId, InputEventTimeline(isDown, eventTime, readTime));
@@ -90,7 +87,8 @@
nsecs_t finishTime) {
const auto it = mTimelines.find(inputEventId);
if (it == mTimelines.end()) {
- // It's possible that an app sends a bad (or late)'Finish' signal, since it's free to do
+ // This could happen if we erased this event when duplicate events were detected. It's
+ // also possible that an app sent a bad (or late) 'Finish' signal, since it's free to do
// anything in its process. Just drop the report and move on.
return;
}
@@ -120,7 +118,8 @@
std::array<nsecs_t, GraphicsTimeline::SIZE> graphicsTimeline) {
const auto it = mTimelines.find(inputEventId);
if (it == mTimelines.end()) {
- // It's possible that an app sends a bad (or late) 'Timeline' signal, since it's free to do
+ // This could happen if we erased this event when duplicate events were detected. It's
+ // also possible that an app sent a bad (or late) 'Timeline' signal, since it's free to do
// anything in its process. Just drop the report and move on.
return;
}
@@ -166,14 +165,6 @@
}
}
-void LatencyTracker::reportNow() {
- for (const auto& [inputEventId, timeline] : mTimelines) {
- mTimelineProcessor->processTimeline(timeline);
- }
- mTimelines.clear();
- mEventTimes.clear();
-}
-
std::string LatencyTracker::dump(const char* prefix) {
return StringPrintf("%sLatencyTracker:\n", prefix) +
StringPrintf("%s mTimelines.size() = %zu\n", prefix, mTimelines.size()) +