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

Commit acb0e27b authored by Daichi Hirono's avatar Daichi Hirono
Browse files

Regard timeout as an error in the MtpDocumentsProvider test. 

Previously if DocumentsProvider found timeout when terminatnig
RootScanner's background thread, it just output it in error log. Thus
the timeout is not regarded as an error in MtpDocumentsProviderTest, and
it makes flaky PipeManagerTest which runs just after
MtpDocumentsProviderTest.

The CL

 * lets MtpDocumentsProvider throw TimeoutException for timeout.
 * removes redundant resumeRootScanner calls to avoid timeout of
   RootScanner#pause.

Also the CL did cleanup the logic that pauses RootScanner when we don't
find any devices. Previously the logic was in
MtpDocumentsProvider#closeInternal but it is not efficient because we
invokes RootScanner#resume just after
MtpDocumentsProvider#closeInternal. Now the CL moves the logic to
RootScanner so that it can pause itself automatically.

BUG=27638500

Change-Id: Ic11bca67c099cbb0f46679db2f035988045d67d6
parent 62006a72

packages/MtpDocumentsProvider/src/com/android/mtp/MtpDocumentsProvider.java

+2 −5
Original line number Diff line number Diff line
@@ -36,7 +36,6 @@ import android.provider.DocumentsContract.Root; 
import android.provider.DocumentsContract;

import android.provider.DocumentsProvider;

import android.provider.Settings;

import android.provider.Settings.SettingNotFoundException;

import android.util.Log;



import com.android.internal.annotations.GuardedBy;
@@ -48,6 +47,7 @@ import java.io.IOException; 
import java.util.HashMap;

import java.util.List;

import java.util.Map;

import java.util.concurrent.TimeoutException;



/**

 * DocumentsProvider for MTP devices.
@@ -426,7 +426,7 @@ public class MtpDocumentsProvider extends DocumentsProvider { 
                    closeDeviceInternal(id);

                }

                mRootScanner.pause();

            } catch (InterruptedException|IOException e) {

            } catch (InterruptedException | IOException | TimeoutException e) {

                // It should fail unit tests by throwing runtime exception.

                throw new RuntimeException(e);

            } finally {
@@ -464,9 +464,6 @@ public class MtpDocumentsProvider extends DocumentsProvider { 
        getDeviceToolkit(deviceId).close();

        mDeviceToolkits.remove(deviceId);

        mMtpManager.closeDevice(deviceId);

        if (mDeviceToolkits.size() == 0) {

            mRootScanner.pause();

        }

    }



    private DeviceToolkit getDeviceToolkit(int deviceId) throws FileNotFoundException {

packages/MtpDocumentsProvider/src/com/android/mtp/RootScanner.java

+12 −4
Original line number Diff line number Diff line
@@ -27,6 +27,7 @@ import java.util.concurrent.CountDownLatch; 
import java.util.concurrent.ExecutorService;

import java.util.concurrent.Executors;

import java.util.concurrent.TimeUnit;

import java.util.concurrent.TimeoutException;



final class RootScanner {

    /**
@@ -95,16 +96,20 @@ final class RootScanner { 
     * Stops background thread and wait for its termination.

     * @throws InterruptedException

     */

    synchronized void pause() throws InterruptedException {

    synchronized void pause() throws InterruptedException, TimeoutException {

        if (mExecutor == null) {

            return;

        }

        mExecutor.shutdownNow();

        try {

            if (!mExecutor.awaitTermination(AWAIT_TERMINATION_TIMEOUT, TimeUnit.MILLISECONDS)) {

            Log.e(MtpDocumentsProvider.TAG, "Failed to terminate RootScanner's background thread.");

                throw new TimeoutException(

                        "Timeout for terminating RootScanner's background thread.");

            }

        } finally {

            mExecutor = null;

        }

    }



    /**

     * Runnable to scan roots and update the database information.
@@ -173,6 +178,9 @@ final class RootScanner { 
                }

                mFirstScanCompleted.countDown();

                pollingCount++;

                if (devices.length == 0) {

                    break;

                }

                try {

                    // Use SHORT_POLLING_PERIOD for the first SHORT_POLLING_TIMES because it is

                    // more likely to add new root just after the device is added.

packages/MtpDocumentsProvider/tests/src/com/android/mtp/MtpDocumentsProviderTest.java

+0 −2
Original line number Diff line number Diff line
@@ -262,11 +262,9 @@ public class MtpDocumentsProviderTest extends AndroidTestCase { 
                null));

        {

            mProvider.openDevice(0);

            mProvider.resumeRootScanner();

            mResolver.waitForNotification(ROOTS_URI, 1);



            mProvider.openDevice(1);

            mProvider.resumeRootScanner();

            mResolver.waitForNotification(ROOTS_URI, 2);



            final Cursor cursor = mProvider.queryRoots(null);

packages/MtpDocumentsProvider/tests/src/com/android/mtp/PipeManagerTest.java

+1 −0
Original line number Diff line number Diff line
@@ -48,6 +48,7 @@ public class PipeManagerTest extends AndroidTestCase { 
    @Override

    protected void tearDown() throws Exception {

        assertTrue(mPipeManager.close());

        mDatabase.close();

    }



    public void testReadDocument_basic() throws Exception {