Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit b0ed5523 authored by Xihou Li's avatar Xihou Li Committed by Brad Ebinger
Browse files

IMS can't recover after IMS relaunch

After ImsService relaunch, some defects would cause issue,
1, no ims features removed when binder(with imsservice) died,
this cause any further binder calls fail;
2, no ims feature status callbacks removed, then later
new MmTelFeatureConnection will add new callbacks, then zombie
callbacks exists;
3, ImsServiceControllerStatiCompat didn't override removeImsFeature,
which cause NPE when remove ims feature

Bug: 109824926
Test: Manual - Crash ImsService, Telephony Tests
Change-Id: I05c5c6f3ec6900eb7974ee1db1f794045d2c1dc2
parent 9c936de4
Loading
Loading
Loading
Loading
+20 −17
Original line number Original line Diff line number Diff line
@@ -78,7 +78,7 @@ public class ImsServiceController {
                mIsBinding = false;
                mIsBinding = false;
                mIsBound = false;
                mIsBound = false;
            }
            }
            notifyAllFeaturesRemoved();
            cleanupAllFeatures();
            cleanUpService();
            cleanUpService();
            startDelayedRebindToService();
            startDelayedRebindToService();
        }
        }
@@ -144,7 +144,7 @@ public class ImsServiceController {
            if (isServiceControllerAvailable()) {
            if (isServiceControllerAvailable()) {
                mImsServiceControllerBinder.unlinkToDeath(mImsDeathRecipient, 0);
                mImsServiceControllerBinder.unlinkToDeath(mImsDeathRecipient, 0);
            }
            }
            notifyAllFeaturesRemoved();
            cleanupAllFeatures();
            cleanUpService();
            cleanUpService();
        }
        }
    }
    }
