From 0856f76846e61ad058e1e9ec0759739812a00600 Mon Sep 17 00:00:00 2001 From: Dmitri Plotnikov Date: Thu, 16 Dec 2021 10:57:09 -0800 Subject: [PATCH] Fix concurrency issue with BatteryUsageStats BatteryUsageStats is created under a BatteryStatsImpl lock. One of the elements of BatteryUsageStats is the battery history buffer Parcel. Once the BatteryUsageStats object is created, the BatteryStatsImpl lock is released and the history buffer parcel continues to be appended by BatteryStatsImpl. The Parcel may even be reset altogether if the battery stats session is reset. The BatteryUsageStats object is parceled during the getBatteryUsageStats binder call. Any modification of the history buffer concurrent with parceling causes a crash. Bug: 194256984 Test: atest FrameworksCoreTests:BatteryUsageStatsTest FrameworksCoreTests:BatteryUsageStatsProviderTest Change-Id: I262c4608cd02943f926e8daaf3e782c6fe6eaee7 Merged-In: Ifb03a32275dfbea172cd28309a42349d6dd4bcd5 --- .../internal/os/BatteryUsageStatsProvider.java | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/core/java/com/android/internal/os/BatteryUsageStatsProvider.java b/core/java/com/android/internal/os/BatteryUsageStatsProvider.java index 980aec196079b..27bc31861f447 100644 --- a/core/java/com/android/internal/os/BatteryUsageStatsProvider.java +++ b/core/java/com/android/internal/os/BatteryUsageStatsProvider.java @@ -21,6 +21,7 @@ import android.hardware.SensorManager; import android.os.BatteryStats; import android.os.BatteryUsageStats; import android.os.BatteryUsageStatsQuery; +import android.os.Parcel; import android.os.SystemClock; import android.os.UidBatteryConsumer; import android.util.Log; @@ -186,16 +187,23 @@ public class BatteryUsageStatsProvider { } BatteryStatsImpl batteryStatsImpl = (BatteryStatsImpl) mStats; + + // Make a copy of battery history to avoid concurrent modification. + Parcel historyBuffer = Parcel.obtain(); + historyBuffer.appendFrom(batteryStatsImpl.mHistoryBuffer, 0, + batteryStatsImpl.mHistoryBuffer.dataSize()); + ArrayList tags = new ArrayList<>( batteryStatsImpl.mHistoryTagPool.size()); for (Map.Entry entry : batteryStatsImpl.mHistoryTagPool.entrySet()) { - final BatteryStats.HistoryTag tag = entry.getKey(); + final BatteryStats.HistoryTag tag = new BatteryStats.HistoryTag(); + tag.setTo(entry.getKey()); tag.poolIdx = entry.getValue(); tags.add(tag); } - batteryUsageStatsBuilder.setBatteryHistory(batteryStatsImpl.mHistoryBuffer, tags); + batteryUsageStatsBuilder.setBatteryHistory(historyBuffer, tags); } return batteryUsageStatsBuilder.build();