commit f7c14104673750445bd1c2553021fd61aaeb7f2c
author Andy Hung <hunga@google.com> Sat Apr 18 14:54:08 2020 -0700
committer Andy Hung <hunga@google.com> Mon Apr 20 18:07:51 2020 -0700
tree bbdef5be71641b2b894f543a36472f0f581e581a
parent a6127a935639f127db56974851abc2dda360eca2

MediaMetrics: Add thread-safety checking

Test: atest mediametrics_tests
Bug: 70398235
Bug: 149850236
Change-Id: I288ef92e6444785e02043c2629355c229c14e85c

services/mediametrics/TimeMachine.h

diff --git a/services/mediametrics/TimeMachine.h b/services/mediametrics/TimeMachine.h
index 29adeae..b138233 100644
--- a/services/mediametrics/TimeMachine.h
+++ b/services/mediametrics/TimeMachine.h
@@ -23,6 +23,7 @@
 #include <variant>
 #include <vector>
 
+#include <android-base/thread_annotations.h>
 #include <media/MediaMetricsItem.h>
 #include <utils/Timers.h>
 
@@ -92,7 +93,8 @@
         }
 
         template <typename T>
-        status_t getValue(const std::string &property, T* value, int64_t time = 0) const {
+        status_t getValue(const std::string &property, T* value, int64_t time = 0) const
+                REQUIRES(mPseudoKeyHistoryLock) {
             if (time == 0) time = systemTime(SYSTEM_TIME_REALTIME);
             const auto tsptr = mPropertyMap.find(property);
             if (tsptr == mPropertyMap.end()) return BAD_VALUE;
@@ -108,20 +110,22 @@
         }
 
         template <typename T>
-        status_t getValue(const std::string &property, T defaultValue, int64_t time = 0) const {
+        status_t getValue(const std::string &property, T defaultValue, int64_t time = 0) const
+                REQUIRES(mPseudoKeyHistoryLock){
             T value;
             return getValue(property, &value, time) != NO_ERROR ? defaultValue : value;
         }
 
         void putProp(
-                const std::string &name, const mediametrics::Item::Prop &prop, int64_t time = 0) {
+                const std::string &name, const mediametrics::Item::Prop &prop, int64_t time = 0)
+                REQUIRES(mPseudoKeyHistoryLock) {
             //alternatively: prop.visit([&](auto value) { putValue(name, value, time); });
             putValue(name, prop.get(), time);
         }
 
         template <typename T>
-        void putValue(const std::string &property,
-                T&& e, int64_t time = 0) {
+        void putValue(const std::string &property, T&& e, int64_t time = 0)
+                REQUIRES(mPseudoKeyHistoryLock) {
             if (time == 0) time = systemTime(SYSTEM_TIME_REALTIME);
             mLastModificationTime = time;
             if (mPropertyMap.size() >= kKeyMaxProperties &&
@@ -144,7 +148,8 @@
             }
         }
 
-        std::pair<std::string, int32_t> dump(int32_t lines, int64_t time) const {
+        std::pair<std::string, int32_t> dump(int32_t lines, int64_t time) const
+                REQUIRES(mPseudoKeyHistoryLock) {
             std::stringstream ss;
             int32_t ll = lines;
             for (auto& tsPair : mPropertyMap) {
@@ -158,7 +163,9 @@
             return { ss.str(), lines - ll };
         }
 
-        int64_t getLastModificationTime() const { return mLastModificationTime; }
+        int64_t getLastModificationTime() const REQUIRES(mPseudoKeyHistoryLock) {
+            return mLastModificationTime;
+        }
 
     private:
         static std::string dump(
@@ -267,7 +274,7 @@
             if (it == mHistory.end()) {
                 if (!isTrusted) return PERMISSION_DENIED;
 
-                (void)gc_l(garbage);
+                (void)gc(garbage);
 
                 // no keylock needed here as we are sole owner
                 // until placed on mHistory.
@@ -435,7 +442,8 @@
 private:
 
     // Obtains the lock for a KeyHistory.
-    std::mutex &getLockForKey(const std::string &key) const {
+    std::mutex &getLockForKey(const std::string &key) const
+            RETURN_CAPABILITY(mPseudoKeyHistoryLock) {
         return mKeyLocks[std::hash<std::string>{}(key) % std::size(mKeyLocks)];
     }
 
@@ -459,7 +467,6 @@
         return it->second;
     }
 
-    // GUARDED_BY mLock
     /**
      * Garbage collects if the TimeMachine size exceeds the high water mark.
      *
@@ -471,7 +478,7 @@
      *
      * \return true if garbage collection was done.
      */
-    bool gc_l(std::vector<std::any>& garbage) {
+    bool gc(std::vector<std::any>& garbage) REQUIRES(mLock) {
         // TODO: something better than this for garbage collection.
         if (mHistory.size() < mKeyHighWaterMark) return false;
 
@@ -540,12 +547,17 @@
      */
 
     mutable std::mutex mLock;           // Lock for mHistory
-    History mHistory;                   // GUARDED_BY mLock
+    History mHistory GUARDED_BY(mLock);
 
     // KEY_LOCKS is the number of mutexes for keys.
     // It need not be a power of 2, but faster that way.
     static inline constexpr size_t KEY_LOCKS = 256;
     mutable std::mutex mKeyLocks[KEY_LOCKS];  // Hash-striped lock for KeyHistory based on key.
+
+    // Used for thread-safety analysis, we create a fake mutex object to represent
+    // the hash stripe lock mechanism, which is then tracked by the compiler.
+    class CAPABILITY("mutex") PseudoLock {};
+    static inline PseudoLock mPseudoKeyHistoryLock;
 };
 
 } // namespace android::mediametrics