Merge "Don't hold hibernation lock when calling other services" into sc-v2-dev

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

@@ -277,16 +277,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);
+        final int realUserId = handleIncomingUser(userId, methodName);
-        if (!checkUserStatesExist(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;
             }
 
@@ -294,13 +294,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,
@@ -308,8 +312,8 @@ public final class AppHibernationService extends SystemService {
                         userIdSnapshot,
                         stateSnapshot.hibernated);
             });
-            List<UserLevelState> states = new ArrayList<>(mUserStates.get(userId).values());
+            List<UserLevelState> states = new ArrayList<>(mUserStates.get(realUserId).values());
-            mUserDiskStores.get(userId).scheduleWriteHibernationStates(states);
+            mUserDiskStores.get(realUserId).scheduleWriteHibernationStates(states);
         }
     }
 
@@ -334,10 +338,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);
@@ -374,20 +380,16 @@ public final class AppHibernationService extends SystemService {
     }
 
     /**
-     * Put an app into hibernation for a given user, allowing user-level optimizations to occur.
+     * 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.
-     * @param pkgState package hibernation state
      */
-    @GuardedBy("mLock")
+    private void hibernatePackageForUser(@NonNull String packageName, int userId) {
-    private void hibernatePackageForUser(@NonNull String packageName, int userId,
-            @NonNull UserLevelState pkgState) {
         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);
@@ -398,16 +400,11 @@ public final class AppHibernationService extends SystemService {
     }
 
     /**
-     * Remove a package from hibernation for a given user.
+     * Remove a package from hibernation for a given user. Do not hold {@link #mLock} while calling
-     *
+     * this.
-     * @param pkgState package hibernation state
      */
-    @GuardedBy("mLock")
+    private void unhibernatePackageForUser(@NonNull String packageName, int userId) {
-    private void unhibernatePackageForUser(@NonNull String packageName, int userId,
-            UserLevelState pkgState) {
         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.
@@ -458,29 +455,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;
+        synchronized (mLock) {
-        Trace.traceEnd(Trace.TRACE_TAG_SYSTEM_SERVER);
+            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

@@ -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