Handle Dead Objects from Phone Crashes better
1) Check to make sure ImsService is available when calling
changeMmTelCapability.
2) Remove caching of ImsConfig in ImsManager. It is already
cached properly in MmTelFeatureConnection.
3) Create a DeathRecipient for MmTelFeature in
MmTelFeatureConnection that cleans up all caches and notifies
ImsManager that the connection is now unavailable.
Bug: 77941698
Test: Manual
Merged-In: Ib26ce5308ec75113ad02c281b2a19cba5f94cdac
Change-Id: I9c3721fe275a94f7f8841e8e84a2f9d8a3c4823b
diff --git a/src/java/com/android/ims/ImsManager.java b/src/java/com/android/ims/ImsManager.java
index 3ad6c72..a01bbbb 100644
--- a/src/java/com/android/ims/ImsManager.java
+++ b/src/java/com/android/ims/ImsManager.java
@@ -368,18 +368,16 @@
private int mPhoneId;
private final boolean mConfigDynamicBind;
private @Nullable MmTelFeatureConnection mMmTelFeatureConnection = null;
- private ImsServiceDeathRecipient mDeathRecipient = new ImsServiceDeathRecipient();
- // Ut interface for the supplementary service configuration
- private ImsUt mUt = null;
- // Interface to get/set ims config items
- private ImsConfig mConfig = null;
private boolean mConfigUpdated = false;
private ImsConfigListener mImsConfigListener;
+ //TODO: Move these caches into the MmTelFeature Connection and restrict their lifetimes to the
+ // lifetime of the MmTelFeature.
+ // Ut interface for the supplementary service configuration
+ private ImsUt mUt = null;
// ECBM interface
private ImsEcbm mEcbm = null;
-
private ImsMultiEndpoint mMultiEndpoint = null;
private Set<MmTelFeatureConnection.IFeatureUpdate> mStatusCallbacks =
@@ -774,7 +772,7 @@
log("setVtSetting(b) : imsServiceAllowTurnOff -> turnOffIms");
turnOffIms();
}
- } catch (ImsException | RemoteException e) {
+ } catch (ImsException e) {
// The ImsService is down. Since the SubscriptionManager already recorded the user's
// preference, it will be resent in updateImsServiceConfig when the ImsPhoneCallTracker
// reconnects.
@@ -902,7 +900,7 @@
}
setWfcModeInternal(imsWfcModeFeatureValue);
- } catch (ImsException | RemoteException e) {
+ } catch (ImsException e) {
loge("setWfcSetting(): ", e);
}
}
@@ -1301,7 +1299,7 @@
}
mConfigUpdated = true;
- } catch (ImsException | RemoteException e) {
+ } catch (ImsException e) {
loge("updateImsServiceConfig: ", e);
mConfigUpdated = false;
}
@@ -1313,7 +1311,7 @@
* @return whether feature is On
* @throws ImsException
*/
- private boolean updateVolteFeatureValue() throws RemoteException {
+ private boolean updateVolteFeatureValue() throws ImsException {
boolean available = isVolteEnabledByPlatform();
boolean enabled = isEnhanced4gLteModeSettingEnabledByUser();
boolean isNonTty = isNonTtyOrTtyOnVolteEnabled();
@@ -1334,7 +1332,7 @@
* @return whether feature is On
* @throws ImsException
*/
- private boolean updateVideoCallFeatureValue() throws RemoteException {
+ private boolean updateVideoCallFeatureValue() throws ImsException {
boolean available = isVtEnabledByPlatform();
boolean enabled = isVtEnabledByUser();
boolean isNonTty = isNonTtyOrTtyOnVolteEnabled();
@@ -1361,7 +1359,7 @@
* @return whether feature is On
* @throws ImsException
*/
- private boolean updateWfcFeatureAndProvisionedValues() throws RemoteException {
+ private boolean updateWfcFeatureAndProvisionedValues() throws ImsException {
TelephonyManager tm = new TelephonyManager(mContext, getSubId());
boolean isNetworkRoaming = tm.isNetworkRoaming();
boolean available = isWfcEnabledByPlatform();
@@ -1652,7 +1650,6 @@
mMmTelFeatureConnection.closeConnection();
}
mUt = null;
- mConfig = null;
mEcbm = null;
mMultiEndpoint = null;
}
@@ -1801,41 +1798,44 @@
* @throws ImsException if getting the setting interface results in an error.
*/
public ImsConfig getConfigInterface() throws ImsException {
- if (mConfig != null && mConfig.isBinderAlive()) {
- return mConfig;
- }
-
checkAndThrowExceptionIfServiceUnavailable();
+
try {
IImsConfig config = mMmTelFeatureConnection.getConfigInterface();
if (config == null) {
throw new ImsException("getConfigInterface()",
ImsReasonInfo.CODE_LOCAL_SERVICE_UNAVAILABLE);
}
- mConfig = new ImsConfig(config, mContext);
+ return new ImsConfig(config);
} catch (RemoteException e) {
throw new ImsException("getConfigInterface()", e,
ImsReasonInfo.CODE_LOCAL_IMS_SERVICE_DOWN);
}
- return mConfig;
}
public void changeMmTelCapability(
@MmTelFeature.MmTelCapabilities.MmTelCapability int capability,
@ImsRegistrationImplBase.ImsRegistrationTech int radioTech,
- boolean isEnabled) throws RemoteException {
+ boolean isEnabled) throws ImsException {
+ checkAndThrowExceptionIfServiceUnavailable();
+
CapabilityChangeRequest request = new CapabilityChangeRequest();
if (isEnabled) {
request.addCapabilitiesToEnableForTech(capability, radioTech);
} else {
request.addCapabilitiesToDisableForTech(capability, radioTech);
}
- mMmTelFeatureConnection.changeEnabledCapabilities(request, null);
- if (mImsConfigListener != null) {
- mImsConfigListener.onSetFeatureResponse(capability,
- mMmTelFeatureConnection.getRegistrationTech(),
- isEnabled ? ImsConfig.FeatureValueConstants.ON
- : ImsConfig.FeatureValueConstants.OFF, -1);
+ try {
+ mMmTelFeatureConnection.changeEnabledCapabilities(request, null);
+ if (mImsConfigListener != null) {
+ mImsConfigListener.onSetFeatureResponse(capability,
+ mMmTelFeatureConnection.getRegistrationTech(),
+ isEnabled ? ImsConfig.FeatureValueConstants.ON
+ : ImsConfig.FeatureValueConstants.OFF, -1);
+ }
+ } catch (RemoteException e) {
+ throw new ImsException("changeMmTelCapability()", e,
+ ImsReasonInfo.CODE_LOCAL_IMS_SERVICE_DOWN);
}
}
@@ -2150,20 +2150,6 @@
}
/**
- * Death recipient class for monitoring IMS service.
- */
- private class ImsServiceDeathRecipient implements IBinder.DeathRecipient {
- @Override
- public void binderDied() {
- mMmTelFeatureConnection = null;
- mUt = null;
- mConfig = null;
- mEcbm = null;
- mMultiEndpoint = null;
- }
- }
-
- /**
* Gets the ECBM interface to request ECBM exit.
*
* @return the ECBM interface instance
diff --git a/src/java/com/android/ims/MmTelFeatureConnection.java b/src/java/com/android/ims/MmTelFeatureConnection.java
index 0706921..d6954f3 100644
--- a/src/java/com/android/ims/MmTelFeatureConnection.java
+++ b/src/java/com/android/ims/MmTelFeatureConnection.java
@@ -75,6 +75,11 @@
private IImsRegistration mRegistrationBinder;
private IImsConfig mConfigBinder;
+ private IBinder.DeathRecipient mDeathRecipient = () -> {
+ Log.w(TAG, "DeathRecipient triggered, binder died.");
+ onRemovedOrDied();
+ };
+
private abstract class CallbackAdapterManager<T> {
private static final String TAG = "CallbackAdapterManager";
@@ -337,14 +342,8 @@
}
switch (feature) {
case ImsFeature.FEATURE_MMTEL: {
- if (mIsAvailable) {
- Log.i(TAG, "MmTel disabled on slotId: " + slotId);
- mIsAvailable = false;
- mmTelFeatureRemoved();
- if (mStatusCallback != null) {
- mStatusCallback.notifyUnavailable();
- }
- }
+ Log.i(TAG, "MmTel removed on slotId: " + slotId);
+ onRemovedOrDied();
break;
}
case ImsFeature.FEATURE_EMERGENCY_MMTEL : {
@@ -376,12 +375,23 @@
mContext = context;
}
- // Called when the MmTelFeatureConnection has received an unavailable notification.
- private void mmTelFeatureRemoved() {
+ /**
+ * Called when the MmTelFeature has either been removed by Telephony or crashed.
+ */
+ private void onRemovedOrDied() {
synchronized (mLock) {
- // invalidate caches.
- mRegistrationBinder = null;
- mConfigBinder = null;
+ if (mIsAvailable) {
+ mIsAvailable = false;
+ // invalidate caches.
+ mRegistrationBinder = null;
+ mConfigBinder = null;
+ if (mBinder != null) {
+ mBinder.unlinkToDeath(mDeathRecipient, 0);
+ }
+ if (mStatusCallback != null) {
+ mStatusCallback.notifyUnavailable();
+ }
+ }
}
}
@@ -434,7 +444,16 @@
}
public void setBinder(IBinder binder) {
- mBinder = binder;
+ synchronized (mLock) {
+ mBinder = binder;
+ try {
+ if (mBinder != null) {
+ mBinder.linkToDeath(mDeathRecipient, 0);
+ }
+ } catch (RemoteException e) {
+ // No need to do anything if the binder is already dead.
+ }
+ }
}
/**
diff --git a/tests/Android.mk b/tests/Android.mk
index e07cab5..a0b998b 100644
--- a/tests/Android.mk
+++ b/tests/Android.mk
@@ -25,7 +25,9 @@
LOCAL_MODULE_TAGS := tests
-LOCAL_JAVA_LIBRARIES := ims-common android.test.runner
+LOCAL_JAVA_LIBRARIES := ims-common \
+ android.test.runner \
+ android.test.base
LOCAL_STATIC_JAVA_LIBRARIES := \
android-support-test \
diff --git a/tests/src/com/android/ims/ImsConfigTest.java b/tests/src/com/android/ims/ImsConfigTest.java
index 4cf7a92..31d9bcb 100644
--- a/tests/src/com/android/ims/ImsConfigTest.java
+++ b/tests/src/com/android/ims/ImsConfigTest.java
@@ -18,6 +18,7 @@
import android.support.test.runner.AndroidJUnit4;
import android.telephony.ims.aidl.IImsConfig;
+import android.test.suitebuilder.annotation.SmallTest;
import org.junit.After;
import org.junit.Before;
@@ -42,7 +43,7 @@
@Override
public void setUp() throws Exception {
super.setUp();
- mTestImsConfig = new ImsConfig(mMockImsConfigInterface, mContext);
+ mTestImsConfig = new ImsConfig(mMockImsConfigInterface);
}
@After
@@ -53,10 +54,11 @@
}
@Test
+ @SmallTest
public void testImsConfigGetProvisionedValue() throws Exception {
int testItem = 0;
- mTestImsConfig.getProvisionedValue(testItem);
+ mTestImsConfig.getConfigInt(testItem);
verify(mMockImsConfigInterface).getConfigInt(eq(testItem));
}
