commit 38eb5cdeb62a55bdf86e7fc76491015718626f1e
author Rambo Wang <rambowang@google.com> Mon Mar 29 16:39:14 2021 -0700
committer Rambo Wang <rambowang@google.com> Tue Mar 30 03:40:33 2021 +0000
tree 42074b7898dccbf154a4240c3eeb0759ccecb562
parent f8f0e5d0b9771afe3b054b77dd40f3730469bd2e

Shared state modified in two threads without synchronization

The method overrideConfig(int, PersistableBundle, boolean) directly
modifies the shared states (both mOverrideConfigs and
mPersistentOverrideConfigs) in IPC thread without synchronization.

To avoid the data race, the operations that modify shared states
are posted to handler thread on which all states are designed to
be confined.

Bug: 1624359
Test: atest com.android.phone.CarrierConfigLoaderTest
Change-Id: I950772f0000e84ec67208c647353513308567393

src/com/android/phone/CarrierConfigLoader.java

diff --git a/src/com/android/phone/CarrierConfigLoader.java b/src/com/android/phone/CarrierConfigLoader.java
index 5196c59..4ff7316 100644
--- a/src/com/android/phone/CarrierConfigLoader.java
+++ b/src/com/android/phone/CarrierConfigLoader.java
@@ -1225,25 +1225,29 @@
             logd("Ignore invalid phoneId: " + phoneId + " for subId: " + subscriptionId);
             return;
         }
-        overrideConfig(mOverrideConfigs, phoneId, overrides);
+        // Post to run on handler thread on which all states should be confined.
+        mHandler.post(() -> {
+            overrideConfig(mOverrideConfigs, phoneId, overrides);
 
-        if (persistent) {
-            overrideConfig(mPersistentOverrideConfigs, phoneId, overrides);
+            if (persistent) {
+                overrideConfig(mPersistentOverrideConfigs, phoneId, overrides);
 
-            if (overrides != null) {
-                final CarrierIdentifier carrierId = getCarrierIdentifierForPhoneId(phoneId);
-                saveConfigToXml(mPlatformCarrierConfigPackage, OVERRIDE_PACKAGE_ADDITION, phoneId,
-                        carrierId, mPersistentOverrideConfigs[phoneId]);
-            } else {
-                final String iccid = getIccIdForPhoneId(phoneId);
-                final int cid = getSpecificCarrierIdForPhoneId(phoneId);
-                String fileName = getFilenameForConfig(mPlatformCarrierConfigPackage,
-                        OVERRIDE_PACKAGE_ADDITION, iccid, cid);
-                File fileToDelete = new File(mContext.getFilesDir(), fileName);
-                fileToDelete.delete();
+                if (overrides != null) {
+                    final CarrierIdentifier carrierId = getCarrierIdentifierForPhoneId(phoneId);
+                    saveConfigToXml(mPlatformCarrierConfigPackage, OVERRIDE_PACKAGE_ADDITION,
+                            phoneId,
+                            carrierId, mPersistentOverrideConfigs[phoneId]);
+                } else {
+                    final String iccid = getIccIdForPhoneId(phoneId);
+                    final int cid = getSpecificCarrierIdForPhoneId(phoneId);
+                    String fileName = getFilenameForConfig(mPlatformCarrierConfigPackage,
+                            OVERRIDE_PACKAGE_ADDITION, iccid, cid);
+                    File fileToDelete = new File(mContext.getFilesDir(), fileName);
+                    fileToDelete.delete();
+                }
             }
-        }
-        notifySubscriptionInfoUpdater(phoneId);
+            notifySubscriptionInfoUpdater(phoneId);
+        });
     }
 
     private void overrideConfig(@NonNull PersistableBundle[] currentOverrides, int phoneId,