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

Commit 5510b021 authored by rambowang's avatar rambowang Committed by Rambo Wang
Browse files

Clean up OpenLogicalChannelRecord on SIM removal 

This CL cleans up the OpenLogicalChannelRecord when detecting
the UiccPort is disposed or finalized which may occor in scenarios
like SIM removal or modem reset.

In all those scenarios, the underneath logical channel tracked in the
record has been released. Without the clean-up, the client crash later may
trigger a LC closure while the LC has been re-assigned to other clients.

Bug: 335046531
Test: atest FrameworksTelephonyTests
Test: Manual test to reproduce and verify the issue sceanrios
Test: Basic functional test (activation, call, sms, data...)
Change-Id: I4c7b150e80c6509ee549785595394b2cebbd7197
parent 9f3a7999

flags/uicc.aconfig

+8 −0
Original line number Diff line number Diff line
@@ -42,3 +42,11 @@ flag { 
    description: "This flag controls eSIM available memory feature."

    bug:"318348580"

}



# OWNER=rambowang TARGET=24Q3

flag {

    name: "cleanup_open_logical_channel_record_on_dispose"

    namespace: "telephony"

    description: "This flag cleans up the OpenLogicalChannelRecord once SIM is removed"

    bug:"335046531"

}

src/java/com/android/internal/telephony/uicc/UiccPort.java

+32 −18
Original line number Diff line number Diff line
@@ -31,6 +31,7 @@ import com.android.internal.telephony.IccLogicalChannelRequest; 
import com.android.internal.telephony.TelephonyComponentFactory;

import com.android.internal.telephony.flags.FeatureFlags;

import com.android.internal.telephony.flags.FeatureFlagsImpl;

import com.android.internal.telephony.flags.Flags;

import com.android.telephony.Rlog;



