Merge "Fix ImsService Callback synchronization issues"
diff --git a/src/java/com/android/internal/telephony/ims/ImsServiceController.java b/src/java/com/android/internal/telephony/ims/ImsServiceController.java
index 440757f..ea3d302 100644
--- a/src/java/com/android/internal/telephony/ims/ImsServiceController.java
+++ b/src/java/com/android/internal/telephony/ims/ImsServiceController.java
@@ -45,6 +45,7 @@
 import java.util.HashSet;
 import java.util.Iterator;
 import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
 
 /**
  * Manages the Binding lifecycle of one ImsService as well as the relevant ImsFeatures that the
@@ -201,7 +202,7 @@
     private IBinder mImsServiceControllerBinder;
     private ImsServiceConnection mImsServiceConnection;
     private ImsDeathRecipient mImsDeathRecipient;
-    private Set<IImsServiceFeatureCallback> mImsStatusCallbacks = new HashSet<>();
+    private Set<IImsServiceFeatureCallback> mImsStatusCallbacks = ConcurrentHashMap.newKeySet();
     // Only added or removed, never accessed on purpose.
     private Set<ImsFeatureStatusCallback> mFeatureStatusCallbacks = new HashSet<>();
 
@@ -456,8 +457,8 @@
      * Add a callback to ImsManager that signals a new feature that the ImsServiceProxy can handle.
      */
     public void addImsServiceFeatureCallback(IImsServiceFeatureCallback callback) {
+        mImsStatusCallbacks.add(callback);
         synchronized (mLock) {
-            mImsStatusCallbacks.add(callback);
             if (mImsFeatures == null || mImsFeatures.isEmpty()) {
                 return;
             }
@@ -587,9 +588,7 @@
 
     @VisibleForTesting
     public void removeImsServiceFeatureCallbacks() {
-        synchronized (mLock) {
             mImsStatusCallbacks.clear();
-        }
     }
 
     // Only add a new rebind if there are no pending rebinds waiting.
@@ -613,52 +612,46 @@
     }
 
     private void sendImsFeatureCreatedCallback(int slot, int feature) {
-        synchronized (mLock) {
-            for (Iterator<IImsServiceFeatureCallback> i = mImsStatusCallbacks.iterator();
-                    i.hasNext(); ) {
-                IImsServiceFeatureCallback callbacks = i.next();
-                try {
-                    callbacks.imsFeatureCreated(slot, feature);
-                } catch (RemoteException e) {
-                    // binder died, remove callback.
-                    Log.w(LOG_TAG, "sendImsFeatureCreatedCallback: Binder died, removing "
-                            + "callback. Exception:" + e.getMessage());
-                    i.remove();
-                }
+        for (Iterator<IImsServiceFeatureCallback> i = mImsStatusCallbacks.iterator();
+                i.hasNext(); ) {
+            IImsServiceFeatureCallback callbacks = i.next();
+            try {
+                callbacks.imsFeatureCreated(slot, feature);
+            } catch (RemoteException e) {
+                // binder died, remove callback.
+                Log.w(LOG_TAG, "sendImsFeatureCreatedCallback: Binder died, removing "
+                        + "callback. Exception:" + e.getMessage());
+                i.remove();
             }
         }
     }
 
     private void sendImsFeatureRemovedCallback(int slot, int feature) {
-        synchronized (mLock) {
-            for (Iterator<IImsServiceFeatureCallback> i = mImsStatusCallbacks.iterator();
-                    i.hasNext(); ) {
-                IImsServiceFeatureCallback callbacks = i.next();
-                try {
-                    callbacks.imsFeatureRemoved(slot, feature);
-                } catch (RemoteException e) {
-                    // binder died, remove callback.
-                    Log.w(LOG_TAG, "sendImsFeatureRemovedCallback: Binder died, removing "
-                            + "callback. Exception:" + e.getMessage());
-                    i.remove();
-                }
+        for (Iterator<IImsServiceFeatureCallback> i = mImsStatusCallbacks.iterator();
+                i.hasNext(); ) {
+            IImsServiceFeatureCallback callbacks = i.next();
+            try {
+                callbacks.imsFeatureRemoved(slot, feature);
+            } catch (RemoteException e) {
+                // binder died, remove callback.
+                Log.w(LOG_TAG, "sendImsFeatureRemovedCallback: Binder died, removing "
+                        + "callback. Exception:" + e.getMessage());
+                i.remove();
             }
         }
     }
 
     private void sendImsFeatureStatusChanged(int slot, int feature, int status) {
-        synchronized (mLock) {
-            for (Iterator<IImsServiceFeatureCallback> i = mImsStatusCallbacks.iterator();
-                    i.hasNext(); ) {
-                IImsServiceFeatureCallback callbacks = i.next();
-                try {
-                    callbacks.imsStatusChanged(slot, feature, status);
-                } catch (RemoteException e) {
-                    // binder died, remove callback.
-                    Log.w(LOG_TAG, "sendImsFeatureStatusChanged: Binder died, removing "
-                            + "callback. Exception:" + e.getMessage());
-                    i.remove();
-                }
+        for (Iterator<IImsServiceFeatureCallback> i = mImsStatusCallbacks.iterator();
+                i.hasNext(); ) {
+            IImsServiceFeatureCallback callbacks = i.next();
+            try {
+                callbacks.imsStatusChanged(slot, feature, status);
+            } catch (RemoteException e) {
+                // binder died, remove callback.
+                Log.w(LOG_TAG, "sendImsFeatureStatusChanged: Binder died, removing "
+                        + "callback. Exception:" + e.getMessage());
+                i.remove();
             }
         }
     }