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

Commit d4dbb28c authored by Tomasz Wasilczyk's avatar Tomasz Wasilczyk
Browse files

Fix LeakedClosableViolations in UICC handling 

Bug: 378009798
Test: atest FrameworksTelephonyTests
Flag: EXEMPT bugfix
Change-Id: I66e6b0e5a911f4a6872b852369e197d1190b01dd
parent 95907a56

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();