Remove unecessary locking to avoid dead lock.

Bug: 160252067
Bug: 173421434
Test: manual - switch to dsds multiple times to make sure no error
happens

Change-Id: I15def5c401811190f0f2ca8d1fa7e44d6387407e
Merged-In: I15def5c401811190f0f2ca8d1fa7e44d6387407e

1 files changed, 49 insertions, 46 deletions

diff --git a/src/java/com/android/internal/telephony/SubscriptionController.java b/src/java/com/android/internal/telephony/SubscriptionController.java
index 457c17d96d..88c80a1964 100644
--- a/src/java/com/android/internal/telephony/SubscriptionController.java
+++ b/src/java/com/android/internal/telephony/SubscriptionController.java
@@ -700,6 +700,12 @@ public class SubscriptionController extends ISub.Stub {
         }
     }
 
+    private List<SubscriptionInfo> makeCacheListCopyWithLock(List<SubscriptionInfo> cacheSubList) {
+        synchronized (mSubInfoListLock) {
+            return new ArrayList<>(cacheSubList);
+        }
+    }
+
     /**
      * Get the SubInfoRecord(s) of the currently active SIM(s) - which include both local
      * and remote SIMs.
@@ -709,7 +715,8 @@ public class SubscriptionController extends ISub.Stub {
     @UnsupportedAppUsage
     @Override
     public List<SubscriptionInfo> getActiveSubscriptionInfoList(String callingPackage) {
-        return getSubscriptionInfoListFromCacheHelper(callingPackage, mCacheActiveSubInfoList);
+        return getSubscriptionInfoListFromCacheHelper(callingPackage,
+                makeCacheListCopyWithLock(mCacheActiveSubInfoList));
     }
 
     /**
@@ -720,13 +727,13 @@ public class SubscriptionController extends ISub.Stub {
     public void refreshCachedActiveSubscriptionInfoList() {
         boolean opptSubListChanged;
 
-        synchronized (mSubInfoListLock) {
-            List<SubscriptionInfo> activeSubscriptionInfoList = getSubInfo(
-                    SubscriptionManager.SIM_SLOT_INDEX + ">=0 OR "
-                    + SubscriptionManager.SUBSCRIPTION_TYPE + "="
-                    + SubscriptionManager.SUBSCRIPTION_TYPE_REMOTE_SIM,
-                    null);
+        List<SubscriptionInfo> activeSubscriptionInfoList = getSubInfo(
+                SubscriptionManager.SIM_SLOT_INDEX + ">=0 OR "
+                + SubscriptionManager.SUBSCRIPTION_TYPE + "="
+                + SubscriptionManager.SUBSCRIPTION_TYPE_REMOTE_SIM,
+                null);
 
+        synchronized (mSubInfoListLock) {
             if (activeSubscriptionInfoList != null) {
                 // Log when active sub info changes.
                 if (mCacheActiveSubInfoList.size() != activeSubscriptionInfoList.size()
@@ -741,10 +748,6 @@ public class SubscriptionController extends ISub.Stub {
                 logd("activeSubscriptionInfoList is null.");
                 mCacheActiveSubInfoList.clear();
             }
-
-            // Refresh cached opportunistic sub list and detect whether it's changed.
-            refreshCachedOpportunisticSubscriptionInfoList();
-
             if (DBG_CACHE) {
                 if (!mCacheActiveSubInfoList.isEmpty()) {
                     for (SubscriptionInfo si : mCacheActiveSubInfoList) {
@@ -756,6 +759,9 @@ public class SubscriptionController extends ISub.Stub {
                 }
             }
         }
+
+        // Refresh cached opportunistic sub list and detect whether it's changed.
+        refreshCachedOpportunisticSubscriptionInfoList();
     }
 
     /**
@@ -2909,8 +2915,8 @@ public class SubscriptionController extends ISub.Stub {
 
     @Override
     public List<SubscriptionInfo> getOpportunisticSubscriptions(String callingPackage) {
-        return getSubscriptionInfoListFromCacheHelper(
-                callingPackage, mCacheOpportunisticSubInfoList);
+        return getSubscriptionInfoListFromCacheHelper(callingPackage,
+                makeCacheListCopyWithLock(mCacheOpportunisticSubInfoList));
     }
 
     /**
@@ -3568,36 +3574,34 @@ public class SubscriptionController extends ISub.Stub {
             // the identifier and phone number access checks are not required.
         }
 
-        synchronized (mSubInfoListLock) {
-            // If the caller can read all phone state, just return the full list.
-            if (canReadIdentifiers) {
-                return new ArrayList<>(cacheSubList);
-            }
-            // Filter the list to only include subscriptions which the caller can manage.
-            List<SubscriptionInfo> subscriptions = new ArrayList<>(cacheSubList.size());
-            for (SubscriptionInfo subscriptionInfo : cacheSubList) {
-                int subId = subscriptionInfo.getSubscriptionId();
-                boolean hasCarrierPrivileges = TelephonyPermissions.checkCarrierPrivilegeForSubId(
-                        subId);
-                // If the caller does not have the READ_PHONE_STATE permission nor carrier
-                // privileges then they cannot access the current subscription.
-                if (!canReadPhoneState && !hasCarrierPrivileges) {
-                    continue;
-                }
-                // If the caller has carrier privileges then they are granted access to all
-                // identifiers for their subscription.
-                if (hasCarrierPrivileges) {
-                    subscriptions.add(subscriptionInfo);
-                } else {
-                    // The caller does not have carrier privileges for this subId, filter the
-                    // identifiers in the subscription based on the results of the initial
-                    // permission checks.
-                    subscriptions.add(
-                            conditionallyRemoveIdentifiers(subscriptionInfo, canReadIdentifiers));
-                }
+        // If the caller can read all phone state, just return the full list.
+        if (canReadIdentifiers) {
+            return cacheSubList;
+        }
+        // Filter the list to only include subscriptions which the caller can manage.
+        List<SubscriptionInfo> subscriptions = new ArrayList<>(cacheSubList.size());
+        for (SubscriptionInfo subscriptionInfo : cacheSubList) {
+            int subId = subscriptionInfo.getSubscriptionId();
+            boolean hasCarrierPrivileges = TelephonyPermissions.checkCarrierPrivilegeForSubId(
+                    subId);
+            // If the caller does not have the READ_PHONE_STATE permission nor carrier
+            // privileges then they cannot access the current subscription.
+            if (!canReadPhoneState && !hasCarrierPrivileges) {
+                continue;
+            }
+            // If the caller has carrier privileges then they are granted access to all
+            // identifiers for their subscription.
+            if (hasCarrierPrivileges) {
+                subscriptions.add(subscriptionInfo);
+            } else {
+                // The caller does not have carrier privileges for this subId, filter the
+                // identifiers in the subscription based on the results of the initial
+                // permission checks.
+                subscriptions.add(
+                        conditionallyRemoveIdentifiers(subscriptionInfo, canReadIdentifiers));
             }
-            return subscriptions;
         }
+        return subscriptions;
     }
 
     /**
@@ -3697,14 +3701,13 @@ public class SubscriptionController extends ISub.Stub {
     }
 
     private void refreshCachedOpportunisticSubscriptionInfoList() {
+        List<SubscriptionInfo> subList = getSubInfo(
+                SubscriptionManager.IS_OPPORTUNISTIC + "=1 AND ("
+                        + SubscriptionManager.SIM_SLOT_INDEX + ">=0 OR "
+                        + SubscriptionManager.IS_EMBEDDED + "=1)", null);
         synchronized (mSubInfoListLock) {
             List<SubscriptionInfo> oldOpptCachedList = mCacheOpportunisticSubInfoList;
 
-            List<SubscriptionInfo> subList = getSubInfo(
-                    SubscriptionManager.IS_OPPORTUNISTIC + "=1 AND ("
-                            + SubscriptionManager.SIM_SLOT_INDEX + ">=0 OR "
-                            + SubscriptionManager.IS_EMBEDDED + "=1)", null);
-
             if (subList != null) {
                 subList.sort(SUBSCRIPTION_INFO_COMPARATOR);
             } else {