From 5bb1ec89d3f4435c0e91387388dc76f0494d749c Mon Sep 17 00:00:00 2001 From: Adam Bookatz Date: Wed, 24 Feb 2021 01:35:16 +0000 Subject: [PATCH] BatteryStats custom buckets: don't revive dead Uids It is possible for old uids to be reported to BatteryStats in updateCustomMeasuredEnergyDataLocked(). This can happen, e.g., if a uid was uninstalled between successive custom energy updates, so that the uid did use non-zero energy but BatteryStats doesn't care anymore since the uid is gone. If this happens, we don't want to recreate any removed Uid objects, so we need to be careful to only consider Uids that aren't removed. In practice, this method will only be called after the CPU update, which means that BatteryStats should already know about all active uids (assuming they used CPU). Therefore, there shouldn't be any unknown uids actively using custom energy buckets at this point. We therefore simply ignore any unknown uids (just log them), on the assumption that they refer to removed uids or stopped users. Test: atest BatteryStatsTests Bug: 180030409 Change-Id: If4fee7a1d11b148ca16d464d6ce4720c5a251130 --- .../android/internal/os/BatteryStatsImpl.java | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/core/java/com/android/internal/os/BatteryStatsImpl.java b/core/java/com/android/internal/os/BatteryStatsImpl.java index 1f8ffe08a2574..54095bd86887f 100644 --- a/core/java/com/android/internal/os/BatteryStatsImpl.java +++ b/core/java/com/android/internal/os/BatteryStatsImpl.java @@ -12520,6 +12520,7 @@ public class BatteryStatsImpl extends BatteryStats { * @param totalEnergyUJ energy (microjoules) used for this bucket since this was last called. * @param uidEnergies map of uid->energy (microjoules) for this bucket since last called. * Data inside uidEnergies will not be modified (treated immutable). + * Uids not already known to BatteryStats will be ignored. */ public void updateCustomMeasuredEnergyDataLocked(int customEnergyBucket, long totalEnergyUJ, @Nullable SparseLongArray uidEnergies) { @@ -12540,10 +12541,20 @@ public class BatteryStatsImpl extends BatteryStats { final int uidInt = mapUid(uidEnergies.keyAt(i)); final long uidEnergyUJ = uidEnergies.valueAt(i); if (uidEnergyUJ == 0) continue; - // TODO(b/180030409): Worry about dead Uids (no longer in BSI) being revived by this, - // or converse problem of not creating a new Uid if its first blame is recorded here. - final Uid uidObj = getUidStatsLocked(uidInt); - uidObj.addEnergyToCustomBucketLocked(uidEnergyUJ, customEnergyBucket, true); + final Uid uidObj = getAvailableUidStatsLocked(uidInt); + if (uidObj != null) { + uidObj.addEnergyToCustomBucketLocked(uidEnergyUJ, customEnergyBucket, true); + } else { + // Ignore any uid not already known to BatteryStats, rather than creating a new Uid. + // Otherwise we could end up reviving dead Uids. Note that the CPU data is updated + // first, so any uid that has used any CPU should already be known to BatteryStats. + // Recently removed uids (especially common for isolated uids) can reach this path + // and are ignored. + if (!Process.isIsolated(uidInt)) { + Slog.w(TAG, "Received measured energy " + totalEnergyUJ + " for custom bucket " + + customEnergyBucket + " for non-existent uid " + uidInt); + } + } } }