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

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

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

ContentProvider is a singleton of the process. So it may live longer
than Service. We could not close database when the service is destroyed.

BUG=25730042

Change-Id: I591250c1a1e7c5705eb2585c71cac2598c0c0fb9
parent 26398b91
Loading
Loading
Loading
Loading
+46 −18
Original line number Diff line number Diff line
@@ -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,10 +224,12 @@ public class MtpDocumentsProvider extends DocumentsProvider {

    @Override
    public void onTrimMemory(int level) {
        synchronized (mDeviceToolkits) {
            for (final DeviceToolkit toolkit : mDeviceToolkits.values()) {
                toolkit.mDocumentLoader.clearCompletedTasks();
            }
        }
    }

    @Override
    public String createDocument(String parentDocumentId, String mimeType, String displayName)
@@ -254,21 +258,28 @@ public class MtpDocumentsProvider extends DocumentsProvider {
    }

    void openDevice(int deviceId) throws IOException {
        synchronized (mDeviceToolkits) {
            mMtpManager.openDevice(deviceId);
            mDeviceToolkits.put(deviceId, new DeviceToolkit(mMtpManager, mResolver, mDatabase));
        mRootScanner.scanNow();
        }
        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).
        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,12 +325,14 @@ public class MtpDocumentsProvider extends DocumentsProvider {
    }

    private DeviceToolkit getDeviceToolkit(int deviceId) throws FileNotFoundException {
        synchronized (mDeviceToolkits) {
            final DeviceToolkit toolkit = mDeviceToolkits.get(deviceId);
            if (toolkit == null) {
                throw new FileNotFoundException();
            }
            return toolkit;
        }
    }

    private PipeManager getPipeManager(Identifier identifier) throws FileNotFoundException {
        return getDeviceToolkit(identifier.mDeviceId).mPipeManager;
+5 −5
Original line number Diff line number Diff line
@@ -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();
            }
+74 −55
Original line number Diff line number Diff line
@@ -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) {
    /**
     * Stops background thread and wait for its termination.
     * @throws InterruptedException
     */
    synchronized void pause() throws InterruptedException {
        if (mExecutor == null) {
            return;
        }
            notify();
        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) {
            int pollingCount = 0;
            while (!Thread.interrupted()) {
                final int[] deviceIds = mManager.getOpenedDeviceIds();
                if (deviceIds.length == 0) {
                        break;
                    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) {
                            if (mDatabase.putRootDocuments(deviceId, mResources, roots)) {
                                changed = true;
                            }
                        } finally {
                            if (mDatabase.stopAddingRootDocuments(deviceId)) {
                                changed = true;
                            }
                        }
                    } 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, exp.getMessage());
                            continue;
                        } finally {
                            changed = mDatabase.stopAddingRootDocuments(deviceId) || changed;
                        Log.e(MtpDocumentsProvider.TAG, exception.getMessage());
                    }
                }
                if (changed) {
                    notifyChange();
                }
                    mPollingCount++;
                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.
                        RootScanner.this.wait(
                                mPollingCount > SHORT_POLLING_TIMES ?
                    Thread.sleep(pollingCount > SHORT_POLLING_TIMES ?
                        LONG_POLLING_INTERVAL : SHORT_POLLING_INTERVAL);
                    } catch (InterruptedException exception) {
                        break;
                    }
                } catch (InterruptedException exp) {
                    // The while condition handles the interrupted flag.
                    continue;
                }

                mBackgroundThread = null;
            }
        }
    }
+1 −5
Original line number Diff line number Diff line
@@ -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 {