Fix the memory leak in BatteryStatsImpl.Timebase.mObservers.

The observer does not always remove themself after adding themself into
mObservers list. The strong reference in mObservers causes memory leak.
The fix is to change mObservers to hold WeakReference.

Fix: 80443940
Test: "adb shell cmd battery unplug" and "adb shell cmd battery set ac",
observe memory usage from Android Monitor.

Change-Id: I27436e350fa0d211c4c64114411a6603824ce599

1 files changed, 25 insertions, 12 deletions

diff --git a/core/java/com/android/internal/os/BatteryStatsImpl.java b/core/java/com/android/internal/os/BatteryStatsImpl.java
index d7db2e2a90e5..31c271e655ec 100644
--- a/core/java/com/android/internal/os/BatteryStatsImpl.java
+++ b/core/java/com/android/internal/os/BatteryStatsImpl.java
@@ -107,6 +107,7 @@ import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.PrintWriter;
 import java.nio.charset.StandardCharsets;
+import java.lang.ref.WeakReference;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Calendar;
@@ -1068,7 +1069,7 @@ public class BatteryStatsImpl extends BatteryStats {
 
     // methods are protected not private to be VisibleForTesting
     public static class TimeBase {
-        protected final ArrayList<TimeBaseObs> mObservers = new ArrayList<>();
+        protected final ArrayList<WeakReference<TimeBaseObs>> mObservers = new ArrayList<>();
 
         protected long mUptime;
         protected long mRealtime;
@@ -1112,17 +1113,24 @@ public class BatteryStatsImpl extends BatteryStats {
         }
 
         public void add(TimeBaseObs observer) {
-            mObservers.add(observer);
+            mObservers.add(new WeakReference<TimeBaseObs>(observer));
         }
 
         public void remove(TimeBaseObs observer) {
-            if (!mObservers.remove(observer)) {
-                Slog.wtf(TAG, "Removed unknown observer: " + observer);
-            }
+           if (!mObservers.removeIf(ref -> ref.get() == observer)) {
+             Slog.wtf(TAG, "Removed unknown observer: " + observer);
+           }
         }
 
         public boolean hasObserver(TimeBaseObs observer) {
-            return mObservers.contains(observer);
+            Iterator<WeakReference<TimeBaseObs>> i = mObservers.iterator();
+            while (i.hasNext()) {
+                TimeBaseObs obs = i.next().get();
+                if (obs == observer) {
+                    return true;
+                }
+            }
+            return false;
         }
 
         public void init(long uptime, long realtime) {
@@ -1211,9 +1219,11 @@ public class BatteryStatsImpl extends BatteryStats {
                     mRealtimeStart = realtime;
                     long batteryUptime = mUnpluggedUptime = getUptime(uptime);
                     long batteryRealtime = mUnpluggedRealtime = getRealtime(realtime);
-
-                    for (int i = mObservers.size() - 1; i >= 0; i--) {
-                        mObservers.get(i).onTimeStarted(realtime, batteryUptime, batteryRealtime);
+                    for (WeakReference<TimeBaseObs> ref : mObservers) {
+                        TimeBaseObs obs = ref.get();
+                        if (obs != null) {
+                            obs.onTimeStarted(realtime, batteryUptime, batteryRealtime);
+                        }
                     }
                 } else {
                     mPastUptime += uptime - mUptimeStart;
@@ -1221,11 +1231,14 @@ public class BatteryStatsImpl extends BatteryStats {
 
                     long batteryUptime = getUptime(uptime);
                     long batteryRealtime = getRealtime(realtime);
-
-                    for (int i = mObservers.size() - 1; i >= 0; i--) {
-                        mObservers.get(i).onTimeStopped(realtime, batteryUptime, batteryRealtime);
+                    for (WeakReference<TimeBaseObs> ref : mObservers) {
+                        TimeBaseObs obs = ref.get();
+                        if (obs != null) {
+                            obs.onTimeStopped(realtime, batteryUptime, batteryRealtime);
+                        }
                     }
                 }
+                mObservers.removeIf(ref -> ref.get() == null);
                 return true;
             }
             return false;