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

Commit 96355514 authored by Tomasz Wasilczyk's avatar Tomasz Wasilczyk Committed by Android (Google) Code Review
Browse files

Merge "Fix LeakedClosableViolations in UICC handling" into main

parents daf24583 d4dbb28c

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

+36 −4
Original line number Diff line number Diff line
@@ -80,6 +80,7 @@ import java.io.PrintWriter; 
import java.util.ArrayList;

import java.util.Arrays;

import java.util.List;

import java.util.Objects;

import java.util.UUID;

import java.util.stream.Collectors;

import java.util.stream.IntStream;
@@ -162,8 +163,7 @@ public class UiccController extends Handler { 
    // this needs to be here, because on bootup we dont know which index maps to which UiccSlot

    @UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.R, trackingBug = 170729553)

    private CommandsInterface[] mCis;

    @VisibleForTesting

    public UiccSlot[] mUiccSlots;

    private UiccSlot[] mUiccSlots;

    private int[] mPhoneIdToSlotId;

    private boolean mIsSlotStatusSupported = true;
@@ -490,6 +490,27 @@ public class UiccController extends Handler { 
        }

    }



    /**

     * Set UiccSlot object for a specific physical slot index on the device.

     *

     * This is only supposed to be used internally and by unit tests.

     *

     * @param slotId Slot index

     * @param slot Slot object

     */

    @VisibleForTesting

    public void setUiccSlot(int slotId, @NonNull UiccSlot slot) {

        synchronized (mLock) {

            if (!isValidSlotIndex(slotId)) {

                throw new ArrayIndexOutOfBoundsException("Invalid slot index: " + slotId);

            }

            if (mUiccSlots[slotId] != null) {

                mUiccSlots[slotId].dispose();

            }

            mUiccSlots[slotId] = Objects.requireNonNull(slot);

        }

    }



    /**

     * API to get UiccSlot object for a given phone id

     * @return UiccSlot object for the given phone id
@@ -1076,7 +1097,7 @@ public class UiccController extends Handler { 
                log("Creating mUiccSlots[" + slotId + "]; mUiccSlots.length = "

                        + mUiccSlots.length);

            }

            mUiccSlots[slotId] = new UiccSlot(mContext, true);

            setUiccSlot(slotId, new UiccSlot(mContext, true));

        }



        mUiccSlots[slotId].update(mCis[index], status, index, slotId);
@@ -1353,7 +1374,7 @@ public class UiccController extends Handler { 
                if (VDBG) {

                    log("Creating mUiccSlot[" + i + "]; mUiccSlots.length = " + mUiccSlots.length);

                }

                mUiccSlots[i] = new UiccSlot(mContext, isActive);

                setUiccSlot(i, new UiccSlot(mContext, isActive));

            }



            if (isActive) { // check isActive flag so that we don't have to iterate through all
@@ -1803,6 +1824,17 @@ public class UiccController extends Handler { 
        return mCardStrings;

    }



    /**

     * Release resources. Must be called each time this class is used.

     */

    @VisibleForTesting

    public void dispose() {

        for (var slot : mUiccSlots) {

            slot.dispose();

        }

        mUiccSlots = null;

    }



    public void dump(FileDescriptor fd, PrintWriter printWriter, String[] args) {

        IndentingPrintWriter pw = new IndentingPrintWriter(printWriter, "  ");

        pw.println("mIsCdmaSupported=" + isCdmaSupported(mContext));

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

+6 −0
Original line number Diff line number Diff line
@@ -440,7 +440,13 @@ public class UiccPort { 
     * removal or modem reset. The obsoleted records may trigger a redundant release of logical

     * channel that may have been assigned to other client.

     */

    @SuppressWarnings("GuardedBy")

    private void cleanupOpenLogicalChannelRecordsIfNeeded() {

        // This check may raise GuardedBy warning, but we need it as long as this method is called

        // from finalize(). We can remove it from there once UiccPort is fully protected against

        // resource leak (e.g. with CloseGuard) and all (direct and indirect) users are fixed.

        if (mOpenChannelRecords == null) return;



        synchronized (mOpenChannelRecords) {

            for (OpenLogicalChannelRecord record : mOpenChannelRecords) {

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

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

+7 −0
Original line number Diff line number Diff line
@@ -391,6 +391,13 @@ public class UiccSlot extends Handler { 
        }

    }



    /**

     * Release resources. Must be called each time this class is used.

     */

    public void dispose() {

        nullifyUiccCard(false);

    }



    public boolean isStateUnknown() {

        // CardState is not specific to any port index, use default port.

        CardState cardState = mCardState.get(TelephonyManager.DEFAULT_PORT_INDEX);

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

+1 −0
Original line number Diff line number Diff line
@@ -69,6 +69,7 @@ public class UiccCardTest extends TelephonyTest { 


    @After

    public void tearDown() throws Exception {

        mUiccCard.dispose();

        mUiccCard = null;

        mIccIoResult = null;

        super.tearDown();

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

+21 −33
Original line number Diff line number Diff line
@@ -15,8 +15,6 @@ 
 */

package com.android.internal.telephony.uicc;



import static junit.framework.Assert.fail;



import static org.junit.Assert.assertEquals;

import static org.junit.Assert.assertFalse;

import static org.junit.Assert.assertNotNull;
@@ -131,6 +129,7 @@ public class UiccControllerTest extends TelephonyTest { 


    @After

    public void tearDown() throws Exception {

        if (mUiccControllerUT != null) mUiccControllerUT.dispose();

        mUiccControllerUT = null;

        super.tearDown();

    }
@@ -145,6 +144,7 @@ public class UiccControllerTest extends TelephonyTest { 
                com.android.internal.R.array.non_removable_euicc_slots,

                nonRemovableEuiccSlots);

        replaceInstance(UiccController.class, "mInstance", null, null);

        mUiccControllerUT.dispose();

        mUiccControllerUT = UiccController.make(mContext, mFeatureFlags);

        processAllMessages();

    }
@@ -250,7 +250,7 @@ public class UiccControllerTest extends TelephonyTest { 
        mUiccControllerUT.mContext = InstrumentationRegistry.getContext();



        // Mock out UiccSlots

        mUiccControllerUT.mUiccSlots[0] = mMockSlot;

        mUiccControllerUT.setUiccSlot(0, mMockSlot);

        doReturn(mMockCard).when(mMockSlot).getUiccCard();

        doReturn(mMockPort).when(mMockCard).getUiccPort(0);

        doReturn("A1B2C3D4").when(mMockPort).getIccId();
@@ -296,7 +296,7 @@ public class UiccControllerTest extends TelephonyTest { 
        mUiccControllerUT.mContext = InstrumentationRegistry.getContext();



        // Mock out UiccSlots

        mUiccControllerUT.mUiccSlots[0] = mMockSlot;

        mUiccControllerUT.setUiccSlot(0, mMockSlot);

        doReturn(true).when(mMockSlot).isEuicc();



        // simulate slot status loaded so that the UiccController sets the card ID
@@ -323,7 +323,7 @@ public class UiccControllerTest extends TelephonyTest { 
        mUiccControllerUT.mContext = InstrumentationRegistry.getContext();



        // Mock out UiccSlots

        mUiccControllerUT.mUiccSlots[0] = mMockSlot;

        mUiccControllerUT.setUiccSlot(0, mMockSlot);

        doReturn(true).when(mMockSlot).isEuicc();



        // simulate slot status loaded so that the UiccController sets the card ID
@@ -351,7 +351,7 @@ public class UiccControllerTest extends TelephonyTest { 
        mUiccControllerUT.mContext = InstrumentationRegistry.getContext();



        // Mock out UiccSlots

        mUiccControllerUT.mUiccSlots[0] = mMockSlot;

        mUiccControllerUT.setUiccSlot(0, mMockSlot);

        doReturn(false).when(mMockSlot).isEuicc();

        doReturn(mMockCard).when(mMockSlot).getUiccCard();

        doReturn("ASDF1234").when(mMockCard).getCardId();
@@ -402,7 +402,7 @@ public class UiccControllerTest extends TelephonyTest { 
        mUiccControllerUT.mContext = InstrumentationRegistry.getContext();



        // Mock out UiccSlots

        mUiccControllerUT.mUiccSlots[0] = mMockSlot;

        mUiccControllerUT.setUiccSlot(0, mMockSlot);

        doReturn(false).when(mMockSlot).isEuicc();

        doReturn(mMockCard).when(mMockSlot).getUiccCard();

        doReturn("ASDF1234").when(mMockCard).getCardId();
@@ -453,7 +453,7 @@ public class UiccControllerTest extends TelephonyTest { 
        mUiccControllerUT.mContext = InstrumentationRegistry.getContext();



        // Mock out UiccSlots

        mUiccControllerUT.mUiccSlots[0] = mMockSlot;

        mUiccControllerUT.setUiccSlot(0, mMockSlot);

        doReturn(true).when(mMockSlot).isEuicc();

        doReturn(null).when(mMockSlot).getUiccCard();

        doReturn(false).when(mMockSlot).isRemovable();
@@ -499,21 +499,17 @@ public class UiccControllerTest extends TelephonyTest { 
     * The default eUICC should not be the removable slot if there is a built-in eUICC.

     */

    @Test

    public void testDefaultEuiccIsNotRemovable() {

        try {

    public void testDefaultEuiccIsNotRemovable() throws Exception {

        reconfigureSlots(2, new int[]{ 1 } /* non-removable slot */);

        } catch (Exception e) {

            fail("Unable to reconfigure slots.");

        }



        // Give UiccController a real context so it can use shared preferences

        mUiccControllerUT.mContext = InstrumentationRegistry.getContext();



        // Mock out UiccSlots so that [0] is a removable eUICC and [1] is built-in

        mUiccControllerUT.mUiccSlots[0] = mMockRemovableEuiccSlot;

        mUiccControllerUT.setUiccSlot(0, mMockRemovableEuiccSlot);

        doReturn(true).when(mMockRemovableEuiccSlot).isEuicc();

        doReturn(true).when(mMockRemovableEuiccSlot).isRemovable();

        mUiccControllerUT.mUiccSlots[1] = mMockSlot;

        mUiccControllerUT.setUiccSlot(1, mMockSlot);

        doReturn(true).when(mMockSlot).isEuicc();

        doReturn(false).when(mMockSlot).isRemovable();
@@ -550,21 +546,17 @@ public class UiccControllerTest extends TelephonyTest { 
     * not depend on the order of the slots.

     */

    @Test

    public void testDefaultEuiccIsNotRemovable_swapSlotOrder() {

        try {

    public void testDefaultEuiccIsNotRemovable_swapSlotOrder() throws Exception {

        reconfigureSlots(2, new int[]{ 0 } /* non-removable slot */);

        } catch (Exception e) {

            fail("Unable to reconfigure slots.");

        }



        // Give UiccController a real context so it can use shared preferences

        mUiccControllerUT.mContext = InstrumentationRegistry.getContext();



        // Mock out UiccSlots so that [0] is a built-in eUICC and [1] is removable

        mUiccControllerUT.mUiccSlots[0] = mMockSlot;

        mUiccControllerUT.setUiccSlot(0, mMockSlot);

        doReturn(true).when(mMockSlot).isEuicc();

        doReturn(false).when(mMockSlot).isRemovable();

        mUiccControllerUT.mUiccSlots[1] = mMockRemovableEuiccSlot;

        mUiccControllerUT.setUiccSlot(1, mMockRemovableEuiccSlot);

        doReturn(true).when(mMockRemovableEuiccSlot).isEuicc();

        doReturn(true).when(mMockRemovableEuiccSlot).isRemovable();
@@ -603,21 +595,17 @@ public class UiccControllerTest extends TelephonyTest { 
     * the removable eUICC.

     */

    @Test

    public void testDefaultEuiccIsNotRemovable_EuiccIsInactive() {

        try {

    public void testDefaultEuiccIsNotRemovable_EuiccIsInactive() throws Exception {

        reconfigureSlots(2, new int[]{ 1 } /* non-removable slot */);

        } catch (Exception e) {

            fail();

        }



        // Give UiccController a real context so it can use shared preferences

        mUiccControllerUT.mContext = InstrumentationRegistry.getContext();



        // Mock out UiccSlots. Slot 0 is inactive here.

        mUiccControllerUT.mUiccSlots[0] = mMockSlot;

        mUiccControllerUT.setUiccSlot(0, mMockSlot);

        doReturn(true).when(mMockSlot).isEuicc();

        doReturn(false).when(mMockSlot).isRemovable();

        mUiccControllerUT.mUiccSlots[1] = mMockRemovableEuiccSlot;

        mUiccControllerUT.setUiccSlot(1, mMockRemovableEuiccSlot);

        doReturn(true).when(mMockRemovableEuiccSlot).isEuicc();

        doReturn(true).when(mMockRemovableEuiccSlot).isRemovable();
@@ -669,7 +657,7 @@ public class UiccControllerTest extends TelephonyTest { 
        mUiccControllerUT.mContext = InstrumentationRegistry.getContext();



        // Mock out UiccSlots

        mUiccControllerUT.mUiccSlots[0] = mMockSlot;

        mUiccControllerUT.setUiccSlot(0, mMockSlot);

        doReturn(true).when(mMockSlot).isEuicc();

        doReturn(null).when(mMockSlot).getUiccCard();

        //doReturn("123451234567890").when(mMockSlot).getIccId();