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

Commit 707c372f authored by Jeff Davidson's avatar Jeff Davidson
Browse files

Allow EuiccService IPCs to happen in parallel.

The (currently) undocumented behavior of oneway AIDLs is to serialize
IPCs made from the same client IBinder object. This is undesirable in
EuiccService as we want to respond to short-lived calls (like listing
subscriptions or getting the EID) while long-lived calls (like
download) are in progress. Introduce our own thread pool to work
around this behavior.

This also fixes a bug where uncaught exceptions thrown in the
implementation of an EuiccService method would be logged but never
trigger a callback on the client. Such exceptions now correctly crash
the app and trigger error callbacks in any active callers.

Bug: 62535655
Test: TreeHugger + verified subscription list during a download
Change-Id: I8b75af7fdc72117a163d81f7bf1447f6cc12ec6d
parent ae448fbf
Loading
Loading
Loading
Loading
+161 −67
Original line number Diff line number Diff line
@@ -25,6 +25,12 @@ import android.telephony.euicc.DownloadableSubscription;
import android.telephony.euicc.EuiccInfo;
import android.util.ArraySet;

import java.util.concurrent.LinkedBlockingQueue;
import java.util.concurrent.ThreadFactory;
import java.util.concurrent.ThreadPoolExecutor;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;

