From e56740dc081cd3024fa2a0c72689febd49ee1c9f Mon Sep 17 00:00:00 2001 From: Daichi Hirono Date: Thu, 2 Feb 2017 13:56:45 +0900 Subject: [PATCH] Use a single thread to handle multiple AppFuse mounts. 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 --- .../android/server/StorageManagerService.java | 144 ++++++--------- .../android/server/storage/AppFuseBridge.java | 165 +++++++++++++----- ...m_android_server_storage_AppFuseBridge.cpp | 110 ++++++++++-- 3 files changed, 266 insertions(+), 153 deletions(-) diff --git a/services/core/java/com/android/server/StorageManagerService.java b/services/core/java/com/android/server/StorageManagerService.java index 8e6310fdacbb8..891a13bc68789 100644 --- a/services/core/java/com/android/server/StorageManagerService.java +++ b/services/core/java/com/android/server/StorageManagerService.java @@ -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 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 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; - } + class AppFuseMountScope extends AppFuseBridge.MountScope { + public AppFuseMountScope(int uid, int pid, int mountId) + throws NativeDaemonConnectorException { + super(uid, pid, mountId, mountAppFuse(uid, mountId)); } @Override - public void close() throws NativeDaemonConnectorException { - try { - IoUtils.closeQuietly(mDeviceFd); - mConnector.execute( - "appfuse", "unmount", mUid, Process.myPid(), mName); - } finally { - synchronized (mLock) { - mAppFusePids.delete(mName); - } - } - } - - @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 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 channel = new ArrayBlockingQueue<>(1); - final ParcelFileDescriptor[] fds = ParcelFileDescriptor.createSocketPair(); - try (CloseableHolder 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()); } } diff --git a/services/core/java/com/android/server/storage/AppFuseBridge.java b/services/core/java/com/android/server/storage/AppFuseBridge.java index 5a1f473b5ea28..904d9159e5786 100644 --- a/services/core/java/com/android/server/storage/AppFuseBridge.java +++ b/services/core/java/com/android/server/storage/AppFuseBridge.java @@ -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 mChannel; + @GuardedBy("this") + private final SparseArray 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 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(); + } + + public ParcelFileDescriptor addBridge(MountScope mountScope) + throws BridgeException { + try { + 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 { + IoUtils.closeQuietly(mountScope); + } } @Override public void run() { - try { - // deviceFd and proxyFd must be closed in native_start_loop. - native_start_loop( - mMountScope.getDeviceFileDescriptor().detachFd(), - mProxyFd.detachFd()); - } finally { - close(); + 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); - } - - private static File getMountPoint(int uid, int mountId) { - return new File(String.format(APPFUSE_MOUNT_NAME_TEMPLATE, uid, mountId)); - } - - @Override - public void close() { - IoUtils.closeQuietly(mMountScope); - IoUtils.closeQuietly(mProxyFd); - // Invoke countDown here in case where close is invoked before mount. - mChannel.offer(false); + return ParcelFileDescriptor.open(new File(scope.mountPoint, String.valueOf(fileId)), mode); } // Used by com_android_server_storage_AppFuse.cpp. - private void onMount() { - mChannel.offer(true); + synchronized private void onMount(int mountId) { + final MountScope scope = mScopes.get(mountId); + if (scope != null) { + scope.setMountResultLocked(true); + } } - public static interface IMountScope extends AutoCloseable { - ParcelFileDescriptor getDeviceFileDescriptor(); + // 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); + } } - private native boolean native_start_loop(int deviceFd, int proxyFd); + 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 class BridgeException extends Exception { + public BridgeException(String message) { + super(message); + } + } + + 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); } diff --git a/services/core/jni/com_android_server_storage_AppFuseBridge.cpp b/services/core/jni/com_android_server_storage_AppFuseBridge.cpp index 2f20ecd1954df..c8f842dde7ae5 100644 --- a/services/core/jni/com_android_server_storage_AppFuseBridge.cpp +++ b/services/core/jni/com_android_server_storage_AppFuseBridge.cpp @@ -19,16 +19,19 @@ #include #include +#include #include #include +#include #include 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(new fuse::FuseBridgeLoop()); +} + +void com_android_server_storage_AppFuseBridge_delete(JNIEnv* env, jobject self, jlong java_loop) { + fuse::FuseBridgeLoop* const loop = reinterpret_cast(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(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(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(com_android_server_storage_AppFuseBridge_new) + }, + { + "native_delete", + "(J)V", + reinterpret_cast(com_android_server_storage_AppFuseBridge_delete) + }, { "native_start_loop", - "(II)Z", - (void *) com_android_server_storage_AppFuseBridge_start_loop + "(J)V", + reinterpret_cast(com_android_server_storage_AppFuseBridge_start_loop) + }, + { + "native_add_bridge", + "(JII)I", + reinterpret_cast(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