import java.io.FileDescriptor;
@@ -57,8 +58,9 @@ public class UiccPort { 
    private int mPortIdx;

    private int mPhysicalSlotIndex;



    // The list of the opened logical channel record. The channels will be closed by us when

    // detecting client died without closing them in advance.

    // The list of the opened logical channel record.

    // The channels will be closed by us when detecting client died without closing them in advance.

    // The same lock should be used to protect both access of the list and the individual record.

    @GuardedBy("mOpenChannelRecords")

    private final List<OpenLogicalChannelRecord> mOpenChannelRecords = new ArrayList<>();
@@ -101,11 +103,13 @@ public class UiccPort { 
            }

            mUiccProfile = null;

        }

        cleanupOpenLogicalChannelRecordsIfNeeded();

    }



    @Override

    protected void finalize() {

        if (DBG) log("UiccPort finalized");

        cleanupOpenLogicalChannelRecordsIfNeeded();

    }



    /**
@@ -389,8 +393,10 @@ public class UiccPort { 
    public void onLogicalChannelOpened(@NonNull IccLogicalChannelRequest request) {

        OpenLogicalChannelRecord record = new OpenLogicalChannelRecord(request);

        try {

            synchronized (mOpenChannelRecords) {

                request.binder.linkToDeath(record, /*flags=*/ 0);

            addOpenLogicalChannelRecord(record);

                mOpenChannelRecords.add(record);

            }

            if (DBG) log("onLogicalChannelOpened: monitoring client " + record);

        } catch (RemoteException | NullPointerException ex) {

            loge("IccOpenLogicChannel client has died, clean up manually");
@@ -406,11 +412,13 @@ public class UiccPort { 
     */

    public void onLogicalChannelClosed(int channelId) {

        OpenLogicalChannelRecord record = getOpenLogicalChannelRecord(channelId);

        synchronized (mOpenChannelRecords) {

            if (record != null && record.mRequest != null && record.mRequest.binder != null) {

                if (DBG) log("onLogicalChannelClosed: stop monitoring client " + record);

                record.mRequest.binder.unlinkToDeath(record, /*flags=*/ 0);

            removeOpenLogicalChannelRecord(record);

                record.mRequest.binder = null;

                mOpenChannelRecords.remove(record);

            }

        }

    }
@@ -428,15 +436,21 @@ public class UiccPort { 
        return null;

    }



    private void addOpenLogicalChannelRecord(OpenLogicalChannelRecord record) {

    /**

     * Clean up records when logical channels underneath have been released, in cases like SIM

     * removal or modem reset. The obsoleted records may trigger a redundant release of logical

     * channel that may have been assigned to other client.

     */

    private void cleanupOpenLogicalChannelRecordsIfNeeded() {

        if (Flags.cleanupOpenLogicalChannelRecordOnDispose()) {

            synchronized (mOpenChannelRecords) {

            mOpenChannelRecords.add(record);

                for (OpenLogicalChannelRecord record : mOpenChannelRecords) {

                    if (DBG) log("Clean up " + record);

                    record.mRequest.binder.unlinkToDeath(record, /*flags=*/ 0);

                    record.mRequest.binder = null;

                }

                mOpenChannelRecords.clear();

            }



    private void removeOpenLogicalChannelRecord(OpenLogicalChannelRecord record) {

        synchronized (mOpenChannelRecords) {

            mOpenChannelRecords.remove(record);

        }

    }

tests/telephonytests/src/com/android/internal/telephony/uicc/UiccPortTest.java

+25 −0
Original line number Diff line number Diff line
@@ -20,11 +20,16 @@ import static com.google.common.truth.Truth.assertThat; 
import static org.junit.Assert.assertEquals;

import static org.junit.Assert.assertFalse;

import static org.junit.Assert.assertNull;

import static org.mockito.ArgumentMatchers.any;

import static org.mockito.ArgumentMatchers.anyBoolean;

import static org.mockito.ArgumentMatchers.anyInt;

import static org.mockito.Matchers.eq;

import static org.mockito.Mockito.mock;

import static org.mockito.Mockito.never;

import static org.mockito.Mockito.verify;



import android.os.Binder;

import android.platform.test.flag.junit.SetFlagsRule;

import android.telephony.TelephonyManager;

import android.testing.AndroidTestingRunner;

import android.testing.TestableLooper;
@@ -33,9 +38,11 @@ import androidx.test.filters.SmallTest; 


import com.android.internal.telephony.IccLogicalChannelRequest;

import com.android.internal.telephony.TelephonyTest;

import com.android.internal.telephony.flags.Flags;



import org.junit.After;

import org.junit.Before;

import org.junit.Rule;

import org.junit.Test;

import org.junit.runner.RunWith;
@@ -54,9 +61,13 @@ public class UiccPortTest extends TelephonyTest { 


    private int mPhoneId = 0;



    @Rule

    public final SetFlagsRule mSetFlagsRule = new SetFlagsRule();



    @Before

    public void setUp() throws Exception {

        super.setUp(getClass().getSimpleName());

        mSetFlagsRule.enableFlags(Flags.FLAG_CLEANUP_OPEN_LOGICAL_CHANNEL_RECORD_ON_DISPOSE);

        mUiccCard = mock(UiccCard.class);

        mIccCardStatus = mock(IccCardStatus.class);

        /* initially there are no application available */
@@ -144,6 +155,20 @@ public class UiccPortTest extends TelephonyTest { 
        verify(mUiccProfile).iccCloseLogicalChannel(eq(CHANNEL_ID), eq(false), eq(null));

    }



    @Test

    @SmallTest

    public void testOnOpenLogicalChannel_withPortDisposed_noRecordLeft() {

        IccLogicalChannelRequest request = getIccLogicalChannelRequest();



        mUiccPort.onLogicalChannelOpened(request);

        mUiccPort.dispose();



        UiccPort.OpenLogicalChannelRecord record = mUiccPort.getOpenLogicalChannelRecord(

                CHANNEL_ID);

        assertThat(record).isNull();

        verify(mUiccProfile, never()).iccCloseLogicalChannel(anyInt(), anyBoolean(), any());

    }



    private IccLogicalChannelRequest getIccLogicalChannelRequest() {

        IccLogicalChannelRequest request = new IccLogicalChannelRequest();

        request.channel = CHANNEL_ID;