commit 3c1c15c9a20fc7c4e9c44b5ae4b253b53736e577
author Brad Ebinger <breadley@google.com> Mon Apr 27 15:44:32 2020 -0700
committer Brad Ebinger <breadley@google.com> Tue May 26 15:52:09 2020 -0700
tree ebb0f53c4ca3a0572146b38b293bdcaf20d622a2
parent a26485fa50b6be4ec0c4943155cf4540e62e18d4

Fix possible deadlock in incoming call

When notifying the framework of an incoming call, a lock is held
in both the MmTelFeature and MmTelFeatureConnection. This can cause
a deadlock if the MmTelFeatureConnection is also processing an
event, such as sending/acknowleging an SMS message.

Remove the lock around the listener methods to the framework, since
it is not needed.

Bug: 155083563
Fixes: 155819733
Test: atest CtsTelephonyTestCases:ImsServiceTest FrameworksTelephonyTests
Change-Id: I39f7192c6f79e215ef989797870f5f501197cd08
Merged-In: I39f7192c6f79e215ef989797870f5f501197cd08

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

diff --git a/telephony/java/android/telephony/ims/feature/MmTelFeature.java b/telephony/java/android/telephony/ims/feature/MmTelFeature.java
index 9ec3f61..6840e8e 100644
--- a/telephony/java/android/telephony/ims/feature/MmTelFeature.java
+++ b/telephony/java/android/telephony/ims/feature/MmTelFeature.java
@@ -430,7 +430,6 @@
     /**
      * @param listener A {@link Listener} used when the MmTelFeature receives an incoming call and
      *     notifies the framework.
-     * @hide
      */
     private void setListener(IImsMmTelListener listener) {
         synchronized (mLock) {
@@ -442,6 +441,16 @@
     }
 
     /**
+     * @return the listener associated with this MmTelFeature. May be null if it has not been set
+     * by the framework yet.
+     */
+    private IImsMmTelListener getListener() {
+        synchronized (mLock) {
+            return mListener;
+        }
+    }
+
+    /**
      * The current capability status that this MmTelFeature has defined is available. This
      * configuration will be used by the platform to figure out which capabilities are CURRENTLY
      * available to be used.
@@ -489,15 +498,14 @@
             throw new IllegalArgumentException("ImsCallSessionImplBase and Bundle can not be "
                     + "null.");
         }
-        synchronized (mLock) {
-            if (mListener == null) {
-                throw new IllegalStateException("Session is not available.");
-            }
-            try {
-                mListener.onIncomingCall(c.getServiceImpl(), extras);
-            } catch (RemoteException e) {
-                throw new RuntimeException(e);
-            }
+        IImsMmTelListener listener = getListener();
+        if (listener == null) {
+            throw new IllegalStateException("Session is not available.");
+        }
+        try {
+            listener.onIncomingCall(c.getServiceImpl(), extras);
+        } catch (RemoteException e) {
+            throw new RuntimeException(e);
         }
     }
 
@@ -516,15 +524,14 @@
             throw new IllegalArgumentException("ImsCallProfile and ImsReasonInfo must not be "
                     + "null.");
         }
-        synchronized (mLock) {
-            if (mListener == null) {
-                throw new IllegalStateException("Session is not available.");
-            }
-            try {
-                mListener.onRejectedCall(callProfile, reason);
-            } catch (RemoteException e) {
-                throw new RuntimeException(e);
-            }
+        IImsMmTelListener listener = getListener();
+        if (listener == null) {
+            throw new IllegalStateException("Session is not available.");
+        }
+        try {
+            listener.onRejectedCall(callProfile, reason);
+        } catch (RemoteException e) {
+            throw new RuntimeException(e);
         }
     }
 
@@ -533,15 +540,14 @@
      * @hide
      */
     public final void notifyIncomingCallSession(IImsCallSession c, Bundle extras) {
-        synchronized (mLock) {
-            if (mListener == null) {
-                throw new IllegalStateException("Session is not available.");
-            }
-            try {
-                mListener.onIncomingCall(c, extras);
-            } catch (RemoteException e) {
-                throw new RuntimeException(e);
-            }
+        IImsMmTelListener listener = getListener();
+        if (listener == null) {
+            throw new IllegalStateException("Session is not available.");
+        }
+        try {
+            listener.onIncomingCall(c, extras);
+        } catch (RemoteException e) {
+            throw new RuntimeException(e);
         }
     }
 
@@ -552,15 +558,14 @@
      */
     @SystemApi @TestApi
     public final void notifyVoiceMessageCountUpdate(int count) {
-        synchronized (mLock) {
-            if (mListener == null) {
-                throw new IllegalStateException("Session is not available.");
-            }
-            try {
-                mListener.onVoiceMessageCountUpdate(count);
-            } catch (RemoteException e) {
-                throw new RuntimeException(e);
-            }
+        IImsMmTelListener listener = getListener();
+        if (listener == null) {
+            throw new IllegalStateException("Session is not available.");
+        }
+        try {
+            listener.onVoiceMessageCountUpdate(count);
+        } catch (RemoteException e) {
+            throw new RuntimeException(e);
         }
     }