OSDN Git Service

Use a single thread to handle multiple AppFuse mounts.
authorDaichi Hirono <hirono@google.com>
Thu, 2 Feb 2017 04:56:45 +0000 (13:56 +0900)
committerDaichi Hirono <hirono@google.com>
Fri, 24 Mar 2017 00:36:48 +0000 (09:36 +0900)
Previously StorageManagerService launched a thread per AppFuse mount
point. The CL changes the behavior so that it just launches a single
thread to handle multiple AppFuse mounts.

Bug: 34903085
Test: CTS and manually testing
Change-Id: I2d9e7232834b0c8eff633b85110a3b38534171f7

services/core/java/com/android/server/StorageManagerService.java
services/core/java/com/android/server/storage/AppFuseBridge.java
services/core/jni/com_android_server_storage_AppFuseBridge.cpp

index 8e6310f..891a13b 100644 (file)
@@ -97,6 +97,7 @@ import android.util.Xml;
 import com.android.internal.annotations.GuardedBy;
 import com.android.internal.app.IMediaContainerService;
 import com.android.internal.os.AppFuseMount;
+import com.android.internal.os.FuseAppLoop;
 import com.android.internal.os.SomeArgs;
 import com.android.internal.os.Zygote;
 import com.android.internal.util.ArrayUtils;
@@ -350,7 +351,7 @@ class StorageManagerService extends IStorageManager.Stub
     private int mNextAppFuseName = 0;
 
     @GuardedBy("mAppFuseLock")
-    private final SparseArray<Integer> mAppFusePids = new SparseArray<>();
+    private AppFuseBridge mAppFuseBridge = null;
 
     private VolumeInfo findVolumeByIdOrThrow(String id) {
         synchronized (mLock) {
@@ -2991,124 +2992,79 @@ class StorageManagerService extends IStorageManager.Stub
         }
     }
 
