diff options
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 { |