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: Ifb03a32275dfbea172cd28309a42349d6dd4bcd5
This commit is contained in:
@@ -21,11 +21,13 @@ import android.hardware.SensorManager;
|
|||||||
import android.os.BatteryStats;
|
import android.os.BatteryStats;
|
||||||
import android.os.BatteryUsageStats;
|
import android.os.BatteryUsageStats;
|
||||||
import android.os.BatteryUsageStatsQuery;
|
import android.os.BatteryUsageStatsQuery;
|
||||||
|
import android.os.Parcel;
|
||||||
import android.os.SystemClock;
|
import android.os.SystemClock;
|
||||||
import android.os.UidBatteryConsumer;
|
import android.os.UidBatteryConsumer;
|
||||||
import android.util.Log;
|
import android.util.Log;
|
||||||
import android.util.SparseArray;
|
import android.util.SparseArray;
|
||||||
|
|
||||||
|
import com.android.internal.annotations.GuardedBy;
|
||||||
import com.android.internal.annotations.VisibleForTesting;
|
import com.android.internal.annotations.VisibleForTesting;
|
||||||
|
|
||||||
import java.util.ArrayList;
|
import java.util.ArrayList;
|
||||||
@@ -133,9 +135,12 @@ public class BatteryUsageStatsProvider {
|
|||||||
*/
|
*/
|
||||||
@VisibleForTesting
|
@VisibleForTesting
|
||||||
public BatteryUsageStats getBatteryUsageStats(BatteryUsageStatsQuery query) {
|
public BatteryUsageStats getBatteryUsageStats(BatteryUsageStatsQuery query) {
|
||||||
return getBatteryUsageStats(query, currentTimeMillis());
|
synchronized (mStats) {
|
||||||
|
return getBatteryUsageStats(query, currentTimeMillis());
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@GuardedBy("mStats")
|
||||||
private BatteryUsageStats getBatteryUsageStats(BatteryUsageStatsQuery query,
|
private BatteryUsageStats getBatteryUsageStats(BatteryUsageStatsQuery query,
|
||||||
long currentTimeMs) {
|
long currentTimeMs) {
|
||||||
if (query.getToTimestamp() == 0) {
|
if (query.getToTimestamp() == 0) {
|
||||||
@@ -145,6 +150,7 @@ public class BatteryUsageStatsProvider {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@GuardedBy("mStats")
|
||||||
private BatteryUsageStats getCurrentBatteryUsageStats(BatteryUsageStatsQuery query,
|
private BatteryUsageStats getCurrentBatteryUsageStats(BatteryUsageStatsQuery query,
|
||||||
long currentTimeMs) {
|
long currentTimeMs) {
|
||||||
final long realtimeUs = elapsedRealtime() * 1000;
|
final long realtimeUs = elapsedRealtime() * 1000;
|
||||||
@@ -189,7 +195,12 @@ public class BatteryUsageStatsProvider {
|
|||||||
}
|
}
|
||||||
|
|
||||||
BatteryStatsImpl batteryStatsImpl = (BatteryStatsImpl) mStats;
|
BatteryStatsImpl batteryStatsImpl = (BatteryStatsImpl) mStats;
|
||||||
batteryUsageStatsBuilder.setBatteryHistory(batteryStatsImpl.mHistoryBuffer);
|
|
||||||
|
// Make a copy of battery history to avoid concurrent modification.
|
||||||
|
Parcel historyBuffer = Parcel.obtain();
|
||||||
|
historyBuffer.appendFrom(batteryStatsImpl.mHistoryBuffer, 0,
|
||||||
|
batteryStatsImpl.mHistoryBuffer.dataSize());
|
||||||
|
batteryUsageStatsBuilder.setBatteryHistory(historyBuffer);
|
||||||
}
|
}
|
||||||
|
|
||||||
return batteryUsageStatsBuilder.build();
|
return batteryUsageStatsBuilder.build();
|
||||||
|
|||||||
Reference in New Issue
Block a user