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

Commit e5c38399 authored by virkumar's avatar virkumar Committed by Hakjun Choi
Browse files

(ImsService API changes for Better IMS Threading) ImsService to execute binder calls in Executor.

In order to avoid undefined behavior when the framework calls a method through IPC, the ImsService
will now be able to define an Executor that the ImsService can be used to execute the methods

By default all ImsService level method calls will use this Executor.
Sub-classes will also use this Executor unless specified via constuctor which takes executor as an argument.

Test: atest CtsTelephonyTestCases:ImsCallingTest
Bug: 171037053
Merged-In: I10621f9a767ba5bc55373f49caf426e66adbec77
Change-Id: I10621f9a767ba5bc55373f49caf426e66adbec77
parent 81568fa2
Loading
Loading
Loading
Loading
+6 −1
Original line number Diff line number Diff line
@@ -12926,6 +12926,7 @@ package android.telephony.ims {
    method public void disableIms(int);
    method public void enableIms(int);
    method public android.telephony.ims.stub.ImsConfigImplBase getConfig(int);
    method @NonNull public java.util.concurrent.Executor getExecutor();
    method public long getImsServiceCapabilities();
    method public android.telephony.ims.stub.ImsRegistrationImplBase getRegistration(int);
    method @Nullable public android.telephony.ims.stub.SipTransportImplBase getSipTransport(int);
@@ -13546,6 +13547,7 @@ package android.telephony.ims.feature {
  public class MmTelFeature extends android.telephony.ims.feature.ImsFeature {
    ctor public MmTelFeature();
    ctor public MmTelFeature(@NonNull java.util.concurrent.Executor);
    method public void changeEnabledCapabilities(@NonNull android.telephony.ims.feature.CapabilityChangeRequest, @NonNull android.telephony.ims.feature.ImsFeature.CapabilityCallbackProxy);
    method public void changeOfferedRtpHeaderExtensionTypes(@NonNull java.util.Set<android.telephony.ims.RtpHeaderExtensionType>);
    method @Nullable public android.telephony.ims.ImsCallProfile createCallProfile(int, int);
@@ -13579,7 +13581,7 @@ package android.telephony.ims.feature {
  }
  public class RcsFeature extends android.telephony.ims.feature.ImsFeature {
    ctor @Deprecated public RcsFeature();
    ctor public RcsFeature();
    ctor public RcsFeature(@NonNull java.util.concurrent.Executor);
    method public void changeEnabledCapabilities(@NonNull android.telephony.ims.feature.CapabilityChangeRequest, @NonNull android.telephony.ims.feature.ImsFeature.CapabilityCallbackProxy);
    method @NonNull public android.telephony.ims.stub.RcsCapabilityExchangeImplBase createCapabilityExchangeImpl(@NonNull android.telephony.ims.stub.CapabilityExchangeEventListener);
@@ -13684,6 +13686,7 @@ package android.telephony.ims.stub {
  }
  public class ImsConfigImplBase {
    ctor public ImsConfigImplBase(@NonNull java.util.concurrent.Executor);
    ctor public ImsConfigImplBase();
    method public int getConfigInt(int);
    method public String getConfigString(int);
@@ -13736,6 +13739,7 @@ package android.telephony.ims.stub {
  public class ImsRegistrationImplBase {
    ctor public ImsRegistrationImplBase();
    ctor public ImsRegistrationImplBase(@NonNull java.util.concurrent.Executor);
    method public final void onDeregistered(android.telephony.ims.ImsReasonInfo);
    method public final void onRegistered(int);
    method public final void onRegistered(@NonNull android.telephony.ims.ImsRegistrationAttributes);
@@ -13848,6 +13852,7 @@ package android.telephony.ims.stub {
  }
  public class SipTransportImplBase {
    ctor public SipTransportImplBase();
    ctor public SipTransportImplBase(@NonNull java.util.concurrent.Executor);
    method public void createSipDelegate(int, @NonNull android.telephony.ims.DelegateRequest, @NonNull android.telephony.ims.DelegateStateCallback, @NonNull android.telephony.ims.DelegateMessageCallback);
    method public void destroySipDelegate(@NonNull android.telephony.ims.stub.SipDelegate, int);
+111 −22
Original line number Diff line number Diff line
@@ -17,6 +17,7 @@
package android.telephony.ims;

import android.annotation.LongDef;
import android.annotation.NonNull;
import android.annotation.Nullable;
import android.annotation.SuppressLint;
import android.annotation.SystemApi;
@@ -44,11 +45,18 @@ import android.util.SparseArray;

import com.android.ims.internal.IImsFeatureStatusCallback;
import com.android.internal.annotations.VisibleForTesting;
import com.android.internal.telephony.util.TelephonyUtils;

import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.util.HashMap;
import java.util.Map;
import java.util.concurrent.CancellationException;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.CompletionException;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.Executor;
import java.util.function.Supplier;

/**
 * Main ImsService implementation, which binds via the Telephony ImsResolver. Services that extend
@@ -173,7 +181,21 @@ public class ImsService extends Service {
    private final SparseArray<SparseArray<ImsFeature>> mFeaturesBySlot = new SparseArray<>();

    private IImsServiceControllerListener mListener;
    private Executor mExecutor;

    /**
     * Create a new ImsService.
     * <p>
     * Method stubs called from the framework will be called asynchronously. Vendor specifies the
     * {@link Executor} that the methods stubs will be called. If mExecutor is set to null by
     * vendor use Runnable::run.
     */
    public ImsService() {
        mExecutor = ImsService.this.getExecutor();
        if (mExecutor == null) {
            mExecutor = Runnable::run;
        }
    }

    /**
     * Listener that notifies the framework of ImsService changes.
@@ -201,38 +223,45 @@ public class ImsService extends Service {

        @Override
        public IImsMmTelFeature createMmTelFeature(int slotId) {
            return createMmTelFeatureInternal(slotId);
            return executeMethodAsyncForResult(() -> createMmTelFeatureInternal(slotId),
                "createMmTelFeature");
        }

        @Override
        public IImsRcsFeature createRcsFeature(int slotId) {
            return createRcsFeatureInternal(slotId);
            return executeMethodAsyncForResult(() -> createRcsFeatureInternal(slotId),
                "createRcsFeature");
        }

        @Override
        public void addFeatureStatusCallback(int slotId, int featureType,
                IImsFeatureStatusCallback c) {
            ImsService.this.addImsFeatureStatusCallback(slotId, featureType, c);
            executeMethodAsync(() -> ImsService.this.addImsFeatureStatusCallback(
                    slotId, featureType, c), "addFeatureStatusCallback");
        }

        @Override
        public void removeFeatureStatusCallback(int slotId, int featureType,
                IImsFeatureStatusCallback c) {
            ImsService.this.removeImsFeatureStatusCallback(slotId, featureType, c);
            executeMethodAsync(() -> ImsService.this.removeImsFeatureStatusCallback(
                    slotId, featureType, c), "removeFeatureStatusCallback");
        }

        @Override
        public void removeImsFeature(int slotId, int featureType) {
            ImsService.this.removeImsFeature(slotId, featureType);
            executeMethodAsync(() -> ImsService.this.removeImsFeature(slotId, featureType),
                    "removeImsFeature");
        }

        @Override
        public ImsFeatureConfiguration querySupportedImsFeatures() {
            return ImsService.this.querySupportedImsFeatures();
            return executeMethodAsyncForResult(() -> ImsService.this.querySupportedImsFeatures(),
                    "ImsFeatureConfiguration");
        }

        @Override
        public long getImsServiceCapabilities() {
            return executeMethodAsyncForResult(() -> {
                long caps = ImsService.this.getImsServiceCapabilities();
                long sanitizedCaps = sanitizeCapabilities(caps);
                if (caps != sanitizedCaps) {
@@ -240,39 +269,86 @@ public class ImsService extends Service {
                            + Long.toHexString(caps ^ sanitizedCaps));
                }
                return sanitizedCaps;
            }, "getImsServiceCapabilities");
        }

        @Override
        public void notifyImsServiceReadyForFeatureCreation() {
            ImsService.this.readyForFeatureCreation();
            executeMethodAsync(() -> ImsService.this.readyForFeatureCreation(),
                    "notifyImsServiceReadyForFeatureCreation");
        }

        @Override
        public IImsConfig getConfig(int slotId) {
            return executeMethodAsyncForResult(() -> {
                ImsConfigImplBase c = ImsService.this.getConfig(slotId);
            return c != null ? c.getIImsConfig() : null;
                if (c != null) {
                    c.setDefaultExecutor(mExecutor);
                    return c.getIImsConfig();
                } else {
                    return null;
                }
            }, "getConfig");
        }

        @Override
        public IImsRegistration getRegistration(int slotId) {
            return executeMethodAsyncForResult(() -> {
                ImsRegistrationImplBase r =  ImsService.this.getRegistration(slotId);
            return r != null ? r.getBinder() : null;
                if (r != null) {
                    r.setDefaultExecutor(mExecutor);
                    return r.getBinder();
                } else {
                    return null;
                }
            }, "getRegistration");
        }

        @Override
        public ISipTransport getSipTransport(int slotId) {
            return executeMethodAsyncForResult(() -> {
                SipTransportImplBase s =  ImsService.this.getSipTransport(slotId);
            return s != null ? s.getBinder() : null;
                if (s != null) {
                    s.setDefaultExecutor(mExecutor);
                    return s.getBinder();
                } else {
                    return null;
                }
            }, "getSipTransport");
        }

        @Override
        public void enableIms(int slotId) {
            ImsService.this.enableIms(slotId);
            executeMethodAsync(() -> ImsService.this.enableIms(slotId), "enableIms");
        }

        @Override
        public void disableIms(int slotId) {
            ImsService.this.disableIms(slotId);
            executeMethodAsync(() -> ImsService.this.disableIms(slotId), "disableIms");
        }

        // Call the methods with a clean calling identity on the executor and wait indefinitely for
        // the future to return.
        private void executeMethodAsync(Runnable r, String errorLogName) {
            try {
                CompletableFuture.runAsync(
                        () -> TelephonyUtils.runWithCleanCallingIdentity(r), mExecutor).join();
            } catch (CancellationException | CompletionException e) {
                Log.w(LOG_TAG, "ImsService Binder - " + errorLogName + " exception: "
                        + e.getMessage());
            }
        }

        private <T> T executeMethodAsyncForResult(Supplier<T> r, String errorLogName) {
            CompletableFuture<T> future = CompletableFuture.supplyAsync(
                    () -> TelephonyUtils.runWithCleanCallingIdentity(r), mExecutor);
            try {
                return future.get();
            } catch (ExecutionException | InterruptedException e) {
                Log.w(LOG_TAG, "ImsService Binder - " + errorLogName + " exception: "
                        + e.getMessage());
                return null;
            }
        }
    };

@@ -300,6 +376,7 @@ public class ImsService extends Service {
        MmTelFeature f = createMmTelFeature(slotId);
        if (f != null) {
            setupFeature(f, slotId, ImsFeature.FEATURE_MMTEL);
            f.setDefaultExecutor(mExecutor);
            return f.getBinder();
        } else {
            Log.e(LOG_TAG, "createMmTelFeatureInternal: null feature returned.");
@@ -310,6 +387,7 @@ public class ImsService extends Service {
    private IImsRcsFeature createRcsFeatureInternal(int slotId) {
        RcsFeature f = createRcsFeature(slotId);
        if (f != null) {
            f.setDefaultExecutor(mExecutor);
            setupFeature(f, slotId, ImsFeature.FEATURE_RCS);
            return f.getBinder();
        } else {
@@ -562,4 +640,15 @@ public class ImsService extends Service {
        result.append("}");
        return result.toString();
    }

    /**
     * The ImsService will now be able to define an Executor that the ImsService can be used to
     * execute the methods. By default all ImsService level method calls will use this Executor.
     * The ImsService has set the default executor as Runnable::run,
     * Should be override or default executor will be used.
     *  @return an Executor used to execute methods called remotely by the framework.
     */
    public @NonNull Executor getExecutor() {
        return Runnable::run;
    }
}
+207 −64

File changed.

Preview size limit exceeded, changes collapsed.

+15 −6
Original line number Diff line number Diff line
@@ -70,7 +70,7 @@ public class RcsFeature extends ImsFeature {
        // Reference the outer class in order to have better test coverage metrics instead of
        // creating a inner class referencing the outer class directly.
        private final RcsFeature mReference;
        private final Executor mExecutor;
        private Executor mExecutor;

        RcsFeatureBinder(RcsFeature classRef, @CallbackExecutor Executor executor) {
            mReference = classRef;
@@ -259,7 +259,7 @@ public class RcsFeature extends ImsFeature {
        }
    }

    private final Executor mExecutor;
    private Executor mExecutor;
    private final RcsFeatureBinder mImsRcsBinder;
    private RcsCapabilityExchangeImplBase mCapabilityExchangeImpl;
    private CapabilityExchangeEventListener mCapExchangeEventListener;
@@ -270,13 +270,9 @@ public class RcsFeature extends ImsFeature {
     * Method stubs called from the framework will be called asynchronously. To specify the
     * {@link Executor} that the methods stubs will be called, use
     * {@link RcsFeature#RcsFeature(Executor)} instead.
     *
     * @deprecated Use {@link #RcsFeature(Executor)} to create the RcsFeature.
     */
    @Deprecated
    public RcsFeature() {
        super();
        mExecutor = Runnable::run;
        // Run on the Binder threads that call them.
        mImsRcsBinder = new RcsFeatureBinder(this, mExecutor);
    }
@@ -477,4 +473,17 @@ public class RcsFeature extends ImsFeature {
            return mCapabilityExchangeImpl;
        }
    }

    /**
     * Set default Executor from ImsService.
     * @param executor The default executor for the framework to use when executing the methods
     * overridden by the implementation of RcsFeature.
     * @hide
     */
    public final void setDefaultExecutor(@NonNull Executor executor) {
        if (mImsRcsBinder.mExecutor == null) {
            mExecutor = executor;
            mImsRcsBinder.mExecutor = executor;
        }
    }
}
+108 −38

File changed.

Preview size limit exceeded, changes collapsed.

Loading