Avoid lambda code to cache computer snapshot
The lambda will automatically retain a reference for use within
the code block. In addition, snapshotComputer() also creates a
new object for the snapshot everytime. If the lambda is called
multiple times and some places keep them for a long time, the
snapshot object becomes huge. It causes OOM in the system_server
process and leads to platform reset.
To avoid the problem, use Supplier instead and then gain snapshot
the supplier.
Bug: 381996338
Flag: EXEMPT bugfix
Test: atest FrameworksMockingServicesTests:com.android.server.pm
Change-Id: I57422392167012905243db0856b6eb5648b56489
diff --git a/services/core/java/com/android/server/pm/BroadcastHelper.java b/services/core/java/com/android/server/pm/BroadcastHelper.java
index 6d54be8..d78e98b 100644
--- a/services/core/java/com/android/server/pm/BroadcastHelper.java
+++ b/services/core/java/com/android/server/pm/BroadcastHelper.java
@@ -78,6 +78,7 @@
import java.util.Arrays;
import java.util.Collections;
import java.util.function.BiFunction;
+import java.util.function.Supplier;
/**
* Helper class to send broadcasts for various situations.
@@ -216,7 +217,7 @@
filterExtrasForReceiver, bOptions);
}
- void sendResourcesChangedBroadcast(@NonNull Computer snapshot,
+ void sendResourcesChangedBroadcast(@NonNull Supplier<Computer> snapshotSupplier,
boolean mediaStatus,
boolean replacing,
@NonNull String[] pkgNames,
@@ -236,7 +237,7 @@
null /* targetPkg */, null /* finishedReceiver */, null /* userIds */,
null /* instantUserIds */, null /* broadcastAllowList */,
(callingUid, intentExtras) -> filterExtrasChangedPackageList(
- snapshot, callingUid, intentExtras),
+ snapshotSupplier, callingUid, intentExtras),
null /* bOptions */, null /* requiredPermissions */);
}
@@ -544,7 +545,7 @@
});
}
- void sendPostInstallBroadcasts(@NonNull Computer snapshot,
+ void sendPostInstallBroadcasts(@NonNull Supplier<Computer> snapshotSupplier,
@NonNull InstallRequest request,
@NonNull String packageName,
@NonNull String requiredPermissionControllerPackage,
@@ -567,8 +568,8 @@
final int[] uids = new int[]{request.getRemovedInfo().mUid};
notifyResourcesChanged(
false /* mediaStatus */, true /* replacing */, pkgNames, uids);
- sendResourcesChangedBroadcast(
- snapshot, false /* mediaStatus */, true /* replacing */, pkgNames, uids);
+ sendResourcesChangedBroadcast(snapshotSupplier,
+ false /* mediaStatus */, true /* replacing */, pkgNames, uids);
}
sendPackageRemovedBroadcasts(
request.getRemovedInfo(), packageSender, isKillApp, false /*removedBySystem*/,
@@ -608,6 +609,7 @@
null /* broadcastAllowList */, null);
}
+ final Computer snapshot = snapshotSupplier.get();
// Send installed broadcasts if the package is not a static shared lib.
if (staticSharedLibraryName == null) {
// Send PACKAGE_ADDED broadcast for users that see the package for the first time
@@ -732,7 +734,7 @@
if (!isArchived) {
final String[] pkgNames = new String[]{packageName};
final int[] uids = new int[]{request.getPkg().getUid()};
- sendResourcesChangedBroadcast(snapshot,
+ sendResourcesChangedBroadcast(snapshotSupplier,
true /* mediaStatus */, true /* replacing */, pkgNames, uids);
notifyResourcesChanged(true /* mediaStatus */,
true /* replacing */, pkgNames, uids);
@@ -860,8 +862,8 @@
* access all the packages in the extras.
*/
@Nullable
- private static Bundle filterExtrasChangedPackageList(@NonNull Computer snapshot, int callingUid,
- @NonNull Bundle extras) {
+ private static Bundle filterExtrasChangedPackageList(
+ @NonNull Supplier<Computer> snapshotSupplier, int callingUid, @NonNull Bundle extras) {
if (UserHandle.isCore(callingUid)) {
// see all
return extras;
@@ -873,6 +875,7 @@
final int userId = extras.getInt(
Intent.EXTRA_USER_HANDLE, UserHandle.getUserId(callingUid));
final int[] uids = extras.getIntArray(Intent.EXTRA_CHANGED_UID_LIST);
+ final Computer snapshot = snapshotSupplier.get();
final Pair<String[], int[]> filteredPkgs =
filterPackages(snapshot, pkgs, uids, callingUid, userId);
if (ArrayUtils.isEmpty(filteredPkgs.first)) {
@@ -952,10 +955,12 @@
final int[] instantUserIds = isInstantApp ? new int[] { userId } : EMPTY_INT_ARRAY;
final SparseArray<int[]> broadcastAllowList =
isInstantApp ? null : snapshot.getVisibilityAllowLists(packageName, userIds);
+ final String[] sharedUserPackages =
+ snapshot.getSharedUserPackagesForPackage(packageName, userId);
mHandler.post(() -> sendPackageChangedBroadcastInternal(
packageName, dontKillApp, componentNames, packageUid, reason, userIds,
instantUserIds, broadcastAllowList, setting.getPkg(),
- snapshot.getSharedUserPackagesForPackage(packageName, userId)));
+ sharedUserPackages));
mPackageMonitorCallbackHelper.notifyPackageChanged(packageName, dontKillApp, componentNames,
packageUid, reason, userIds, instantUserIds, broadcastAllowList, mHandler);
}
@@ -1120,7 +1125,7 @@
* @param uidList The uids of packages which have suspension changes.
* @param userId The user where packages reside.
*/
- void sendPackagesSuspendedOrUnsuspendedForUser(@NonNull Computer snapshot,
+ void sendPackagesSuspendedOrUnsuspendedForUser(@NonNull Supplier<Computer> snapshotSupplier,
@NonNull String intent,
@NonNull String[] pkgList,
@NonNull int[] uidList,
@@ -1138,7 +1143,7 @@
.toBundle();
BiFunction<Integer, Bundle, Bundle> filterExtrasForReceiver =
(callingUid, intentExtras) -> BroadcastHelper.filterExtrasChangedPackageList(
- snapshot, callingUid, intentExtras);
+ snapshotSupplier, callingUid, intentExtras);
mHandler.post(() -> sendPackageBroadcast(intent, null /* pkg */,
extras, flags, null /* targetPkg */, null /* finishedReceiver */,
new int[]{userId}, null /* instantUserIds */, null /* broadcastAllowList */,
@@ -1148,7 +1153,7 @@
null /* instantUserIds */, null /* broadcastAllowList */, filterExtrasForReceiver);
}
- void sendMyPackageSuspendedOrUnsuspended(@NonNull Computer snapshot,
+ void sendMyPackageSuspendedOrUnsuspended(@NonNull Supplier<Computer> snapshotSupplier,
@NonNull String[] affectedPackages,
boolean suspended,
int userId) {
@@ -1163,6 +1168,7 @@
return;
}
final int[] targetUserIds = new int[] {userId};
+ final Computer snapshot = snapshotSupplier.get();
for (String packageName : affectedPackages) {
final Bundle appExtras = suspended
? SuspendPackageHelper.getSuspendedPackageAppExtras(
@@ -1192,7 +1198,7 @@
* @param uidList The uids of packages which have suspension changes.
* @param userId The user where packages reside.
*/
- void sendDistractingPackagesChanged(@NonNull Computer snapshot,
+ void sendDistractingPackagesChanged(@NonNull Supplier<Computer> snapshotSupplier,
@NonNull String[] pkgList,
@NonNull int[] uidList,
int userId,
@@ -1208,11 +1214,11 @@
null /* finishedReceiver */, new int[]{userId}, null /* instantUserIds */,
null /* broadcastAllowList */,
(callingUid, intentExtras) -> filterExtrasChangedPackageList(
- snapshot, callingUid, intentExtras),
+ snapshotSupplier, callingUid, intentExtras),
null /* bOptions */, null /* requiredPermissions */));
}
- void sendResourcesChangedBroadcastAndNotify(@NonNull Computer snapshot,
+ void sendResourcesChangedBroadcastAndNotify(@NonNull Supplier<Computer> snapshotSupplier,
boolean mediaStatus,
boolean replacing,
@NonNull ArrayList<AndroidPackage> packages) {
@@ -1224,7 +1230,7 @@
packageNames[i] = pkg.getPackageName();
packageUids[i] = pkg.getUid();
}
- sendResourcesChangedBroadcast(snapshot, mediaStatus,
+ sendResourcesChangedBroadcast(snapshotSupplier, mediaStatus,
replacing, packageNames, packageUids);
notifyResourcesChanged(mediaStatus, replacing, packageNames, packageUids);
}
diff --git a/services/core/java/com/android/server/pm/DistractingPackageHelper.java b/services/core/java/com/android/server/pm/DistractingPackageHelper.java
index c5ec73b..c4e981d 100644
--- a/services/core/java/com/android/server/pm/DistractingPackageHelper.java
+++ b/services/core/java/com/android/server/pm/DistractingPackageHelper.java
@@ -123,7 +123,7 @@
if (!changedPackagesList.isEmpty()) {
final String[] changedPackages = changedPackagesList.toArray(
new String[changedPackagesList.size()]);
- mBroadcastHelper.sendDistractingPackagesChanged(mPm.snapshotComputer(),
+ mBroadcastHelper.sendDistractingPackagesChanged(mPm::snapshotComputer,
changedPackages, changedUids.toArray(), userId, restrictionFlags);
mPm.scheduleWritePackageRestrictions(userId);
}
@@ -198,7 +198,7 @@
if (!changedPackages.isEmpty()) {
final String[] packageArray = changedPackages.toArray(
new String[changedPackages.size()]);
- mBroadcastHelper.sendDistractingPackagesChanged(mPm.snapshotComputer(),
+ mBroadcastHelper.sendDistractingPackagesChanged(mPm::snapshotComputer,
packageArray, changedUids.toArray(), userId, RESTRICTION_NONE);
mPm.scheduleWritePackageRestrictions(userId);
}
diff --git a/services/core/java/com/android/server/pm/InstallPackageHelper.java b/services/core/java/com/android/server/pm/InstallPackageHelper.java
index 69c6ce8..183ceae 100644
--- a/services/core/java/com/android/server/pm/InstallPackageHelper.java
+++ b/services/core/java/com/android/server/pm/InstallPackageHelper.java
@@ -3084,7 +3084,7 @@
mPm.mProcessLoggingHandler.invalidateBaseApkHash(request.getPkg().getBaseApkPath());
}
- mBroadcastHelper.sendPostInstallBroadcasts(mPm.snapshotComputer(), request, packageName,
+ mBroadcastHelper.sendPostInstallBroadcasts(mPm::snapshotComputer, request, packageName,
mPm.mRequiredPermissionControllerPackage, mPm.mRequiredVerifierPackages,
mPm.mRequiredInstallerPackage,
/* packageSender= */ mPm, launchedForRestore, killApp, update, archived);
diff --git a/services/core/java/com/android/server/pm/StorageEventHelper.java b/services/core/java/com/android/server/pm/StorageEventHelper.java
index 951986f..a09d477 100644
--- a/services/core/java/com/android/server/pm/StorageEventHelper.java
+++ b/services/core/java/com/android/server/pm/StorageEventHelper.java
@@ -227,7 +227,7 @@
}
if (DEBUG_INSTALL) Slog.d(TAG, "Loaded packages " + loaded);
- mBroadcastHelper.sendResourcesChangedBroadcastAndNotify(mPm.snapshotComputer(),
+ mBroadcastHelper.sendResourcesChangedBroadcastAndNotify(mPm::snapshotComputer,
true /* mediaStatus */, false /* replacing */, loaded);
synchronized (mLoadedVolumes) {
mLoadedVolumes.add(vol.getId());
@@ -279,7 +279,7 @@
}
if (DEBUG_INSTALL) Slog.d(TAG, "Unloaded packages " + unloaded);
- mBroadcastHelper.sendResourcesChangedBroadcastAndNotify(mPm.snapshotComputer(),
+ mBroadcastHelper.sendResourcesChangedBroadcastAndNotify(mPm::snapshotComputer,
false /* mediaStatus */, false /* replacing */, unloaded);
synchronized (mLoadedVolumes) {
mLoadedVolumes.remove(vol.getId());
diff --git a/services/core/java/com/android/server/pm/SuspendPackageHelper.java b/services/core/java/com/android/server/pm/SuspendPackageHelper.java
index 4e70cc5..88fd1aa 100644
--- a/services/core/java/com/android/server/pm/SuspendPackageHelper.java
+++ b/services/core/java/com/android/server/pm/SuspendPackageHelper.java
@@ -192,21 +192,20 @@
}
});
- final Computer newSnapshot = mPm.snapshotComputer();
if (!notifyPackagesList.isEmpty()) {
final String[] changedPackages =
notifyPackagesList.toArray(new String[0]);
- mBroadcastHelper.sendPackagesSuspendedOrUnsuspendedForUser(newSnapshot,
+ mBroadcastHelper.sendPackagesSuspendedOrUnsuspendedForUser(mPm::snapshotComputer,
suspended ? Intent.ACTION_PACKAGES_SUSPENDED
: Intent.ACTION_PACKAGES_UNSUSPENDED,
changedPackages, notifyUids.toArray(), quarantined, targetUserId);
- mBroadcastHelper.sendMyPackageSuspendedOrUnsuspended(newSnapshot, changedPackages,
- suspended, targetUserId);
+ mBroadcastHelper.sendMyPackageSuspendedOrUnsuspended(mPm::snapshotComputer,
+ changedPackages, suspended, targetUserId);
mPm.scheduleWritePackageRestrictions(targetUserId);
}
// Send the suspension changed broadcast to ensure suspension state is not stale.
if (!changedPackagesList.isEmpty()) {
- mBroadcastHelper.sendPackagesSuspendedOrUnsuspendedForUser(newSnapshot,
+ mBroadcastHelper.sendPackagesSuspendedOrUnsuspendedForUser(mPm::snapshotComputer,
Intent.ACTION_PACKAGES_SUSPENSION_CHANGED,
changedPackagesList.toArray(new String[0]), changedUids.toArray(), quarantined,
targetUserId);
@@ -343,13 +342,12 @@
});
mPm.scheduleWritePackageRestrictions(targetUserId);
- final Computer newSnapshot = mPm.snapshotComputer();
if (!unsuspendedPackages.isEmpty()) {
final String[] packageArray = unsuspendedPackages.toArray(
new String[unsuspendedPackages.size()]);
- mBroadcastHelper.sendMyPackageSuspendedOrUnsuspended(newSnapshot, packageArray,
- false, targetUserId);
- mBroadcastHelper.sendPackagesSuspendedOrUnsuspendedForUser(newSnapshot,
+ mBroadcastHelper.sendMyPackageSuspendedOrUnsuspended(mPm::snapshotComputer,
+ packageArray, false, targetUserId);
+ mBroadcastHelper.sendPackagesSuspendedOrUnsuspendedForUser(mPm::snapshotComputer,
Intent.ACTION_PACKAGES_UNSUSPENDED,
packageArray, unsuspendedUids.toArray(), false, targetUserId);
}
diff --git a/services/tests/mockingservicestests/src/com/android/server/pm/DistractingPackageHelperTest.kt b/services/tests/mockingservicestests/src/com/android/server/pm/DistractingPackageHelperTest.kt
index de029e0..90e1263 100644
--- a/services/tests/mockingservicestests/src/com/android/server/pm/DistractingPackageHelperTest.kt
+++ b/services/tests/mockingservicestests/src/com/android/server/pm/DistractingPackageHelperTest.kt
@@ -56,8 +56,8 @@
testHandler.flush()
verify(pms).scheduleWritePackageRestrictions(eq(TEST_USER_ID))
- verify(broadcastHelper).sendDistractingPackagesChanged(any(Computer::class.java),
- pkgListCaptor.capture(), any(), any(), flagsCaptor.capture())
+ verify(broadcastHelper).sendDistractingPackagesChanged(
+ any(), pkgListCaptor.capture(), any(), any(), flagsCaptor.capture())
val modifiedPackages = pkgListCaptor.value
val distractionFlags = flagsCaptor.value
@@ -158,8 +158,7 @@
verify(pms).scheduleWritePackageRestrictions(eq(TEST_USER_ID))
verify(broadcastHelper).sendDistractingPackagesChanged(
- any(Computer::class.java), pkgListCaptor.capture(), any(), eq(TEST_USER_ID),
- flagsCaptor.capture())
+ any(), pkgListCaptor.capture(), any(), eq(TEST_USER_ID), flagsCaptor.capture())
val modifiedPackages = pkgListCaptor.value
val distractionFlags = flagsCaptor.value
assertThat(modifiedPackages).asList().containsExactly(TEST_PACKAGE_1, TEST_PACKAGE_2)
@@ -198,12 +197,12 @@
@Test
fun sendDistractingPackagesChanged() {
- broadcastHelper.sendDistractingPackagesChanged(pms.snapshotComputer(),
+ broadcastHelper.sendDistractingPackagesChanged(pms::snapshotComputer,
packagesToChange, uidsToChange, TEST_USER_ID,
PackageManager.RESTRICTION_HIDE_NOTIFICATIONS)
testHandler.flush()
- verify(broadcastHelper).sendDistractingPackagesChanged(any(Computer::class.java),
- pkgListCaptor.capture(), uidsCaptor.capture(), eq(TEST_USER_ID), any())
+ verify(broadcastHelper).sendDistractingPackagesChanged(
+ any(), pkgListCaptor.capture(), uidsCaptor.capture(), eq(TEST_USER_ID), any())
var changedPackages = pkgListCaptor.value
var changedUids = uidsCaptor.value
diff --git a/services/tests/mockingservicestests/src/com/android/server/pm/SuspendPackageHelperTest.kt b/services/tests/mockingservicestests/src/com/android/server/pm/SuspendPackageHelperTest.kt
index 7444403..60e8250 100644
--- a/services/tests/mockingservicestests/src/com/android/server/pm/SuspendPackageHelperTest.kt
+++ b/services/tests/mockingservicestests/src/com/android/server/pm/SuspendPackageHelperTest.kt
@@ -58,8 +58,8 @@
testHandler.flush()
verify(pms).scheduleWritePackageRestrictions(eq(TEST_USER_ID))
- verify(broadcastHelper).sendPackagesSuspendedOrUnsuspendedForUser(any(Computer::class.java),
- eq(Intent.ACTION_PACKAGES_SUSPENDED), pkgListCaptor.capture(), any(), any(), any())
+ verify(broadcastHelper).sendPackagesSuspendedOrUnsuspendedForUser(any(),
+ eq(Intent.ACTION_PACKAGES_SUSPENDED), pkgListCaptor.capture(), any(), any(), any())
var modifiedPackages = pkgListCaptor.value
assertThat(modifiedPackages).asList().containsExactly(TEST_PACKAGE_1, TEST_PACKAGE_2)
@@ -149,11 +149,11 @@
testHandler.flush()
verify(pms, times(2)).scheduleWritePackageRestrictions(eq(TEST_USER_ID))
- verify(broadcastHelper).sendPackagesSuspendedOrUnsuspendedForUser(any(Computer::class.java),
- eq(Intent.ACTION_PACKAGES_UNSUSPENDED), pkgListCaptor.capture(), any(), any(),
- any())
- verify(broadcastHelper).sendMyPackageSuspendedOrUnsuspended(any(Computer::class.java),
- any(), any(), any())
+ verify(broadcastHelper).sendPackagesSuspendedOrUnsuspendedForUser(
+ any(), eq(Intent.ACTION_PACKAGES_UNSUSPENDED),
+ pkgListCaptor.capture(), any(), any(), any())
+ verify(broadcastHelper).sendMyPackageSuspendedOrUnsuspended(
+ any(), any(), any(), any())
var modifiedPackages = pkgListCaptor.value
assertThat(modifiedPackages).asList().containsExactly(TEST_PACKAGE_1, TEST_PACKAGE_2)
@@ -230,10 +230,10 @@
testHandler.flush()
verify(pms, times(2)).scheduleWritePackageRestrictions(eq(TEST_USER_ID))
- verify(broadcastHelper).sendPackagesSuspendedOrUnsuspendedForUser(any(Computer::class.java),
- eq(Intent.ACTION_PACKAGES_UNSUSPENDED), any(), any(), any(), any())
- verify(broadcastHelper).sendMyPackageSuspendedOrUnsuspended(any(Computer::class.java),
- any(), any(), any())
+ verify(broadcastHelper).sendPackagesSuspendedOrUnsuspendedForUser(
+ any(), eq(Intent.ACTION_PACKAGES_UNSUSPENDED), any(), any(), any(), any())
+ verify(broadcastHelper).sendMyPackageSuspendedOrUnsuspended(
+ any(), any(), any(), any())
assertThat(suspendPackageHelper.getSuspendingPackage(pms.snapshotComputer(),
TEST_PACKAGE_1, TEST_USER_ID, deviceOwnerUid)).isNull()