Fixed subscription database cache out of sync
The subscription reloading from database could cause cache
to be out-of-sync. Fixed by making reloading synchronous and
only reload when necessary (e.g. Backup restore actually changes
the database).
Fix: 290295550
Fix: 290176403
Test: atest SubscriptionManagerServiceTest TelephonyProviderTests
Test: Manually test backup/restore
Test: Telephony basic functionality tests
Test: Re-tested passed on b/290295550#comment23
Test: Re-tested passed on b/290176403#comment28
Merged-In: Iee0355a4069ce6d5c85ecc1bd727bc457c3bb902
Change-Id: Iee0355a4069ce6d5c85ecc1bd727bc457c3bb902
diff --git a/src/java/com/android/internal/telephony/subscription/SubscriptionDatabaseManager.java b/src/java/com/android/internal/telephony/subscription/SubscriptionDatabaseManager.java
index b90dc5e..124437c 100644
--- a/src/java/com/android/internal/telephony/subscription/SubscriptionDatabaseManager.java
+++ b/src/java/com/android/internal/telephony/subscription/SubscriptionDatabaseManager.java
@@ -2018,21 +2018,21 @@
}
/**
- * Reload the database from content provider to the cache.
+ * Reload the database from content provider to the cache. This must be a synchronous operation
+ * to prevent cache/database out-of-sync. Callers should be cautious to call this method because
+ * it might take longer time to complete.
*/
- public void reloadDatabase() {
- if (mAsyncMode) {
- post(this::loadDatabaseInternal);
- } else {
- loadDatabaseInternal();
- }
+ public void reloadDatabaseSync() {
+ logl("reloadDatabaseSync");
+ // Synchronously load the database into the cache.
+ loadDatabaseInternal();
}
/**
* Load the database from content provider to the cache.
*/
private void loadDatabaseInternal() {
- log("loadDatabaseInternal");
+ logl("loadDatabaseInternal");
try (Cursor cursor = mContext.getContentResolver().query(
SimInfo.CONTENT_URI, null, null, null, null)) {
mReadWriteLock.writeLock().lock();
diff --git a/src/java/com/android/internal/telephony/subscription/SubscriptionManagerService.java b/src/java/com/android/internal/telephony/subscription/SubscriptionManagerService.java
index 8e773c0..3872b2c 100644
--- a/src/java/com/android/internal/telephony/subscription/SubscriptionManagerService.java
+++ b/src/java/com/android/internal/telephony/subscription/SubscriptionManagerService.java
@@ -1420,12 +1420,15 @@
}
// Attempt to restore SIM specific settings when SIM is loaded.
- mContext.getContentResolver().call(
+ Bundle result = mContext.getContentResolver().call(
SubscriptionManager.SIM_INFO_BACKUP_AND_RESTORE_CONTENT_URI,
SubscriptionManager.RESTORE_SIM_SPECIFIC_SETTINGS_METHOD_NAME,
iccId, null);
- log("Reload the database.");
- mSubscriptionDatabaseManager.reloadDatabase();
+ if (result != null && result.getBoolean(
+ SubscriptionManager.RESTORE_SIM_SPECIFIC_SETTINGS_DATABASE_UPDATED)) {
+ logl("Sim specific settings changed the database.");
+ mSubscriptionDatabaseManager.reloadDatabaseSync();
+ }
}
log("updateSubscription: " + mSubscriptionDatabaseManager
@@ -3707,12 +3710,16 @@
Bundle bundle = new Bundle();
bundle.putByteArray(SubscriptionManager.KEY_SIM_SPECIFIC_SETTINGS_DATA, data);
logl("restoreAllSimSpecificSettingsFromBackup");
- mContext.getContentResolver().call(
+ Bundle result = mContext.getContentResolver().call(
SubscriptionManager.SIM_INFO_BACKUP_AND_RESTORE_CONTENT_URI,
SubscriptionManager.RESTORE_SIM_SPECIFIC_SETTINGS_METHOD_NAME,
null, bundle);
- // After restoring, we need to reload the content provider into the cache.
- mSubscriptionDatabaseManager.reloadDatabase();
+
+ if (result != null && result.getBoolean(
+ SubscriptionManager.RESTORE_SIM_SPECIFIC_SETTINGS_DATABASE_UPDATED)) {
+ logl("Sim specific settings changed the database.");
+ mSubscriptionDatabaseManager.reloadDatabaseSync();
+ }
} finally {
Binder.restoreCallingIdentity(token);
}
diff --git a/tests/telephonytests/src/com/android/internal/telephony/subscription/SubscriptionDatabaseManagerTest.java b/tests/telephonytests/src/com/android/internal/telephony/subscription/SubscriptionDatabaseManagerTest.java
index 9898353..0358809 100644
--- a/tests/telephonytests/src/com/android/internal/telephony/subscription/SubscriptionDatabaseManagerTest.java
+++ b/tests/telephonytests/src/com/android/internal/telephony/subscription/SubscriptionDatabaseManagerTest.java
@@ -269,6 +269,8 @@
private final List<String> mAllColumns;
+ private boolean mDatabaseChanged;
+
SubscriptionProvider() {
mAllColumns = SimInfo.getAllColumns();
}
@@ -378,7 +380,17 @@
@Override
public Bundle call(String method, @Nullable String args, @Nullable Bundle bundle) {
- return new Bundle();
+ Bundle result = new Bundle();
+ if (method.equals(SubscriptionManager.RESTORE_SIM_SPECIFIC_SETTINGS_METHOD_NAME)) {
+ result.putBoolean(
+ SubscriptionManager.RESTORE_SIM_SPECIFIC_SETTINGS_DATABASE_UPDATED,
+ mDatabaseChanged);
+ }
+ return result;
+ }
+
+ public void setRestoreDatabaseChanged(boolean changed) {
+ mDatabaseChanged = changed;
}
}
@@ -422,7 +434,7 @@
.that(mDatabaseManagerUT.getSubscriptionInfoInternal(subId)).isEqualTo(subInfo);
// Load subscription info from the database.
- mDatabaseManagerUT.reloadDatabase();
+ mDatabaseManagerUT.reloadDatabaseSync();
processAllMessages();
// Verify the database value is same as the inserted one.
diff --git a/tests/telephonytests/src/com/android/internal/telephony/subscription/SubscriptionManagerServiceTest.java b/tests/telephonytests/src/com/android/internal/telephony/subscription/SubscriptionManagerServiceTest.java
index 3abfac7..ba6f046 100644
--- a/tests/telephonytests/src/com/android/internal/telephony/subscription/SubscriptionManagerServiceTest.java
+++ b/tests/telephonytests/src/com/android/internal/telephony/subscription/SubscriptionManagerServiceTest.java
@@ -69,8 +69,10 @@
import android.app.AppOpsManager;
import android.app.PropertyInvalidatedCache;
import android.compat.testing.PlatformCompatChangeRule;
+import android.content.ContentValues;
import android.content.Intent;
import android.content.pm.PackageManager;
+import android.net.Uri;
import android.os.Build;
import android.os.Bundle;
import android.os.Handler;
@@ -2212,16 +2214,40 @@
}
@Test
- public void testRestoreAllSimSpecificSettingsFromBackup() {
+ public void testRestoreAllSimSpecificSettingsFromBackup() throws Exception {
assertThrows(SecurityException.class, ()
-> mSubscriptionManagerServiceUT.restoreAllSimSpecificSettingsFromBackup(
new byte[0]));
+ insertSubscription(FAKE_SUBSCRIPTION_INFO1);
mContextFixture.addCallingOrSelfPermission(Manifest.permission.MODIFY_PHONE_STATE);
- // TODO: Briefly copy the logic from TelephonyProvider to
- // SubscriptionDatabaseManagerTest.SubscriptionProvider
+
+ // getSubscriptionDatabaseManager().setWifiCallingEnabled(1, 0);
+
+ // Simulate restoration altered the database directly.
+ ContentValues cvs = new ContentValues();
+ cvs.put(SimInfo.COLUMN_WFC_IMS_ENABLED, 0);
+ mSubscriptionProvider.update(Uri.withAppendedPath(SimInfo.CONTENT_URI, "1"), cvs, null,
+ null);
+
+ // Setting this to false to prevent database reload.
+ mSubscriptionProvider.setRestoreDatabaseChanged(false);
mSubscriptionManagerServiceUT.restoreAllSimSpecificSettingsFromBackup(
new byte[0]);
+
+ SubscriptionInfoInternal subInfo = mSubscriptionManagerServiceUT
+ .getSubscriptionInfoInternal(1);
+ // Since reload didn't happen, WFC should remains enabled.
+ assertThat(subInfo.getWifiCallingEnabled()).isEqualTo(1);
+
+ // Now the database reload should happen
+ mSubscriptionProvider.setRestoreDatabaseChanged(true);
+ mSubscriptionManagerServiceUT.restoreAllSimSpecificSettingsFromBackup(
+ new byte[0]);
+
+ subInfo = mSubscriptionManagerServiceUT.getSubscriptionInfoInternal(1);
+ // Since reload didn't happen, WFC should remains enabled.
+ assertThat(subInfo.getWifiCallingEnabled()).isEqualTo(0);
}
@Test
