OSDN Git Service

Fix potential crash when per-procstate cpu times tracking is turned on.
authorSudheer Shanka <sudheersai@google.com>
Tue, 17 Jul 2018 01:00:46 +0000 (18:00 -0700)
committerSudheer Shanka <sudheersai@google.com>
Sat, 16 Feb 2019 02:27:38 +0000 (18:27 -0800)
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
Merged-In: Id014f23fbe31fed64fba769f14ba4396a003092e
(cherry picked from commit 020239df85931c3d5d5a89259f2e321fa48de353)

core/java/com/android/internal/os/BatteryStatsImpl.java
core/java/com/android/internal/os/KernelSingleUidTimeReader.java

index 8b06c47..4ad4545 100644 (file)
@@ -230,6 +230,15 @@ public class BatteryStatsImpl extends BatteryStats {
     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.
      *
      * Contains uid -> procState mappings.
@@ -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;
             }
@@ -13430,7 +13439,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;
index 4283917..ad62852 100644 (file)
@@ -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();