Fix race in AppFuseTest.

Previously IllegalStateException is thrown in app fuse main loop, if the
loop thread starts just after closing device FD.

BUG=None

Change-Id: Ia5232857d29f9f324446aa38acf3c062f359d406

diff --git a/packages/MtpDocumentsProvider/jni/com_android_mtp_AppFuse.cpp b/packages/MtpDocumentsProvider/jni/com_android_mtp_AppFuse.cpp
index cde28fc..ba75cc2 100644 (file)
--- a/packages/MtpDocumentsProvider/jni/com_android_mtp_AppFuse.cpp
+++ b/packages/MtpDocumentsProvider/jni/com_android_mtp_AppFuse.cpp
@@ -379,7 +379,7 @@ private:
 
 jboolean com_android_mtp_AppFuse_start_app_fuse_loop(
         JNIEnv* env, jobject self, jint jfd) {
-    ScopedFd fd(dup(static_cast<int>(jfd)));
+    ScopedFd fd(static_cast<int>(jfd));
     AppFuse appfuse(env, self);
 
     ALOGD("Start fuse loop.");

diff --git a/packages/MtpDocumentsProvider/src/com/android/mtp/AppFuse.java b/packages/MtpDocumentsProvider/src/com/android/mtp/AppFuse.java
index 01d1301..1300c47 100644 (file)
--- a/packages/MtpDocumentsProvider/src/com/android/mtp/AppFuse.java
+++ b/packages/MtpDocumentsProvider/src/com/android/mtp/AppFuse.java
@@ -21,6 +21,7 @@ import android.os.Process;
 import android.os.storage.StorageManager;
 import android.util.Log;
 import com.android.internal.annotations.VisibleForTesting;
+import com.android.internal.util.Preconditions;
 import com.android.mtp.annotations.UsedByNative;
 import java.io.File;
 import java.io.FileNotFoundException;
@@ -39,22 +40,18 @@ public class AppFuse {
 
     private final String mName;
     private final Callback mCallback;
-    private final Thread mMessageThread;
+    private Thread mMessageThread;
     private ParcelFileDescriptor mDeviceFd;
 
     AppFuse(String name, Callback callback) {
         mName = name;
         mCallback = callback;
-        mMessageThread = new Thread(new Runnable() {
-            @Override
-            public void run() {
-                native_start_app_fuse_loop(mDeviceFd.getFd());
-            }
-        });
     }
 
-    void mount(StorageManager storageManager) {
+    void mount(StorageManager storageManager) throws IOException {
+        Preconditions.checkState(mDeviceFd == null);
         mDeviceFd = storageManager.mountAppFuse(mName);
+        mMessageThread = new AppFuseMessageThread(mDeviceFd.dup().detachFd());
         mMessageThread.start();
     }
 
@@ -80,7 +77,6 @@ public class AppFuse {
                 ParcelFileDescriptor.MODE_READ_ONLY);
     }
 
-    @VisibleForTesting
     File getMountPoint() {
         return new File("/mnt/appfuse/" + Process.myUid() + "_" + mName);
     }
@@ -112,4 +108,22 @@ public class AppFuse {
     }
 
     private native boolean native_start_app_fuse_loop(int fd);
+
+    private class AppFuseMessageThread extends Thread {
+        /**
+         * File descriptor used by native loop.
+         * It's owned by native loop and does not need to close here.
+         */
+        private final int mRawFd;
+
+        AppFuseMessageThread(int fd) {
+            super("AppFuseMessageThread");
+            mRawFd = fd;
+        }
+
+        @Override
+        public void run() {
+            native_start_app_fuse_loop(mRawFd);
+        }
+    }
 }

diff --git a/packages/MtpDocumentsProvider/src/com/android/mtp/MtpDocumentsProvider.java b/packages/MtpDocumentsProvider/src/com/android/mtp/MtpDocumentsProvider.java
index afef3de..3381656 100644 (file)
--- a/packages/MtpDocumentsProvider/src/com/android/mtp/MtpDocumentsProvider.java
+++ b/packages/MtpDocumentsProvider/src/com/android/mtp/MtpDocumentsProvider.java
@@ -92,7 +92,12 @@ public class MtpDocumentsProvider extends DocumentsProvider {
         mRootScanner = new RootScanner(mResolver, mResources, mMtpManager, mDatabase);
         mAppFuse = new AppFuse(TAG, new AppFuseCallback());
         // TODO: Mount AppFuse on demands.
-        mAppFuse.mount(getContext().getSystemService(StorageManager.class));
+        try {
+            mAppFuse.mount(getContext().getSystemService(StorageManager.class));
+        } catch (IOException e) {
+            Log.e(TAG, "Failed to start app fuse.", e);
+            return false;
+        }
         resume();
         return true;
     }

diff --git a/packages/MtpDocumentsProvider/tests/src/com/android/mtp/AppFuseTest.java b/packages/MtpDocumentsProvider/tests/src/com/android/mtp/AppFuseTest.java
index 5e1a796..6354880 100644 (file)
--- a/packages/MtpDocumentsProvider/tests/src/com/android/mtp/AppFuseTest.java
+++ b/packages/MtpDocumentsProvider/tests/src/com/android/mtp/AppFuseTest.java
@@ -30,7 +30,7 @@ import java.util.Arrays;
 
 @MediumTest
 public class AppFuseTest extends AndroidTestCase {
-    public void testMount() throws ErrnoException {
+    public void testMount() throws ErrnoException, IOException {
         final StorageManager storageManager = getContext().getSystemService(StorageManager.class);
         final AppFuse appFuse = new AppFuse("test", new TestCallback());
         appFuse.mount(storageManager);
@@ -61,7 +61,7 @@ public class AppFuseTest extends AndroidTestCase {
         appFuse.close();
     }
 
-    public void testOpenFile_error() {
+    public void testOpenFile_fileNotFound() throws IOException {
         final StorageManager storageManager = getContext().getSystemService(StorageManager.class);
         final int INODE = 10;
         final AppFuse appFuse = new AppFuse("test", new TestCallback());