From fe6da864da360a339edfb09288483704e87cc76f Mon Sep 17 00:00:00 2001
From: Brad Ebinger <breadley@google.com>
Date: Mon, 27 Apr 2020 15:44:32 -0700
Subject: [PATCH] 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
Test: atest CtsTelephonyTestCases:ImsServiceTest FrameworksTelephonyTests
Change-Id: I39f7192c6f79e215ef989797870f5f501197cd08
---
 .../telephony/ims/feature/MmTelFeature.java   | 79 ++++++++++---------
 1 file changed, 42 insertions(+), 37 deletions(-)

diff --git a/telephony/java/android/telephony/ims/feature/MmTelFeature.java b/telephony/java/android/telephony/ims/feature/MmTelFeature.java
index 7ff8735d7b739..0b25d6f8275b2 100644
--- a/telephony/java/android/telephony/ims/feature/MmTelFeature.java
+++ b/telephony/java/android/telephony/ims/feature/MmTelFeature.java
@@ -430,7 +430,6 @@ public class MmTelFeature extends ImsFeature {
     /**
      * @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) {
@@ -441,6 +440,16 @@ public class MmTelFeature extends ImsFeature {
         }
     }
 
+    /**
+     * @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
@@ -489,15 +498,14 @@ public class MmTelFeature extends ImsFeature {
             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 @@ public class MmTelFeature extends ImsFeature {
             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 @@ public class MmTelFeature extends ImsFeature {
      * @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 @@ public class MmTelFeature extends ImsFeature {
      */
     @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);
         }
     }