From c76bb78758f57b064b3e442f8e8e12af84eddbf2 Mon Sep 17 00:00:00 2001 From: jackqdyulei Date: Wed, 14 Feb 2018 12:43:09 -0800 Subject: [PATCH] Make BatteryDatabaseManager singleton In BatteryTipLoader, two threads may access BatteryDatabaseManager simultaneously. In this case thread A may close the database thread B holds, then settings will crash. In this cl, we make the BatteryDatabaseManager as singleton and synchronize all the database related method. Then it shouldn't have the crash anymore. Bug: 73346734 Test: RunSettingsRoboTests Change-Id: Ib53b2894b25155cca0c6ec60d1a816663d27a578 --- .../batterytip/BatteryDatabaseManager.java | 24 ++++++++++++++----- .../batterytip/actions/RestrictAppAction.java | 2 +- .../detectors/RestrictAppDetector.java | 2 +- .../fuelgauge/BatteryDatabaseManagerTest.java | 2 +- .../settings/testutils/DatabaseTestUtils.java | 14 +++++++++++ 5 files changed, 35 insertions(+), 9 deletions(-) diff --git a/src/com/android/settings/fuelgauge/batterytip/BatteryDatabaseManager.java b/src/com/android/settings/fuelgauge/batterytip/BatteryDatabaseManager.java index 2019b9d7d64..87c248820da 100644 --- a/src/com/android/settings/fuelgauge/batterytip/BatteryDatabaseManager.java +++ b/src/com/android/settings/fuelgauge/batterytip/BatteryDatabaseManager.java @@ -38,14 +38,26 @@ import java.util.List; /** * Database manager for battery data. Now it only contains anomaly data stored in {@link AppInfo}. + * + * This manager may be accessed by multi-threads. All the database related methods are synchronized + * so each operation won't be interfered by other threads. */ public class BatteryDatabaseManager { - private final AnomalyDatabaseHelper mDatabaseHelper; + private static BatteryDatabaseManager sSingleton; - public BatteryDatabaseManager(Context context) { + private AnomalyDatabaseHelper mDatabaseHelper; + + private BatteryDatabaseManager(Context context) { mDatabaseHelper = AnomalyDatabaseHelper.getInstance(context); } + public static BatteryDatabaseManager getInstance(Context context) { + if (sSingleton == null) { + sSingleton = new BatteryDatabaseManager(context); + } + return sSingleton; + } + /** * Insert an anomaly log to database. * @@ -53,7 +65,7 @@ public class BatteryDatabaseManager { * @param type the type of the anomaly * @param timestampMs the time when it is happened */ - public void insertAnomaly(String packageName, int type, long timestampMs) { + public synchronized void insertAnomaly(String packageName, int type, long timestampMs) { try (SQLiteDatabase db = mDatabaseHelper.getWritableDatabase()) { ContentValues values = new ContentValues(); values.put(PACKAGE_NAME, packageName); @@ -67,7 +79,7 @@ public class BatteryDatabaseManager { /** * Query all the anomalies that happened after {@code timestampMsAfter} and with {@code state}. */ - public List queryAllAnomalies(long timestampMsAfter, int state) { + public synchronized List queryAllAnomalies(long timestampMsAfter, int state) { final List appInfos = new ArrayList<>(); try (SQLiteDatabase db = mDatabaseHelper.getReadableDatabase()) { final String[] projection = {PACKAGE_NAME, ANOMALY_TYPE}; @@ -90,7 +102,7 @@ public class BatteryDatabaseManager { return appInfos; } - public void deleteAllAnomaliesBeforeTimeStamp(long timestampMs) { + public synchronized void deleteAllAnomaliesBeforeTimeStamp(long timestampMs) { try (SQLiteDatabase db = mDatabaseHelper.getWritableDatabase()) { db.delete(TABLE_ANOMALY, TIME_STAMP_MS + " < ?", new String[]{String.valueOf(timestampMs)}); @@ -103,7 +115,7 @@ public class BatteryDatabaseManager { * @param appInfos represents the anomalies * @param state which state to update to */ - public void updateAnomalies(List appInfos, int state) { + public synchronized void updateAnomalies(List appInfos, int state) { if (!appInfos.isEmpty()) { final int size = appInfos.size(); final String[] whereArgs = new String[size]; diff --git a/src/com/android/settings/fuelgauge/batterytip/actions/RestrictAppAction.java b/src/com/android/settings/fuelgauge/batterytip/actions/RestrictAppAction.java index 886a6d534e6..37f4b2a5bea 100644 --- a/src/com/android/settings/fuelgauge/batterytip/actions/RestrictAppAction.java +++ b/src/com/android/settings/fuelgauge/batterytip/actions/RestrictAppAction.java @@ -42,7 +42,7 @@ public class RestrictAppAction extends BatteryTipAction { super(context); mRestrictAppTip = tip; mBatteryUtils = BatteryUtils.getInstance(context); - mBatteryDatabaseManager = new BatteryDatabaseManager(context); + mBatteryDatabaseManager = BatteryDatabaseManager.getInstance(context); } /** diff --git a/src/com/android/settings/fuelgauge/batterytip/detectors/RestrictAppDetector.java b/src/com/android/settings/fuelgauge/batterytip/detectors/RestrictAppDetector.java index e3c9b9ed612..0bb1f17aa68 100644 --- a/src/com/android/settings/fuelgauge/batterytip/detectors/RestrictAppDetector.java +++ b/src/com/android/settings/fuelgauge/batterytip/detectors/RestrictAppDetector.java @@ -43,7 +43,7 @@ public class RestrictAppDetector implements BatteryTipDetector { public RestrictAppDetector(Context context, BatteryTipPolicy policy) { mPolicy = policy; - mBatteryDatabaseManager = new BatteryDatabaseManager(context); + mBatteryDatabaseManager = BatteryDatabaseManager.getInstance(context); } @Override diff --git a/tests/robotests/src/com/android/settings/fuelgauge/BatteryDatabaseManagerTest.java b/tests/robotests/src/com/android/settings/fuelgauge/BatteryDatabaseManagerTest.java index 92332f26822..e835e65a9b1 100644 --- a/tests/robotests/src/com/android/settings/fuelgauge/BatteryDatabaseManagerTest.java +++ b/tests/robotests/src/com/android/settings/fuelgauge/BatteryDatabaseManagerTest.java @@ -59,7 +59,7 @@ public class BatteryDatabaseManagerTest { MockitoAnnotations.initMocks(this); mContext = RuntimeEnvironment.application; - mBatteryDatabaseManager = spy(new BatteryDatabaseManager(mContext)); + mBatteryDatabaseManager = spy(BatteryDatabaseManager.getInstance(mContext)); } @After diff --git a/tests/robotests/src/com/android/settings/testutils/DatabaseTestUtils.java b/tests/robotests/src/com/android/settings/testutils/DatabaseTestUtils.java index 499a2f78f34..2ffdc605aef 100644 --- a/tests/robotests/src/com/android/settings/testutils/DatabaseTestUtils.java +++ b/tests/robotests/src/com/android/settings/testutils/DatabaseTestUtils.java @@ -19,6 +19,7 @@ package com.android.settings.testutils; import android.content.Context; import com.android.settings.fuelgauge.batterytip.AnomalyDatabaseHelper; +import com.android.settings.fuelgauge.batterytip.BatteryDatabaseManager; import com.android.settings.search.IndexDatabaseHelper; import com.android.settings.slices.SlicesDatabaseHelper; @@ -30,6 +31,7 @@ public class DatabaseTestUtils { clearSearchDb(context); clearSlicesDb(context); clearAnomalyDb(context); + clearAnomalyDbManager(); } private static void clearSlicesDb(Context context) { @@ -76,4 +78,16 @@ public class DatabaseTestUtils { throw new RuntimeException(); } } + + private static void clearAnomalyDbManager() { + Field instance; + Class clazz = BatteryDatabaseManager.class; + try { + instance = clazz.getDeclaredField("sSingleton"); + instance.setAccessible(true); + instance.set(null, null); + } catch (Exception e) { + throw new RuntimeException(); + } + } }