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

Commit d41db181 authored by Kevin Han's avatar Kevin Han
Browse files

Don't hold hibernation lock when calling other services 

Fix deadlock issues by ensuring that calls to activity manager and
package manager are done without holding the hibernation lock.

This will loosen some guarantees on the hibernation state (e.g. there
may be a brief period of time where an app is hibernated but not
force-stopped), but none of the callers currently need that strong
guarantee.

Bug: 196535754
Test: atest AppHibernationIntegrationTest
Test: atest AppHibernationServiceTest
Test: adb shell cmd jobscheduler run -u 0 -f
com.google.android.permissioncontroller 2

Change-Id: Ic67d9a93565a935c077707e3c34a4c6605f041d1
parent 3c618772

services/core/java/com/android/server/apphibernation/AppHibernationService.java

+30 −42
Original line number Diff line number Diff line
@@ -269,16 +269,16 @@ public final class AppHibernationService extends SystemService { 
        getContext().enforceCallingOrSelfPermission(

                android.Manifest.permission.MANAGE_APP_HIBERNATION,

                "Caller does not have MANAGE_APP_HIBERNATION permission.");

        userId = handleIncomingUser(userId, methodName);

        if (!checkUserStatesExist(userId, methodName)) {

        final int realUserId = handleIncomingUser(userId, methodName);

        if (!checkUserStatesExist(realUserId, methodName)) {

            return;

        }

        synchronized (mLock) {

            final Map<String, UserLevelState> packageStates = mUserStates.get(userId);

            final Map<String, UserLevelState> packageStates = mUserStates.get(realUserId);

            final UserLevelState pkgState = packageStates.get(packageName);

            if (pkgState == null) {

                Slog.e(TAG, String.format("Package %s is not installed for user %s",

                        packageName, userId));

                        packageName, realUserId));

                return;

            }
@@ -286,13 +286,17 @@ public final class AppHibernationService extends SystemService { 
                return;

            }



            pkgState.hibernated = isHibernating;

            if (isHibernating) {

                hibernatePackageForUser(packageName, userId, pkgState);

                mBackgroundExecutor.execute(() -> hibernatePackageForUser(packageName, realUserId));

            } else {

                unhibernatePackageForUser(packageName, userId, pkgState);

                mBackgroundExecutor.execute(

                        () -> unhibernatePackageForUser(packageName, realUserId));

                pkgState.lastUnhibernatedMs = System.currentTimeMillis();

            }



            final UserLevelState stateSnapshot = new UserLevelState(pkgState);

            final int userIdSnapshot = userId;

            final int userIdSnapshot = realUserId;

            mBackgroundExecutor.execute(() -> {

                FrameworkStatsLog.write(

                        FrameworkStatsLog.USER_LEVEL_HIBERNATION_STATE_CHANGED,
@@ -300,8 +304,8 @@ public final class AppHibernationService extends SystemService { 
                        userIdSnapshot,

                        stateSnapshot.hibernated);

            });

            List<UserLevelState> states = new ArrayList<>(mUserStates.get(userId).values());

            mUserDiskStores.get(userId).scheduleWriteHibernationStates(states);

            List<UserLevelState> states = new ArrayList<>(mUserStates.get(realUserId).values());

            mUserDiskStores.get(realUserId).scheduleWriteHibernationStates(states);

        }

    }
