Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 8c44db9e authored by Omar Eissa's avatar Omar Eissa
Browse files

Pass volume with correct user id to StorageSessionController 

When remounting public volumes, we pass the volume after setting its
mount user id to the new user, it's going to be mounted to. This is
incorrect, as it doesn't really allow StorageSessionController
clean up the old session from the old user, but tries to clean it up
from the new user which doesn't do thing at the end.

Luckily, we rely on vold to do the real logic of unmounting fuse paths,
which in turn takes care of existing the corresponding fuse daemon.
But the incorrect clean up could cause some unnecessary delays
when further reset operation happens, as we try to kill stale sessions.

Bug: 327173406
Bug: 369519866
Test: atest StorageManagerService & manual
Flag: EXEMPT bug fix
Change-Id: Ie4851f316ea384433f97da84e47cbcbf4c367a7a
parent d9f8e5f2

services/core/java/com/android/server/StorageManagerService.java

+26 −11
Original line number Diff line number Diff line
@@ -161,6 +161,7 @@ import com.android.server.memory.ZramMaintenance; 
import com.android.server.pm.Installer;

import com.android.server.pm.UserManagerInternal;

import com.android.server.storage.AppFuseBridge;

import com.android.server.storage.ImmutableVolumeInfo;

import com.android.server.storage.StorageSessionController;

import com.android.server.storage.StorageSessionController.ExternalStorageServiceException;

import com.android.server.storage.WatchedVolumeInfo;
@@ -778,7 +779,7 @@ class StorageManagerService extends IStorageManager.Stub 
                    break;

                }

                case H_VOLUME_UNMOUNT: {

                    final WatchedVolumeInfo vol = (WatchedVolumeInfo) msg.obj;

                    final ImmutableVolumeInfo vol = (ImmutableVolumeInfo) msg.obj;

                    unmount(vol);

                    break;

                }
@@ -899,8 +900,14 @@ class StorageManagerService extends IStorageManager.Stub 
                        for (int i = 0; i < size; i++) {

                            final WatchedVolumeInfo vol = mVolumes.valueAt(i);

                            if (vol.getMountUserId() == userId) {

                                // Capture the volume before we set mount user id to null,

                                // so that StorageSessionController remove the session from

                                // the correct user (old mount user id)

                                final ImmutableVolumeInfo volToUnmount

                                        = vol.getClonedImmutableVolumeInfo();

                                vol.setMountUserId(UserHandle.USER_NULL);

                                mHandler.obtainMessage(H_VOLUME_UNMOUNT, vol).sendToTarget();

                                mHandler.obtainMessage(H_VOLUME_UNMOUNT, volToUnmount)

                                        .sendToTarget();

                            }

                        }

                    }
@@ -1296,7 +1303,12 @@ class StorageManagerService extends IStorageManager.Stub 
    }



    private void maybeRemountVolumes(int userId) {

        List<WatchedVolumeInfo> volumesToRemount = new ArrayList<>();

        // We need to keep 2 lists

        // 1. List of volumes before we set the mount user Id so that

        // StorageSessionController is able to remove the session from the correct user (old one)

        // 2. List of volumes to mount which should have the up to date info

        List<ImmutableVolumeInfo> volumesToUnmount = new ArrayList<>();

        List<WatchedVolumeInfo> volumesToMount = new ArrayList<>();

        synchronized (mLock) {

            for (int i = 0; i < mVolumes.size(); i++) {

                final WatchedVolumeInfo vol = mVolumes.valueAt(i);
@@ -1304,16 +1316,19 @@ class StorageManagerService extends IStorageManager.Stub 
                        && vol.getMountUserId() != mCurrentUserId) {

                    // If there's a visible secondary volume mounted,

                    // we need to update the currentUserId and remount

                    // But capture the volume with the old user id first to use it in unmounting

                    volumesToUnmount.add(vol.getClonedImmutableVolumeInfo());

                    vol.setMountUserId(mCurrentUserId);

                    volumesToRemount.add(vol);

                    volumesToMount.add(vol);

                }

            }

        }



        for (WatchedVolumeInfo vol : volumesToRemount) {

            Slog.i(TAG, "Remounting volume for user: " + userId + ". Volume: " + vol);

            mHandler.obtainMessage(H_VOLUME_UNMOUNT, vol).sendToTarget();

            mHandler.obtainMessage(H_VOLUME_MOUNT, vol).sendToTarget();

        for (int i = 0; i < volumesToMount.size(); i++) {

            Slog.i(TAG, "Remounting volume for user: " + userId + ". Volume: "

                    + volumesToUnmount.get(i));

            mHandler.obtainMessage(H_VOLUME_UNMOUNT, volumesToUnmount.get(i)).sendToTarget();

            mHandler.obtainMessage(H_VOLUME_MOUNT, volumesToMount.get(i)).sendToTarget();

        }

    }
@@ -2431,10 +2446,10 @@ class StorageManagerService extends IStorageManager.Stub 
        super.unmount_enforcePermission();



        final WatchedVolumeInfo vol = findVolumeByIdOrThrow(volId);

        unmount(vol);

        unmount(vol.getClonedImmutableVolumeInfo());

    }



    private void unmount(WatchedVolumeInfo vol) {

    private void unmount(ImmutableVolumeInfo vol) {

        try {

            try {

                if (vol.getType() == VolumeInfo.TYPE_PRIVATE) {
@@ -2444,7 +2459,7 @@ class StorageManagerService extends IStorageManager.Stub 
                Slog.e(TAG, "Failed unmount mirror data", e);

            }

            mVold.unmount(vol.getId());

            mStorageSessionController.onVolumeUnmount(vol.getImmutableVolumeInfo());

            mStorageSessionController.onVolumeUnmount(vol);

        } catch (Exception e) {

            Slog.wtf(TAG, e);

        }

services/core/java/com/android/server/storage/WatchedVolumeInfo.java

+4 −0
Original line number Diff line number Diff line
@@ -68,6 +68,10 @@ public class WatchedVolumeInfo extends WatchableImpl { 
        return ImmutableVolumeInfo.fromVolumeInfo(mVolumeInfo);

    }



    public ImmutableVolumeInfo getClonedImmutableVolumeInfo() {

        return ImmutableVolumeInfo.fromVolumeInfo(mVolumeInfo.clone());

    }



    public StorageVolume buildStorageVolume(Context context, int userId, boolean reportUnmounted) {

        return mVolumeInfo.buildStorageVolume(context, userId, reportUnmounted);

    }