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

Commit b3dca5cc authored by Meng Wang's avatar Meng Wang
Browse files

Close logical channels when EuiccService impl app crashes / is unbound. 

EuiccService implementation app can crash and individual endSession() calls won't happen in EuiccConnector. This CL adds endAllSessions() to EuiccSession to close all logical channels in such cases.

For normal unbinding, also call endAllSessions() as a defense of unexpected lingering sessions.

Flag: com.android.internal.telephony.flags.optimization_apdu_sender
Bug: 335257880
Test: unit test
Test: manual, make LPA crash in esim download and verify channel closed. Also keep device idle to verify unbind(). See b/335257880#comment27
Change-Id: Id42c9fca1567a2c861ec4abc0f9d2ae1e8a0e494
parent 5d94e6cf

src/java/com/android/internal/telephony/euicc/EuiccConnector.java

+2 −0
Original line number Diff line number Diff line
@@ -749,10 +749,12 @@ public class EuiccConnector extends StateMachine implements ServiceConnection { 
        @Override

        public boolean processMessage(Message message) {

            if (message.what == CMD_SERVICE_DISCONNECTED) {

                EuiccSession.get().endAllSessions();

                mEuiccService = null;

                transitionTo(mDisconnectedState);

                return HANDLED;

            } else if (message.what == CMD_LINGER_TIMEOUT) {

                EuiccSession.get().endAllSessions();

                unbind();

                transitionTo(mAvailableState);

                return HANDLED;

src/java/com/android/internal/telephony/euicc/EuiccSession.java

+52 −10
Original line number Diff line number Diff line
@@ -15,6 +15,7 @@ 
 */

package com.android.internal.telephony.euicc;



import android.annotation.Nullable;

import android.util.ArraySet;



import com.android.internal.annotations.GuardedBy;
@@ -85,14 +86,18 @@ public class EuiccSession { 


    /** Returns {@code true} if there is at least one session ongoing. */

    public boolean hasSession() {

        boolean hasSession;

        synchronized(this) {

            hasSession = !mSessions.isEmpty();

        }

        boolean hasSession = hasSessionInternal();

        Rlog.i(TAG, "hasSession: " + hasSession);

        return hasSession;

    }



    // The bare metal implementation of hasSession() without logging.

    private boolean hasSessionInternal() {

        synchronized(this) {

            return !mSessions.isEmpty();

        }

    }



    /**

     * Notes that a logical channel may be opened by the {@code apduSender}, which will

     * be used to close the channel when session ends (see {@link #endSession()}).
@@ -104,7 +109,7 @@ public class EuiccSession { 
    public void noteChannelOpen(ApduSender apduSender) {

        Rlog.i(TAG, "noteChannelOpen: " + apduSender);

        synchronized(this) {

            if (hasSession()) {

            if (hasSessionInternal()) {

                mApduSenders.add(apduSender);

            }

        }
@@ -112,19 +117,41 @@ public class EuiccSession { 


    /**

     * Marks the end of a eUICC transaction session. If this ends the last ongoing session,

     * try to close the logical channel using the noted {@code apduSender}

     * try to close the logical channel using the noted {@code apduSender}s

     * (see {@link #noteChannelOpen()}).

     *

     * @param sessionId The session ID.

     */

    public void endSession(String sessionId) {

        Rlog.i(TAG, "endSession: " + sessionId);

        endSessionInternal(sessionId);

    }



    /**

     * Marks the end of all eUICC transaction sessions and close the logical

     * channels using the noted {@code apduSender}s

     * (see {@link #noteChannelOpen()}).

     *

     * <p>This is useful in global cleanup e.g. when EuiccService

     * implementation app crashes and indivial {@link #endSession()} calls

     * won't happen in {@link EuiccConnector}.

     */

    public void endAllSessions() {

        Rlog.i(TAG, "endAllSessions");

        endSessionInternal(null);

    }



    // The implementation of endSession(sessionId) or endAllSessions() when the sessionId is null,

    // without logging.

    private void endSessionInternal(@Nullable String sessionId) {

        ApduSender[] apduSenders = new ApduSender[0];

        synchronized(this) {

            boolean sessionRemoved = mSessions.remove(sessionId);

            // sessionRemoved is false if the `sessionId` was never started or there was

            // no session at all i.e. `sessions` is empty. Don't bother invoke `apduSender`.

            if (sessionRemoved && mSessions.isEmpty()) {

            boolean sessionRemoved = removeOrClear(mSessions, sessionId);

            // 1. sessionRemoved is false if the `sessionId` was never started or there was

            // no session. Don't bother invoke `apduSender`.

            // 2. If some session is removed, and as a result there is no more session, we

            // can clsoe channels.

            if (sessionRemoved && !hasSessionInternal()) {

                // copy mApduSenders to a local variable so we don't call closeAnyOpenChannel()

                // which can take time in synchronized block.

                apduSenders = mApduSenders.toArray(apduSenders);
@@ -136,6 +163,21 @@ public class EuiccSession { 
        }

    }



    /**

     * Removes the given element from the set. If the element is null, clears the set.

     *

     * @return true if the set changed as a result of the call

     */

    private static boolean removeOrClear(Set<String> collection, @Nullable String element) {

        if (element == null) {

            boolean collectionChanged = !collection.isEmpty();

            collection.clear();

            return collectionChanged;

        } else {

            return collection.remove(element);

        }

    }



    @VisibleForTesting

    public EuiccSession() {}

}

tests/telephonytests/src/com/android/internal/telephony/euicc/EuiccSessionTest.java

+37 −1
Original line number Diff line number Diff line
@@ -103,6 +103,7 @@ public class EuiccSessionTest extends TelephonyTest { 


    private EuiccSession mEuiccSession;

    @Mock private ApduSender mApduSender;

    @Mock private ApduSender mApduSender2;



    @Before

    public void setUp() throws Exception {
@@ -114,6 +115,7 @@ public class EuiccSessionTest extends TelephonyTest { 
    public void startOneSession_featureDisabled_noop() throws Exception {

        mEuiccSession.startSession(SESSION_ID_1);

        mEuiccSession.noteChannelOpen(mApduSender);

        mEuiccSession.noteChannelOpen(mApduSender2);



        assertThat(mEuiccSession.hasSession()).isFalse();
@@ -121,6 +123,7 @@ public class EuiccSessionTest extends TelephonyTest { 


        assertThat(mEuiccSession.hasSession()).isFalse();

        verify(mApduSender, never()).closeAnyOpenChannel();

        verify(mApduSender2, never()).closeAnyOpenChannel();

    }



    @Test
@@ -128,6 +131,7 @@ public class EuiccSessionTest extends TelephonyTest { 
    public void startOneSession_endSession_hasSession() throws Exception {

        mEuiccSession.startSession(SESSION_ID_1);

        mEuiccSession.noteChannelOpen(mApduSender);

        mEuiccSession.noteChannelOpen(mApduSender2);



        assertThat(mEuiccSession.hasSession()).isTrue();
@@ -140,6 +144,7 @@ public class EuiccSessionTest extends TelephonyTest { 


        assertThat(mEuiccSession.hasSession()).isFalse();

        verify(mApduSender).closeAnyOpenChannel();

        verify(mApduSender2).closeAnyOpenChannel();

    }



    @Test
@@ -164,7 +169,24 @@ public class EuiccSessionTest extends TelephonyTest { 


    @Test

    @EnableFlags(Flags.FLAG_OPTIMIZATION_APDU_SENDER)

    public void noteChannelOpen_noSession_noop() throws Exception {

    public void startTwoSessions_endAllSessions_hasSession() throws Exception {

        mEuiccSession.startSession(SESSION_ID_1);

        mEuiccSession.noteChannelOpen(mApduSender);

        mEuiccSession.startSession(SESSION_ID_2);

        mEuiccSession.noteChannelOpen(mApduSender2);



        assertThat(mEuiccSession.hasSession()).isTrue();



        mEuiccSession.endAllSessions();



        assertThat(mEuiccSession.hasSession()).isFalse();

        verify(mApduSender).closeAnyOpenChannel();

        verify(mApduSender2).closeAnyOpenChannel();

    }



    @Test

    @EnableFlags(Flags.FLAG_OPTIMIZATION_APDU_SENDER)

    public void noteChannelOpen_noSession_endSession_noop() throws Exception {

        // noteChannelOpen called without a session started

        mEuiccSession.noteChannelOpen(mApduSender);
@@ -175,4 +197,18 @@ public class EuiccSessionTest extends TelephonyTest { 
        assertThat(mEuiccSession.hasSession()).isFalse();

        verify(mApduSender, never()).closeAnyOpenChannel();

    }



    @Test

    @EnableFlags(Flags.FLAG_OPTIMIZATION_APDU_SENDER)

    public void endAllSessions_noSession_endAllSessions_noOp() throws Exception {

        // noteChannelOpen called without a session started

        mEuiccSession.noteChannelOpen(mApduSender);



        assertThat(mEuiccSession.hasSession()).isFalse();



        mEuiccSession.endAllSessions();



        assertThat(mEuiccSession.hasSession()).isFalse();

        verify(mApduSender, never()).closeAnyOpenChannel();

    }

}