-    class CloseableHolder<T extends AutoCloseable> implements AutoCloseable {
-        @Nullable T mCloseable;
-
-        CloseableHolder(T closeable) {
-            mCloseable = closeable;
-        }
-
-        @Nullable T get() {
-            return mCloseable;
-        }
-
-        @Nullable T release() {
-            final T result = mCloseable;
-            mCloseable = null;
-            return result;
-        }
-
-        @Override
-        public void close() {
-            if (mCloseable != null) {
-                IoUtils.closeQuietly(mCloseable);
-            }
+    private ParcelFileDescriptor mountAppFuse(int uid, int mountId)
+            throws NativeDaemonConnectorException {
+        final NativeDaemonEvent event = StorageManagerService.this.mConnector.execute(
+                "appfuse", "mount", uid, Process.myPid(), mountId);
+        if (event.getFileDescriptors() == null ||
+            event.getFileDescriptors().length == 0) {
+            throw new NativeDaemonConnectorException("Cannot obtain device FD");
         }
+        return new ParcelFileDescriptor(event.getFileDescriptors()[0]);
     }
 
-    class AppFuseMountScope implements AppFuseBridge.IMountScope {
-        final int mUid;
-        final int mName;
-        final ParcelFileDescriptor mDeviceFd;
-
-        AppFuseMountScope(int uid, int pid, int name) throws NativeDaemonConnectorException {
-            final NativeDaemonEvent event = mConnector.execute(
-                    "appfuse", "mount", uid, Process.myPid(), name);
-            mUid = uid;
-            mName = name;
-            synchronized (mLock) {
-                mAppFusePids.put(name, pid);
-            }
-            if (event.getFileDescriptors() != null &&
-                    event.getFileDescriptors().length > 0) {
-                mDeviceFd = new ParcelFileDescriptor(event.getFileDescriptors()[0]);
-            } else {
-                mDeviceFd = null;
-            }
-        }
-
-        @Override
-        public void close() throws NativeDaemonConnectorException {
-            try {
-                IoUtils.closeQuietly(mDeviceFd);
-                mConnector.execute(
-                        "appfuse", "unmount", mUid, Process.myPid(), mName);
-            } finally {
-                synchronized (mLock) {
-                    mAppFusePids.delete(mName);
-                }
-            }
+    class AppFuseMountScope extends AppFuseBridge.MountScope {
+        public AppFuseMountScope(int uid, int pid, int mountId)
+                throws NativeDaemonConnectorException {
+            super(uid, pid, mountId, mountAppFuse(uid, mountId));
         }
 
         @Override
-        public ParcelFileDescriptor getDeviceFileDescriptor() {
-            return mDeviceFd;
+        public void close() throws Exception {
+            super.close();
+            mConnector.execute("appfuse", "unmount", uid, Process.myPid(), mountId);
         }
     }
 
     @Override
     public AppFuseMount mountProxyFileDescriptorBridge() throws RemoteException {
+        Slog.v(TAG, "mountProxyFileDescriptorBridge");
         final int uid = Binder.getCallingUid();
         final int pid = Binder.getCallingPid();
-        final int name;
-        synchronized (mAppFuseLock) {
-            name = mNextAppFuseName++;
-        }
-        try (CloseableHolder<AppFuseMountScope> mountScope =
-                new CloseableHolder<>(new AppFuseMountScope(uid, pid, name))) {
-            if (mountScope.get().getDeviceFileDescriptor() == null) {
-                throw new RemoteException("Failed to obtain device FD");
-            }
 
-            // Create communication channel.
-            final ArrayBlockingQueue<Boolean> channel = new ArrayBlockingQueue<>(1);
-            final ParcelFileDescriptor[] fds = ParcelFileDescriptor.createSocketPair();
-            try (CloseableHolder<ParcelFileDescriptor> remote = new CloseableHolder<>(fds[0])) {
-                new Thread(
-                        new AppFuseBridge(mountScope.release(), fds[1], channel),
-                        AppFuseBridge.TAG).start();
-                if (!channel.take()) {
-                    throw new RemoteException("Failed to init AppFuse mount point");
+        while (true) {
+            synchronized (mAppFuseLock) {
+                boolean newlyCreated = false;
+                if (mAppFuseBridge == null) {
+                    mAppFuseBridge = new AppFuseBridge();
+                    new Thread(mAppFuseBridge, AppFuseBridge.TAG).start();
+                    newlyCreated = true;
+                }
+                try {
+                    final int name = mNextAppFuseName++;
+                    try {
+                        return new AppFuseMount(
+                            name,
+                            mAppFuseBridge.addBridge(new AppFuseMountScope(uid, pid, name)));
+                    } catch (AppFuseBridge.BridgeException e) {
+                        if (newlyCreated) {
+                            // If newly created bridge fails, it's a real error.
+                            throw new RemoteException(e.getMessage());
+                        }
+                        // It seems the thread of mAppFuseBridge has already been terminated.
+                        mAppFuseBridge = null;
+                    }
+                } catch (NativeDaemonConnectorException e) {
+                    throw e.rethrowAsParcelableException();
                 }
-
-                return new AppFuseMount(name, remote.release());
             }
-        } catch (NativeDaemonConnectorException e){
-            throw e.rethrowAsParcelableException();
-        } catch (IOException | InterruptedException error) {
-            throw new RemoteException(error.getMessage());
         }
     }
 
     @Override
-    public ParcelFileDescriptor openProxyFileDescriptor(int mountId, int fileId, int mode) {
-        final int uid = Binder.getCallingUid();
+    public ParcelFileDescriptor openProxyFileDescriptor(int mountId, int fileId, int mode)
+            throws RemoteException {
+        Slog.v(TAG, "mountProxyFileDescriptorBridge");
         final int pid = Binder.getCallingPid();
         try {
             synchronized (mAppFuseLock) {
-                final int expectedPid = mAppFusePids.get(mountId, -1);
-                if (expectedPid == -1) {
-                    Slog.i(TAG, "The mount point has already been unmounted");
-                    return null;
-                }
-                if (expectedPid != pid) {
-                    throw new SecurityException("Mount point was not created by this process.");
+                if (mAppFuseBridge == null) {
+                    throw new RemoteException("Cannot find mount point");
                 }
+                return mAppFuseBridge.openFile(pid, mountId, fileId, mode);
             }
-            return AppFuseBridge.openFile(uid, mountId, fileId, mode);
-        } catch (FileNotFoundException error) {
-            Slog.e(TAG, "Failed to openProxyFileDescriptor", error);
-            return null;
+        } catch (FileNotFoundException | SecurityException | InterruptedException error) {
+            throw new RemoteException(error.getMessage());
         }
     }
 
index 5a1f473..904d915 100644 (file)
@@ -19,18 +19,20 @@ package com.android.server.storage;
 import android.os.ParcelFileDescriptor;
 import android.system.ErrnoException;
 import android.system.Os;
+import android.util.SparseArray;
+import com.android.internal.annotations.GuardedBy;
 import com.android.internal.util.Preconditions;
 import libcore.io.IoUtils;
 import java.io.File;
 import java.io.FileNotFoundException;
-import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.CountDownLatch;
 
 /**
  * Runnable that delegates FUSE command from the kernel to application.
  * run() blocks until all opened files on the FUSE mount point are closed. So this should be run in
  * a separated thread.
  */
-public class AppFuseBridge implements Runnable, AutoCloseable {
+public class AppFuseBridge implements Runnable {
     public static final String TAG = "AppFuseBridge";
 
     /**
@@ -41,71 +43,138 @@ public class AppFuseBridge implements Runnable, AutoCloseable {
      */
     private static final String APPFUSE_MOUNT_NAME_TEMPLATE = "/mnt/appfuse/%d_%d";
 
-    private final IMountScope mMountScope;
-    private final ParcelFileDescriptor mProxyFd;
-    private final BlockingQueue<Boolean> mChannel;
+    @GuardedBy("this")
+    private final SparseArray<MountScope> mScopes = new SparseArray<>();
 
-    /**
-     * @param mountScope Listener to unmount mount point.
-     * @param proxyFd FD of socket pair. Ownership of FD is taken by AppFuseBridge.
-     * @param channel Channel that the runnable send mount result to.
-     */
-    public AppFuseBridge(
-            IMountScope mountScope, ParcelFileDescriptor proxyFd, BlockingQueue<Boolean> channel) {
-        Preconditions.checkNotNull(mountScope);
-        Preconditions.checkNotNull(proxyFd);
-        Preconditions.checkNotNull(channel);
-        mMountScope = mountScope;
-        mProxyFd = proxyFd;
-        mChannel = channel;
+    @GuardedBy("this")
+    private long mNativeLoop;
+
+    public AppFuseBridge() {
+        mNativeLoop = native_new();
     }
 
-    @Override
-    public void run() {
+    public ParcelFileDescriptor addBridge(MountScope mountScope)
+            throws BridgeException {
         try {
-            // deviceFd and proxyFd must be closed in native_start_loop.
-            native_start_loop(
-                    mMountScope.getDeviceFileDescriptor().detachFd(),
-                    mProxyFd.detachFd());
+            synchronized (this) {
+                Preconditions.checkArgument(mScopes.indexOfKey(mountScope.mountId) < 0);
+                if (mNativeLoop == 0) {
+                    throw new BridgeException("The thread has already been terminated");
+                }
+                final int fd = native_add_bridge(
+                        mNativeLoop, mountScope.mountId, mountScope.deviceFd.detachFd());
+                if (fd == -1) {
+                    throw new BridgeException("Failed to invoke native_add_bridge");
+                }
+                final ParcelFileDescriptor result = ParcelFileDescriptor.adoptFd(fd);
+                mScopes.put(mountScope.mountId, mountScope);
+                mountScope = null;
+                return result;
+            }
         } finally {
-            close();
+            IoUtils.closeQuietly(mountScope);
+        }
+    }
+
+    @Override
+    public void run() {
+        native_start_loop(mNativeLoop);
+        synchronized (this) {
+            native_delete(mNativeLoop);
+            mNativeLoop = 0;
         }
     }
 
-    public static ParcelFileDescriptor openFile(int uid, int mountId, int fileId, int mode)
-            throws FileNotFoundException {
-        final File mountPoint = getMountPoint(uid, mountId);
+    public ParcelFileDescriptor openFile(int pid, int mountId, int fileId, int mode)
+            throws FileNotFoundException, SecurityException, InterruptedException {
+        final MountScope scope;
+        synchronized (this) {
+            scope = mScopes.get(mountId);
+            if (scope == null) {
+                throw new FileNotFoundException("Cannot find mount point");
+            }
+        }
+        if (scope.pid != pid) {
+            throw new SecurityException("PID does not match");
+        }
+        final boolean result = scope.waitForMount();
+        if (result == false) {
+            throw new FileNotFoundException("Mount failed");
+        }
         try {
-            if (Os.stat(mountPoint.getPath()).st_ino != 1) {
+            if (Os.stat(scope.mountPoint.getPath()).st_ino != 1) {
                 throw new FileNotFoundException("Could not find bridge mount point.");
             }
         } catch (ErrnoException e) {
             throw new FileNotFoundException(
-                    "Failed to stat mount point: " + mountPoint.getParent());
+                    "Failed to stat mount point: " + scope.mountPoint.getParent());
         }
-        return ParcelFileDescriptor.open(new File(mountPoint, String.valueOf(fileId)), mode);
+        return ParcelFileDescriptor.open(new File(scope.mountPoint, String.valueOf(fileId)), mode);
     }
 
-    private static File getMountPoint(int uid, int mountId) {
-        return new File(String.format(APPFUSE_MOUNT_NAME_TEMPLATE,  uid, mountId));
+    // Used by com_android_server_storage_AppFuse.cpp.
+    synchronized private void onMount(int mountId) {
+        final MountScope scope = mScopes.get(mountId);
+        if (scope != null) {
+            scope.setMountResultLocked(true);
+        }
     }
 
-    @Override
-    public void close() {
-        IoUtils.closeQuietly(mMountScope);
-        IoUtils.closeQuietly(mProxyFd);
-        // Invoke countDown here in case where close is invoked before mount.
-        mChannel.offer(false);
+    // Used by com_android_server_storage_AppFuse.cpp.
+    synchronized private void onClosed(int mountId) {
+        final MountScope scope = mScopes.get(mountId);
+        if (scope != null) {
+            scope.setMountResultLocked(false);
+            IoUtils.closeQuietly(scope);
+            mScopes.remove(mountId);
+        }
     }
 
-    // Used by com_android_server_storage_AppFuse.cpp.
-    private void onMount() {
-        mChannel.offer(true);
+    public static class MountScope implements AutoCloseable {
+        public final int uid;
+        public final int pid;
+        public final int mountId;
+        public final ParcelFileDescriptor deviceFd;
+        public final File mountPoint;
+        private final CountDownLatch mMounted = new CountDownLatch(1);
+        private boolean mMountResult = false;
+
+        public MountScope(int uid, int pid, int mountId, ParcelFileDescriptor deviceFd) {
+            this.uid = uid;
+            this.pid = pid;
+            this.mountId = mountId;
+            this.deviceFd = deviceFd;
+            this.mountPoint = new File(String.format(APPFUSE_MOUNT_NAME_TEMPLATE,  uid, mountId));
+        }
+
+        @GuardedBy("AppFuseBridge.this")
+        void setMountResultLocked(boolean result) {
+            if (mMounted.getCount() == 0) {
+                return;
+            }
+            mMountResult = result;
+            mMounted.countDown();
+        }
+
+        boolean waitForMount() throws InterruptedException {
+            mMounted.await();
+            return mMountResult;
+        }
+
+        @Override
+        public void close() throws Exception {
+            deviceFd.close();
+        }
     }
 
-    public static interface IMountScope extends AutoCloseable {
-        ParcelFileDescriptor getDeviceFileDescriptor();
+    public static class BridgeException extends Exception {
+        public BridgeException(String message) {
+            super(message);
+        }
     }
 
-    private native boolean native_start_loop(int deviceFd, int proxyFd);
+    private native long native_new();
+    private native void native_delete(long loop);
+    private native void native_start_loop(long loop);
+    private native int native_add_bridge(long loop, int mountId, int deviceId);
 }
index 2f20ecd..c8f842d 100644 (file)
 
 #include <android_runtime/Log.h>
 #include <android-base/logging.h>
+#include <android-base/unique_fd.h>
 #include <core_jni_helpers.h>
 #include <libappfuse/FuseBridgeLoop.h>
+#include <libappfuse/FuseBuffer.h>
 #include <nativehelper/JNIHelp.h>
 
 namespace android {
 namespace {
 
 constexpr const char* CLASS_NAME = "com/android/server/storage/AppFuseBridge";
-static jclass appFuseClass;
-static jmethodID appFuseOnMount;
+static jclass gAppFuseClass;
+static jmethodID gAppFuseOnMount;
+static jmethodID gAppFuseOnClosed;
 
 class Callback : public fuse::FuseBridgeLoopCallback {
     JNIEnv* mEnv;
@@ -36,8 +39,16 @@ class Callback : public fuse::FuseBridgeLoopCallback {
 
 public:
     Callback(JNIEnv* env, jobject self) : mEnv(env), mSelf(self) {}
-    void OnMount() override {
-        mEnv->CallVoidMethod(mSelf, appFuseOnMount);
+    void OnMount(int mount_id) override {
+        mEnv->CallVoidMethod(mSelf, gAppFuseOnMount, mount_id);
+        if (mEnv->ExceptionCheck()) {
+            LOGE_EX(mEnv, nullptr);
+            mEnv->ExceptionClear();
+        }
+    }
+
+    void OnClosed(int mount_id) override {
+        mEnv->CallVoidMethod(mSelf, gAppFuseOnClosed, mount_id);
         if (mEnv->ExceptionCheck()) {
             LOGE_EX(mEnv, nullptr);
             mEnv->ExceptionClear();
@@ -45,17 +56,93 @@ public:
     }
 };
 
-jboolean com_android_server_storage_AppFuseBridge_start_loop(
-        JNIEnv* env, jobject self, jint devJavaFd, jint proxyJavaFd) {
+class MonitorScope final {
+public:
+    MonitorScope(JNIEnv* env, jobject obj) : mEnv(env), mObj(obj), mLocked(false) {
+        if (mEnv->MonitorEnter(obj) == JNI_OK) {
+            mLocked = true;
+        } else {
+            LOG(ERROR) << "Failed to enter monitor.";
+        }
+    }
+
+    ~MonitorScope() {
+        if (mLocked) {
+            if (mEnv->MonitorExit(mObj) != JNI_OK) {
+                LOG(ERROR) << "Failed to exit monitor.";
+            }
+        }
+    }
+
+    operator bool() {
+        return mLocked;
+    }
+
+private:
+    // Lifetime of |MonitorScope| must be shorter than the reference of mObj.
+    JNIEnv* mEnv;
+    jobject mObj;
+    bool mLocked;
+
+    DISALLOW_COPY_AND_ASSIGN(MonitorScope);
+};
+
+jlong com_android_server_storage_AppFuseBridge_new(JNIEnv* env, jobject self) {
+    return reinterpret_cast<jlong>(new fuse::FuseBridgeLoop());
+}
+
+void com_android_server_storage_AppFuseBridge_delete(JNIEnv* env, jobject self, jlong java_loop) {
+    fuse::FuseBridgeLoop* const loop = reinterpret_cast<fuse::FuseBridgeLoop*>(java_loop);
+    CHECK(loop);
+    delete loop;
+}
+
+void com_android_server_storage_AppFuseBridge_start_loop(
+        JNIEnv* env, jobject self, jlong java_loop) {
+    fuse::FuseBridgeLoop* const loop = reinterpret_cast<fuse::FuseBridgeLoop*>(java_loop);
+    CHECK(loop);
     Callback callback(env, self);
-    return fuse::StartFuseBridgeLoop(devJavaFd, proxyJavaFd, &callback);
+    loop->Start(&callback);
+}
+
+jint com_android_server_storage_AppFuseBridge_add_bridge(
+        JNIEnv* env, jobject self, jlong java_loop, jint mountId, jint javaDevFd) {
+    base::unique_fd devFd(javaDevFd);
+    fuse::FuseBridgeLoop* const loop = reinterpret_cast<fuse::FuseBridgeLoop*>(java_loop);
+    CHECK(loop);
+
+    base::unique_fd proxyFd[2];
+    if (!fuse::SetupMessageSockets(&proxyFd)) {
+        return -1;
+    }
+
+    if (!loop->AddBridge(mountId, std::move(devFd), std::move(proxyFd[0]))) {
+        return -1;
+    }
+
+    return proxyFd[1].release();
 }
 
 const JNINativeMethod methods[] = {
     {
+        "native_new",
+        "()J",
+        reinterpret_cast<void*>(com_android_server_storage_AppFuseBridge_new)
+    },
+    {
+        "native_delete",
+        "(J)V",
+        reinterpret_cast<void*>(com_android_server_storage_AppFuseBridge_delete)
+    },
+    {
         "native_start_loop",
-        "(II)Z",
-        (void *) com_android_server_storage_AppFuseBridge_start_loop
+        "(J)V",
+        reinterpret_cast<void*>(com_android_server_storage_AppFuseBridge_start_loop)
+    },
+    {
+        "native_add_bridge",
+        "(JII)I",
+        reinterpret_cast<void*>(com_android_server_storage_AppFuseBridge_add_bridge)
     }
 };
 
@@ -64,8 +151,9 @@ const JNINativeMethod methods[] = {
 void register_android_server_storage_AppFuse(JNIEnv* env) {
     CHECK(env != nullptr);
 
-    appFuseClass = MakeGlobalRefOrDie(env, FindClassOrDie(env, CLASS_NAME));
-    appFuseOnMount = GetMethodIDOrDie(env, appFuseClass, "onMount", "()V");
+    gAppFuseClass = MakeGlobalRefOrDie(env, FindClassOrDie(env, CLASS_NAME));
+    gAppFuseOnMount = GetMethodIDOrDie(env, gAppFuseClass, "onMount", "(I)V");
+    gAppFuseOnClosed = GetMethodIDOrDie(env, gAppFuseClass, "onClosed", "(I)V");
     RegisterMethodsOrDie(env, CLASS_NAME, methods, NELEM(methods));
 }
 }  // namespace android