Merge "Don't close database when all devices have been detached."

4 files changed, 126 insertions, 83 deletions

diff --git a/packages/MtpDocumentsProvider/src/com/android/mtp/MtpDocumentsProvider.java b/packages/MtpDocumentsProvider/src/com/android/mtp/MtpDocumentsProvider.java
index f0f816173586..7c0676f23a94 100644
--- a/packages/MtpDocumentsProvider/src/com/android/mtp/MtpDocumentsProvider.java
+++ b/packages/MtpDocumentsProvider/src/com/android/mtp/MtpDocumentsProvider.java
@@ -31,6 +31,7 @@ import android.provider.DocumentsContract;
 import android.provider.DocumentsProvider;
 import android.util.Log;
 
+import com.android.internal.annotations.GuardedBy;
 import com.android.internal.annotations.VisibleForTesting;
 
 import java.io.FileNotFoundException;
@@ -59,6 +60,7 @@ public class MtpDocumentsProvider extends DocumentsProvider {
 
     private MtpManager mMtpManager;
     private ContentResolver mResolver;
+    @GuardedBy("mDeviceToolkits")
     private Map<Integer, DeviceToolkit> mDeviceToolkits;
     private RootScanner mRootScanner;
     private Resources mResources;
@@ -222,9 +224,11 @@ public class MtpDocumentsProvider extends DocumentsProvider {
 
     @Override
     public void onTrimMemory(int level) {
-      for (final DeviceToolkit toolkit : mDeviceToolkits.values()) {
-          toolkit.mDocumentLoader.clearCompletedTasks();
-      }
+        synchronized (mDeviceToolkits) {
+            for (final DeviceToolkit toolkit : mDeviceToolkits.values()) {
+                toolkit.mDocumentLoader.clearCompletedTasks();
+            }
+        }
     }
 
     @Override
@@ -254,21 +258,28 @@ public class MtpDocumentsProvider extends DocumentsProvider {
     }
 
     void openDevice(int deviceId) throws IOException {
-        mMtpManager.openDevice(deviceId);
-        mDeviceToolkits.put(deviceId, new DeviceToolkit(mMtpManager, mResolver, mDatabase));
-        mRootScanner.scanNow();
+        synchronized (mDeviceToolkits) {
+            mMtpManager.openDevice(deviceId);
+            mDeviceToolkits.put(deviceId, new DeviceToolkit(mMtpManager, mResolver, mDatabase));
+        }
+        mRootScanner.resume();
     }
 
-    void closeDevice(int deviceId) throws IOException {
+    void closeDevice(int deviceId) throws IOException, InterruptedException {
         // TODO: Flush the device before closing (if not closed externally).
-        getDeviceToolkit(deviceId).mDocumentLoader.clearTasks();
-        mDeviceToolkits.remove(deviceId);
-        mDatabase.removeDeviceRows(deviceId);
-        mMtpManager.closeDevice(deviceId);
+        synchronized (mDeviceToolkits) {
+            getDeviceToolkit(deviceId).mDocumentLoader.clearTasks();
+            mDeviceToolkits.remove(deviceId);
+            mDatabase.removeDeviceRows(deviceId);
+            mMtpManager.closeDevice(deviceId);
+        }
         mRootScanner.notifyChange();
+        if (!hasOpenedDevices()) {
+            mRootScanner.pause();
+        }
     }
 
-    void close() throws InterruptedException {
+    synchronized void closeAllDevices() throws InterruptedException {
         boolean closed = false;
         for (int deviceId : mMtpManager.getOpenedDeviceIds()) {
             try {
@@ -282,15 +293,30 @@ public class MtpDocumentsProvider extends DocumentsProvider {
         }
         if (closed) {
             mRootScanner.notifyChange();
+            mRootScanner.pause();
         }
-        mRootScanner.close();
-        mDatabase.close();
     }
 
     boolean hasOpenedDevices() {
         return mMtpManager.getOpenedDeviceIds().length != 0;
     }
 
+    /**
+     * Finalize the content provider for unit tests.
+     */
+    @Override
+    public void shutdown() {
+        try {
+            closeAllDevices();
+        } catch (InterruptedException e) {
+            // It should fail unit tests by throwing runtime exception.
+            throw new RuntimeException(e.getMessage());
+        } finally {
+            mDatabase.close();
+            super.shutdown();
+        }
+    }
+
     private void notifyChildDocumentsChange(String parentDocumentId) {
         mResolver.notifyChange(
                 DocumentsContract.buildChildDocumentsUri(AUTHORITY, parentDocumentId),
@@ -299,11 +325,13 @@ public class MtpDocumentsProvider extends DocumentsProvider {
     }
 
     private DeviceToolkit getDeviceToolkit(int deviceId) throws FileNotFoundException {
-        final DeviceToolkit toolkit = mDeviceToolkits.get(deviceId);
-        if (toolkit == null) {
-            throw new FileNotFoundException();
+        synchronized (mDeviceToolkits) {
+            final DeviceToolkit toolkit = mDeviceToolkits.get(deviceId);
+            if (toolkit == null) {
+                throw new FileNotFoundException();
+            }
+            return toolkit;
         }
-        return toolkit;
     }
 
     private PipeManager getPipeManager(Identifier identifier) throws FileNotFoundException {
diff --git a/packages/MtpDocumentsProvider/src/com/android/mtp/MtpDocumentsService.java b/packages/MtpDocumentsProvider/src/com/android/mtp/MtpDocumentsService.java
index 2d1af26ba3d2..723dc149c15d 100644
--- a/packages/MtpDocumentsProvider/src/com/android/mtp/MtpDocumentsService.java
+++ b/packages/MtpDocumentsProvider/src/com/android/mtp/MtpDocumentsService.java
@@ -67,10 +67,10 @@ public class MtpDocumentsService extends Service {
                 provider.openDevice(device.getDeviceId());
                 return START_STICKY;
             } catch (IOException error) {
-                Log.d(MtpDocumentsProvider.TAG, error.getMessage());
+                Log.e(MtpDocumentsProvider.TAG, error.getMessage());
             }
         } else {
-            Log.d(MtpDocumentsProvider.TAG, "Received unknown intent action.");
+            Log.w(MtpDocumentsProvider.TAG, "Received unknown intent action.");
         }
         stopSelfIfNeeded();
         return Service.START_NOT_STICKY;
@@ -82,7 +82,7 @@ public class MtpDocumentsService extends Service {
         unregisterReceiver(mReceiver);
         mReceiver = null;
         try {
-            provider.close();
+            provider.closeAllDevices();
         } catch (InterruptedException e) {
             Log.e(MtpDocumentsProvider.TAG, e.getMessage());
         }
@@ -105,8 +105,8 @@ public class MtpDocumentsService extends Service {
                 final MtpDocumentsProvider provider = MtpDocumentsProvider.getInstance();
                 try {
                     provider.closeDevice(device.getDeviceId());
-                } catch (IOException error) {
-                    Log.d(MtpDocumentsProvider.TAG, error.getMessage());
+                } catch (IOException | InterruptedException error) {
+                    Log.e(MtpDocumentsProvider.TAG, error.getMessage());
                 }
                 stopSelfIfNeeded();
             }
diff --git a/packages/MtpDocumentsProvider/src/com/android/mtp/RootScanner.java b/packages/MtpDocumentsProvider/src/com/android/mtp/RootScanner.java
index d9ed4ab5dc4a..b0962dd148b9 100644
--- a/packages/MtpDocumentsProvider/src/com/android/mtp/RootScanner.java
+++ b/packages/MtpDocumentsProvider/src/com/android/mtp/RootScanner.java
@@ -9,6 +9,10 @@ import android.provider.DocumentsContract;
 import android.util.Log;
 
 import java.io.IOException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.FutureTask;
+import java.util.concurrent.TimeUnit;
 
 final class RootScanner {
     /**
@@ -27,13 +31,18 @@ final class RootScanner {
      */
     private final static long SHORT_POLLING_TIMES = 10;
 
+    /**
+     * Milliseconds we wait for background thread when pausing.
+     */
+    private final static long AWAIT_TERMINATION_TIMEOUT = 2000;
+
     final ContentResolver mResolver;
     final Resources mResources;
     final MtpManager mManager;
     final MtpDatabase mDatabase;
-    boolean mClosed = false;
-    int mPollingCount;
-    Thread mBackgroundThread;
+
+    ExecutorService mExecutor;
+    FutureTask<Void> mCurrentTask;
 
     RootScanner(
             ContentResolver resolver,
@@ -46,6 +55,9 @@ final class RootScanner {
         mDatabase = database;
     }
 
+    /**
+     * Notifies a change of the roots list via ContentResolver.
+     */
     void notifyChange() {
         final Uri uri =
                 DocumentsContract.buildRootsUri(MtpDocumentsProvider.AUTHORITY);
@@ -56,74 +68,81 @@ final class RootScanner {
      * Starts to check new changes right away.
      * If the background thread has already gone, it restarts another background thread.
      */
-    synchronized void scanNow() {
-        if (mClosed) {
-            return;
+    synchronized void resume() {
+        if (mExecutor == null) {
+            // Only single thread updates the database.
+            mExecutor = Executors.newSingleThreadExecutor();
         }
-        mPollingCount = 0;
-        if (mBackgroundThread == null) {
-            mBackgroundThread = new BackgroundLoaderThread();
-            mBackgroundThread.start();
-        } else {
-            notify();
+        if (mCurrentTask != null) {
+            // Cancel previous task.
+            mCurrentTask.cancel(true);
         }
+        mCurrentTask = new FutureTask<Void>(new UpdateRootsRunnable(), null);
+        mExecutor.submit(mCurrentTask);
     }
 
-    void close() throws InterruptedException {
-        Thread thread;
-        synchronized (this) {
-            mClosed = true;
-            thread = mBackgroundThread;
-            if (mBackgroundThread == null) {
-                return;
-            }
-            notify();
+    /**
+     * Stops background thread and wait for its termination.
+     * @throws InterruptedException
+     */
+    synchronized void pause() throws InterruptedException {
+        if (mExecutor == null) {
+            return;
+        }
+        mExecutor.shutdownNow();
+        if (!mExecutor.awaitTermination(AWAIT_TERMINATION_TIMEOUT, TimeUnit.MILLISECONDS)) {
+            Log.e(MtpDocumentsProvider.TAG, "Failed to terminate RootScanner's background thread.");
         }
-        thread.join();
+        mExecutor = null;
     }
 
-    private final class BackgroundLoaderThread extends Thread {
+    /**
+     * Runnable to scan roots and update the database information.
+     */
+    private final class UpdateRootsRunnable implements Runnable {
         @Override
         public void run() {
             Process.setThreadPriority(Process.THREAD_PRIORITY_BACKGROUND);
-            synchronized (RootScanner.this) {
-                while (!mClosed) {
-                    final int[] deviceIds = mManager.getOpenedDeviceIds();
-                    if (deviceIds.length == 0) {
-                        break;
-                    }
-                    boolean changed = false;
-                    for (int deviceId : deviceIds) {
+            int pollingCount = 0;
+            while (!Thread.interrupted()) {
+                final int[] deviceIds = mManager.getOpenedDeviceIds();
+                if (deviceIds.length == 0) {
+                    return;
+                }
+                boolean changed = false;
+                for (int deviceId : deviceIds) {
+                    try {
+                        final MtpRoot[] roots = mManager.getRoots(deviceId);
                         mDatabase.startAddingRootDocuments(deviceId);
                         try {
-                            changed = mDatabase.putRootDocuments(
-                                    deviceId, mResources, mManager.getRoots(deviceId)) || changed;
-                        } catch (IOException|SQLiteException exp) {
-                            // The error may happen on the device. We would like to continue getting
-                            // roots for other devices.
-                            Log.e(MtpDocumentsProvider.TAG, exp.getMessage());
-                            continue;
+                            if (mDatabase.putRootDocuments(deviceId, mResources, roots)) {
+                                changed = true;
+                            }
                         } finally {
-                            changed = mDatabase.stopAddingRootDocuments(deviceId) || changed;
+                            if (mDatabase.stopAddingRootDocuments(deviceId)) {
+                                changed = true;
+                            }
                         }
-                    }
-                    if (changed) {
-                        notifyChange();
-                    }
-                    mPollingCount++;
-                    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.
-                        // TODO: Use short interval only for a device that is just added.
-                        RootScanner.this.wait(
-                                mPollingCount > SHORT_POLLING_TIMES ?
-                                        LONG_POLLING_INTERVAL : SHORT_POLLING_INTERVAL);
-                    } catch (InterruptedException exception) {
-                        break;
+                    } catch (IOException | SQLiteException exception) {
+                        // The error may happen on the device. We would like to continue getting
+                        // roots for other devices.
+                        Log.e(MtpDocumentsProvider.TAG, exception.getMessage());
                     }
                 }
-
-                mBackgroundThread = null;
+                if (changed) {
+                    notifyChange();
+                }
+                pollingCount++;
+                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.
+                    // TODO: Use short interval only for a device that is just added.
+                    Thread.sleep(pollingCount > SHORT_POLLING_TIMES ?
+                        LONG_POLLING_INTERVAL : SHORT_POLLING_INTERVAL);
+                } catch (InterruptedException exp) {
+                    // The while condition handles the interrupted flag.
+                    continue;
+                }
             }
         }
     }
diff --git a/packages/MtpDocumentsProvider/tests/src/com/android/mtp/MtpDocumentsProviderTest.java b/packages/MtpDocumentsProvider/tests/src/com/android/mtp/MtpDocumentsProviderTest.java
index 82e08cd26927..cabb08de9a04 100644
--- a/packages/MtpDocumentsProvider/tests/src/com/android/mtp/MtpDocumentsProviderTest.java
+++ b/packages/MtpDocumentsProvider/tests/src/com/android/mtp/MtpDocumentsProviderTest.java
@@ -49,11 +49,7 @@ public class MtpDocumentsProviderTest extends AndroidTestCase {
 
     @Override
     public void tearDown() {
-        try {
-            mProvider.close();
-        } catch (InterruptedException e) {
-            fail();
-        }
+        mProvider.shutdown();
     }
 
     public void testOpenAndCloseDevice() throws Exception {