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

Commit 216cf453 authored by Hieu Dang's avatar Hieu Dang
Browse files

Fix IllegalArgumentException in BluetoothOppServiceTest 

trimDatabase() is called in "trimDatabase" thread that's started in
BluetoothOppService#start(). This thread can be ran even when provider
is not enabled, and even not in the same time frame with the current
test. This might raise exception, resulting in the tests following the
test that initiate this thread to fail.

This cl also fix similar problems with other threads like mUpdateThread
and NotificationUpdateThread

This cl also fix memory issue mentioned in b/266459454, where
ContentProviderClient is used but not freed

Test: atest BluetoothOppServiceTest --rerun-until-failure=100
Bug: 281311423
Bug: 257539713
Bug: 266459454
Tag: #refactor
Change-Id: Ieb8129bf29a7e1841ba3d6c40f677cec7dd9b897
Merged-In: 23705047
parent 3daf6927

android/app/src/com/android/bluetooth/BluetoothMethodProxy.java

+5 −0
Original line number Diff line number Diff line
@@ -258,4 +258,9 @@ public class BluetoothMethodProxy { 
            ContextMap map, GattService service) {

        return new AppAdvertiseStats(appUid, id, name, map, service);

    }



    /** Proxies {@link Thread#start()}. */

    public void threadStart(Thread thread) {

        thread.start();

    }

}

android/app/src/com/android/bluetooth/opp/BluetoothOppService.java

+67 −34
Original line number Diff line number Diff line
@@ -55,6 +55,7 @@ import android.os.Process; 
import android.sysprop.BluetoothProperties;

import android.util.Log;



import com.android.bluetooth.BluetoothMethodProxy;

import com.android.bluetooth.BluetoothObexTransport;

import com.android.bluetooth.IObexConnectionHandler;

import com.android.bluetooth.ObexServerSockets;
@@ -130,12 +131,11 @@ public class BluetoothOppService extends ProfileService implements IObexConnecti 


    private boolean mPendingUpdate;



    private UpdateThread mUpdateThread;

    @VisibleForTesting UpdateThread mUpdateThread;



    private boolean mUpdateThreadRunning;



    private ArrayList<BluetoothOppShareInfo> mShares;



    private ArrayList<BluetoothOppBatch> mBatches;



    private BluetoothOppTransfer mTransfer;
@@ -181,8 +181,15 @@ public class BluetoothOppService extends ProfileService implements IObexConnecti 
                    + BluetoothShare.USER_CONFIRMATION_PENDING;



    private static final String WHERE_INVISIBLE_UNCONFIRMED =

            "(" + BluetoothShare.STATUS + " > " + BluetoothShare.STATUS_SUCCESS + " AND "

                    + INVISIBLE + ") OR (" + WHERE_CONFIRM_PENDING_INBOUND + ")";

            "("

                    + BluetoothShare.STATUS

                    + " > "

                    + BluetoothShare.STATUS_SUCCESS

                    + " AND "

                    + INVISIBLE

                    + ") OR ("

                    + WHERE_CONFIRM_PENDING_INBOUND

                    + ")";



    private static BluetoothOppService sBluetoothOppService;
@@ -609,7 +616,7 @@ public class BluetoothOppService extends ProfileService implements IObexConnecti 
            mPendingUpdate = true;

            if (mUpdateThread == null) {

                mUpdateThread = new UpdateThread();

                mUpdateThread.start();

                BluetoothMethodProxy.getInstance().threadStart(mUpdateThread);

                mUpdateThreadRunning = true;

            }

        }
