Stop FUSE sessions properly
Now when we call ExternalStorageService#onEndSession
we block to ensure the FUSE loop has exited. This allows
us avoid racing mount and unmount events.
Typical usage: Vold#unmount -> ExternalStorageService#waitForExit
We don't use fuse_session_exit provided by libfuse because it needs
to be called from a signal handler or a FUSE operation handler.
Those are the only ways the loop can break out of a blocking read(2)
to accept the set 'exit' flag.
Test: atest AdoptableHostTest
Bug: 142109745
Change-Id: Ib0d525792eecd813ed4c7c30db5cb9d3053668c0
diff --git a/jni/FuseDaemon.cpp b/jni/FuseDaemon.cpp
index a9c9506..16976d3 100644
--- a/jni/FuseDaemon.cpp
+++ b/jni/FuseDaemon.cpp
@@ -1455,8 +1455,6 @@
FuseDaemon::FuseDaemon(JNIEnv* env, jobject mediaProvider) : mp(env, mediaProvider) {}
-void FuseDaemon::Stop() {}
-
void FuseDaemon::Start(const int fd, const std::string& path) {
struct fuse_args args;
struct fuse_cmdline_opts opts;
@@ -1513,22 +1511,30 @@
fuse_to_android_loglevel.insert({FUSE_LOG_DEBUG, ANDROID_LOG_VERBOSE});
fuse_set_log_func(fuse_logger);
- LOG(INFO) << "Starting fuse...";
struct fuse_session
* se = fuse_session_new(&args, &ops, sizeof(ops), &fuse_default);
+ if (!se) {
+ PLOG(ERROR) << "Failed to create session ";
+ return;
+ }
se->fd = fd;
se->mountpoint = strdup(path.c_str());
// Single thread. Useful for debugging
// fuse_session_loop(se);
// Multi-threaded
+ LOG(INFO) << "Starting fuse...";
fuse_session_loop_mt(se, &config);
+ LOG(INFO) << "Ending fuse...";
if (munmap(fuse_default.zero_addr, MAX_READ_SIZE)) {
PLOG(ERROR) << "munmap failed!";
}
- LOG(INFO) << "Ending fuse...";
+ fuse_opt_free_args(&args);
+ fuse_session_destroy(se);
+ LOG(INFO) << "Ended fuse";
+ return;
}
} //namespace fuse
} // namespace mediaprovider
diff --git a/jni/FuseDaemon.h b/jni/FuseDaemon.h
index 89134a2..d3ff966 100644
--- a/jni/FuseDaemon.h
+++ b/jni/FuseDaemon.h
@@ -29,17 +29,12 @@
public:
FuseDaemon(JNIEnv* env, jobject mediaProvider);
- ~FuseDaemon() {
- // TODO(b/135341433): Ensure daemon is stopped and all resources are cleaned up
- }
+ ~FuseDaemon() = default;
+
/**
* Start the FUSE daemon loop that will handle filesystem calls.
*/
void Start(const int fd, const std::string& path);
- /**
- * Stop the FUSE daemon and clean up resources.
- */
- void Stop();
private:
FuseDaemon(const FuseDaemon&) = delete;
diff --git a/jni/com_android_providers_media_FuseDaemon.cpp b/jni/com_android_providers_media_FuseDaemon.cpp
index a6a8d18..1633b99 100644
--- a/jni/com_android_providers_media_FuseDaemon.cpp
+++ b/jni/com_android_providers_media_FuseDaemon.cpp
@@ -15,7 +15,7 @@
*/
// Need to use LOGE_EX.
-#define LOG_TAG "FuseDaemon"
+#define LOG_TAG "FuseDaemonJNI"
#include <nativehelper/scoped_utf_chars.h>
@@ -49,12 +49,6 @@
daemon->Start(fd, string_path);
}
-void com_android_providers_media_FuseDaemon_stop(JNIEnv* env, jobject self, jlong java_daemon) {
- LOG(DEBUG) << "Stopping the FUSE daemon...";
- fuse::FuseDaemon* const daemon = reinterpret_cast<fuse::FuseDaemon*>(java_daemon);
- daemon->Stop();
-}
-
void com_android_providers_media_FuseDaemon_delete(JNIEnv* env, jobject self, jlong java_daemon) {
LOG(DEBUG) << "Destroying the FUSE daemon...";
fuse::FuseDaemon* const daemon = reinterpret_cast<fuse::FuseDaemon*>(java_daemon);
@@ -66,8 +60,6 @@
reinterpret_cast<void*>(com_android_providers_media_FuseDaemon_new)},
{"native_start", "(JILjava/lang/String;)V",
reinterpret_cast<void*>(com_android_providers_media_FuseDaemon_start)},
- {"native_stop", "(J)V",
- reinterpret_cast<void*>(com_android_providers_media_FuseDaemon_stop)},
{"native_delete", "(J)V",
reinterpret_cast<void*>(com_android_providers_media_FuseDaemon_delete)}};
} // namespace
diff --git a/src/com/android/providers/media/MediaProvider.java b/src/com/android/providers/media/MediaProvider.java
index 428c8d1..efc65d1 100644
--- a/src/com/android/providers/media/MediaProvider.java
+++ b/src/com/android/providers/media/MediaProvider.java
@@ -274,9 +274,13 @@
}
public static File getVolumePath(String volumeName) throws FileNotFoundException {
- synchronized (sCacheLock) {
- return MediaStore.getVolumePath(sCachedVolumes, volumeName);
- }
+ // TODO(b/144275217): A more performant invocation is
+ // MediaStore#getVolumePath(sCachedVolumes, volumeName) since we avoid a binder
+ // to StorageManagerService to getVolumeList. We need to delay the mount broadcasts
+ // from StorageManagerService so that sCachedVolumes is up to date in
+ // onVolumeStateChanged before we to call this method, otherwise we would crash
+ // when we don't find volumeName yet
+ return MediaStore.getVolumePath(volumeName);
}
public static Set<String> getExternalVolumeNames() {
@@ -501,7 +505,7 @@
final File path = getVolumePath(volumeName);
final StorageVolume vol = mStorageManager.getStorageVolume(path);
final String key;
- if (VolumeInfo.ID_EMULATED_INTERNAL.equals(vol.getId())) {
+ if (vol == null || vol.isPrimary()) {
key = "created_default_folders";
} else {
key = "created_default_folders_" + vol.getNormalizedUuid();
diff --git a/src/com/android/providers/media/fuse/ExternalStorageServiceImpl.java b/src/com/android/providers/media/fuse/ExternalStorageServiceImpl.java
index 60c5a59..7d6b3f1 100644
--- a/src/com/android/providers/media/fuse/ExternalStorageServiceImpl.java
+++ b/src/com/android/providers/media/fuse/ExternalStorageServiceImpl.java
@@ -16,12 +16,17 @@
package com.android.providers.media.fuse;
+import android.content.ContentProviderClient;
+import android.os.OperationCanceledException;
import android.os.ParcelFileDescriptor;
+import android.provider.MediaStore;
import android.service.storage.ExternalStorageService;
import android.util.Log;
import androidx.annotation.NonNull;
+import com.android.providers.media.MediaProvider;
+
import java.util.HashMap;
import java.util.Map;
@@ -38,34 +43,53 @@
public void onStartSession(String sessionId, @SessionFlag int flag,
@NonNull ParcelFileDescriptor deviceFd, @NonNull String upperFileSystemPath,
@NonNull String lowerFileSystemPath) {
+ MediaProvider mediaProvider = getMediaProvider();
+
synchronized (mLock) {
if (mFuseDaemons.containsKey(sessionId)) {
Log.w(TAG, "Session already started with id: " + sessionId);
- return;
+ } else {
+ Log.i(TAG, "Starting session for id: " + sessionId);
+ // We only use the upperFileSystemPath because the media process is mounted as
+ // REMOUNT_MODE_PASS_THROUGH which guarantees that all /storage paths are bind
+ // mounts of the lower filesystem.
+ FuseDaemon daemon = new FuseDaemon(mediaProvider, this, deviceFd, sessionId,
+ upperFileSystemPath);
+ daemon.start();
+ mFuseDaemons.put(sessionId, daemon);
}
-
- Log.i(TAG, "Starting session for id: " + sessionId);
- // We only use the upperFileSystemPath because the media process is mounted as
- // REMOUNT_MODE_PASS_THROUGH which guarantees that all /storage paths are bind
- // mounts of the lower filesystem.
- FuseDaemon daemon = new FuseDaemon(getContentResolver(), sessionId, this, deviceFd,
- upperFileSystemPath);
- new Thread(daemon).start();
- mFuseDaemons.put(sessionId, daemon);
}
}
@Override
public void onEndSession(String sessionId) {
+ FuseDaemon daemon = onExitSession(sessionId);
+
+ if (daemon == null) {
+ Log.w(TAG, "Session already ended with id: " + sessionId);
+ } else {
+ Log.i(TAG, "Ending session for id: " + sessionId);
+ // The FUSE daemon cannot end the FUSE session itself, but if the FUSE filesystem
+ // is unmounted, the FUSE thread started in #onStartSession will exit and we can
+ // this allows us wait for confirmation. This blocks the client until the session has
+ // exited for sure
+ daemon.waitForExit();
+ }
+ }
+
+ public FuseDaemon onExitSession(String sessionId) {
+ Log.i(TAG, "Exiting session for id: " + sessionId);
synchronized (mLock) {
- FuseDaemon daemon = mFuseDaemons.get(sessionId);
- if (daemon != null) {
- Log.i(TAG, "Ending session for id: " + sessionId);
- daemon.stop();
- mFuseDaemons.remove(sessionId);
- } else {
- Log.w(TAG, "No session with id: " + sessionId);
- }
+ return mFuseDaemons.remove(sessionId);
+ }
+ }
+
+ private MediaProvider getMediaProvider() {
+ try (ContentProviderClient cpc =
+ getContentResolver().acquireContentProviderClient(MediaStore.AUTHORITY)) {
+ return (MediaProvider) cpc.getLocalContentProvider();
+ } catch (OperationCanceledException e) {
+ throw new IllegalStateException("Failed to acquire MediaProvider", e);
}
}
}
diff --git a/src/com/android/providers/media/fuse/FuseDaemon.java b/src/com/android/providers/media/fuse/FuseDaemon.java
index e6fb1a2..b1af04c 100644
--- a/src/com/android/providers/media/fuse/FuseDaemon.java
+++ b/src/com/android/providers/media/fuse/FuseDaemon.java
@@ -16,11 +16,8 @@
package com.android.providers.media.fuse;
-import android.content.ContentProviderClient;
-import android.content.ContentResolver;
-import android.os.OperationCanceledException;
import android.os.ParcelFileDescriptor;
-import android.provider.MediaStore;
+import android.util.Log;
import androidx.annotation.NonNull;
@@ -30,58 +27,66 @@
/**
* Starts a FUSE session to handle FUSE messages from the kernel.
*/
-public final class FuseDaemon implements Runnable {
- public static final String TAG = "FuseDaemon";
+public final class FuseDaemon extends Thread {
+ public static final String TAG = "FuseDaemonThread";
- private final Object mLock = new Object();
- private final ExternalStorageServiceImpl mService;
- private final String mSessionId;
+ private final MediaProvider mMediaProvider;
private final int mFuseDeviceFd;
private final String mPath;
- private long mNativeFuseDaemon;
+ private final ExternalStorageServiceImpl mService;
- public FuseDaemon(@NonNull ContentResolver resolver, @NonNull String sessionId,
+ public FuseDaemon(@NonNull MediaProvider mediaProvider,
@NonNull ExternalStorageServiceImpl service, @NonNull ParcelFileDescriptor fd,
- @NonNull String path) {
- Preconditions.checkNotNull(resolver);
+ @NonNull String sessionId, @NonNull String path) {
+ mMediaProvider = Preconditions.checkNotNull(mediaProvider);
mService = Preconditions.checkNotNull(service);
- mSessionId = Preconditions.checkNotNull(sessionId);;
+ setName(Preconditions.checkNotNull(sessionId));
mFuseDeviceFd = Preconditions.checkNotNull(fd).detachFd();
mPath = Preconditions.checkNotNull(path);
-
- try (ContentProviderClient cpc =
- resolver.acquireContentProviderClient(MediaStore.AUTHORITY)) {
- mNativeFuseDaemon = native_new((MediaProvider) cpc.getLocalContentProvider());
- } catch (OperationCanceledException e) {
- throw new IllegalStateException("Failed to acquire content provider", e);
- }
}
- /** Starts a FUSE session. Does not return until {@link #stop} is called. */
+ /** Starts a FUSE session. Does not return until the lower filesystem is unmounted. */
@Override
public void run() {
- native_start(mNativeFuseDaemon, mFuseDeviceFd, mPath);
- mService.onEndSession(mSessionId);
- }
-
- /** Stops any running FUSE sessions, causing {@link #run} to return. */
- public void stop() {
- native_stop(mNativeFuseDaemon);
- }
-
- // TODO(b/135341433): Don't use finalizer. Consider Cleaner and PhantomReference
- @Override
- public void finalize() throws Throwable {
- synchronized (mLock) {
- if (mNativeFuseDaemon != 0) {
- native_delete(mNativeFuseDaemon);
- mNativeFuseDaemon = 0;
- }
+ long ptr = native_new(mMediaProvider);
+ if (ptr == 0) {
+ return;
}
+
+ Log.i(TAG, "Starting thread for " + getName() + " ...");
+ native_start(ptr, mFuseDeviceFd, mPath); // Blocks
+ Log.i(TAG, "Exiting thread for " + getName() + " ...");
+
+ // Cleanup
+ if (ptr != 0) {
+ native_delete(ptr);
+ }
+ mService.onExitSession(getName());
+ Log.i(TAG, "Exited thread for " + getName());
+ }
+
+ /** Waits for any running FUSE sessions to return. */
+ public void waitForExit() {
+ Log.i(TAG, "Waiting 5s for thread " + getName() + " to exit...");
+
+ try {
+ join(5000);
+ } catch (InterruptedException e) {
+ Log.e(TAG, "Interrupted while waiting for thread " + getName()
+ + " to exit. Terminating process", e);
+ System.exit(1);
+ }
+
+ if (isAlive()) {
+ Log.i(TAG, "Failed to exit thread " + getName()
+ + " successfully. Terminating process");
+ System.exit(1);
+ }
+
+ Log.i(TAG, "Exited thread " + getName() + " successfully");
}
private native long native_new(MediaProvider mediaProvider);
private native void native_start(long daemon, int deviceFd, String path);
- private native void native_stop(long daemon);
private native void native_delete(long daemon);
}