From 5b3dd0e09b9c7bffb83c3d0b89d35c020b274cf3 Mon Sep 17 00:00:00 2001
From: Kevin Han <kevhan@google.com>
Date: Tue, 6 Apr 2021 13:48:06 -0700
Subject: [PATCH] Do not hold PM or AM locks when calling hibernation API

App hibernation calls into activity manager and package manager API. If
package manager calls into AppHibernationService while holding either
the activity manager lock or package manager locks, it can cause a
deadlock. To fix this, we ensure that app hibernation API is never
called with either lock, either by separating out the logic or putting
the app hibernation state mutation on a background thread.

Bug: 184661338
Bug: 182811830
Test: manual, hibernation app, open, see hibernation state is left
Test: atest PackageManagerServiceHibernationTests
Change-Id: I1d7dfb2232a49a63958628dd94f6b6445ece4d55
---
 .../server/pm/PackageManagerService.java      | 23 ++++++++++---------
 .../PackageManagerServiceHibernationTests.kt  | 13 +++++++++--
 2 files changed, 23 insertions(+), 13 deletions(-)

diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java
index cb68cc9d5e453..b0abd402f8537 100644
--- a/services/core/java/com/android/server/pm/PackageManagerService.java
+++ b/services/core/java/com/android/server/pm/PackageManagerService.java
@@ -12272,19 +12272,18 @@ public class PackageManagerService extends IPackageManager.Stub
 
     public ArraySet<String> getOptimizablePackages() {
         ArraySet<String> pkgs = new ArraySet<>();
-        final boolean hibernationEnabled = AppHibernationService.isAppHibernationEnabled();
-        AppHibernationManagerInternal appHibernationManager =
-                mInjector.getLocalService(AppHibernationManagerInternal.class);
         synchronized (mLock) {
             for (AndroidPackage p : mPackages.values()) {
-                // Checking hibernation state is an inexpensive call.
-                boolean isHibernating = hibernationEnabled
-                        && appHibernationManager.isHibernatingGlobally(p.getPackageName());
-                if (PackageDexOptimizer.canOptimizePackage(p) && !isHibernating) {
+                if (PackageDexOptimizer.canOptimizePackage(p)) {
                     pkgs.add(p.getPackageName());
                 }
             }
         }
+        if (AppHibernationService.isAppHibernationEnabled()) {
+            AppHibernationManagerInternal appHibernationManager =
+                    mInjector.getLocalService(AppHibernationManagerInternal.class);
+            pkgs.removeIf(pkgName -> appHibernationManager.isHibernatingGlobally(pkgName));
+        }
         return pkgs;
     }
 
@@ -23465,10 +23464,12 @@ public class PackageManagerService extends IPackageManager.Stub
             }
         }
         if (shouldUnhibernate) {
-            AppHibernationManagerInternal ah =
-                    mInjector.getLocalService(AppHibernationManagerInternal.class);
-            ah.setHibernatingForUser(packageName, userId, false);
-            ah.setHibernatingGlobally(packageName, false);
+            mHandler.post(() -> {
+                AppHibernationManagerInternal ah =
+                        mInjector.getLocalService(AppHibernationManagerInternal.class);
+                ah.setHibernatingForUser(packageName, userId, false);
+                ah.setHibernatingGlobally(packageName, false);
+            });
         }
     }
 
diff --git a/services/tests/mockingservicestests/src/com/android/server/pm/PackageManagerServiceHibernationTests.kt b/services/tests/mockingservicestests/src/com/android/server/pm/PackageManagerServiceHibernationTests.kt
index 46487ea279598..411c31c97120b 100644
--- a/services/tests/mockingservicestests/src/com/android/server/pm/PackageManagerServiceHibernationTests.kt
+++ b/services/tests/mockingservicestests/src/com/android/server/pm/PackageManagerServiceHibernationTests.kt
@@ -17,8 +17,12 @@
 package com.android.server.pm
 
 import android.os.Build
+import android.os.Handler
 import android.provider.DeviceConfig
 import android.provider.DeviceConfig.NAMESPACE_APP_HIBERNATION
+import android.testing.AndroidTestingRunner
+import android.testing.TestableLooper
+import android.testing.TestableLooper.RunWithLooper
 import com.android.server.apphibernation.AppHibernationManagerInternal
 import com.android.server.extendedtestutils.wheneverStatic
 import com.android.server.testutils.whenever
@@ -28,12 +32,12 @@ import org.junit.Before
 import org.junit.Rule
 import org.junit.Test
 import org.junit.runner.RunWith
-import org.junit.runners.JUnit4
 import org.mockito.Mock
 import org.mockito.Mockito.verify
 import org.mockito.MockitoAnnotations
 
-@RunWith(JUnit4::class)
+@RunWith(AndroidTestingRunner::class)
+@RunWithLooper
 class PackageManagerServiceHibernationTests {
 
     companion object {
@@ -60,6 +64,8 @@ class PackageManagerServiceHibernationTests {
         rule.system().stageNominalSystemState()
         whenever(rule.mocks().injector.getLocalService(AppHibernationManagerInternal::class.java))
             .thenReturn(appHibernationManager)
+        whenever(rule.mocks().injector.handler)
+            .thenReturn(Handler(TestableLooper.get(this).looper))
     }
 
     @Test
@@ -74,6 +80,9 @@ class PackageManagerServiceHibernationTests {
         ps!!.setStopped(true, TEST_USER_ID)
 
         pm.setPackageStoppedState(TEST_PACKAGE_NAME, false, TEST_USER_ID)
+
+        TestableLooper.get(this).processAllMessages()
+
         verify(appHibernationManager).setHibernatingForUser(TEST_PACKAGE_NAME, TEST_USER_ID, false)
         verify(appHibernationManager).setHibernatingGlobally(TEST_PACKAGE_NAME, false)
     }