Merge "Protect pulled data in LatencyAggregator with a lock" into main am: fb3686cc9d

Original change: https://android-review.googlesource.com/c/platform/frameworks/native/+/2809993

Change-Id: I79952d4d5cff6eb5bb4f92b5318015ed54ea14c8
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

2 files changed, 13 insertions, 3 deletions

diff --git a/services/inputflinger/dispatcher/LatencyAggregator.cpp b/services/inputflinger/dispatcher/LatencyAggregator.cpp
index 96d78c375b..e09d97a13b 100644
--- a/services/inputflinger/dispatcher/LatencyAggregator.cpp
+++ b/services/inputflinger/dispatcher/LatencyAggregator.cpp
@@ -126,6 +126,7 @@ void LatencyAggregator::processTimeline(const InputEventTimeline& timeline) {
 }
 
 void LatencyAggregator::processStatistics(const InputEventTimeline& timeline) {
+    std::scoped_lock lock(mLock);
     // Before we do any processing, check that we have not yet exceeded MAX_SIZE
     if (mNumSketchEventsProcessed >= MAX_EVENTS_FOR_STATISTICS) {
         return;
@@ -167,6 +168,7 @@ void LatencyAggregator::processStatistics(const InputEventTimeline& timeline) {
 }
 
 AStatsManager_PullAtomCallbackReturn LatencyAggregator::pullData(AStatsEventList* data) {
+    std::scoped_lock lock(mLock);
     std::array<std::unique_ptr<SafeBytesField>, SketchIndex::SIZE> serializedDownData;
     std::array<std::unique_ptr<SafeBytesField>, SketchIndex::SIZE> serializedMoveData;
     for (size_t i = 0; i < SketchIndex::SIZE; i++) {
@@ -257,6 +259,7 @@ void LatencyAggregator::processSlowEvent(const InputEventTimeline& timeline) {
 }
 
 std::string LatencyAggregator::dump(const char* prefix) const {
+    std::scoped_lock lock(mLock);
     std::string sketchDump = StringPrintf("%s  Sketches:\n", prefix);
     for (size_t i = 0; i < SketchIndex::SIZE; i++) {
         const int64_t numDown = mDownSketches[i]->num_values();
diff --git a/services/inputflinger/dispatcher/LatencyAggregator.h b/services/inputflinger/dispatcher/LatencyAggregator.h
index 60b6813158..d6d168602a 100644
--- a/services/inputflinger/dispatcher/LatencyAggregator.h
+++ b/services/inputflinger/dispatcher/LatencyAggregator.h
@@ -16,6 +16,7 @@
 
 #pragma once
 
+#include <android-base/thread_annotations.h>
 #include <kll.h>
 #include <statslog.h>
 #include <utils/Timers.h>
@@ -61,10 +62,13 @@ public:
     ~LatencyAggregator();
 
 private:
+    // Binder call -- called on a binder thread. This is different from the thread where the rest of
+    // the public API is called.
     static AStatsManager_PullAtomCallbackReturn pullAtomCallback(int32_t atom_tag,
                                                                  AStatsEventList* data,
                                                                  void* cookie);
     AStatsManager_PullAtomCallbackReturn pullData(AStatsEventList* data);
+
     // ---------- Slow event handling ----------
     void processSlowEvent(const InputEventTimeline& timeline);
     nsecs_t mLastSlowEventTime = 0;
@@ -74,14 +78,17 @@ private:
     size_t mNumEventsSinceLastSlowEventReport = 0;
 
     // ---------- Statistics handling ----------
+    // Statistics is pulled rather than pushed. It's pulled on a binder thread, and therefore will
+    // be accessed by two different threads. The lock is needed to protect the pulled data.
+    mutable std::mutex mLock;
     void processStatistics(const InputEventTimeline& timeline);
     // Sketches
     std::array<std::unique_ptr<dist_proc::aggregation::KllQuantile>, SketchIndex::SIZE>
-            mDownSketches;
+            mDownSketches GUARDED_BY(mLock);
     std::array<std::unique_ptr<dist_proc::aggregation::KllQuantile>, SketchIndex::SIZE>
-            mMoveSketches;
+            mMoveSketches GUARDED_BY(mLock);
     // How many events have been processed so far
-    size_t mNumSketchEventsProcessed = 0;
+    size_t mNumSketchEventsProcessed GUARDED_BY(mLock) = 0;
 };
 
 } // namespace android::inputdispatcher