From d7616ef73c48a1e5d6ba0e36fc6e85b007832d25 Mon Sep 17 00:00:00 2001 From: Adam Lesinski Date: Mon, 27 Jul 2015 11:43:05 -0700 Subject: [PATCH] BatteryStats: Prevent BatteryService from blocking BatteryService is a low level service, and we shouldn't have it block in BatteryStatsService while collecting external data from components like WiFi. Deadlocks can occur this way. Bug:22559655 Change-Id: I64026453d191695eb583a47163ab6f48f882b085 --- .../server/am/BatteryStatsService.java | 47 +++++++++++-------- 1 file changed, 28 insertions(+), 19 deletions(-) diff --git a/services/core/java/com/android/server/am/BatteryStatsService.java b/services/core/java/com/android/server/am/BatteryStatsService.java index 4b0b924e2ddbc..0707ec855b4de 100644 --- a/services/core/java/com/android/server/am/BatteryStatsService.java +++ b/services/core/java/com/android/server/am/BatteryStatsService.java @@ -854,26 +854,35 @@ public final class BatteryStatsService extends IBatteryStats.Stub public boolean isOnBattery() { return mStats.isOnBattery(); } - - public void setBatteryState(int status, int health, int plugType, int level, - int temp, int volt) { - enforceCallingPermission(); - synchronized (mStats) { - final boolean onBattery = plugType == BatteryStatsImpl.BATTERY_PLUGGED_NONE; - if (mStats.isOnBattery() == onBattery) { - // The battery state has not changed, so we don't need to sync external - // stats immediately. - mStats.setBatteryStateLocked(status, health, plugType, level, temp, volt); - return; - } - } - // Sync external stats first as the battery has changed states. If we don't sync - // immediately here, we may not collect the relevant data later. - updateExternalStats("battery-state", UPDATE_ALL); - synchronized (mStats) { - mStats.setBatteryStateLocked(status, health, plugType, level, temp, volt); - } + @Override + public void setBatteryState(final int status, final int health, final int plugType, + final int level, final int temp, final int volt) { + enforceCallingPermission(); + + // BatteryService calls us here and we may update external state. It would be wrong + // to block such a low level service like BatteryService on external stats like WiFi. + mHandler.post(new Runnable() { + @Override + public void run() { + synchronized (mStats) { + final boolean onBattery = plugType == BatteryStatsImpl.BATTERY_PLUGGED_NONE; + if (mStats.isOnBattery() == onBattery) { + // The battery state has not changed, so we don't need to sync external + // stats immediately. + mStats.setBatteryStateLocked(status, health, plugType, level, temp, volt); + return; + } + } + + // Sync external stats first as the battery has changed states. If we don't sync + // immediately here, we may not collect the relevant data later. + updateExternalStats("battery-state", UPDATE_ALL); + synchronized (mStats) { + mStats.setBatteryStateLocked(status, health, plugType, level, temp, volt); + } + } + }); } public long getAwakeTimeBattery() {