/**
 * Service interface linking the system with an eUICC local profile assistant (LPA) application.
 *
@@ -116,10 +122,45 @@ public abstract class EuiccService extends Service {

    private final IEuiccService.Stub mStubWrapper;

    private ThreadPoolExecutor mExecutor;

    public EuiccService() {
        mStubWrapper = new IEuiccServiceWrapper();
    }

    @Override
    @CallSuper
    public void onCreate() {
        super.onCreate();
        // We use a oneway AIDL interface to avoid blocking phone process binder threads on IPCs to
        // an external process, but doing so means the requests are serialized by binder, which is
        // not desired. Spin up a background thread pool to allow requests to be parallelized.
        // TODO(b/38206971): Consider removing this if basic card-level functions like listing
        // profiles are moved to the platform.
        mExecutor = new ThreadPoolExecutor(
                4 /* corePoolSize */,
                4 /* maxPoolSize */,
                30, TimeUnit.SECONDS, /* keepAliveTime */
                new LinkedBlockingQueue<>(), /* workQueue */
                new ThreadFactory() {
                    private final AtomicInteger mCount = new AtomicInteger(1);

                    @Override
                    public Thread newThread(Runnable r) {
                        return new Thread(r, "EuiccService #" + mCount.getAndIncrement());
                    }
                }
        );
        mExecutor.allowCoreThreadTimeOut(true);
    }

    @Override
    @CallSuper
    public void onDestroy() {
        mExecutor.shutdownNow();
        super.onDestroy();
    }

    /**
     * If overriding this method, call through to the super method for any unknown actions.
     * {@inheritDoc}
@@ -279,6 +320,9 @@ public abstract class EuiccService extends Service {
        public void downloadSubscription(int slotId, DownloadableSubscription subscription,
                boolean switchAfterDownload, boolean forceDeactivateSim,
                IDownloadSubscriptionCallback callback) {
            mExecutor.execute(new Runnable() {
                @Override
                public void run() {
                    int result = EuiccService.this.onDownloadSubscription(
                            slotId, subscription, switchAfterDownload, forceDeactivateSim);
                    try {
@@ -287,9 +331,14 @@ public abstract class EuiccService extends Service {
                        // Can't communicate with the phone process; ignore.
                    }
                }
            });
        }

        @Override
        public void getEid(int slotId, IGetEidCallback callback) {
            mExecutor.execute(new Runnable() {
                @Override
                public void run() {
                    String eid = EuiccService.this.onGetEid(slotId);
                    try {
                        callback.onSuccess(eid);
@@ -297,12 +346,17 @@ public abstract class EuiccService extends Service {
                        // Can't communicate with the phone process; ignore.
                    }
                }
            });
        }

        @Override
        public void getDownloadableSubscriptionMetadata(int slotId,
                DownloadableSubscription subscription,
                boolean forceDeactivateSim,
                IGetDownloadableSubscriptionMetadataCallback callback) {
            mExecutor.execute(new Runnable() {
                @Override
                public void run() {
                    GetDownloadableSubscriptionMetadataResult result =
                            EuiccService.this.onGetDownloadableSubscriptionMetadata(
                                    slotId, subscription, forceDeactivateSim);
@@ -312,10 +366,15 @@ public abstract class EuiccService extends Service {
                        // Can't communicate with the phone process; ignore.
                    }
                }
            });
        }

        @Override
        public void getDefaultDownloadableSubscriptionList(int slotId, boolean forceDeactivateSim,
                IGetDefaultDownloadableSubscriptionListCallback callback) {
            mExecutor.execute(new Runnable() {
                @Override
                public void run() {
                    GetDefaultDownloadableSubscriptionListResult result =
                            EuiccService.this.onGetDefaultDownloadableSubscriptionList(
                                    slotId, forceDeactivateSim);
@@ -325,9 +384,14 @@ public abstract class EuiccService extends Service {
                        // Can't communicate with the phone process; ignore.
                    }
                }
            });
        }

        @Override
        public void getEuiccProfileInfoList(int slotId, IGetEuiccProfileInfoListCallback callback) {
            mExecutor.execute(new Runnable() {
                @Override
                public void run() {
                    GetEuiccProfileInfoListResult result =
                            EuiccService.this.onGetEuiccProfileInfoList(slotId);
                    try {
@@ -336,9 +400,14 @@ public abstract class EuiccService extends Service {
                        // Can't communicate with the phone process; ignore.
                    }
                }
            });
        }

        @Override
        public void getEuiccInfo(int slotId, IGetEuiccInfoCallback callback) {
            mExecutor.execute(new Runnable() {
                @Override
                public void run() {
                    EuiccInfo euiccInfo = EuiccService.this.onGetEuiccInfo(slotId);
                    try {
                        callback.onSuccess(euiccInfo);
@@ -346,10 +415,16 @@ public abstract class EuiccService extends Service {
                        // Can't communicate with the phone process; ignore.
                    }
                }
            });

        }

        @Override
        public void deleteSubscription(int slotId, String iccid,
                IDeleteSubscriptionCallback callback) {
            mExecutor.execute(new Runnable() {
                @Override
                public void run() {
                    int result = EuiccService.this.onDeleteSubscription(slotId, iccid);
                    try {
                        callback.onComplete(result);
@@ -357,32 +432,49 @@ public abstract class EuiccService extends Service {
                        // Can't communicate with the phone process; ignore.
                    }
                }
            });
        }

        @Override
        public void switchToSubscription(int slotId, String iccid, boolean forceDeactivateSim,
                ISwitchToSubscriptionCallback callback) {
            mExecutor.execute(new Runnable() {
                @Override
                public void run() {
                    int result =
                    EuiccService.this.onSwitchToSubscription(slotId, iccid, forceDeactivateSim);
                            EuiccService.this.onSwitchToSubscription(
                                    slotId, iccid, forceDeactivateSim);
                    try {
                        callback.onComplete(result);
                    } catch (RemoteException e) {
                        // Can't communicate with the phone process; ignore.
                    }
                }
            });
        }

        @Override
        public void updateSubscriptionNickname(int slotId, String iccid, String nickname,
                IUpdateSubscriptionNicknameCallback callback) {
            int result = EuiccService.this.onUpdateSubscriptionNickname(slotId, iccid, nickname);
            mExecutor.execute(new Runnable() {
                @Override
                public void run() {
                    int result =
                            EuiccService.this.onUpdateSubscriptionNickname(slotId, iccid, nickname);
                    try {
                        callback.onComplete(result);
                    } catch (RemoteException e) {
                        // Can't communicate with the phone process; ignore.
                    }
                }
            });
        }

        @Override
        public void eraseSubscriptions(int slotId, IEraseSubscriptionsCallback callback) {
            mExecutor.execute(new Runnable() {
                @Override
                public void run() {
                    int result = EuiccService.this.onEraseSubscriptions(slotId);
                    try {
                        callback.onComplete(result);
@@ -390,5 +482,7 @@ public abstract class EuiccService extends Service {
                        // Can't communicate with the phone process; ignore.
                    }
                }
            });
        }
    }
}