From 020239df85931c3d5d5a89259f2e321fa48de353 Mon Sep 17 00:00:00 2001
From: Sudheer Shanka <sudheersai@google.com>
Date: Mon, 16 Jul 2018 18:00:46 -0700
Subject: [PATCH] Fix potential crash when per-procstate cpu times tracking is
 turned on.

When per-procstate cpu times tracking is turned on,
BatteryStatsImpl tries to access mKernelSingleUidTimeReader but
it's possible that mKernelSingleUidTimeReader hasn't been initialized
yet after a reboot and this could lead to a system_server crash.

Bug: 111523951
Test: manual
Change-Id: Id014f23fbe31fed64fba769f14ba4396a003092e
---
 .../android/internal/os/BatteryStatsImpl.java   | 17 +++++++++++++----
 .../internal/os/KernelSingleUidTimeReader.java  | 14 --------------
 2 files changed, 13 insertions(+), 18 deletions(-)

diff --git a/core/java/com/android/internal/os/BatteryStatsImpl.java b/core/java/com/android/internal/os/BatteryStatsImpl.java
index df59f79b0f20c..788ac2244da55 100644
--- a/core/java/com/android/internal/os/BatteryStatsImpl.java
+++ b/core/java/com/android/internal/os/BatteryStatsImpl.java
@@ -229,6 +229,15 @@ public class BatteryStatsImpl extends BatteryStats {
     @GuardedBy("this")
     public boolean mPerProcStateCpuTimesAvailable = true;
 
+    /**
+     * When per process state cpu times tracking is off, cpu times in KernelSingleUidTimeReader are
+     * not updated. So, when the setting is turned on later, we would end up with huge cpu time
+     * deltas. This flag tracks the case where tracking is turned on from off so that we won't
+     * end up attributing the huge deltas to wrong buckets.
+     */
+    @GuardedBy("this")
+    private boolean mIsPerProcessStateCpuDataStale;
+
     /**
      * Uids for which per-procstate cpu times need to be updated.
      *
@@ -402,7 +411,7 @@ public class BatteryStatsImpl extends BatteryStats {
             }
             // If the KernelSingleUidTimeReader has stale cpu times, then we shouldn't try to
             // compute deltas since it might result in mis-attributing cpu times to wrong states.
-            if (mKernelSingleUidTimeReader.hasStaleData()) {
+            if (mIsPerProcessStateCpuDataStale) {
                 mPendingUids.clear();
                 return;
             }
@@ -485,9 +494,9 @@ public class BatteryStatsImpl extends BatteryStats {
                     mKernelUidCpuFreqTimeReader.getAllUidCpuFreqTimeMs();
             // If the KernelSingleUidTimeReader has stale cpu times, then we shouldn't try to
             // compute deltas since it might result in mis-attributing cpu times to wrong states.
-            if (mKernelSingleUidTimeReader.hasStaleData()) {
+            if (mIsPerProcessStateCpuDataStale) {
                 mKernelSingleUidTimeReader.setAllUidsCpuTimesMs(allUidCpuFreqTimesMs);
-                mKernelSingleUidTimeReader.markDataAsStale(false);
+                mIsPerProcessStateCpuDataStale = false;
                 mPendingUids.clear();
                 return;
             }
@@ -13334,7 +13343,7 @@ public class BatteryStatsImpl extends BatteryStats {
         private void updateTrackCpuTimesByProcStateLocked(boolean wasEnabled, boolean isEnabled) {
             TRACK_CPU_TIMES_BY_PROC_STATE = isEnabled;
             if (isEnabled && !wasEnabled) {
-                mKernelSingleUidTimeReader.markDataAsStale(true);
+                mIsPerProcessStateCpuDataStale = true;
                 mExternalSync.scheduleCpuSyncDueToSettingChange();
 
                 mNumSingleUidCpuTimeReads = 0;
diff --git a/core/java/com/android/internal/os/KernelSingleUidTimeReader.java b/core/java/com/android/internal/os/KernelSingleUidTimeReader.java
index 42839171dc535..ad628524f4436 100644
--- a/core/java/com/android/internal/os/KernelSingleUidTimeReader.java
+++ b/core/java/com/android/internal/os/KernelSingleUidTimeReader.java
@@ -53,8 +53,6 @@ public class KernelSingleUidTimeReader {
     private int mReadErrorCounter;
     @GuardedBy("this")
     private boolean mSingleUidCpuTimesAvailable = true;
-    @GuardedBy("this")
-    private boolean mHasStaleData;
     // We use the freq count obtained from /proc/uid_time_in_state to decide how many longs
     // to read from each /proc/uid/<uid>/time_in_state. On the first read, verify if this is
     // correct and if not, set {@link #mSingleUidCpuTimesAvailable} to false. This flag will
@@ -196,18 +194,6 @@ public class KernelSingleUidTimeReader {
         return deltaTimesMs;
     }
 
-    public void markDataAsStale(boolean hasStaleData) {
-        synchronized (this) {
-            mHasStaleData = hasStaleData;
-        }
-    }
-
-    public boolean hasStaleData() {
-        synchronized (this) {
-            return mHasStaleData;
-        }
-    }
-
     public void setAllUidsCpuTimesMs(SparseArray<long[]> allUidsCpuTimesMs) {
         synchronized (this) {
             mLastUidCpuTimeMs.clear();