diff --git a/services/core/java/com/android/server/pm/ComputerEngine.java b/services/core/java/com/android/server/pm/ComputerEngine.java index 1945ed054e210..2128beaa7322e 100644 --- a/services/core/java/com/android/server/pm/ComputerEngine.java +++ b/services/core/java/com/android/server/pm/ComputerEngine.java @@ -146,7 +146,6 @@ import com.android.server.pm.verify.domain.DomainVerificationManagerInternal; import com.android.server.pm.verify.domain.DomainVerificationUtils; import com.android.server.uri.UriGrantsManagerInternal; import com.android.server.utils.WatchedArrayMap; -import com.android.server.utils.WatchedArraySet; import com.android.server.utils.WatchedLongSparseArray; import com.android.server.utils.WatchedSparseBooleanArray; import com.android.server.utils.WatchedSparseIntArray; @@ -333,7 +332,7 @@ public class ComputerEngine implements Computer { private final InstantAppRegistry mInstantAppRegistry; private final ApplicationInfo mLocalAndroidApplication; private final AppsFilter mAppsFilter; - private final WatchedArraySet mFrozenPackages; + private final WatchedArrayMap mFrozenPackages; // Immutable service attribute private final String mAppPredictionServicePackage; @@ -3580,7 +3579,7 @@ public class ComputerEngine implements Computer { return PackageManagerService.PACKAGE_STARTABILITY_NOT_SYSTEM; } - if (mFrozenPackages.contains(packageName)) { + if (mFrozenPackages.containsKey(packageName)) { return PackageManagerService.PACKAGE_STARTABILITY_FROZEN; } diff --git a/services/core/java/com/android/server/pm/DumpHelper.java b/services/core/java/com/android/server/pm/DumpHelper.java index c670e1fe69ff0..47e94f37ec8c8 100644 --- a/services/core/java/com/android/server/pm/DumpHelper.java +++ b/services/core/java/com/android/server/pm/DumpHelper.java @@ -504,6 +504,9 @@ final class DumpHelper { ipw.println("(none)"); } else { for (int i = 0; i < mPm.mFrozenPackages.size(); i++) { + ipw.print("package="); + ipw.print(mPm.mFrozenPackages.keyAt(i)); + ipw.print(", refCounts="); ipw.println(mPm.mFrozenPackages.valueAt(i)); } } diff --git a/services/core/java/com/android/server/pm/MovePackageHelper.java b/services/core/java/com/android/server/pm/MovePackageHelper.java index 19ebb5d1caa2d..652a9ae06124d 100644 --- a/services/core/java/com/android/server/pm/MovePackageHelper.java +++ b/services/core/java/com/android/server/pm/MovePackageHelper.java @@ -130,7 +130,7 @@ public final class MovePackageHelper { "Device admin cannot be moved"); } - if (mPm.mFrozenPackages.contains(packageName)) { + if (mPm.mFrozenPackages.containsKey(packageName)) { throw new PackageManagerException(MOVE_FAILED_OPERATION_PENDING, "Failed to move already frozen package"); } @@ -188,6 +188,7 @@ public final class MovePackageHelper { for (int userId : installedUserIds) { if (StorageManager.isFileEncryptedNativeOrEmulated() && !StorageManager.isUserKeyUnlocked(userId)) { + freezer.close(); throw new PackageManagerException(MOVE_FAILED_LOCKED_USER, "User " + userId + " must be unlocked"); } @@ -230,6 +231,7 @@ public final class MovePackageHelper { final IPackageInstallObserver2 installObserver = new IPackageInstallObserver2.Stub() { @Override public void onUserActionRequired(Intent intent) throws RemoteException { + freezer.close(); throw new IllegalStateException(); } diff --git a/services/core/java/com/android/server/pm/PackageFreezer.java b/services/core/java/com/android/server/pm/PackageFreezer.java index ecc92b7fa7406..1e0a1f2ccf1f7 100644 --- a/services/core/java/com/android/server/pm/PackageFreezer.java +++ b/services/core/java/com/android/server/pm/PackageFreezer.java @@ -31,8 +31,6 @@ import java.util.concurrent.atomic.AtomicBoolean; final class PackageFreezer implements AutoCloseable { private final String mPackageName; - private final boolean mWeFroze; - private final AtomicBoolean mClosed = new AtomicBoolean(); private final CloseGuard mCloseGuard = CloseGuard.get(); @@ -48,7 +46,7 @@ final class PackageFreezer implements AutoCloseable { PackageFreezer(PackageManagerService pm) { mPm = pm; mPackageName = null; - mWeFroze = false; + mClosed.set(true); mCloseGuard.open("close"); } @@ -58,7 +56,9 @@ final class PackageFreezer implements AutoCloseable { mPackageName = packageName; final PackageSetting ps; synchronized (mPm.mLock) { - mWeFroze = mPm.mFrozenPackages.add(mPackageName); + final int refCounts = mPm.mFrozenPackages + .getOrDefault(mPackageName, 0 /* defaultValue */) + 1; + mPm.mFrozenPackages.put(mPackageName, refCounts); ps = mPm.mSettings.getPackageLPr(mPackageName); } if (ps != null) { @@ -82,7 +82,11 @@ final class PackageFreezer implements AutoCloseable { mCloseGuard.close(); if (mClosed.compareAndSet(false, true)) { synchronized (mPm.mLock) { - if (mWeFroze) { + final int refCounts = mPm.mFrozenPackages + .getOrDefault(mPackageName, 0 /* defaultValue */) - 1; + if (refCounts > 0) { + mPm.mFrozenPackages.put(mPackageName, refCounts); + } else { mPm.mFrozenPackages.remove(mPackageName); } } diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index 6f8703bf5a5df..c6ec59a5811f2 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -249,7 +249,6 @@ import com.android.server.utils.TimingsTraceAndSlog; import com.android.server.utils.Watchable; import com.android.server.utils.Watched; import com.android.server.utils.WatchedArrayMap; -import com.android.server.utils.WatchedArraySet; import com.android.server.utils.WatchedLongSparseArray; import com.android.server.utils.WatchedSparseBooleanArray; import com.android.server.utils.WatchedSparseIntArray; @@ -646,15 +645,16 @@ public class PackageManagerService extends IPackageManager.Stub final Settings mSettings; /** - * Set of package names that are currently "frozen", which means active - * surgery is being done on the code/data for that package. The platform - * will refuse to launch frozen packages to avoid race conditions. + * Map of package names to frozen counts that are currently "frozen", + * which means active surgery is being done on the code/data for that + * package. The platform will refuse to launch frozen packages to avoid + * race conditions. * * @see PackageFreezer */ @GuardedBy("mLock") - final WatchedArraySet mFrozenPackages = new WatchedArraySet<>(); - private final SnapshotCache> mFrozenPackagesSnapshot = + final WatchedArrayMap mFrozenPackages = new WatchedArrayMap<>(); + private final SnapshotCache> mFrozenPackagesSnapshot = new SnapshotCache.Auto(mFrozenPackages, mFrozenPackages, "PackageManagerService.mFrozenPackages"); @@ -1016,7 +1016,7 @@ public class PackageManagerService extends IPackageManager.Stub public final AppsFilter appsFilter; public final ComponentResolver componentResolver; public final PackageManagerService service; - public final WatchedArraySet frozenPackages; + public final WatchedArrayMap frozenPackages; Snapshot(int type) { if (type == Snapshot.SNAPPED) { @@ -7255,7 +7255,7 @@ public class PackageManagerService extends IPackageManager.Stub */ void checkPackageFrozen(String packageName) { synchronized (mLock) { - if (!mFrozenPackages.contains(packageName)) { + if (!mFrozenPackages.containsKey(packageName)) { Slog.wtf(TAG, "Expected " + packageName + " to be frozen!", new Throwable()); } } diff --git a/services/tests/mockingservicestests/Android.bp b/services/tests/mockingservicestests/Android.bp index 48a8b1bca99b0..8538603d41807 100644 --- a/services/tests/mockingservicestests/Android.bp +++ b/services/tests/mockingservicestests/Android.bp @@ -59,6 +59,7 @@ android_test { "mockingservicestests-utils-mockito", "servicestests-core-utils", "testables", + "kotlin-test", // TODO: remove once Android migrates to JUnit 4.12, which provides assertThrows "testng", ], diff --git a/services/tests/mockingservicestests/src/com/android/server/pm/PackageFreezerTest.kt b/services/tests/mockingservicestests/src/com/android/server/pm/PackageFreezerTest.kt new file mode 100644 index 0000000000000..edbfecc41b04d --- /dev/null +++ b/services/tests/mockingservicestests/src/com/android/server/pm/PackageFreezerTest.kt @@ -0,0 +1,136 @@ +/* + * Copyright (C) 2021 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.server.pm + +import android.os.Build +import com.android.server.testutils.any +import com.android.server.testutils.spy +import com.android.server.testutils.whenever +import com.google.common.truth.Truth.assertThat +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.Mockito.eq +import org.mockito.Mockito.times +import org.mockito.Mockito.verify +import kotlin.test.assertFailsWith + +@RunWith(JUnit4::class) +class PackageFreezerTest { + + companion object { + const val TEST_PACKAGE = "com.android.test.package" + const val TEST_REASON = "test reason" + const val TEST_USER_ID = 0 + } + + @Rule + @JvmField + val rule = MockSystemRule() + + lateinit var pms: PackageManagerService + + private fun createPackageManagerService(vararg stageExistingPackages: String): + PackageManagerService { + stageExistingPackages.forEach { + rule.system().stageScanExistingPackage(it, 1L, + rule.system().dataAppDirectory) + } + var pms = PackageManagerService(rule.mocks().injector, + false /*coreOnly*/, + false /*factoryTest*/, + MockSystem.DEFAULT_VERSION_INFO.fingerprint, + false /*isEngBuild*/, + false /*isUserDebugBuild*/, + Build.VERSION_CODES.CUR_DEVELOPMENT, + Build.VERSION.INCREMENTAL, + false /*snapshotEnabled*/) + rule.system().validateFinalState() + return pms + } + + private fun frozenMessage(packageName: String) = "Package $packageName is currently frozen!" + + private fun assertThrowContainsMessage( + exceptionClass: kotlin.reflect.KClass, + message: String, + block: () -> Unit + ) { + assertThat(assertFailsWith(exceptionClass, block).message).contains(message) + } + + @Before + @Throws(Exception::class) + fun setup() { + rule.system().stageNominalSystemState() + pms = spy(createPackageManagerService(TEST_PACKAGE)) + whenever(pms.killApplication(any(), any(), any(), any())) + } + + @Test + fun freezePackage() { + val freezer = PackageFreezer(TEST_PACKAGE, TEST_USER_ID, TEST_REASON, pms) + verify(pms, times(1)) + .killApplication(eq(TEST_PACKAGE), any(), eq(TEST_USER_ID), eq(TEST_REASON)) + + assertThrowContainsMessage(SecurityException::class, frozenMessage(TEST_PACKAGE)) { + pms.checkPackageStartable(TEST_PACKAGE, TEST_USER_ID) + } + + freezer.close() + pms.checkPackageStartable(TEST_PACKAGE, TEST_USER_ID) + } + + @Test + fun freezePackage_twice() { + val freezer1 = PackageFreezer(TEST_PACKAGE, TEST_USER_ID, TEST_REASON, pms) + val freezer2 = PackageFreezer(TEST_PACKAGE, TEST_USER_ID, TEST_REASON, pms) + verify(pms, times(2)) + .killApplication(eq(TEST_PACKAGE), any(), eq(TEST_USER_ID), eq(TEST_REASON)) + + assertThrowContainsMessage(SecurityException::class, frozenMessage(TEST_PACKAGE)) { + pms.checkPackageStartable(TEST_PACKAGE, TEST_USER_ID) + } + + freezer1.close() + assertThrowContainsMessage(SecurityException::class, frozenMessage(TEST_PACKAGE)) { + pms.checkPackageStartable(TEST_PACKAGE, TEST_USER_ID) + } + + freezer2.close() + pms.checkPackageStartable(TEST_PACKAGE, TEST_USER_ID) + } + + @Test + fun freezePackage_withoutClosing() { + var freezer: PackageFreezer? = PackageFreezer(TEST_PACKAGE, TEST_USER_ID, TEST_REASON, pms) + verify(pms, times(1)) + .killApplication(eq(TEST_PACKAGE), any(), eq(TEST_USER_ID), eq(TEST_REASON)) + + assertThrowContainsMessage(SecurityException::class, frozenMessage(TEST_PACKAGE)) { + pms.checkPackageStartable(TEST_PACKAGE, TEST_USER_ID) + } + + freezer = null + System.gc() + System.runFinalization() + + pms.checkPackageStartable(TEST_PACKAGE, TEST_USER_ID) + } +} diff --git a/services/tests/mockingservicestests/src/com/android/server/pm/TEST_MAPPING b/services/tests/mockingservicestests/src/com/android/server/pm/TEST_MAPPING new file mode 100644 index 0000000000000..13e255fe4ab83 --- /dev/null +++ b/services/tests/mockingservicestests/src/com/android/server/pm/TEST_MAPPING @@ -0,0 +1,18 @@ +{ + "presubmit": [ + { + "name": "FrameworksMockingServicesTests", + "options": [ + { + "include-filter": "com.android.server.pm" + }, + { + "exclude-annotation": "androidx.test.filters.FlakyTest" + }, + { + "exclude-annotation": "org.junit.Ignore" + } + ] + } + ] +}