Fix multi-user and multi-storage with FUSE

Up until now, the FUSE mount logic has made two assumptions:
1. The primary external volume is an emulated volume on /data/media
2. Only the primary user is running, as user zero

With this change, we are able to handle the following cases:
1. Mount new external storage volumes as as portable storage
(PublicVolume) managed by FUSE. The PublicVolume originally mounted on
/mnt/media_rw/<uuid> is availabe on /storage/<uuid> as a FUSE mount

2. Mount new external storage volumes as adoptable storage
(PrivateVolume) with a stacked EmulatedVolume managed by FUSE. The
EmulatedVolume orignally mounted on /mnt/expand/<uuid>/media will be
available on /storage/emulated if set as the primary storage

3. Run the MediaProvider as a secondary user handling requests on
/mnt/user/<userid>/<volid>

Test: atest AdoptableHostTest
Bug: 135341433
Bug: 140120303

Change-Id: I5fb48616b4143277ea14c6846037e2fd176e16db

6 files changed, 684 insertions, 246 deletions

diff --git a/core/java/android/os/storage/StorageManager.java b/core/java/android/os/storage/StorageManager.java
index 2d8af83c7ca7..ac7a0a8d8b5c 100644
--- a/core/java/android/os/storage/StorageManager.java
+++ b/core/java/android/os/storage/StorageManager.java
@@ -779,7 +779,12 @@ public class StorageManager {
     /** {@hide} */
     public @Nullable VolumeInfo findPrivateForEmulated(VolumeInfo emulatedVol) {
         if (emulatedVol != null) {
-            return findVolumeById(emulatedVol.getId().replace("emulated", "private"));
+            String id = emulatedVol.getId();
+            int idx = id.indexOf(";");
+            if (idx != -1) {
+                id = id.substring(0, idx);
+            }
+            return findVolumeById(id.replace("emulated", "private"));
         } else {
             return null;
         }
@@ -789,7 +794,8 @@ public class StorageManager {
     @UnsupportedAppUsage
     public @Nullable VolumeInfo findEmulatedForPrivate(VolumeInfo privateVol) {
         if (privateVol != null) {
-            return findVolumeById(privateVol.getId().replace("private", "emulated"));
+            return findVolumeById(privateVol.getId().replace("private", "emulated") + ";"
+                    + mContext.getUserId());
         } else {
             return null;
         }
diff --git a/core/java/android/os/storage/VolumeInfo.java b/core/java/android/os/storage/VolumeInfo.java
index 7699a0529826..d6ec52fac8a5 100644
--- a/core/java/android/os/storage/VolumeInfo.java
+++ b/core/java/android/os/storage/VolumeInfo.java
@@ -266,7 +266,7 @@ public class VolumeInfo implements Parcelable {
 
     @UnsupportedAppUsage
     public @Nullable String getDescription() {
-        if (ID_PRIVATE_INTERNAL.equals(id) || ID_EMULATED_INTERNAL.equals(id)) {
+        if (ID_PRIVATE_INTERNAL.equals(id) || id.startsWith(ID_EMULATED_INTERNAL + ";")) {
             return Resources.getSystem().getString(com.android.internal.R.string.storage_internal);
         } else if (!TextUtils.isEmpty(fsLabel)) {
             return fsLabel;
@@ -301,13 +301,20 @@ public class VolumeInfo implements Parcelable {
     }
 
     public boolean isVisibleForUser(int userId) {
-        if ((type == TYPE_PUBLIC || type == TYPE_STUB) && mountUserId == userId) {
-            return isVisible();
-        } else if (type == TYPE_EMULATED) {
+        if ((type == TYPE_PUBLIC || type == TYPE_STUB || type == TYPE_EMULATED)
+                && mountUserId == userId) {
             return isVisible();
-        } else {
-            return false;
         }
+        return false;
+    }
+
+    /**
+     * Returns {@code true} if this volume is the primary emulated volume for {@code userId},
+     * {@code false} otherwise.
+     */
+    @UnsupportedAppUsage
+    public boolean isPrimaryEmulatedForUser(int userId) {
+        return id.equals(ID_EMULATED_INTERNAL + ";" + userId);
     }
 
     public boolean isVisibleForRead(int userId) {
@@ -390,7 +397,7 @@ public class VolumeInfo implements Parcelable {
                 derivedFsUuid = privateVol.fsUuid;
             }
 
-            if (ID_EMULATED_INTERNAL.equals(id)) {
+            if (isPrimaryEmulatedForUser(userId)) {
                 removable = false;
             } else {
                 removable = true;
diff --git a/packages/ExternalStorageProvider/src/com/android/externalstorage/ExternalStorageProvider.java b/packages/ExternalStorageProvider/src/com/android/externalstorage/ExternalStorageProvider.java
index af96982f5426..4a50210d1a60 100644
--- a/packages/ExternalStorageProvider/src/com/android/externalstorage/ExternalStorageProvider.java
+++ b/packages/ExternalStorageProvider/src/com/android/externalstorage/ExternalStorageProvider.java
@@ -162,12 +162,12 @@ public class ExternalStorageProvider extends FileSystemProvider {
             final String title;
             final UUID storageUuid;
             if (volume.getType() == VolumeInfo.TYPE_EMULATED) {
-                // We currently only support a single emulated volume mounted at
+                // We currently only support a single emulated volume per user mounted at
                 // a time, and it's always considered the primary
                 if (DEBUG) Log.d(TAG, "Found primary volume: " + volume);
                 rootId = ROOT_ID_PRIMARY_EMULATED;
 
-                if (VolumeInfo.ID_EMULATED_INTERNAL.equals(volume.getId())) {
+                if (volume.isPrimaryEmulatedForUser(userId)) {
                     // This is basically the user's primary device storage.
                     // Use device name for the volume since this is likely same thing
                     // the user sees when they mount their phone on another device.
diff --git a/services/core/java/com/android/server/StorageManagerService.java b/services/core/java/com/android/server/StorageManagerService.java
index 3916f0d78931..dcc690fa09b4 100644
--- a/services/core/java/com/android/server/StorageManagerService.java
+++ b/services/core/java/com/android/server/StorageManagerService.java
@@ -141,6 +141,7 @@ import com.android.internal.util.Preconditions;
 import com.android.internal.widget.LockPatternUtils;
 import com.android.server.storage.AppFuseBridge;
 import com.android.server.storage.StorageSessionController;
+import com.android.server.storage.StorageSessionController.ExternalStorageServiceException;
 import com.android.server.wm.ActivityTaskManagerInternal;
 import com.android.server.wm.ActivityTaskManagerInternal.ScreenObserver;
 
@@ -196,9 +197,6 @@ class StorageManagerService extends IStorageManager.Stub
     private static final String ZRAM_ENABLED_PROPERTY =
             "persist.sys.zram_enabled";
 
-    private static final boolean IS_FUSE_ENABLED =
-            SystemProperties.getBoolean(StorageManager.PROP_FUSE, false);
-
     private static final boolean ENABLE_ISOLATED_STORAGE = StorageManager.hasIsolatedStorage();
 
     /**
@@ -350,6 +348,10 @@ class StorageManagerService extends IStorageManager.Stub
     @GuardedBy("mLock")
     private ArrayMap<String, CountDownLatch> mDiskScanLatches = new ArrayMap<>();
 
+    /** Map from volume ID to latches */
+    @GuardedBy("mLock")
+    private ArrayMap<String, CountDownLatch> mFuseVolumeReadyLatches = new ArrayMap<>();
+
     @GuardedBy("mLock")
     private IPackageMoveObserver mMoveCallback;
     @GuardedBy("mLock")
@@ -419,7 +421,7 @@ class StorageManagerService extends IStorageManager.Stub
     private @Nullable VolumeInfo findStorageForUuid(String volumeUuid) {
         final StorageManager storage = mContext.getSystemService(StorageManager.class);
         if (Objects.equals(StorageManager.UUID_PRIVATE_INTERNAL, volumeUuid)) {
-            return storage.findVolumeById(VolumeInfo.ID_EMULATED_INTERNAL);
+            return storage.findVolumeById(VolumeInfo.ID_EMULATED_INTERNAL + ";" + 0);
         } else if (Objects.equals(StorageManager.UUID_PRIMARY_PHYSICAL, volumeUuid)) {
             return storage.getPrimaryPhysicalVolume();
         } else {
@@ -462,6 +464,17 @@ class StorageManagerService extends IStorageManager.Stub
         }
     }
 
+    private CountDownLatch findOrCreateFuseVolumeReadyLatch(String volId) {
+        synchronized (mLock) {
+            CountDownLatch latch = mFuseVolumeReadyLatches.get(volId);
+            if (latch == null) {
+                latch = new CountDownLatch(1);
+                mFuseVolumeReadyLatches.put(volId, latch);
+            }
+            return latch;
+        }
+    }
+
     /** List of crypto types.
       * These must match CRYPT_TYPE_XXX in cryptfs.h AND their
       * corresponding commands in CommandListener.cpp */
@@ -514,6 +527,8 @@ class StorageManagerService extends IStorageManager.Stub
     // Not guarded by a lock.
     private final StorageSessionController mStorageSessionController;
 
+    private final boolean mIsFuseEnabled;
+
     class ObbState implements IBinder.DeathRecipient {
         public ObbState(String rawPath, String canonicalPath, int callingUid,
                 IObbActionListener token, int nonce, String volId) {
@@ -597,6 +612,7 @@ class StorageManagerService extends IStorageManager.Stub
     private static final int H_ABORT_IDLE_MAINT = 12;
     private static final int H_BOOT_COMPLETED = 13;
     private static final int H_COMPLETE_UNLOCK_USER = 14;
+    private static final int H_VOLUME_READY = 15;
 
     class StorageManagerServiceHandler extends Handler {
         public StorageManagerServiceHandler(Looper looper) {
@@ -657,6 +673,22 @@ class StorageManagerService extends IStorageManager.Stub
                     }
                     break;
                 }
+                case H_VOLUME_READY: {
+                    final VolumeInfo vol = (VolumeInfo) msg.obj;
+                    try {
+                        mStorageSessionController.onVolumeReady(vol);
+
+                        synchronized (mLock) {
+                            CountDownLatch latch = mFuseVolumeReadyLatches.remove(vol.id);
+                            if (latch != null) {
+                                latch.countDown();
+                            }
+                        }
+                    } catch (IllegalStateException | ExternalStorageServiceException e) {
+                        Slog.i(TAG, "Failed to initialise volume " + vol, e);
+                    }
+                    break;
+                }
                 case H_VOLUME_MOUNT: {
                     final VolumeInfo vol = (VolumeInfo) msg.obj;
                     if (isMountDisallowed(vol)) {
@@ -664,19 +696,12 @@ class StorageManagerService extends IStorageManager.Stub
                         break;
                     }
 
-                    // TODO(b/135341433): Remove paranoid logging when FUSE is stable
-                    Slog.i(TAG, "Mounting volume " + vol);
-                    // TODO(b/135341433): Update to use new vold API that gets or mounts fuse fd
-                    // Ensure that we can pass user of a volume to the new API
-                    mStorageSessionController.onVolumeMounted(mCurrentUserId, mount(vol), vol);
-                    Slog.i(TAG, "Mounted volume " + vol);
-
+                    mount(vol);
                     break;
                 }
                 case H_VOLUME_UNMOUNT: {
                     final VolumeInfo vol = (VolumeInfo) msg.obj;
                     unmount(vol);
-                    mStorageSessionController.onVolumeUnmounted(mCurrentUserId, vol);
                     break;
                 }
                 case H_VOLUME_BROADCAST: {
@@ -757,7 +782,6 @@ class StorageManagerService extends IStorageManager.Stub
                         }
                     }
                     mVold.onUserRemoved(userId);
-                    mStorageSessionController.onUserRemoved(userId);
                 }
             } catch (Exception e) {
                 Slog.wtf(TAG, e);
@@ -978,7 +1002,12 @@ class StorageManagerService extends IStorageManager.Stub
                 + ", mDaemonConnected=" + mDaemonConnected);
         if (mBootCompleted && mDaemonConnected) {
             final List<UserInfo> users = mContext.getSystemService(UserManager.class).getUsers();
-            killMediaProvider(users);
+
+            if (mIsFuseEnabled) {
+                mStorageSessionController.onReset(mVold, mHandler);
+            } else {
+                killMediaProvider(users);
+            }
 
             final int[] systemUnlockedUsers;
             synchronized (mLock) {
@@ -992,7 +1021,7 @@ class StorageManagerService extends IStorageManager.Stub
 
             try {
                 // TODO(b/135341433): Remove paranoid logging when FUSE is stable
-                Slog.i(TAG, "Resetting vold");
+                Slog.i(TAG, "Resetting vold...");
                 mVold.reset();
                 Slog.i(TAG, "Reset vold");
 
@@ -1019,7 +1048,7 @@ class StorageManagerService extends IStorageManager.Stub
         // staging area is ready so it's ready for zygote-forked apps to
         // bind mount against.
         try {
-            mStorageSessionController.onUserStarted(userId);
+            mStorageSessionController.onUnlockUser(userId);
             mVold.onUserStarted(userId);
             mStoraged.onUserStarted(userId);
         } catch (Exception e) {
@@ -1201,10 +1230,12 @@ class StorageManagerService extends IStorageManager.Stub
         }
 
         @Override
-        public void onVolumeCreated(String volId, int type, String diskId, String partGuid) {
+        public void onVolumeCreated(String volId, int type, String diskId, String partGuid,
+                int userId) {
             synchronized (mLock) {
                 final DiskInfo disk = mDisks.get(diskId);
                 final VolumeInfo vol = new VolumeInfo(volId, type, disk, partGuid);
+                vol.mountUserId = userId;
                 mVolumes.put(volId, vol);
                 onVolumeCreatedLocked(vol);
             }
@@ -1258,8 +1289,13 @@ class StorageManagerService extends IStorageManager.Stub
 
         @Override
         public void onVolumeDestroyed(String volId) {
+            VolumeInfo vol = null;
             synchronized (mLock) {
-                mVolumes.remove(volId);
+                vol = mVolumes.remove(volId);
+            }
+
+            if (vol != null) {
+                mStorageSessionController.onVolumeRemove(vol);
             }
         }
     };
@@ -1395,6 +1431,13 @@ class StorageManagerService extends IStorageManager.Stub
             writeSettingsLocked();
         }
 
+        if (mIsFuseEnabled && newState == VolumeInfo.STATE_MOUNTED
+                && (vol.type == VolumeInfo.TYPE_PUBLIC || vol.type == VolumeInfo.TYPE_EMULATED)) {
+            Slog.i(TAG, "Initialising volume " + vol + " ...");
+            // TODO(b/144275217): Delay broadcasts till mount is really ready
+            mHandler.obtainMessage(H_VOLUME_READY, vol).sendToTarget();
+        }
+
         mCallbacks.notifyVolumeStateChanged(vol, oldState, newState);
 
         // Do not broadcast before boot has completed to avoid launching the
@@ -1546,13 +1589,12 @@ class StorageManagerService extends IStorageManager.Stub
         // Snapshot feature flag used for this boot
         SystemProperties.set(StorageManager.PROP_ISOLATED_STORAGE_SNAPSHOT, Boolean.toString(
                 SystemProperties.getBoolean(StorageManager.PROP_ISOLATED_STORAGE, true)));
-
         SystemProperties.set(StorageManager.PROP_FUSE_SNAPSHOT, Boolean.toString(
                 SystemProperties.getBoolean(StorageManager.PROP_FUSE, false)));
 
+        mIsFuseEnabled = SystemProperties.getBoolean(StorageManager.PROP_FUSE_SNAPSHOT, false);
         mContext = context;
         mResolver = mContext.getContentResolver();
-
         mCallbacks = new Callbacks(FgThread.get().getLooper());
         mLockPatternUtils = new LockPatternUtils(mContext);
 
@@ -1563,11 +1605,7 @@ class StorageManagerService extends IStorageManager.Stub
         // Add OBB Action Handler to StorageManagerService thread.
         mObbActionHandler = new ObbActionHandler(IoThread.get().getLooper());
 
-        mStorageSessionController = new StorageSessionController(mContext,
-                userId -> {
-                    Slog.i(TAG, "Storage session ended for user: " + userId + ". Resetting...");
-                    mHandler.obtainMessage(H_RESET).sendToTarget();
-                });
+        mStorageSessionController = new StorageSessionController(mContext, mIsFuseEnabled);
 
         // Initialize the last-fstrim tracking if necessary
         File dataDir = Environment.getDataDirectory();
@@ -1873,21 +1911,36 @@ class StorageManagerService extends IStorageManager.Stub
         if (isMountDisallowed(vol)) {
             throw new SecurityException("Mounting " + volId + " restricted by policy");
         }
+
+        CountDownLatch latch = null;
+        if (mIsFuseEnabled && StorageSessionController.isEmulatedOrPublic(vol)) {
+            latch = findOrCreateFuseVolumeReadyLatch(volId);
+        }
+
         mount(vol);
+
+        if (latch != null) {
+            try {
+                waitForLatch(latch, "mount " + volId, 3 * DateUtils.MINUTE_IN_MILLIS);
+            } catch (TimeoutException e) {
+                Slog.wtf(TAG, e);
+            } finally {
+                synchronized (mLock) {
+                    mFuseVolumeReadyLatches.remove(volId);
+                }
+            }
+        }
     }
 
-    private FileDescriptor mount(VolumeInfo vol) {
+    private void mount(VolumeInfo vol) {
         try {
-            // TODO(b/135341433): Now, emulated (and private?) volumes are shared across users
-            // This means the mountUserId on such volumes is USER_NULL. This breaks fuse which
-            // requires a valid user to mount a volume. Create individual volumes per user in vold
-            // and remove this property check
-            int userId = SystemProperties.getBoolean(StorageManager.PROP_FUSE_SNAPSHOT, false)
-                    ? mCurrentUserId : vol.mountUserId;
-            return mVold.mount(vol.id, vol.mountFlags, userId);
+            // TODO(b/135341433): Remove paranoid logging when FUSE is stable
+            Slog.i(TAG, "Mounting volume " + vol);
+            FileDescriptor fd = mVold.mount(vol.id, vol.mountFlags, vol.mountUserId);
+            Slog.i(TAG, "Mounted volume " + vol);
+            mStorageSessionController.onVolumeMount(fd, vol);
         } catch (Exception e) {
             Slog.wtf(TAG, e);
-            return null;
         }
     }
 
@@ -1902,6 +1955,7 @@ class StorageManagerService extends IStorageManager.Stub
     private void unmount(VolumeInfo vol) {
         try {
             mVold.unmount(vol.id);
+            mStorageSessionController.onVolumeUnmount(vol);
         } catch (Exception e) {
             Slog.wtf(TAG, e);
         }
@@ -3040,6 +3094,14 @@ class StorageManagerService extends IStorageManager.Stub
 
     @Override
     public void mkdirs(String callingPkg, String appPath) {
+        if (mIsFuseEnabled) {
+            // TODO(b/144332951): Calling into Vold is risky because the FUSE daemon can go down
+            // anytime and Vold will hang forever. We should either remove this call
+            // or at least call into the FUSE daemon to mkdir instead
+            Slog.w(TAG, "Not making dir for package " + callingPkg + " with path " + appPath);
+            return;
+        }
+
         final int callingUid = Binder.getCallingUid();
         final int userId = UserHandle.getUserId(callingUid);
         final UserEnvironment userEnv = new UserEnvironment(userId);
@@ -3121,8 +3183,12 @@ class StorageManagerService extends IStorageManager.Stub
                 switch (vol.getType()) {
                     case VolumeInfo.TYPE_PUBLIC:
                     case VolumeInfo.TYPE_STUB:
-                    case VolumeInfo.TYPE_EMULATED:
                         break;
+                    case VolumeInfo.TYPE_EMULATED:
+                        if (vol.getMountUserId() == userId) {
+                            break;
+                        }
+                        // Skip if emulated volume not for userId
                     default:
                         continue;
                 }
@@ -3711,7 +3777,7 @@ class StorageManagerService extends IStorageManager.Stub
                 return Zygote.MOUNT_EXTERNAL_NONE;
             }
 
-            if (IS_FUSE_ENABLED && packageName.equals(mMediaStoreAuthorityPackageName)) {
+            if (mIsFuseEnabled && packageName.equals(mMediaStoreAuthorityPackageName)) {
                 // Determine if caller requires pass_through mount
                 return Zygote.MOUNT_EXTERNAL_PASS_THROUGH;
             }
diff --git a/services/core/java/com/android/server/storage/StorageSessionController.java b/services/core/java/com/android/server/storage/StorageSessionController.java
index 2d36a0dd1be4..72a1b9df3b3c 100644
--- a/services/core/java/com/android/server/storage/StorageSessionController.java
+++ b/services/core/java/com/android/server/storage/StorageSessionController.java
@@ -18,6 +18,8 @@ package com.android.server.storage;
 
 import android.Manifest;
 import android.annotation.Nullable;
+import android.app.ActivityManager;
+import android.app.IActivityManager;
 import android.content.ComponentName;
 import android.content.Context;
 import android.content.Intent;
@@ -25,7 +27,12 @@ import android.content.pm.PackageManager;
 import android.content.pm.ProviderInfo;
 import android.content.pm.ResolveInfo;
 import android.content.pm.ServiceInfo;
+import android.os.Handler;
+import android.os.IVold;
 import android.os.ParcelFileDescriptor;
+import android.os.RemoteException;
+import android.os.ServiceSpecificException;
+import android.os.UserHandle;
 import android.os.storage.VolumeInfo;
 import android.provider.MediaStore;
 import android.service.storage.ExternalStorageService;
@@ -47,27 +54,41 @@ public final class StorageSessionController {
 
     private final Object mLock = new Object();
     private final Context mContext;
-    private final Callback mCallback;
-    @GuardedBy("mLock")
-    private ComponentName mExternalStorageServiceComponent;
     @GuardedBy("mLock")
     private final SparseArray<StorageUserConnection> mConnections = new SparseArray<>();
+    private final boolean mIsFuseEnabled;
+
+    private volatile ComponentName mExternalStorageServiceComponent;
+    private volatile String mExternalStorageServicePackageName;
+    private volatile int mExternalStorageServiceAppId;
+    private volatile boolean mIsResetting;
 
-    public StorageSessionController(Context context, Callback callback) {
+    public StorageSessionController(Context context, boolean isFuseEnabled) {
         mContext = Preconditions.checkNotNull(context);
-        mCallback = Preconditions.checkNotNull(callback);
+        mIsFuseEnabled = isFuseEnabled;
     }
 
     /**
-     * Starts a storage session associated with {@code deviceFd} for {@code vol}.
-     * Does nothing if a session is already started or starting. If the user associated with
-     * {@code vol} is not yet ready, the session will be retried {@link #onUserStarted}.
+     * Creates a storage session associated with {@code deviceFd} for {@code vol}. Sessions can be
+     * started with {@link #onVolumeReady} and removed with {@link #onVolumeUnmount} or
+     * {@link #onVolumeRemove}.
      *
-     * A session must be ended with {@link #endSession} when no longer required.
+     * Does nothing if {@link #shouldHandle} is {@code false}
+     *
+     * @throws IllegalStateException if a session has already been created for {@code vol}
      */
-    public void onVolumeMounted(int userId, FileDescriptor deviceFd, VolumeInfo vol) {
+    public void onVolumeMount(FileDescriptor deviceFd, VolumeInfo vol) {
+        if (!shouldHandle(vol)) {
+            return;
+        }
+
+        Slog.i(TAG, "On volume mount " + vol);
+
+        String sessionId = vol.getId();
+        int userId = vol.getMountUserId();
+
         if (deviceFd == null) {
-            Slog.w(TAG, "Null device fd. Session not started for " + vol);
+            Slog.w(TAG, "Null fd. Session not started for vol: " + vol);
             return;
         }
 
@@ -82,136 +103,320 @@ public final class StorageSessionController {
         }
 
         if ("/dev/null".equals(realPath)) {
-            Slog.i(TAG, "Volume ready for use: " + vol);
+            Slog.i(TAG, "Volume ready for use with id: " + sessionId);
             return;
         }
 
         synchronized (mLock) {
             StorageUserConnection connection = mConnections.get(userId);
             if (connection == null) {
-                Slog.i(TAG, "Creating new session for vol: " + vol);
                 connection = new StorageUserConnection(mContext, userId, this);
                 mConnections.put(userId, connection);
             }
-            try {
-                Slog.i(TAG, "Starting session for vol: " + vol);
-                connection.startSession(deviceFd, vol);
-            } catch (ExternalStorageServiceException e) {
-                Slog.e(TAG, "Failed to start session for vol: " + vol, e);
+            Slog.i(TAG, "Creating session with id: " + sessionId);
+            connection.createSession(sessionId, new ParcelFileDescriptor(deviceFd));
+        }
+    }
+
+    /**
+     * Starts a storage session associated with {@code vol} after {@link #onVolumeMount}.
+     *
+     * Subsequent calls will attempt to start the storage session, but does nothing if already
+     * started. If the user associated with {@code vol} is not yet ready, all pending sesssions
+     * can be restarted with {@link onUnlockUser}.
+     *
+     * Does nothing if {@link #shouldHandle} is {@code false}
+     *
+     * Blocks until the session is started or fails
+     *
+     * @throws ExternalStorageServiceException if the session fails to start
+     */
+    public void onVolumeReady(VolumeInfo vol) throws ExternalStorageServiceException {
+        if (!shouldHandle(vol)) {
+            return;
+        }
+
+        Slog.i(TAG, "On volume ready " + vol);
+        String sessionId = vol.getId();
+
+        StorageUserConnection connection = null;
+        synchronized (mLock) {
+            connection = mConnections.get(vol.getMountUserId());
+            if (connection == null) {
+                Slog.i(TAG, "Volume ready but no associated connection");
+                return;
             }
         }
+
+        connection.initSession(sessionId, vol.getPath().getPath(),
+                vol.getInternalPath().getPath());
+
+        if (isReady()) {
+            connection.startSession(sessionId);
+        } else {
+            Slog.i(TAG, "Controller not initialised, session not started " + sessionId);
+        }
     }
 
     /**
-     * Ends a storage session for {@code vol}. Does nothing if the session is already
-     * ended or ending. Ending a session discards all resources associated with that session.
+     * Removes and returns the {@link StorageUserConnection} for {@code vol}.
+     *
+     * Does nothing if {@link #shouldHandle} is {@code false}
+     *
+     * @return the connection that was removed or {@code null} if nothing was removed
      */
-    public void onVolumeUnmounted(int userId, VolumeInfo vol) {
+    @Nullable
+    public StorageUserConnection onVolumeRemove(VolumeInfo vol) {
+        if (!shouldHandle(vol)) {
+            return null;
+        }
+
+        Slog.i(TAG, "On volume remove " + vol);
+        String sessionId = vol.getId();
+        int userId = vol.getMountUserId();
+
         synchronized (mLock) {
             StorageUserConnection connection = mConnections.get(userId);
             if (connection != null) {
-                Slog.i(TAG, "Ending session for vol: " + vol);
-                try {
-                    if (connection.endSession(vol)) {
-                        mConnections.remove(userId);
-                    }
-                } catch (ExternalStorageServiceException e) {
-                    Slog.e(TAG, "Failed to end session for vol: " + vol, e);
-                }
+                Slog.i(TAG, "Removed session for vol with id: " + sessionId);
+                connection.removeSession(sessionId);
+                return connection;
             } else {
-                Slog.w(TAG, "Session already ended for vol: " + vol);
+                Slog.w(TAG, "Session already removed for vol with id: " + sessionId);
+                return null;
             }
         }
     }
 
-    /** Restarts all sessions for {@code userId}. */
-    public void onUserStarted(int userId) {
-        synchronized (mLock) {
-            StorageUserConnection connection = mConnections.get(userId);
-            if (connection != null) {
+
+    /**
+     * Removes a storage session for {@code vol} and waits for exit.
+     *
+     * Does nothing if {@link #shouldHandle} is {@code false}
+     *
+     * Any errors are ignored
+     *
+     * Call {@link #onVolumeRemove} to remove the connection without waiting for exit
+     */
+    public void onVolumeUnmount(VolumeInfo vol) {
+        StorageUserConnection connection = onVolumeRemove(vol);
+
+        Slog.i(TAG, "On volume unmount " + vol);
+        if (connection != null) {
+            String sessionId = vol.getId();
+
+            if (isReady()) {
                 try {
-                    Slog.i(TAG, "Restarting all sessions for user: " + userId);
-                    connection.startAllSessions();
+                    connection.removeSessionAndWait(sessionId);
                 } catch (ExternalStorageServiceException e) {
-                    Slog.e(TAG, "Failed to start all sessions", e);
+                    Slog.e(TAG, "Failed to end session for vol with id: " + sessionId, e);
                 }
             } else {
-                // TODO(b/135341433): What does this mean in multi-user
+                Slog.i(TAG, "Controller not initialised, session not ended " + sessionId);
             }
         }
     }
 
-    /** Ends all sessions for {@code userId}. */
-    public void onUserRemoved(int userId) {
+    /**
+     * Restarts all sessions for {@code userId}.
+     *
+     * Does nothing if {@link #shouldHandle} is {@code false}
+     *
+     * This call blocks and waits for all sessions to be started, however any failures when starting
+     * a session will be ignored.
+     */
+    public void onUnlockUser(int userId) throws ExternalStorageServiceException {
+        if (!shouldHandle(null)) {
+            return;
+        }
+
+        Slog.i(TAG, "On user unlock " + userId);
+        if (userId == 0) {
+            init();
+        }
+
+        StorageUserConnection connection = null;
         synchronized (mLock) {
-            StorageUserConnection connection = mConnections.get(userId);
-            if (connection != null) {
+            connection = mConnections.get(userId);
+        }
+
+        if (connection != null) {
+            Slog.i(TAG, "Restarting all sessions for user: " + userId);
+            connection.startAllSessions();
+        } else {
+            Slog.w(TAG, "No connection found for user: " + userId);
+        }
+    }
+
+    /**
+     * Resets all sessions for all users and waits for exit. This may kill the
+     * {@link ExternalStorageservice} for a user if necessary to ensure all state has been reset.
+     *
+     * Does nothing if {@link #shouldHandle} is {@code false}
+     **/
+    public void onReset(IVold vold, Handler handler) {
+        if (!shouldHandle(null)) {
+            return;
+        }
+
+        if (!isReady()) {
+            synchronized (mLock) {
+                mConnections.clear();
+            }
+            return;
+        }
+
+        SparseArray<StorageUserConnection> connections = new SparseArray();
+        synchronized (mLock) {
+            mIsResetting = true;
+            Slog.i(TAG, "Started resetting external storage service...");
+            for (int i = 0; i < mConnections.size(); i++) {
+                connections.put(mConnections.keyAt(i), mConnections.valueAt(i));
+            }
+        }
+
+        for (int i = 0; i < connections.size(); i++) {
+            StorageUserConnection connection = connections.valueAt(i);
+            for (String sessionId : connection.getAllSessionIds()) {
                 try {
-                    Slog.i(TAG, "Ending all sessions for user: " + userId);
-                    connection.endAllSessions();
-                    mConnections.remove(userId);
-                } catch (ExternalStorageServiceException e) {
-                    Slog.e(TAG, "Failed to end all sessions", e);
+                    Slog.i(TAG, "Unmounting " + sessionId);
+                    vold.unmount(sessionId);
+                    Slog.i(TAG, "Unmounted " + sessionId);
+                } catch (ServiceSpecificException | RemoteException e) {
+                    // TODO(b/140025078): Hard reset vold?
+                    Slog.e(TAG, "Failed to unmount volume: " + sessionId, e);
+                }
+
+                try {
+                    Slog.i(TAG, "Exiting " + sessionId);
+                    connection.removeSessionAndWait(sessionId);
+                    Slog.i(TAG, "Exited " + sessionId);
+                } catch (IllegalStateException | ExternalStorageServiceException e) {
+                    Slog.e(TAG, "Failed to exit session: " + sessionId
+                            + ". Killing MediaProvider...", e);
+                    // If we failed to confirm the session exited, it is risky to proceed
+                    // We kill the ExternalStorageService as a last resort
+                    killExternalStorageService(connections.keyAt(i));
+                    break;
                 }
-            } else {
-                // TODO(b/135341433): What does this mean in multi-user
             }
+            connection.close();
+        }
+
+        handler.removeCallbacksAndMessages(null);
+        synchronized (mLock) {
+            mConnections.clear();
+            mIsResetting = false;
+            Slog.i(TAG, "Finished resetting external storage service");
         }
     }
 
+    private void init() throws ExternalStorageServiceException {
+        Slog.i(TAG, "Initialialising...");
+        ProviderInfo provider = mContext.getPackageManager().resolveContentProvider(
+                MediaStore.AUTHORITY, PackageManager.MATCH_DIRECT_BOOT_AWARE
+                | PackageManager.MATCH_DIRECT_BOOT_UNAWARE
+                | PackageManager.MATCH_SYSTEM_ONLY);
+        if (provider == null) {
+            throw new ExternalStorageServiceException("No valid MediaStore provider found");
+        }
+
+        mExternalStorageServicePackageName = provider.applicationInfo.packageName;
+        mExternalStorageServiceAppId = UserHandle.getAppId(provider.applicationInfo.uid);
+
+        Intent intent = new Intent(ExternalStorageService.SERVICE_INTERFACE);
+        intent.setPackage(mExternalStorageServicePackageName);
+        ResolveInfo resolveInfo = mContext.getPackageManager().resolveService(intent,
+                PackageManager.GET_SERVICES | PackageManager.GET_META_DATA);
+        if (resolveInfo == null || resolveInfo.serviceInfo == null) {
+            throw new ExternalStorageServiceException(
+                    "No valid ExternalStorageService component found");
+        }
+
+        ServiceInfo serviceInfo = resolveInfo.serviceInfo;
+        ComponentName name = new ComponentName(serviceInfo.packageName, serviceInfo.name);
+        if (!Manifest.permission.BIND_EXTERNAL_STORAGE_SERVICE
+                .equals(serviceInfo.permission)) {
+            throw new ExternalStorageServiceException(name.flattenToShortString()
+                    + " does not require permission "
+                    + Manifest.permission.BIND_EXTERNAL_STORAGE_SERVICE);
+        }
+
+        mExternalStorageServiceComponent = name;
+    }
+
     /** Returns the {@link ExternalStorageService} component name. */
     @Nullable
     public ComponentName getExternalStorageServiceComponentName() {
-        synchronized (mLock) {
-            if (mExternalStorageServiceComponent == null) {
-                ProviderInfo provider = mContext.getPackageManager().resolveContentProvider(
-                        MediaStore.AUTHORITY, PackageManager.MATCH_DIRECT_BOOT_AWARE
-                        | PackageManager.MATCH_DIRECT_BOOT_UNAWARE
-                        | PackageManager.MATCH_SYSTEM_ONLY);
-
-                if (provider == null) {
-                    Slog.e(TAG, "No valid MediaStore provider found.");
-                }
-                String packageName = provider.applicationInfo.packageName;
-
-                Intent intent = new Intent(ExternalStorageService.SERVICE_INTERFACE);
-                intent.setPackage(packageName);
-                ResolveInfo resolveInfo = mContext.getPackageManager().resolveService(intent,
-                        PackageManager.GET_SERVICES | PackageManager.GET_META_DATA);
-                if (resolveInfo == null || resolveInfo.serviceInfo == null) {
-                    Slog.e(TAG, "No valid ExternalStorageService component found.");
-                    return null;
-                }
+        return mExternalStorageServiceComponent;
+    }
 
-                ServiceInfo serviceInfo = resolveInfo.serviceInfo;
-                ComponentName name = new ComponentName(serviceInfo.packageName, serviceInfo.name);
-                if (!Manifest.permission.BIND_EXTERNAL_STORAGE_SERVICE
-                        .equals(serviceInfo.permission)) {
-                    Slog.e(TAG, name.flattenToShortString() + " does not require permission "
-                            + Manifest.permission.BIND_EXTERNAL_STORAGE_SERVICE);
-                    return null;
-                }
-                mExternalStorageServiceComponent = name;
-            }
-            return mExternalStorageServiceComponent;
+    private void killExternalStorageService(int userId) {
+        IActivityManager am = ActivityManager.getService();
+        try {
+            am.killApplication(mExternalStorageServicePackageName, mExternalStorageServiceAppId,
+                    userId, "storage_session_controller reset");
+        } catch (RemoteException e) {
+            Slog.i(TAG, "Failed to kill the ExtenalStorageService for user " + userId);
         }
     }
 
-    /** Returns the {@link StorageManagerService} callback. */
-    public Callback getCallback() {
-        return mCallback;
+    /**
+     * Throws an {@link IllegalStateException} if {@code path} is not ready to be accessed by
+     * {@code userId}.
+     */
+    // TODO(b/144332951): This is not used because it is racy. Right after checking a path
+    // we can call into vold with that path and the FUSE daemon can go down. Improve or remove
+    public void checkPathReadyForUser(int userId, String path) {
+        if (!mIsFuseEnabled) {
+            return;
+        }
+
+        if (mIsResetting) {
+            throw new IllegalStateException("Connection resetting for user " + userId
+                    + " with path " + path);
+        }
+
+        StorageUserConnection connection = null;
+        synchronized (mLock) {
+            connection = mConnections.get(userId);
+        }
+
+        if (connection == null) {
+            throw new IllegalStateException("Connection not ready for user " + userId
+                    + " with path " + path);
+        }
+        connection.checkPathReady(path);
     }
 
-    /** Callback to listen to session events from the {@link StorageSessionController}. */
-    public interface Callback {
-        /** Called when a {@link StorageUserConnection} is disconnected. */
-        void onUserDisconnected(int userId);
+    /**
+     * Returns {@code true} if {@code vol} is an emulated or public volume,
+     * {@code false} otherwise
+     **/
+    public static boolean isEmulatedOrPublic(VolumeInfo vol) {
+        return vol.type == VolumeInfo.TYPE_EMULATED || vol.type == VolumeInfo.TYPE_PUBLIC;
     }
 
-    /** Exception thrown when communication with the {@link ExternalStorageService}. */
+    /** Exception thrown when communication with the {@link ExternalStorageService} fails. */
     public static class ExternalStorageServiceException extends Exception {
         public ExternalStorageServiceException(Throwable cause) {
             super(cause);
         }
+
+        public ExternalStorageServiceException(String message) {
+            super(message);
+        }
+
+        public ExternalStorageServiceException(String message, Throwable cause) {
+            super(message, cause);
+        }
+    }
+
+    private boolean shouldHandle(@Nullable VolumeInfo vol) {
+        return mIsFuseEnabled && !mIsResetting && (vol == null || isEmulatedOrPublic(vol));
+    }
+
+    private boolean isReady() {
+        return mExternalStorageServiceComponent != null;
     }
 }
diff --git a/services/core/java/com/android/server/storage/StorageUserConnection.java b/services/core/java/com/android/server/storage/StorageUserConnection.java
index ff9c900958ea..24b56a48900b 100644
--- a/services/core/java/com/android/server/storage/StorageUserConnection.java
+++ b/services/core/java/com/android/server/storage/StorageUserConnection.java
@@ -33,29 +33,31 @@ import android.os.IBinder;
 import android.os.ParcelFileDescriptor;
 import android.os.ParcelableException;
 import android.os.RemoteCallback;
-import android.os.RemoteException;
 import android.os.UserHandle;
-import android.os.storage.VolumeInfo;
 import android.service.storage.ExternalStorageService;
 import android.service.storage.IExternalStorageService;
+import android.text.TextUtils;
 import android.util.Slog;
 
 import com.android.internal.annotations.GuardedBy;
 import com.android.internal.util.Preconditions;
 
-import java.io.FileDescriptor;
 import java.io.IOException;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.Map;
+import java.util.Set;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
 
 /**
  * Controls the lifecycle of the {@link ActiveConnection} to an {@link ExternalStorageService}
- * for a user and manages storage sessions represented by a {@link Session}.
+ * for a user and manages storage sessions associated with mounted volumes.
  */
 public final class StorageUserConnection {
     private static final String TAG = "StorageUserConnection";
+    private static final int REMOTE_TIMEOUT_SECONDS = 15;
 
     private final Object mLock = new Object();
     private final Context mContext;
@@ -70,68 +72,188 @@ public final class StorageUserConnection {
         mSessionController = controller;
     }
 
-    /** Starts a session for a user */
-    public void startSession(FileDescriptor deviceFd, VolumeInfo vol)
-            throws ExternalStorageServiceException {
-        String sessionId = vol.getId();
-        String upperPath = vol.getPath().getPath();
-        String lowerPath = vol.getInternalPath().getPath();
-        Slog.i(TAG, "Starting session with id: " + sessionId + " and upperPath: " + upperPath
-                + " and lowerPath: " + lowerPath);
-        Session session = new Session(sessionId, deviceFd, upperPath, lowerPath);
+    /**
+     * Creates and stores a storage {@link Session}.
+     *
+     * Created sessions must be initialised with {@link #initSession} before starting with
+     * {@link #startSession}.
+     *
+     * They must also be cleaned up with {@link #removeSession}.
+     *
+     * @throws IllegalArgumentException if a {@code Session} with {@code sessionId} already exists
+     */
+    public void createSession(String sessionId, ParcelFileDescriptor pfd) {
+        Preconditions.checkNotNull(sessionId);
+        Preconditions.checkNotNull(pfd);
+
+        synchronized (mLock) {
+            Preconditions.checkArgument(!mSessions.containsKey(sessionId));
+            mSessions.put(sessionId, new Session(sessionId, pfd));
+        }
+    }
+
+    /**
+     * Initialise a storage {@link Session}.
+     *
+     * Initialised sessions can be started with {@link #startSession}.
+     *
+     * They must also be cleaned up with {@link #removeSession}.
+     *
+     * @throws IllegalArgumentException if {@code sessionId} does not exist or is initialised
+     */
+    public void initSession(String sessionId, String upperPath, String lowerPath) {
+        synchronized (mLock) {
+            Session session = mSessions.get(sessionId);
+            if (session == null) {
+                throw new IllegalStateException("Failed to initialise non existent session. Id: "
+                        + sessionId + ". Upper path: " + upperPath + ". Lower path: " + lowerPath);
+            } else if (session.isInitialisedLocked()) {
+                throw new IllegalStateException("Already initialised session. Id: "
+                        + sessionId + ". Upper path: " + upperPath + ". Lower path: " + lowerPath);
+            } else {
+                session.upperPath = upperPath;
+                session.lowerPath = lowerPath;
+                Slog.i(TAG, "Initialised session: " + session);
+            }
+        }
+    }
+
+    /**
+     * Starts an already created storage {@link Session} for {@code sessionId}.
+     *
+     * It is safe to call this multiple times, however if the session is already started,
+     * subsequent calls will be ignored.
+     *
+     * @throws ExternalStorageServiceException if the session failed to start
+     **/
+    public void startSession(String sessionId) throws ExternalStorageServiceException {
+        Session session;
+        synchronized (mLock) {
+            session = mSessions.get(sessionId);
+        }
+
+        prepareRemote();
         synchronized (mLock) {
-            // TODO(b/135341433): Ensure we don't replace a session without ending the previous
-            mSessions.put(sessionId, session);
-            // TODO(b/135341433): If this fails, maybe its at boot, how to handle if not boot?
             mActiveConnection.startSessionLocked(session);
         }
     }
 
     /**
-     * Ends a session for a user.
+     * Removes a session without ending it or waiting for exit.
      *
-     * @return {@code true} if there are no more sessions for this user, {@code false} otherwise
+     * This should only be used if the session has certainly been ended because the volume was
+     * unmounted or the user running the session has been stopped. Otherwise, wait for session
+     * with {@link #waitForExit}.
      **/
-    public boolean endSession(VolumeInfo vol) throws ExternalStorageServiceException {
+    public Session removeSession(String sessionId) {
         synchronized (mLock) {
-            Session session = mSessions.remove(vol.getId());
+            Session session = mSessions.remove(sessionId);
             if (session != null) {
-                mActiveConnection.endSessionLocked(session);
-                mSessions.remove(session.sessionId);
-            }
-            boolean isAllSessionsEnded = mSessions.isEmpty();
-            if (isAllSessionsEnded) {
-                mActiveConnection.close();
+                session.close();
+                return session;
             }
-            return isAllSessionsEnded;
+            return null;
+        }
+    }
+
+
+    /**
+     * Removes a session and waits for exit
+     *
+     * @throws ExternalStorageServiceException if the session may not have exited
+     **/
+    public void removeSessionAndWait(String sessionId) throws ExternalStorageServiceException {
+        Session session = removeSession(sessionId);
+        if (session == null) {
+            Slog.i(TAG, "No session found for id: " + sessionId);
+            return;
+        }
+
+        Slog.i(TAG, "Waiting for session end " + session + " ...");
+        prepareRemote();
+        synchronized (mLock) {
+            mActiveConnection.endSessionLocked(session);
         }
     }
 
-    /** Starts all available sessions for a user */
-    public void startAllSessions() throws ExternalStorageServiceException {
+    /** Starts all available sessions for a user without blocking. Any failures will be ignored. */
+    public void startAllSessions() {
+        try {
+            prepareRemote();
+        } catch (ExternalStorageServiceException e) {
+            Slog.e(TAG, "Failed to start all sessions for user: " + mUserId, e);
+            return;
+        }
+
         synchronized (mLock) {
+            Slog.i(TAG, "Starting " + mSessions.size() + " sessions for user: " + mUserId + "...");
             for (Session session : mSessions.values()) {
-                mActiveConnection.startSessionLocked(session);
+                try {
+                    mActiveConnection.startSessionLocked(session);
+                } catch (IllegalStateException | ExternalStorageServiceException e) {
+                    // TODO: Don't crash process? We could get into process crash loop
+                    Slog.e(TAG, "Failed to start " + session, e);
+                }
             }
         }
     }
 
-    /** Ends all available sessions for a user */
-    public void endAllSessions() throws ExternalStorageServiceException {
+    /**
+     * Closes the connection to the {@link ExternalStorageService}. The connection will typically
+     * be restarted after close.
+     */
+    public void close() {
+        mActiveConnection.close();
+    }
+
+    /** Throws an {@link IllegalArgumentException} if {@code path} is not ready for access */
+    public void checkPathReady(String path) {
         synchronized (mLock) {
             for (Session session : mSessions.values()) {
-                mActiveConnection.endSessionLocked(session);
-                mSessions.remove(session.sessionId);
+                if (session.upperPath != null && path.startsWith(session.upperPath)) {
+                    if (mActiveConnection.isActiveLocked(session)) {
+                        return;
+                    }
+                }
+            }
+            throw new IllegalStateException("Path not ready " + path);
+        }
+    }
+
+    /** Returns all created sessions. */
+    public Set<String> getAllSessionIds() {
+        synchronized (mLock) {
+            return new HashSet<>(mSessions.keySet());
+        }
+    }
+
+    private void prepareRemote() throws ExternalStorageServiceException {
+        try {
+            waitForLatch(mActiveConnection.bind(), "remote_prepare_user " + mUserId);
+        } catch (IllegalStateException | TimeoutException e) {
+            throw new ExternalStorageServiceException("Failed to prepare remote", e);
+        }
+    }
+
+    private void waitForLatch(CountDownLatch latch, String reason) throws TimeoutException {
+        try {
+            if (!latch.await(REMOTE_TIMEOUT_SECONDS, TimeUnit.SECONDS)) {
+                // TODO(b/140025078): Call ActivityManager ANR API?
+                throw new TimeoutException("Latch wait for " + reason + " elapsed");
             }
-            mActiveConnection.close();
+        } catch (InterruptedException e) {
+            Thread.currentThread().interrupt();
+            throw new IllegalStateException("Latch wait for " + reason + " interrupted");
         }
     }
 
     private final class ActiveConnection implements AutoCloseable {
         // Lifecycle connection to the external storage service, needed to unbind.
-        // We should only try to bind if mServiceConnection is null.
-        // Non-null indicates we are connected or connecting.
         @GuardedBy("mLock") @Nullable private ServiceConnection mServiceConnection;
+        // True if we are connecting, either bound or binding
+        // False && mRemote != null means we are connected
+        // False && mRemote == null means we are neither connecting nor connected
+        @GuardedBy("mLock") @Nullable private boolean mIsConnecting;
         // Binder object representing the external storage service.
         // Non-null indicates we are connected
         @GuardedBy("mLock") @Nullable private IExternalStorageService mRemote;
@@ -141,58 +263,72 @@ public final class StorageUserConnection {
         // (and clear the exception state) with the same lock which we hold during
         // the entire transaction, there is no risk of race.
         @GuardedBy("mLock") @Nullable private ParcelableException mLastException;
+        // Not guarded by any lock intentionally and non final because we cannot
+        // reset latches so need to create a new one after one use
+        private CountDownLatch mLatch;
 
         @Override
         public void close() {
+            ServiceConnection oldConnection = null;
             synchronized (mLock) {
-                if (mServiceConnection != null) {
-                    mContext.unbindService(mServiceConnection);
-                }
+                Slog.i(TAG, "Closing connection for user " + mUserId);
+                mIsConnecting = false;
+                oldConnection = mServiceConnection;
                 mServiceConnection = null;
                 mRemote = null;
             }
+
+            if (oldConnection != null) {
+                mContext.unbindService(oldConnection);
+            }
+        }
+
+        public boolean isActiveLocked(Session session) {
+            if (!session.isInitialisedLocked()) {
+                Slog.i(TAG, "Session not initialised " + session);
+                return false;
+            }
+
+            if (mRemote == null) {
+                throw new IllegalStateException("Valid session with inactive connection");
+            }
+            return true;
         }
 
         public void startSessionLocked(Session session) throws ExternalStorageServiceException {
-            if (mServiceConnection == null || mRemote == null) {
-                if (mServiceConnection == null) {
-                    // Not bound
-                    bindLocked();
-                } // else we are binding. In any case when we bind we'll re-start all sessions
+            if (!isActiveLocked(session)) {
                 return;
             }
 
             CountDownLatch latch = new CountDownLatch(1);
-            try {
+            try (ParcelFileDescriptor dupedPfd = session.pfd.dup()) {
                 mRemote.startSession(session.sessionId,
                         FLAG_SESSION_TYPE_FUSE | FLAG_SESSION_ATTRIBUTE_INDEXABLE,
-                        new ParcelFileDescriptor(session.deviceFd), session.upperPath,
-                        session.lowerPath, new RemoteCallback(result ->
+                        dupedPfd, session.upperPath, session.lowerPath, new RemoteCallback(result ->
                                 setResultLocked(latch, result)));
-
-            } catch (RemoteException e) {
-                throw new ExternalStorageServiceException(e);
+                waitForLatch(latch, "start_session " + session);
+                maybeThrowExceptionLocked();
+            } catch (Exception e) {
+                throw new ExternalStorageServiceException("Failed to start session: " + session, e);
             }
-            waitAndReturnResultLocked(latch);
         }
 
         public void endSessionLocked(Session session) throws ExternalStorageServiceException {
-            if (mRemote == null) {
-                // TODO(b/135341433): This assumes if there is no connection, there are no
-                // session resources held. Need to document in the ExternalStorageService
-                // API that implementors should end all sessions and clean up resources
-                // when the binding is lost, onDestroy?
+            session.close();
+            if (!isActiveLocked(session)) {
+                // Nothing to end, not started yet
                 return;
             }
 
             CountDownLatch latch = new CountDownLatch(1);
             try {
                 mRemote.endSession(session.sessionId, new RemoteCallback(result ->
-                                setResultLocked(latch, result)));
-            } catch (RemoteException e) {
-                throw new ExternalStorageServiceException(e);
+                        setResultLocked(latch, result)));
+                waitForLatch(latch, "end_session " + session);
+                maybeThrowExceptionLocked();
+            } catch (Exception e) {
+                throw new ExternalStorageServiceException("Failed to end session: " + session, e);
             }
-            waitAndReturnResultLocked(latch);
         }
 
         private void setResultLocked(CountDownLatch latch, Bundle result) {
@@ -200,36 +336,38 @@ public final class StorageUserConnection {
             latch.countDown();
         }
 
-        private void waitAndReturnResultLocked(CountDownLatch latch)
-                throws ExternalStorageServiceException {
-            try {
-                // TODO(b/140025078): Call ActivityManager ANR API?
-                latch.await(20, TimeUnit.SECONDS);
-            } catch (InterruptedException e) {
-                Thread.currentThread().interrupt();
-                throw new IllegalStateException(
-                        "Interrupted while waiting for ExternalStorageService result");
-            }
+        private void maybeThrowExceptionLocked() throws IOException {
             if (mLastException != null) {
+                ParcelableException lastException = mLastException;
                 mLastException = null;
                 try {
-                    mLastException.maybeRethrow(IOException.class);
+                    lastException.maybeRethrow(IOException.class);
                 } catch (IOException e) {
-                    throw new ExternalStorageServiceException(e);
+                    throw e;
                 }
-                throw new RuntimeException(mLastException);
+                throw new RuntimeException(lastException);
             }
-            mLastException = null;
         }
 
-        private void bindLocked() {
+        public CountDownLatch bind() throws ExternalStorageServiceException {
             ComponentName name = mSessionController.getExternalStorageServiceComponentName();
             if (name == null) {
-                Slog.i(TAG, "Not ready to bind to the ExternalStorageService for user " + mUserId);
-                return;
+                // Not ready to bind
+                throw new ExternalStorageServiceException(
+                        "Not ready to bind to the ExternalStorageService for user " + mUserId);
             }
 
-            ServiceConnection connection = new ServiceConnection() {
+            synchronized (mLock) {
+                if (mRemote != null || mIsConnecting) {
+                    // Connected or connecting (bound or binding)
+                    // Will wait on a latch that will countdown when we connect, unless we are
+                    // connected and the latch has already countdown, yay!
+                    return mLatch;
+                } // else neither connected nor connecting
+
+                mLatch = new CountDownLatch(1);
+                mIsConnecting = true;
+                mServiceConnection = new ServiceConnection() {
                     @Override
                     public void onServiceConnected(ComponentName name, IBinder service) {
                         Slog.i(TAG, "Service: [" + name + "] connected. User [" + mUserId + "]");
@@ -255,65 +393,81 @@ public final class StorageUserConnection {
 
                     @Override
                     public void onNullBinding(ComponentName name) {
-                        // Should never happen. Service returned null from #onBind.
                         Slog.wtf(TAG, "Service: [" + name + "] is null. User [" + mUserId + "]");
                     }
 
                     private void handleConnection(IBinder service) {
                         synchronized (mLock) {
-                            if (mServiceConnection != null) {
+                            if (mIsConnecting) {
                                 mRemote = IExternalStorageService.Stub.asInterface(service);
-                            } else {
-                                Slog.wtf(TAG, "Service connected without a connection object??");
+                                mIsConnecting = false;
+                                mLatch.countDown();
+                                // Separate thread so we don't block the main thead
+                                return;
                             }
                         }
-
-                        try {
-                            startAllSessions();
-                        } catch (ExternalStorageServiceException e) {
-                            Slog.e(TAG, "Failed to start all sessions", e);
-                        }
+                        Slog.wtf(TAG, "Connection closed to the ExternalStorageService for user "
+                                + mUserId);
                     }
 
                     private void handleDisconnection() {
-                        close();
                         // Clear all sessions because we will need a new device fd since
                         // StorageManagerService will reset the device mount state and #startSession
                         // will be called for any required mounts.
-                        synchronized (mLock) {
-                            mSessions.clear();
-                        }
                         // Notify StorageManagerService so it can restart all necessary sessions
-                        mSessionController.getCallback().onUserDisconnected(mUserId);
+                        close();
+                        new Thread(StorageUserConnection.this::startAllSessions).start();
                     }
                 };
+            }
 
             Slog.i(TAG, "Binding to the ExternalStorageService for user " + mUserId);
-            // TODO(b/135341433): Verify required service flags BIND_IMPORTANT?
-            if (mContext.bindServiceAsUser(new Intent().setComponent(name), connection,
-                            Context.BIND_AUTO_CREATE, UserHandle.of(mUserId))) {
+            if (mContext.bindServiceAsUser(new Intent().setComponent(name), mServiceConnection,
+                            Context.BIND_AUTO_CREATE | Context.BIND_IMPORTANT,
+                            UserHandle.of(mUserId))) {
                 Slog.i(TAG, "Bound to the ExternalStorageService for user " + mUserId);
-                mServiceConnection = connection;
-                // Reset the remote, we will set when we connect
-                mRemote = null;
+                return mLatch;
             } else {
-                Slog.w(TAG, "Failed to bind to the ExternalStorageService for user " + mUserId);
+                synchronized (mLock) {
+                    mIsConnecting = false;
+                }
+                throw new ExternalStorageServiceException(
+                        "Failed to bind to the ExternalStorageService for user " + mUserId);
             }
         }
     }
 
-    private static final class Session {
+    private static final class Session implements AutoCloseable {
         public final String sessionId;
-        public final FileDescriptor deviceFd;
-        public final String lowerPath;
-        public final String upperPath;
+        public final ParcelFileDescriptor pfd;
+        @GuardedBy("mLock")
+        public String lowerPath;
+        @GuardedBy("mLock")
+        public String upperPath;
 
-        Session(String sessionId, FileDescriptor deviceFd, String upperPath,
-                String lowerPath) {
+        Session(String sessionId, ParcelFileDescriptor pfd) {
             this.sessionId = sessionId;
-            this.upperPath = upperPath;
-            this.lowerPath = lowerPath;
-            this.deviceFd = deviceFd;
+            this.pfd = pfd;
+        }
+
+        @Override
+        public void close() {
+            try {
+                pfd.close();
+            } catch (IOException e) {
+                Slog.i(TAG, "Failed to close session: " + this);
+            }
+        }
+
+        @Override
+        public String toString() {
+            return "[SessionId: " + sessionId + ". UpperPath: " + upperPath + ". LowerPath: "
+                    + lowerPath + "]";
+        }
+
+        @GuardedBy("mLock")
+        public boolean isInitialisedLocked() {
+            return !TextUtils.isEmpty(upperPath) && !TextUtils.isEmpty(lowerPath);
         }
     }
 }