@@ -373,6 +373,7 @@ public class ImsServiceController {
                    boolean bindSucceeded = startBindToService(imsServiceIntent,
                    boolean bindSucceeded = startBindToService(imsServiceIntent,
                            mImsServiceConnection, serviceFlags);
                            mImsServiceConnection, serviceFlags);
                    if (!bindSucceeded) {
                    if (!bindSucceeded) {
                        mIsBinding = false;
                        mBackoff.notifyFailed();
                        mBackoff.notifyFailed();
                    }
                    }
                    return bindSucceeded;
                    return bindSucceeded;
@@ -700,8 +701,7 @@ public class ImsServiceController {
    }
    }


    // This method should only be called when synchronized on mLock
    // This method should only be called when synchronized on mLock
    private void removeImsServiceFeature(ImsFeatureConfiguration.FeatureSlotPair featurePair)
    private void removeImsServiceFeature(ImsFeatureConfiguration.FeatureSlotPair featurePair) {
            throws RemoteException {
        if (!isServiceControllerAvailable() || mCallbacks == null) {
        if (!isServiceControllerAvailable() || mCallbacks == null) {
            Log.w(LOG_TAG, "removeImsServiceFeature called with null values.");
            Log.w(LOG_TAG, "removeImsServiceFeature called with null values.");
            return;
            return;
@@ -714,11 +714,18 @@ public class ImsServiceController {
            if (callbackToRemove != null) {
            if (callbackToRemove != null) {
                mFeatureStatusCallbacks.remove(callbackToRemove);
                mFeatureStatusCallbacks.remove(callbackToRemove);
            }
            }
            removeImsFeature(featurePair.slotId, featurePair.featureType,
                    (callbackToRemove != null ? callbackToRemove.getCallback() : null));
            removeImsFeatureBinder(featurePair.slotId, featurePair.featureType);
            removeImsFeatureBinder(featurePair.slotId, featurePair.featureType);
            // Signal ImsResolver to change supported ImsFeatures for this ImsServiceController
            // Signal ImsResolver to change supported ImsFeatures for this ImsServiceController
            mCallbacks.imsServiceFeatureRemoved(featurePair.slotId, featurePair.featureType, this);
            mCallbacks.imsServiceFeatureRemoved(featurePair.slotId, featurePair.featureType, this);
            try {
                removeImsFeature(featurePair.slotId, featurePair.featureType,
                        (callbackToRemove != null ? callbackToRemove.getCallback() : null));
            } catch (RemoteException e) {
                // The connection to this ImsService doesn't exist. This may happen if the service
                // has died and we are removing features.
                Log.i(LOG_TAG, "Couldn't remove feature {" + featurePair.featureType
                        + "}, connection is down: " + e.getMessage());
            }
        } else {
        } else {
            // Don't update ImsService for emergency MMTEL feature.
            // Don't update ImsService for emergency MMTEL feature.
            Log.i(LOG_TAG, "doesn't support emergency calling on slot " + featurePair.slotId);
            Log.i(LOG_TAG, "doesn't support emergency calling on slot " + featurePair.slotId);
@@ -773,19 +780,15 @@ public class ImsServiceController {
                .findFirst().orElse(null);
                .findFirst().orElse(null);
    }
    }


    private void notifyAllFeaturesRemoved() {
    private void cleanupAllFeatures() {
        if (mCallbacks == null) {
            Log.w(LOG_TAG, "notifyAllFeaturesRemoved called with invalid callbacks.");
            return;
        }
        synchronized (mLock) {
        synchronized (mLock) {
            for (ImsFeatureConfiguration.FeatureSlotPair feature : mImsFeatures) {
            // Remove all features and clean up all associated Binders.
                if (feature.featureType != ImsFeature.FEATURE_EMERGENCY_MMTEL) {
            for (ImsFeatureConfiguration.FeatureSlotPair i : mImsFeatures) {
                    // don't update ImsServiceController for emergency MMTEL.
                removeImsServiceFeature(i);
                    mCallbacks.imsServiceFeatureRemoved(feature.slotId, feature.featureType, this);
                }
                sendImsFeatureRemovedCallback(feature.slotId, feature.featureType);
            }
            }
            // remove all MmTelFeatureConnection callbacks, since we have already sent removed
            // callback.
            removeImsServiceFeatureCallbacks();
        }
        }
    }
    }


+12 −9
Original line number Original line Diff line number Diff line
@@ -61,7 +61,7 @@ public class ImsServiceControllerCompat extends ImsServiceController {
    }
    }


    @Override
    @Override
    protected String getServiceInterface() {
    protected final String getServiceInterface() {
        // Return compatibility version of String.
        // Return compatibility version of String.
        return ImsService.SERVICE_INTERFACE;
        return ImsService.SERVICE_INTERFACE;
    }
    }
@@ -70,7 +70,7 @@ public class ImsServiceControllerCompat extends ImsServiceController {
     * Converts the new command to {@link MMTelFeature#turnOnIms()}.
     * Converts the new command to {@link MMTelFeature#turnOnIms()}.
     */
     */
    @Override
    @Override
    public void enableIms(int slotId) {
    public final void enableIms(int slotId) {
        MmTelFeatureCompatAdapter adapter = mMmTelCompatAdapters.get(slotId);
        MmTelFeatureCompatAdapter adapter = mMmTelCompatAdapters.get(slotId);
        if (adapter == null) {
        if (adapter == null) {
            Log.w(TAG, "enableIms: adapter null for slot :" + slotId);
            Log.w(TAG, "enableIms: adapter null for slot :" + slotId);
@@ -87,7 +87,7 @@ public class ImsServiceControllerCompat extends ImsServiceController {
     * Converts the new command to {@link MMTelFeature#turnOffIms()}.
     * Converts the new command to {@link MMTelFeature#turnOffIms()}.
     */
     */
    @Override
    @Override
    public void disableIms(int slotId) {
    public final void disableIms(int slotId) {
        MmTelFeatureCompatAdapter adapter = mMmTelCompatAdapters.get(slotId);
        MmTelFeatureCompatAdapter adapter = mMmTelCompatAdapters.get(slotId);
        if (adapter == null) {
        if (adapter == null) {
            Log.w(TAG, "enableIms: adapter null for slot :" + slotId);
            Log.w(TAG, "enableIms: adapter null for slot :" + slotId);
@@ -104,7 +104,7 @@ public class ImsServiceControllerCompat extends ImsServiceController {
     * @return the IImsRegistration that corresponds to the slot id specified.
     * @return the IImsRegistration that corresponds to the slot id specified.
     */
     */
    @Override
    @Override
    public IImsRegistration getRegistration(int slotId) throws RemoteException {
    public final IImsRegistration getRegistration(int slotId) {
        ImsRegistrationCompatAdapter adapter = mRegCompatAdapters.get(slotId);
        ImsRegistrationCompatAdapter adapter = mRegCompatAdapters.get(slotId);
        if (adapter == null) {
        if (adapter == null) {
            Log.w(TAG, "getRegistration: Registration does not exist for slot " + slotId);
            Log.w(TAG, "getRegistration: Registration does not exist for slot " + slotId);
@@ -117,7 +117,7 @@ public class ImsServiceControllerCompat extends ImsServiceController {
     * @return the IImsConfig that corresponds to the slot id specified.
     * @return the IImsConfig that corresponds to the slot id specified.
     */
     */
    @Override
    @Override
    public IImsConfig getConfig(int slotId) throws RemoteException {
    public final IImsConfig getConfig(int slotId) {
        ImsConfigCompatAdapter adapter = mConfigCompatAdapters.get(slotId);
        ImsConfigCompatAdapter adapter = mConfigCompatAdapters.get(slotId);
        if (adapter == null) {
        if (adapter == null) {
            Log.w(TAG, "getConfig: Config does not exist for slot " + slotId);
            Log.w(TAG, "getConfig: Config does not exist for slot " + slotId);
@@ -127,13 +127,14 @@ public class ImsServiceControllerCompat extends ImsServiceController {
    }
    }


    @Override
    @Override
    protected void notifyImsServiceReady() throws RemoteException {
    protected final void notifyImsServiceReady() {
        Log.d(TAG, "notifyImsServiceReady");
        Log.d(TAG, "notifyImsServiceReady");
        // don't do anything for compat impl.
        // don't do anything for compat impl.
    }
    }


    @Override
    @Override
    protected IInterface createImsFeature(int slotId, int featureType, IImsFeatureStatusCallback c)
    protected final IInterface createImsFeature(int slotId, int featureType,
            IImsFeatureStatusCallback c)
            throws RemoteException {
            throws RemoteException {
        switch (featureType) {
        switch (featureType) {
            case ImsFeature.MMTEL: {
            case ImsFeature.MMTEL: {
@@ -148,15 +149,17 @@ public class ImsServiceControllerCompat extends ImsServiceController {
    }
    }


    @Override
    @Override
    protected void removeImsFeature(int slotId, int featureType, IImsFeatureStatusCallback c)
    protected final void removeImsFeature(int slotId, int featureType, IImsFeatureStatusCallback c)
            throws RemoteException {
            throws RemoteException {
        if (featureType == ImsFeature.MMTEL) {
        if (featureType == ImsFeature.MMTEL) {
            mMmTelCompatAdapters.remove(slotId);
            mMmTelCompatAdapters.remove(slotId);
            mRegCompatAdapters.remove(slotId);
            mRegCompatAdapters.remove(slotId);
            mConfigCompatAdapters.remove(slotId);
            mConfigCompatAdapters.remove(slotId);
        }
        }
        if (mServiceController != null) {
            mServiceController.removeImsFeature(slotId, featureType, c);
            mServiceController.removeImsFeature(slotId, featureType, c);
        }
        }
    }


    @Override
    @Override
    protected void setServiceController(IBinder serviceController) {
    protected void setServiceController(IBinder serviceController) {
+7 −3
Original line number Original line Diff line number Diff line
@@ -20,7 +20,6 @@ import android.content.ComponentName;
import android.content.Context;
import android.content.Context;
import android.content.Intent;
import android.content.Intent;
import android.os.IBinder;
import android.os.IBinder;
import android.os.RemoteException;
import android.os.ServiceManager;
import android.os.ServiceManager;
import android.util.Log;
import android.util.Log;


@@ -66,8 +65,13 @@ public class ImsServiceControllerStaticCompat extends ImsServiceControllerCompat
    }
    }


    @Override
    @Override
    protected MmTelInterfaceAdapter getInterface(int slotId, IImsFeatureStatusCallback c)
    // used for add/remove features and cleanup in ImsServiceController.
            throws RemoteException {
    protected boolean isServiceControllerAvailable() {
        return mImsServiceCompat != null;
    }

    @Override
    protected MmTelInterfaceAdapter getInterface(int slotId, IImsFeatureStatusCallback c) {
        if (mImsServiceCompat == null) {
        if (mImsServiceCompat == null) {
            Log.w(TAG, "getInterface: IImsService returned null.");
            Log.w(TAG, "getInterface: IImsService returned null.");
            return null;
            return null;