@@ -326,10 +330,12 @@ public final class AppHibernationService extends SystemService { 
                return;

            }

            if (state.hibernated != isHibernating) {

                state.hibernated = isHibernating;

                if (isHibernating) {

                    hibernatePackageGlobally(packageName, state);

                    mBackgroundExecutor.execute(() -> hibernatePackageGlobally(packageName, state));

                } else {

                    unhibernatePackageGlobally(packageName, state);

                    state.savedByte = 0;

                    state.lastUnhibernatedMs = System.currentTimeMillis();

                }

                List<GlobalLevelState> states = new ArrayList<>(mGlobalHibernationStates.values());

                mGlobalLevelHibernationDiskStore.scheduleWriteHibernationStates(states);
@@ -366,20 +372,16 @@ public final class AppHibernationService extends SystemService { 
    }



    /**

     * Put an app into hibernation for a given user, allowing user-level optimizations to occur.

     *

     * @param pkgState package hibernation state

     * Put an app into hibernation for a given user, allowing user-level optimizations to occur. Do

     * not hold {@link #mLock} while calling this to avoid deadlock scenarios.

     */

    @GuardedBy("mLock")

    private void hibernatePackageForUser(@NonNull String packageName, int userId,

            @NonNull UserLevelState pkgState) {

    private void hibernatePackageForUser(@NonNull String packageName, int userId) {

        Trace.traceBegin(Trace.TRACE_TAG_SYSTEM_SERVER, "hibernatePackage");

        final long caller = Binder.clearCallingIdentity();

        try {

            mIActivityManager.forceStopPackage(packageName, userId);

            mIPackageManager.deleteApplicationCacheFilesAsUser(packageName, userId,

                    null /* observer */);

            pkgState.hibernated = true;

        } catch (RemoteException e) {

            throw new IllegalStateException(

                    "Failed to hibernate due to manager not being available", e);
@@ -390,16 +392,11 @@ public final class AppHibernationService extends SystemService { 
    }



    /**

     * Remove a package from hibernation for a given user.

     *

     * @param pkgState package hibernation state

     * Remove a package from hibernation for a given user. Do not hold {@link #mLock} while calling

     * this.

     */

    @GuardedBy("mLock")

    private void unhibernatePackageForUser(@NonNull String packageName, int userId,

            UserLevelState pkgState) {

    private void unhibernatePackageForUser(@NonNull String packageName, int userId) {

        Trace.traceBegin(Trace.TRACE_TAG_SYSTEM_SERVER, "unhibernatePackage");

        pkgState.hibernated = false;

        pkgState.lastUnhibernatedMs = System.currentTimeMillis();

        final long caller = Binder.clearCallingIdentity();

        // Deliver LOCKED_BOOT_COMPLETE AND BOOT_COMPLETE broadcast so app can re-register

        // their alarms/jobs/etc.
@@ -450,29 +447,20 @@ public final class AppHibernationService extends SystemService { 
    }



    /**

     * Put a package into global hibernation, optimizing its storage at a package / APK level.

     * Put a package into global hibernation, optimizing its storage at a package / APK level. Do

     * not hold {@link #mLock} while calling this.

     */

    @GuardedBy("mLock")

    private void hibernatePackageGlobally(@NonNull String packageName, GlobalLevelState state) {

        Trace.traceBegin(Trace.TRACE_TAG_SYSTEM_SERVER, "hibernatePackageGlobally");

        long savedBytes = 0;

        if (mOatArtifactDeletionEnabled) {

            state.savedByte = Math.max(

            savedBytes = Math.max(

                    mPackageManagerInternal.deleteOatArtifactsOfPackage(packageName),

                    0);

        }

        state.hibernated = true;

        Trace.traceEnd(Trace.TRACE_TAG_SYSTEM_SERVER);

        synchronized (mLock) {

            state.savedByte = savedBytes;

        }



    /**

     * Unhibernate a package from global hibernation.

     */

    @GuardedBy("mLock")

    private void unhibernatePackageGlobally(@NonNull String packageName, GlobalLevelState state) {

        Trace.traceBegin(Trace.TRACE_TAG_SYSTEM_SERVER, "unhibernatePackageGlobally");

        state.hibernated = false;

        state.savedByte = 0;

        state.lastUnhibernatedMs = System.currentTimeMillis();

        Trace.traceEnd(Trace.TRACE_TAG_SYSTEM_SERVER);

    }

services/tests/servicestests/src/com/android/server/apphibernation/AppHibernationServiceTest.java

+5 −1
Original line number Diff line number Diff line
@@ -146,12 +146,15 @@ public final class AppHibernationServiceTest { 
    }



    @Test

    public void testSetHibernatingForUser_packageIsHibernating() {

    public void testSetHibernatingForUser_packageIsHibernating() throws Exception {

        // WHEN we hibernate a package for a user

        mAppHibernationService.setHibernatingForUser(PACKAGE_NAME_1, USER_ID_1, true);



        // THEN the package is marked hibernating for the user

        assertTrue(mAppHibernationService.isHibernatingForUser(PACKAGE_NAME_1, USER_ID_1));

        verify(mIActivityManager).forceStopPackage(PACKAGE_NAME_1, USER_ID_1);

        verify(mIPackageManager).deleteApplicationCacheFilesAsUser(

                eq(PACKAGE_NAME_1), eq(USER_ID_1), any());

    }



    @Test
@@ -204,6 +207,7 @@ public final class AppHibernationServiceTest { 


        // THEN the package is marked hibernating for the user

        assertTrue(mAppHibernationService.isHibernatingGlobally(PACKAGE_NAME_1));

        verify(mPackageManagerInternal).deleteOatArtifactsOfPackage(PACKAGE_NAME_1);

    }



    @Test