commit 166663a1d2879a5a0dfc74cec0d68e3221708723
author Atneya Nair <atneya@google.com> Tue Jun 27 19:16:24 2023 -0700
committer Atneya Nair <atneya@google.com> Wed Jun 28 18:53:41 2023 -0700
tree aee280f88b4ecf8db7172f390ef6b720a6c13065
parent 8f83345cf48112a710039558a5f08d46e60a3fbe

Prevent audioflinger battery notes leak

Ensure battery notes from audioflinger are encapsulated in an RAII
object so noteStart is always properly matched.

Remove uid refcounting in audioserver, since it already exists in
the battery notifier utility class, and is error-prone. As a byproduct,
notes occur during track transitions rather than in the threadloop,
which happens to avoid problematic blocking.

Bump refcount mismatch logging to error.

Bug: 259449389
Test: Manual verification of battery stats notes w/ multiple refs
Change-Id: Ifc931a9ac46f37d662e42dd4696f0a62eec22c25

services/audioflinger/Threads.cpp

diff --git a/services/audioflinger/Threads.cpp b/services/audioflinger/Threads.cpp
index 700bdd2..45a5c09 100644
--- a/services/audioflinger/Threads.cpp
+++ b/services/audioflinger/Threads.cpp
@@ -1877,7 +1877,7 @@
     logTrack("add", track);
     mActiveTracksGeneration++;
     mLatestActiveTrack = track;
-    ++mBatteryCounter[track->uid()].second;
+    track->beginBatteryAttribution();
     mHasChanged = true;
     return mActiveTracks.add(track);
 }
@@ -1891,7 +1891,7 @@
     }
     logTrack("remove", track);
     mActiveTracksGeneration++;
-    --mBatteryCounter[track->uid()].second;
+    track->endBatteryAttribution();
     // mLatestActiveTrack is not cleared even if is the same as track.
     mHasChanged = true;
 #ifdef TEE_SINK
@@ -1904,14 +1904,13 @@
 template <typename T>
 void AudioFlinger::ThreadBase::ActiveTracks<T>::clear() {
     for (const sp<T> &track : mActiveTracks) {
-        BatteryNotifier::getInstance().noteStopAudio(track->uid());
+        track->endBatteryAttribution();
         logTrack("clear", track);
     }
     mLastActiveTracksGeneration = mActiveTracksGeneration;
     if (!mActiveTracks.empty()) { mHasChanged = true; }
     mActiveTracks.clear();
     mLatestActiveTrack.clear();
-    mBatteryCounter.clear();
 }
 
 template <typename T>
@@ -1922,27 +1921,6 @@
         thread->updateWakeLockUids_l(getWakeLockUids());
         mLastActiveTracksGeneration = mActiveTracksGeneration;
     }
-
-    // Updates BatteryNotifier uids
-    for (auto it = mBatteryCounter.begin(); it != mBatteryCounter.end();) {
-        const uid_t uid = it->first;
-        ssize_t &previous = it->second.first;
-        ssize_t &current = it->second.second;
-        if (current > 0) {
-            if (previous == 0) {
-                BatteryNotifier::getInstance().noteStartAudio(uid);
-            }
-            previous = current;
-            ++it;
-        } else if (current == 0) {
-            if (previous > 0) {
-                BatteryNotifier::getInstance().noteStopAudio(uid);
-            }
-            it = mBatteryCounter.erase(it); // std::map<> is stable on iterator erase.
-        } else /* (current < 0) */ {
-            LOG_ALWAYS_FATAL("negative battery count %zd", current);
-        }
-    }
 }
 
 template <typename T>