OSDN Git Service

Fix Possible Deadlock with getFeatureState
authorBrad Ebinger <breadley@google.com>
Thu, 21 Jun 2018 17:18:21 +0000 (10:18 -0700)
committerBrad Ebinger <breadley@google.com>
Fri, 22 Jun 2018 23:14:17 +0000 (16:14 -0700)
Use the ImsFeature lock for concurrency in MmTelFeature
instead of declaring its own lock.

Also, fixed up some other concurrency code.

Bug: 110507525
Test: Manual, Telephony Unit Tests
Change-Id: I5a7ef1beca619873a7c228bed0e1b82843d4de63

telephony/java/android/telephony/ims/feature/ImsFeature.java
telephony/java/android/telephony/ims/feature/MmTelFeature.java

index d537699..b77881e 100644 (file)
@@ -341,15 +341,15 @@ public abstract class ImsFeature {
         }
     }
 
+    /** @hide */
+    protected Context mContext;
+    /** @hide */
+    protected final Object mLock = new Object();
+
     private final Set<IImsFeatureStatusCallback> mStatusCallbacks = Collections.newSetFromMap(
             new WeakHashMap<IImsFeatureStatusCallback, Boolean>());
     private @ImsState int mState = STATE_UNAVAILABLE;
     private int mSlotId = SubscriptionManager.INVALID_SIM_SLOT_INDEX;
-    /**
-     * @hide
-     */
-    protected Context mContext;
-    private final Object mLock = new Object();
     private final RemoteCallbackList<IImsCapabilityCallback> mCapabilityCallbacks
             = new RemoteCallbackList<>();
     private Capabilities mCapabilityStatus = new Capabilities();
index bc790fb..7681aef 100644 (file)
@@ -22,25 +22,25 @@ import android.os.Bundle;
 import android.os.Message;
 import android.os.RemoteException;
 import android.telecom.TelecomManager;
-import android.telephony.ims.stub.ImsRegistrationImplBase;
-import android.telephony.ims.stub.ImsCallSessionImplBase;
-import android.telephony.ims.stub.ImsSmsImplBase;
+import android.telephony.ims.ImsCallProfile;
+import android.telephony.ims.ImsCallSession;
+import android.telephony.ims.ImsReasonInfo;
 import android.telephony.ims.aidl.IImsCapabilityCallback;
 import android.telephony.ims.aidl.IImsMmTelFeature;
 import android.telephony.ims.aidl.IImsMmTelListener;
 import android.telephony.ims.aidl.IImsSmsListener;
+import android.telephony.ims.stub.ImsCallSessionImplBase;
 import android.telephony.ims.stub.ImsEcbmImplBase;
 import android.telephony.ims.stub.ImsMultiEndpointImplBase;
+import android.telephony.ims.stub.ImsRegistrationImplBase;
+import android.telephony.ims.stub.ImsSmsImplBase;
 import android.telephony.ims.stub.ImsUtImplBase;
 import android.util.Log;
 
-import android.telephony.ims.ImsCallProfile;
-import android.telephony.ims.ImsReasonInfo;
 import com.android.ims.internal.IImsCallSession;
 import com.android.ims.internal.IImsEcbm;
 import com.android.ims.internal.IImsMultiEndpoint;
 import com.android.ims.internal.IImsUt;
-import android.telephony.ims.ImsCallSession;
 import com.android.internal.annotations.VisibleForTesting;
 
 import java.lang.annotation.Retention;
@@ -61,20 +61,16 @@ public class MmTelFeature extends ImsFeature {
     private final IImsMmTelFeature mImsMMTelBinder = new IImsMmTelFeature.Stub() {
 
         @Override
-        public void setListener(IImsMmTelListener l) throws RemoteException {
-            synchronized (mLock) {
-                MmTelFeature.this.setListener(l);
-            }
+        public void setListener(IImsMmTelListener l) {
+            MmTelFeature.this.setListener(l);
         }
 
         @Override
         public int getFeatureState() throws RemoteException {
-            synchronized (mLock) {
-                try {
-                    return MmTelFeature.this.getFeatureState();
-                } catch (Exception e) {
-                    throw new RemoteException(e.getMessage());
-                }
+            try {
+                return MmTelFeature.this.getFeatureState();
+            } catch (Exception e) {
+                throw new RemoteException(e.getMessage());
             }
         }
 
@@ -138,10 +134,8 @@ public class MmTelFeature extends ImsFeature {
         }
 
         @Override
-        public int queryCapabilityStatus() throws RemoteException {
-            synchronized (mLock) {
-                return MmTelFeature.this.queryCapabilityStatus().mCapabilities;
-            }
+        public int queryCapabilityStatus() {
+            return MmTelFeature.this.queryCapabilityStatus().mCapabilities;
         }
 
         @Override
@@ -158,7 +152,7 @@ public class MmTelFeature extends ImsFeature {
 
         @Override
         public void changeCapabilitiesConfiguration(CapabilityChangeRequest request,
-                IImsCapabilityCallback c) throws RemoteException {
+                IImsCapabilityCallback c) {
             synchronized (mLock) {
                 MmTelFeature.this.requestChangeEnabledCapabilities(request, c);
             }
@@ -173,10 +167,8 @@ public class MmTelFeature extends ImsFeature {
         }
 
         @Override
-        public void setSmsListener(IImsSmsListener l) throws RemoteException {
-            synchronized (mLock) {
-                MmTelFeature.this.setSmsListener(l);
-            }
+        public void setSmsListener(IImsSmsListener l) {
+            MmTelFeature.this.setSmsListener(l);
         }
 
         @Override
@@ -364,9 +356,6 @@ public class MmTelFeature extends ImsFeature {
     @Retention(RetentionPolicy.SOURCE)
     public @interface ProcessCallResult {}
 
-
-    // Lock for feature synchronization
-    private final Object mLock = new Object();
     private IImsMmTelListener mListener;
 
     /**
@@ -376,9 +365,9 @@ public class MmTelFeature extends ImsFeature {
     private void setListener(IImsMmTelListener listener) {
         synchronized (mLock) {
             mListener = listener;
-        }
-        if (mListener != null) {
-            onFeatureReady();
+            if (mListener != null) {
+                onFeatureReady();
+            }
         }
     }