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

Commit 76be46f4 authored by Daichi Hirono's avatar Daichi Hirono
Browse files

Fix crash when deleting multiple files. 

When deleting files, MtpDocumentsProvider clears LoadingTask in
DocumentsLoader to update directory contents list. Previously it can
clear ongoing task, and it skips calling Mapper#stopAddingDocuments.
Since Mapper#startAddingDocuments and Mapper#stopAddingDocuments must be
called 1 to 1, it causes precondition check failure at the next call of
Mapper#startAddingDocuments.

Change-Id: I23e2b117da826297e45404be4db4cc29f96e5510
Fix: 28076320
parent 75aee0a6

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

+34 −28
Original line number Diff line number Diff line
@@ -33,7 +33,6 @@ import com.android.internal.util.Preconditions; 
import java.io.FileNotFoundException;

import java.io.IOException;

import java.util.ArrayList;

import java.util.Arrays;

import java.util.Date;

import java.util.LinkedList;
@@ -118,10 +117,11 @@ class DocumentLoader implements AutoCloseable { 
    synchronized @Nullable LoaderTask getNextTaskOrReleaseBackgroundThread() {

        Preconditions.checkState(mBackgroundThread != null);



        final LoaderTask task = mTaskList.findRunningTask();

        if (task != null) {

        for (final LoaderTask task : mTaskList) {

            if (task.getState() == LoaderTask.STATE_LOADING) {

                return task;

            }

        }



        final Identifier identifier = mDatabase.getUnmappedDocumentsParent(mDevice.deviceId);

        if (identifier != null) {
@@ -161,8 +161,21 @@ class DocumentLoader implements AutoCloseable { 
        mTaskList.clearCompletedTasks();

    }



    synchronized void clearTask(Identifier parentIdentifier) {

        mTaskList.clearTask(parentIdentifier);

    /**

     * Cancels the task for |parentIdentifier|.

     *

     * Task is removed from the cached list and it will create new task when |parentIdentifier|'s

     * children are queried next.

     */

    void cancelTask(Identifier parentIdentifier) {

        final LoaderTask task;

        synchronized (this) {

            task = mTaskList.findTask(parentIdentifier);

        }

        if (task != null) {

            task.cancel();

            mTaskList.remove(task);

        }

    }



    /**
@@ -205,14 +218,6 @@ class DocumentLoader implements AutoCloseable { 
            return null;

        }



        LoaderTask findRunningTask() {

            for (int i = 0; i < size(); i++) {

                if (get(i).getState() == LoaderTask.STATE_LOADING)

                    return get(i);

            }

            return null;

        }



        void clearCompletedTasks() {

            int i = 0;

            while (i < size()) {
@@ -223,17 +228,6 @@ class DocumentLoader implements AutoCloseable { 
                }

            }

        }



        void clearTask(Identifier parentIdentifier) {

            for (int i = 0; i < size(); i++) {

                final LoaderTask task = get(i);

                if (task.mIdentifier.mDeviceId == parentIdentifier.mDeviceId &&

                        task.mIdentifier.mObjectHandle == parentIdentifier.mObjectHandle) {

                    remove(i);

                    return;

                }

            }

        }

    }



    /**
@@ -245,6 +239,7 @@ class DocumentLoader implements AutoCloseable { 
        static final int STATE_LOADING = 1;

        static final int STATE_COMPLETED = 2;

        static final int STATE_ERROR = 3;

        static final int STATE_CANCELLED = 4;



        final MtpManager mManager;

        final MtpDatabase mDatabase;
@@ -272,6 +267,7 @@ class DocumentLoader implements AutoCloseable { 


        synchronized void loadObjectHandles() {

            assert mState == STATE_START;

            mPosition = 0;

            int parentHandle = mIdentifier.mObjectHandle;

            // Need to pass the special value MtpManager.OBJECT_HANDLE_ROOT_CHILDREN to

            // getObjectHandles if we would like to obtain children under the root.
@@ -303,12 +299,10 @@ class DocumentLoader implements AutoCloseable { 
                case STATE_ERROR:

                    throw mError;

            }



            final Cursor cursor =

                    mDatabase.queryChildDocuments(columnNames, mIdentifier.mDocumentId);

            cursor.setExtras(extras);

            cursor.setNotificationUri(resolver, createUri());

            cursor.respond(extras);



            return cursor;

        }
@@ -374,6 +368,10 @@ class DocumentLoader implements AutoCloseable { 
                }

            }

            synchronized (this) {

                // Check if the task is cancelled or not.

                if (mState != STATE_LOADING) {

                    return;

                }

                try {

                    mDatabase.getMapper().putChildDocuments(

                            mIdentifier.mDeviceId,
@@ -402,6 +400,14 @@ class DocumentLoader implements AutoCloseable { 
            }

        }



        /**

         * Cancels the task.

         */

        synchronized void cancel() {

            mDatabase.getMapper().cancelAddingDocuments(mIdentifier.mDocumentId);

            mState = STATE_CANCELLED;

        }



        /**

         * Returns a state of the task.

         */

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

+35 −0
Original line number Diff line number Diff line
@@ -362,6 +362,41 @@ class Mapper { 
        }

    }



    /**

     * Cancels adding documents.

     * @param parentId

     */

    void cancelAddingDocuments(@Nullable String parentId) {

        final String selection;

        final String[] args;

        if (parentId != null) {

            selection = COLUMN_PARENT_DOCUMENT_ID + " = ?";

            args = strings(parentId);

        } else {

            selection = COLUMN_PARENT_DOCUMENT_ID + " IS NULL";

            args = EMPTY_ARGS;

        }



        final SQLiteDatabase database = mDatabase.getSQLiteDatabase();

        database.beginTransaction();

        try {

            if (!mInMappingIds.contains(parentId)) {

                return;

            }

            mInMappingIds.remove(parentId);

            final ContentValues values = new ContentValues();

            values.put(COLUMN_ROW_STATE, ROW_STATE_VALID);

            mDatabase.getSQLiteDatabase().update(

                    TABLE_DOCUMENTS,

                    values,

                    selection + " AND " + COLUMN_ROW_STATE + " = ?",

                    DatabaseUtils.appendSelectionArgs(args, strings(ROW_STATE_INVALIDATED)));

            database.setTransactionSuccessful();

        } finally {

            database.endTransaction();

        }

    }



    /**

     * Queries candidate for each mappingKey, and returns the first cursor that includes a

     * candidate.

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

+2 −2
Original line number Diff line number Diff line
@@ -308,7 +308,7 @@ public class MtpDocumentsProvider extends DocumentsProvider { 
            final Identifier parentIdentifier = mDatabase.getParentIdentifier(documentId);

            mMtpManager.deleteDocument(identifier.mDeviceId, identifier.mObjectHandle);

            mDatabase.deleteDocument(documentId);

            getDocumentLoader(parentIdentifier).clearTask(parentIdentifier);

            getDocumentLoader(parentIdentifier).cancelTask(parentIdentifier);

            notifyChildDocumentsChange(parentIdentifier.mDocumentId);

            if (parentIdentifier.mDocumentType == MtpDatabaseConstants.DOCUMENT_TYPE_STORAGE) {

                // If the parent is storage, the object might be appeared as child of device because
@@ -402,7 +402,7 @@ public class MtpDocumentsProvider extends DocumentsProvider { 
            final String documentId = mDatabase.putNewDocument(

                    parentId.mDeviceId, parentDocumentId, record.operationsSupported,

                    infoWithHandle, 0l);

            getDocumentLoader(parentId).clearTask(parentId);

            getDocumentLoader(parentId).cancelTask(parentId);

            notifyChildDocumentsChange(parentDocumentId);

            return documentId;

        } catch (FileNotFoundException | RuntimeException error) {

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

+29 −0
Original line number Diff line number Diff line
@@ -21,6 +21,7 @@ import android.database.Cursor; 
import android.mtp.MtpObjectInfo;

import android.net.Uri;

import android.provider.DocumentsContract;

import android.provider.DocumentsContract.Document;

import android.test.AndroidTestCase;

import android.test.suitebuilder.annotation.MediumTest;
@@ -28,6 +29,7 @@ import java.io.IOException; 
import java.util.HashMap;

import java.util.Map;

import java.util.concurrent.CountDownLatch;

import java.util.concurrent.TimeoutException;



@MediumTest

public class DocumentLoaderTest extends AndroidTestCase {
@@ -141,6 +143,33 @@ public class DocumentLoaderTest extends AndroidTestCase { 
        }

    }



    public void testCancelTask() throws IOException, InterruptedException {

        setUpDocument(mManager,

                DocumentLoader.NUM_INITIAL_ENTRIES + DocumentLoader.NUM_LOADING_ENTRIES + 1);



        // Block the first iteration in the background thread.

        mManager.blockDocument(

                0, DocumentLoader.NUM_INITIAL_ENTRIES + 1);

        setUpLoader();

        try (final Cursor cursor = mLoader.queryChildDocuments(

                MtpDocumentsProvider.DEFAULT_DOCUMENT_PROJECTION, mParentIdentifier)) {

            assertTrue(cursor.getExtras().getBoolean(DocumentsContract.EXTRA_LOADING));

        }

        Thread.sleep(DocumentLoader.NOTIFY_PERIOD_MS);



        // Clear task while the first iteration is being blocked.

        mManager.unblockDocument(

                0, DocumentLoader.NUM_INITIAL_ENTRIES + 1);

        mLoader.cancelTask(mParentIdentifier);



        Thread.sleep(DocumentLoader.NOTIFY_PERIOD_MS * 2);



        // Check if it's OK to query invalidated task.

        try (final Cursor cursor = mLoader.queryChildDocuments(

                MtpDocumentsProvider.DEFAULT_DOCUMENT_PROJECTION, mParentIdentifier)) {

        }

    }



    private void setUpLoader() {

        mLoader = new DocumentLoader(

                new MtpDeviceRecord(