diff options
author | Jiakai Zhang <jiakaiz@google.com> | 2024-04-29 15:37:44 +0100 |
---|---|---|
committer | Jiakai Zhang <jiakaiz@google.com> | 2024-04-30 16:58:17 +0000 |
commit | 0441f9b87bd2c207d21027ebb19bdc630f8dc9e0 (patch) | |
tree | c53ee6fdf4af4973bd40697b7cef9185e11b5eb4 | |
parent | f65b1c4e254b3fcd262ed6f26420d4d13b127709 (diff) | |
download | art-0441f9b87bd2c207d21027ebb19bdc630f8dc9e0.tar.gz |
Support native ABI set change when processing secondary dex use records.
It's possible that a device's native ABI set is changed by an OTA update
(e.g., from 64/32 bit to 64 bit only) after we recorded the ABIs for
secondary dex use.
After this change, we filter out ABIs that are no longer native when
secondary dex use is being queried, and clean them up later.
Bug: 336510004
Test: atest ArtServiceTests
Change-Id: Ie04e82ef9149774c65ac86118a8af53dc7279d78
3 files changed, 74 insertions, 8 deletions
diff --git a/libartservice/service/java/com/android/server/art/DexUseManagerLocal.java b/libartservice/service/java/com/android/server/art/DexUseManagerLocal.java index 7c231c9ce0..c6394d3bd6 100644 --- a/libartservice/service/java/com/android/server/art/DexUseManagerLocal.java +++ b/libartservice/service/java/com/android/server/art/DexUseManagerLocal.java @@ -1023,6 +1023,16 @@ public class DexUseManagerLocal { var record = new SecondaryDexUseRecord(); record.fromProto(recordProto); + + if (!Utils.isNativeAbi(record.mAbiName)) { + // The native ABI set has changed by an OTA since the ABI name was recorded. + Log.i(TAG, + String.format("Ignoring secondary dex use record with non-native ABI " + + "'%s' for '%s'", + record.mAbiName, proto.getDexFile())); + continue; + } + mRecordByLoader.put( DexLoader.create(Utils.assertNonEmpty(recordProto.getLoadingPackageName()), recordProto.getIsolatedProcess()), diff --git a/libartservice/service/java/com/android/server/art/Utils.java b/libartservice/service/java/com/android/server/art/Utils.java index b05a597c7d..6c01bbcb40 100644 --- a/libartservice/service/java/com/android/server/art/Utils.java +++ b/libartservice/service/java/com/android/server/art/Utils.java @@ -196,7 +196,7 @@ public final class Utils { throw new IllegalStateException(String.format("Non-native isa '%s'", isa)); } - private static boolean isNativeAbi(@NonNull String abiName) { + public static boolean isNativeAbi(@NonNull String abiName) { return abiName.equals(Constants.getNative64BitAbi()) || abiName.equals(Constants.getNative32BitAbi()); } diff --git a/libartservice/service/javatests/com/android/server/art/DexUseManagerTest.java b/libartservice/service/javatests/com/android/server/art/DexUseManagerTest.java index 5bd2edc6d2..11810dfb35 100644 --- a/libartservice/service/javatests/com/android/server/art/DexUseManagerTest.java +++ b/libartservice/service/javatests/com/android/server/art/DexUseManagerTest.java @@ -398,6 +398,38 @@ public class DexUseManagerTest { assertThat(dexInfoList.get(0).classLoaderContext()).isNull(); } + @Test + public void testSecondaryDexNativeAbiSetChange() { + when(mInjector.getCurrentTimeMillis()).thenReturn(1000l); + + mDexUseManager.notifyDexContainersLoaded( + mSnapshot, OWNING_PKG_NAME, Map.of(mCeDir + "/foo.apk", "CLC")); + mDexUseManager.notifyDexContainersLoaded( + mSnapshot, LOADING_PKG_NAME, Map.of(mCeDir + "/foo.apk", "CLC2")); + + // Initially, both loaders are recorded. + assertThat(mDexUseManager.getSecondaryDexInfo(OWNING_PKG_NAME).get(0).loaders()).hasSize(2); + + // Save the file. + mMockClock.advanceTime(DexUseManagerLocal.INTERVAL_MS); + + // Simulate that 32 bit is no longer supported. + when(Constants.getNative32BitAbi()).thenReturn(null); + + // Reload the file. + mDexUseManager = new DexUseManagerLocal(mInjector); + + // The info about `LOADING_PKG_NAME` should be filtered out. There should be one loader + // left. + List<? extends SecondaryDexInfo> dexInfoList = + mDexUseManager.getSecondaryDexInfo(OWNING_PKG_NAME); + assertThat(dexInfoList) + .containsExactly(CheckedSecondaryDexInfo.create(mCeDir + "/foo.apk", mUserHandle, + "CLC", Set.of("arm64-v8a"), + Set.of(DexLoader.create(OWNING_PKG_NAME, false /* isolatedProcess */)), + false /* isUsedByOtherApps */, FileVisibility.OTHER_READABLE)); + } + /** Checks that it ignores and dedups things correctly. */ @Test public void testSecondaryDexMultipleEntries() throws Exception { @@ -612,7 +644,7 @@ public class DexUseManagerTest { } @Test - public void testCleanup() throws Exception { + public void testCleanupDeletedPackage() throws Exception { PackageState pkgState = createPackageState( "com.example.deletedpackage", "arm64-v8a", true /* hasPackage */); addPackage("com.example.deletedpackage", pkgState); @@ -633,6 +665,11 @@ public class DexUseManagerTest { // Simulate that the package is then deleted. removePackage("com.example.deletedpackage"); + verifyCleanup(); + } + + @Test + public void testCleanupDeletedPrimaryDex() throws Exception { // Simulate that a primary dex file is loaded and then deleted. lenient() .when(mArtd.getDexFileVisibility(SPLIT_APK)) @@ -643,6 +680,11 @@ public class DexUseManagerTest { mSnapshot, LOADING_PKG_NAME, Map.of(SPLIT_APK, "CLC")); lenient().when(mArtd.getDexFileVisibility(SPLIT_APK)).thenReturn(FileVisibility.NOT_FOUND); + verifyCleanup(); + } + + @Test + public void testCleanupDeletedSecondaryDex() throws Exception { // Simulate that a secondary dex file is loaded and then deleted. lenient() .when(mArtd.getDexFileVisibility(mCeDir + "/foo.apk")) @@ -655,13 +697,11 @@ public class DexUseManagerTest { .when(mArtd.getDexFileVisibility(mCeDir + "/foo.apk")) .thenReturn(FileVisibility.NOT_FOUND); - // Create an entry that should be kept. - lenient() - .when(mArtd.getDexFileVisibility(mCeDir + "/bar.apk")) - .thenReturn(FileVisibility.NOT_OTHER_READABLE); - mDexUseManager.notifyDexContainersLoaded( - mSnapshot, OWNING_PKG_NAME, Map.of(mCeDir + "/bar.apk", "CLC")); + verifyCleanup(); + } + @Test + public void testCleanupPrivateSecondaryDex() throws Exception { // Simulate that a secondary dex file is loaded by another package and then made private. lenient() .when(mArtd.getDexFileVisibility(mCeDir + "/baz.apk")) @@ -672,6 +712,11 @@ public class DexUseManagerTest { .when(mArtd.getDexFileVisibility(mCeDir + "/baz.apk")) .thenReturn(FileVisibility.NOT_OTHER_READABLE); + verifyCleanup(); + } + + @Test + public void testCleanupDeletedAllDex() throws Exception { // Simulate that all the files of a package are deleted. The whole container entry of the // package should be cleaned up, though the package still exists. lenient() @@ -685,6 +730,17 @@ public class DexUseManagerTest { "/somewhere/app/" + LOADING_PKG_NAME + "/base.apk")) .thenReturn(FileVisibility.NOT_FOUND); + verifyCleanup(); + } + + private void verifyCleanup() throws Exception { + // Create an entry that should be kept. + lenient() + .when(mArtd.getDexFileVisibility(mCeDir + "/bar.apk")) + .thenReturn(FileVisibility.NOT_OTHER_READABLE); + mDexUseManager.notifyDexContainersLoaded( + mSnapshot, OWNING_PKG_NAME, Map.of(mCeDir + "/bar.apk", "CLC")); + // Run cleanup. mDexUseManager.cleanup(); |