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

Commit 02c4a81e authored by Hai Zhang's avatar Hai Zhang
Browse files

Revert "Revert "Persist permission state for updated apps synchronously."" 

This reverts commit cebec6c3.

The root cause for the previous regression is that when a new user is
added, the replace parameter will be true for every package (despite they
aren't being actually replaced),  thus a synchronous write will be done
for every one of them and the PM lock will be blocked and main thread
will be held for a very long time, causing ANR in apps calling into
system server at the same time.

The proper fix would be renaming the replace parameter and the relevant
flags like UPDATE_PERMISSIONS_REPLACE(_ALL) into something like
regrantInstallPermissions that better describes its use, and then
introduce another parameter that actually represents whether the package
is being replaced. But since we are late in T now and naming is hard,
this double revert simply re-purposes the existing changing package name
to indicate whether there is actually a changing package, and only write
synchronously in that case.

Fixes: 221899913
Test: android.platform.test.scenario.sysui.multiuser.AddGuest
Change-Id: Ifa40b435c2ad8d04050fd5c5972408e61aeea64d
parent e020d397

services/core/java/com/android/server/pm/permission/PermissionManagerServiceImpl.java

+20 −8
Original line number Diff line number Diff line
@@ -2493,13 +2493,13 @@ public class PermissionManagerServiceImpl implements PermissionManagerServiceInt 
     * @param pkg the package the permissions belong to

     * @param replace if the package is getting replaced (this might change the requested

     *                permissions of this package)

     * @param packageOfInterest If this is the name of {@code pkg} add extra logging

     * @param changingPackageName the name of the package that is changing

     * @param callback Result call back

     * @param filterUserId If not {@link UserHandle.USER_ALL}, only restore the permission state for

     *                     this particular user

     */

    private void restorePermissionState(@NonNull AndroidPackage pkg, boolean replace,

            @Nullable String packageOfInterest, @Nullable PermissionCallback callback,

            @Nullable String changingPackageName, @Nullable PermissionCallback callback,

            @UserIdInt int filterUserId) {

        // IMPORTANT: There are two types of permissions: install and runtime.

        // Install time permissions are granted when the app is installed to
@@ -2521,6 +2521,7 @@ public class PermissionManagerServiceImpl implements PermissionManagerServiceInt 
        final int[] userIds = filterUserId == UserHandle.USER_ALL ? getAllUserIds()

                : new int[] { filterUserId };



        boolean installPermissionsChanged = false;

        boolean runtimePermissionsRevoked = false;

        int[] updatedUserIds = EMPTY_INT_ARRAY;
@@ -2639,7 +2640,7 @@ public class PermissionManagerServiceImpl implements PermissionManagerServiceInt 


                UidPermissionState origState = uidState;



                boolean changedInstallPermission = false;

                boolean installPermissionsChangedForUser = false;



                if (replace) {

                    userState.setInstallPermissionsFixed(ps.getPackageName(), false);
@@ -2680,7 +2681,7 @@ public class PermissionManagerServiceImpl implements PermissionManagerServiceInt 
                    //  permission is present, because otherwise the permission should have been

                    //  removed.

                    if (bp == null /*|| getSourcePackageSetting(bp) == null*/) {

                        if (packageOfInterest == null || packageOfInterest.equals(

                        if (changingPackageName == null || changingPackageName.equals(

                                pkg.getPackageName())) {

                            if (DEBUG_PERMISSIONS) {

                                Slog.i(TAG, "Unknown permission " + permName
@@ -2805,7 +2806,7 @@ public class PermissionManagerServiceImpl implements PermissionManagerServiceInt 
                                                    && origState.isPermissionGranted(permName))))) {

                        // Grant an install permission.

                        if (uidState.grantPermission(bp)) {

                            changedInstallPermission = true;

                            installPermissionsChangedForUser = true;

                        }

                    } else if (bp.isRuntime()) {

                        boolean hardRestricted = bp.isHardRestricted();
@@ -2945,12 +2946,12 @@ public class PermissionManagerServiceImpl implements PermissionManagerServiceInt 
                            }

                        }

                        if (uidState.removePermissionState(bp.getName())) {

                            changedInstallPermission = true;

                            installPermissionsChangedForUser = true;

                        }

                    }

                }



                if ((changedInstallPermission || replace)

                if ((installPermissionsChangedForUser || replace)

                        && !userState.areInstallPermissionsFixed(ps.getPackageName())

                        && !ps.isSystem() || ps.getTransientState().isUpdatedSystemApp()) {

                    // This is the first that we have heard about this package, so the
@@ -2959,6 +2960,12 @@ public class PermissionManagerServiceImpl implements PermissionManagerServiceInt 
                    userState.setInstallPermissionsFixed(ps.getPackageName(), true);

                }



                if (installPermissionsChangedForUser) {

                    installPermissionsChanged = true;

                    if (changingPackageName != null && replace) {

                        updatedUserIds = ArrayUtils.appendInt(updatedUserIds, userId);

                    }

                }

                updatedUserIds = revokePermissionsNoLongerImplicitLocked(uidState,

                        pkg.getPackageName(), uidImplicitPermissions, uidTargetSdkVersion, userId,

                        updatedUserIds);
@@ -2975,8 +2982,13 @@ public class PermissionManagerServiceImpl implements PermissionManagerServiceInt 
        // Persist the runtime permissions state for users with changes. If permissions

        // were revoked because no app in the shared user declares them we have to

        // write synchronously to avoid losing runtime permissions state.

        // Also write synchronously if we changed any install permission for an updated app, because

        // the install permission state is likely already fixed before update, and if we lose the

        // changes here the app won't be reconsidered for newly-added install permissions.

        if (callback != null) {

            callback.onPermissionUpdated(updatedUserIds, runtimePermissionsRevoked);

            callback.onPermissionUpdated(updatedUserIds,

                    (changingPackageName != null && replace && installPermissionsChanged)

                            || runtimePermissionsRevoked);

        }



        for (int userId : updatedUserIds) {