diff --git a/services/core/java/com/android/server/StorageManagerService.java b/services/core/java/com/android/server/StorageManagerService.java index dcc690fa09b44..9c1d2fec16402 100644 --- a/services/core/java/com/android/server/StorageManagerService.java +++ b/services/core/java/com/android/server/StorageManagerService.java @@ -80,6 +80,7 @@ import android.os.IBinder; import android.os.IStoraged; import android.os.IVold; import android.os.IVoldListener; +import android.os.IVoldMountCallback; import android.os.IVoldTaskListener; import android.os.Looper; import android.os.Message; @@ -348,10 +349,6 @@ class StorageManagerService extends IStorageManager.Stub @GuardedBy("mLock") private ArrayMap mDiskScanLatches = new ArrayMap<>(); - /** Map from volume ID to latches */ - @GuardedBy("mLock") - private ArrayMap mFuseVolumeReadyLatches = new ArrayMap<>(); - @GuardedBy("mLock") private IPackageMoveObserver mMoveCallback; @GuardedBy("mLock") @@ -464,17 +461,6 @@ 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 */ @@ -612,7 +598,6 @@ 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) { @@ -673,22 +658,6 @@ 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)) { @@ -1431,13 +1400,6 @@ 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 @@ -1912,33 +1874,29 @@ class StorageManagerService extends IStorageManager.Stub 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 void mount(VolumeInfo vol) { try { // 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); + mVold.mount(vol.id, vol.mountFlags, vol.mountUserId, new IVoldMountCallback.Stub() { + @Override + public boolean onVolumeChecking(FileDescriptor deviceFd, String path, + String internalPath) { + vol.path = path; + vol.internalPath = internalPath; + try { + mStorageSessionController.onVolumeMount(deviceFd, vol); + return true; + } catch (ExternalStorageServiceException e) { + Slog.i(TAG, "Failed to mount volume " + vol, e); + return false; + } + } + }); Slog.i(TAG, "Mounted volume " + vol); - mStorageSessionController.onVolumeMount(fd, vol); } catch (Exception e) { Slog.wtf(TAG, e); } @@ -3648,7 +3606,7 @@ class StorageManagerService extends IStorageManager.Stub try { mObbState.volId = mVold.createObb(mObbState.canonicalPath, binderKey, mObbState.ownerGid); - mVold.mount(mObbState.volId, 0, -1); + mVold.mount(mObbState.volId, 0, -1, null); if (DEBUG_OBB) Slog.d(TAG, "Successfully mounted OBB " + mObbState.canonicalPath); diff --git a/services/core/java/com/android/server/storage/StorageSessionController.java b/services/core/java/com/android/server/storage/StorageSessionController.java index 72a1b9df3b3c3..d76836f90e6b0 100644 --- a/services/core/java/com/android/server/storage/StorageSessionController.java +++ b/services/core/java/com/android/server/storage/StorageSessionController.java @@ -69,15 +69,21 @@ public final class StorageSessionController { } /** - * 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}. + * Creates and starts 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}. + * + * Throws an {@link IllegalStateException} if a session for {@code vol} has already been created * * Does nothing if {@link #shouldHandle} is {@code false} * + * Blocks until the session is started or fails + * + * @throws ExternalStorageServiceException if the session fails to start * @throws IllegalStateException if a session has already been created for {@code vol} */ - public void onVolumeMount(FileDescriptor deviceFd, VolumeInfo vol) { + public void onVolumeMount(FileDescriptor deviceFd, VolumeInfo vol) + throws ExternalStorageServiceException { if (!shouldHandle(vol)) { return; } @@ -87,71 +93,22 @@ public final class StorageSessionController { String sessionId = vol.getId(); int userId = vol.getMountUserId(); - if (deviceFd == null) { - Slog.w(TAG, "Null fd. Session not started for vol: " + vol); - return; - } - - // Get realpath for the fd, paths that are not /dev/null need additional - // setup by the ExternalStorageService before they can be ready - String realPath; - try { - realPath = ParcelFileDescriptor.getFile(deviceFd).getPath(); - } catch (IOException e) { - Slog.wtf(TAG, "Could not get real path from fd: " + deviceFd, e); - return; - } - - if ("/dev/null".equals(realPath)) { - Slog.i(TAG, "Volume ready for use with id: " + sessionId); - return; - } - + StorageUserConnection connection = null; synchronized (mLock) { - StorageUserConnection connection = mConnections.get(userId); + connection = mConnections.get(userId); if (connection == null) { + Slog.i(TAG, "Creating connection for user: " + userId); connection = new StorageUserConnection(mContext, userId, this); mConnections.put(userId, connection); } 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; + connection.createSession(sessionId, new ParcelFileDescriptor(deviceFd), + vol.getPath().getPath(), vol.getInternalPath().getPath()); } - 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()) { + // At boot, a volume can be mounted before user is unlocked, in that case, we create it + // above and save it so that we can restart all sessions when the user is unlocked + if (mExternalStorageServiceComponent != null) { connection.startSession(sessionId); } else { Slog.i(TAG, "Controller not initialised, session not started " + sessionId); @@ -205,14 +162,10 @@ public final class StorageSessionController { if (connection != null) { String sessionId = vol.getId(); - if (isReady()) { - try { - connection.removeSessionAndWait(sessionId); - } catch (ExternalStorageServiceException e) { - Slog.e(TAG, "Failed to end session for vol with id: " + sessionId, e); - } - } else { - Slog.i(TAG, "Controller not initialised, session not ended " + sessionId); + try { + connection.removeSessionAndWait(sessionId); + } catch (ExternalStorageServiceException e) { + Slog.e(TAG, "Failed to end session for vol with id: " + sessionId, e); } } } @@ -232,7 +185,7 @@ public final class StorageSessionController { Slog.i(TAG, "On user unlock " + userId); if (userId == 0) { - init(); + initExternalStorageServiceComponent(); } StorageUserConnection connection = null; @@ -259,13 +212,6 @@ public final class StorageSessionController { return; } - if (!isReady()) { - synchronized (mLock) { - mConnections.clear(); - } - return; - } - SparseArray connections = new SparseArray(); synchronized (mLock) { mIsResetting = true; @@ -311,7 +257,7 @@ public final class StorageSessionController { } } - private void init() throws ExternalStorageServiceException { + private void initExternalStorageServiceComponent() throws ExternalStorageServiceException { Slog.i(TAG, "Initialialising..."); ProviderInfo provider = mContext.getPackageManager().resolveContentProvider( MediaStore.AUTHORITY, PackageManager.MATCH_DIRECT_BOOT_AWARE @@ -415,8 +361,4 @@ public final class StorageSessionController { 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 24b56a48900b2..7c4773033518e 100644 --- a/services/core/java/com/android/server/storage/StorageUserConnection.java +++ b/services/core/java/com/android/server/storage/StorageUserConnection.java @@ -75,46 +75,20 @@ public final class StorageUserConnection { /** * 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) { + public void createSession(String sessionId, ParcelFileDescriptor pfd, String upperPath, + String lowerPath) { Preconditions.checkNotNull(sessionId); Preconditions.checkNotNull(pfd); + Preconditions.checkNotNull(upperPath); + Preconditions.checkNotNull(lowerPath); 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); - } + mSessions.put(sessionId, new Session(sessionId, pfd, upperPath, lowerPath)); } } @@ -440,14 +414,14 @@ public final class StorageUserConnection { private static final class Session implements AutoCloseable { public final String sessionId; public final ParcelFileDescriptor pfd; - @GuardedBy("mLock") - public String lowerPath; - @GuardedBy("mLock") - public String upperPath; + public final String lowerPath; + public final String upperPath; - Session(String sessionId, ParcelFileDescriptor pfd) { + Session(String sessionId, ParcelFileDescriptor pfd, String upperPath, String lowerPath) { this.sessionId = sessionId; this.pfd = pfd; + this.upperPath = upperPath; + this.lowerPath = lowerPath; } @Override