@@ -985,9 +992,7 @@ public class BluetoothOppService extends ProfileService implements IObexConnecti 
        }

    }



    /**

     * Removes the local copy of the info about a share.

     */

    /** Removes the local copy of the info about a share. */

    private void deleteShare(int arrayPos) {

        BluetoothOppShareInfo info = mShares.get(arrayPos);
@@ -1113,8 +1118,17 @@ public class BluetoothOppService extends ProfileService implements IObexConnecti 


    // Run in a background thread at boot.

    private static void trimDatabase(ContentResolver contentResolver) {

        // Try-catch is important because trimDatabase can run even when the OPP_PROVIDER is

        // disabled (by OPP service, shell command, etc.).

        // At the sametime, it's ok to retry trimDatabase later when the service restart

        try {

            // remove the invisible/unconfirmed inbound shares

        int delNum = contentResolver.delete(BluetoothShare.CONTENT_URI, WHERE_INVISIBLE_UNCONFIRMED,

            int delNum =

                    BluetoothMethodProxy.getInstance()

                            .contentResolverDelete(

                                    contentResolver,

                                    BluetoothShare.CONTENT_URI,

                                    WHERE_INVISIBLE_UNCONFIRMED,

                                    null);

            if (V) {

                Log.v(TAG, "Deleted shares, number = " + delNum);
@@ -1122,8 +1136,14 @@ public class BluetoothOppService extends ProfileService implements IObexConnecti 


            // Keep the latest inbound and successful shares.

            Cursor cursor =

                contentResolver.query(BluetoothShare.CONTENT_URI, new String[]{BluetoothShare._ID},

                        WHERE_INBOUND_SUCCESS, null, BluetoothShare._ID); // sort by id

                    BluetoothMethodProxy.getInstance()

                            .contentResolverQuery(

                                    contentResolver,

                                    BluetoothShare.CONTENT_URI,

                                    new String[] {BluetoothShare._ID},

                                    WHERE_INBOUND_SUCCESS,

                                    null,

                                    BluetoothShare._ID); // sort by id

            if (cursor == null) {

                return;

            }
@@ -1134,14 +1154,22 @@ public class BluetoothOppService extends ProfileService implements IObexConnecti 
                if (cursor.moveToPosition(numToDelete)) {

                    int columnId = cursor.getColumnIndexOrThrow(BluetoothShare._ID);

                    long id = cursor.getLong(columnId);

                delNum = contentResolver.delete(BluetoothShare.CONTENT_URI,

                        BluetoothShare._ID + " < " + id, null);

                    delNum =

                            BluetoothMethodProxy.getInstance()

                                    .contentResolverDelete(

                                            contentResolver,

                                            BluetoothShare.CONTENT_URI,

                                            BluetoothShare._ID + " < " + id,

                                            null);

                    if (V) {

                        Log.v(TAG, "Deleted old inbound success share: " + delNum);

                    }

                }

            }

            cursor.close();

        } catch (Exception e) {

            Log.e(TAG, "Exception when trimming database: ", e);

        }

    }



    private static class MediaScannerNotifier implements MediaScannerConnectionClient {
@@ -1229,7 +1257,12 @@ public class BluetoothOppService extends ProfileService implements IObexConnecti 
    public boolean onConnect(BluetoothDevice device, BluetoothSocket socket) {



        if (D) {

            Log.d(TAG, " onConnect BluetoothSocket :" + socket + " \n :device :" + device.getIdentityAddress());

            Log.d(

                    TAG,

                    " onConnect BluetoothSocket :"

                            + socket

                            + " \n :device :"

                            + device.getIdentityAddress());

        }

        if (!mAcceptNewConnections) {

            Log.d(TAG, " onConnect BluetoothSocket :" + socket + " rejected");

android/app/tests/unit/src/com/android/bluetooth/opp/BluetoothOppServiceTest.java

+20 −3
Original line number Diff line number Diff line
@@ -15,23 +15,23 @@ 
 */

package com.android.bluetooth.opp;



import static org.mockito.ArgumentMatchers.any;

import static org.mockito.Mockito.anyString;

import static org.mockito.Mockito.doNothing;

import static org.mockito.Mockito.doReturn;



import android.bluetooth.BluetoothAdapter;

import android.content.Context;



import androidx.test.filters.MediumTest;

import androidx.test.rule.ServiceTestRule;

import androidx.test.runner.AndroidJUnit4;



import com.android.bluetooth.R;

import com.android.bluetooth.BluetoothMethodProxy;

import com.android.bluetooth.TestUtils;

import com.android.bluetooth.btservice.AdapterService;



import org.junit.After;

import org.junit.Assert;

import org.junit.Assume;

import org.junit.Before;

import org.junit.Rule;

import org.junit.Test;
@@ -48,12 +48,24 @@ public class BluetoothOppServiceTest { 
    @Rule

    public final ServiceTestRule mServiceRule = new ServiceTestRule();



    @Mock BluetoothMethodProxy mBluetoothMethodProxy;



    @Mock

    private AdapterService mAdapterService;



    @Before

    public void setUp() throws Exception {

        MockitoAnnotations.initMocks(this);



        BluetoothMethodProxy.setInstanceForTesting(mBluetoothMethodProxy);



        // BluetoothOppService can create a UpdateThread, which will call

        // BluetoothOppNotification#updateNotification(), which in turn create a new

        // NotificationUpdateThread. Both threads may cause the tests to fail because they try to

        // access to ContentProvider in multiple places (ContentProvider might be disabled & there

        // is no mocking). Since we have no intention to test those threads, avoid running them

        doNothing().when(mBluetoothMethodProxy).threadStart(any());



        TestUtils.setAdapterService(mAdapterService);

        doReturn(true, false).when(mAdapterService).isStartedProfile(anyString());

        TestUtils.startService(mServiceRule, BluetoothOppService.class);
@@ -66,6 +78,11 @@ public class BluetoothOppServiceTest { 


    @After

    public void tearDown() throws Exception {

        // Since the update thread is not run (we mocked it), it will not clean itself on interrupt

        // (normally, the service will wait for the update thread to clean itself after

        // being interrupted). We clean it manually here

        mService.mUpdateThread = null;

        BluetoothMethodProxy.setInstanceForTesting(null);

        TestUtils.stopService(mServiceRule, BluetoothOppService.class);

        TestUtils.clearAdapterService(mAdapterService);

    }