From 61cb9ddfac3591df9e75cd0989e80d5e5bbebc65 Mon Sep 17 00:00:00 2001 From: Nikita Ioffe Date: Thu, 21 Apr 2022 17:58:12 +0100 Subject: [PATCH] apk-in-apex-cache 1/n: Fix isCacheUpToDate for apk-in-apex In case an APK resides on the APEX mount point it's mtime will always be zero. This breaks the isCacheUpToDate check, which is based on comparing the mtime of the apk to the mtime of the cache. This cl fixes the issue by using the mtime of the APEX file backing the /apex mount point the apk resides on. Also noticed some inefficiencies in ApexManager and added TODO's to fix them in the follow-up cls. Bug: 225435110 Test: atest PackageParserTest Test: atest ApexManagerTest Change-Id: I8a0ab8952454652048076a26e81097b95991ae60 --- .../com/android/server/pm/ApexManager.java | 53 ++++++++++++++++--- .../server/pm/parsing/PackageCacher.java | 13 +++++ .../android/server/pm/ApexManagerTest.java | 43 +++++++++++++++ 3 files changed, 102 insertions(+), 7 deletions(-) diff --git a/services/core/java/com/android/server/pm/ApexManager.java b/services/core/java/com/android/server/pm/ApexManager.java index 54b6cab05595d..3d75bef758a5c 100644 --- a/services/core/java/com/android/server/pm/ApexManager.java +++ b/services/core/java/com/android/server/pm/ApexManager.java @@ -62,6 +62,7 @@ import java.io.File; import java.io.PrintWriter; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; +import java.nio.file.Path; import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -122,17 +123,19 @@ public abstract class ApexManager { public final File apexDirectory; public final File preInstalledApexPath; public final boolean isFactory; + public final File apexFile; - private ActiveApexInfo(File apexDirectory, File preInstalledApexPath) { - this(null, apexDirectory, preInstalledApexPath, true); + private ActiveApexInfo(File apexDirectory, File preInstalledApexPath, File apexFile) { + this(null, apexDirectory, preInstalledApexPath, true, apexFile); } private ActiveApexInfo(@Nullable String apexModuleName, File apexDirectory, - File preInstalledApexPath, boolean isFactory) { + File preInstalledApexPath, boolean isFactory, File apexFile) { this.apexModuleName = apexModuleName; this.apexDirectory = apexDirectory; this.preInstalledApexPath = preInstalledApexPath; this.isFactory = isFactory; + this.apexFile = apexFile; } private ActiveApexInfo(ApexInfo apexInfo) { @@ -141,7 +144,8 @@ public abstract class ApexManager { new File(Environment.getApexDirectory() + File.separator + apexInfo.moduleName), new File(apexInfo.preinstalledModulePath), - apexInfo.isFactory); + apexInfo.isFactory, + new File(apexInfo.modulePath)); } } @@ -370,6 +374,15 @@ public abstract class ApexManager { */ public abstract List getApexSystemServices(); + /** + * Returns an APEX file backing the mount point {@code file} is located on, or {@code null} if + * {@code file} doesn't belong to a {@code /apex} mount point. + * + *

Also returns {@code null} if device doesn't support updatable APEX packages. + */ + @Nullable + public abstract File getBackingApexFile(@NonNull File file); + /** * Dumps various state information to the provided {@link PrintWriter} object. * @@ -392,6 +405,7 @@ public abstract class ApexManager { protected static class ApexManagerImpl extends ApexManager { private final Object mLock = new Object(); + // TODO(ioffe): this should be either List or ArrayMap. @GuardedBy("mLock") private Set mActiveApexInfosCache; @@ -943,6 +957,25 @@ public abstract class ApexManager { } } + @Override + public File getBackingApexFile(File file) { + Path path = file.toPath(); + if (!path.startsWith(Environment.getApexDirectory().toPath())) { + return null; + } + if (path.getNameCount() < 2) { + return null; + } + String moduleName = file.toPath().getName(1).toString(); + final List apexes = getActiveApexInfos(); + for (int i = 0; i < apexes.size(); i++) { + if (apexes.get(i).apexModuleName.equals(moduleName)) { + return apexes.get(i).apexFile; + } + } + return null; + } + @Override void dump(PrintWriter pw) { final IndentingPrintWriter ipw = new IndentingPrintWriter(pw, " ", 120); @@ -987,7 +1020,8 @@ public abstract class ApexManager { * An implementation of {@link ApexManager} that should be used in case device does not support * updating APEX packages. */ - private static final class ApexManagerFlattenedApex extends ApexManager { + @VisibleForTesting + static final class ApexManagerFlattenedApex extends ApexManager { @Override ApexInfo[] getAllApexInfos() { return null; @@ -1016,7 +1050,8 @@ public abstract class ApexManager { // In flattened configuration, init special-cases the art directory // and bind-mounts com.android.art.debug to com.android.art. && !file.getName().equals("com.android.art.debug")) { - result.add(new ActiveApexInfo(file, Environment.getRootDirectory())); + result.add( + new ActiveApexInfo(file, Environment.getRootDirectory(), file)); } } } @@ -1173,7 +1208,11 @@ public abstract class ApexManager { @Override void dump(PrintWriter pw) { - // No-op + } + + @Override + public File getBackingApexFile(File file) { + return null; } } } diff --git a/services/core/java/com/android/server/pm/parsing/PackageCacher.java b/services/core/java/com/android/server/pm/parsing/PackageCacher.java index 59b7cbd5d94ec..f7a6eef29deec 100644 --- a/services/core/java/com/android/server/pm/parsing/PackageCacher.java +++ b/services/core/java/com/android/server/pm/parsing/PackageCacher.java @@ -18,6 +18,7 @@ package com.android.server.pm.parsing; import android.annotation.NonNull; import android.content.pm.PackageParserCacheHelper; +import android.os.Environment; import android.os.FileUtils; import android.os.Parcel; import android.system.ErrnoException; @@ -27,6 +28,7 @@ import android.system.StructStat; import android.util.Slog; import com.android.internal.annotations.VisibleForTesting; +import com.android.server.pm.ApexManager; import com.android.server.pm.parsing.pkg.PackageImpl; import com.android.server.pm.parsing.pkg.ParsedPackage; @@ -118,6 +120,17 @@ public class PackageCacher { */ private static boolean isCacheUpToDate(File packageFile, File cacheFile) { try { + // In case packageFile is located on one of /apex mount points it's mtime will always be + // 0. Instead, we can use mtime of the APEX file backing the corresponding mount point. + if (packageFile.toPath().startsWith(Environment.getApexDirectory().toPath())) { + File backingApexFile = ApexManager.getInstance().getBackingApexFile(packageFile); + if (backingApexFile == null) { + Slog.w(TAG, + "Failed to find APEX file backing " + packageFile.getAbsolutePath()); + } else { + packageFile = backingApexFile; + } + } // NOTE: We don't use the File.lastModified API because it has the very // non-ideal failure mode of returning 0 with no excepions thrown. // The nio2 Files API is a little better but is considerably more expensive. diff --git a/services/tests/servicestests/src/com/android/server/pm/ApexManagerTest.java b/services/tests/servicestests/src/com/android/server/pm/ApexManagerTest.java index 1538f0619e1ed..03d4e9e3ebac7 100644 --- a/services/tests/servicestests/src/com/android/server/pm/ApexManagerTest.java +++ b/services/tests/servicestests/src/com/android/server/pm/ApexManagerTest.java @@ -39,6 +39,7 @@ import android.apex.IApexService; import android.content.Context; import android.content.pm.ApplicationInfo; import android.content.pm.PackageInfo; +import android.os.Environment; import android.os.RemoteException; import android.os.ServiceSpecificException; import android.platform.test.annotations.Presubmit; @@ -531,6 +532,48 @@ public class ApexManagerTest { .isEqualTo(TEST_APEX_PKG); } + @Test + public void testGetBackingApexFiles() throws Exception { + final ApexInfo apex = createApexInfoForTestPkg(true, true, 37); + when(mApexService.getActivePackages()).thenReturn(new ApexInfo[]{apex}); + + final File backingApexFile = mApexManager.getBackingApexFile( + new File("/apex/" + TEST_APEX_PKG + "/apk/App/App.apk")); + assertThat(backingApexFile.getAbsolutePath()).isEqualTo(apex.modulePath); + } + + @Test + public void testGetBackingApexFile_fileNotOnApexMountPoint_returnsNull() throws Exception { + File result = mApexManager.getBackingApexFile( + new File("/data/local/tmp/whatever/does-not-matter")); + assertThat(result).isNull(); + } + + @Test + public void testGetBackingApexFiles_unknownApex_returnsNull() throws Exception { + final ApexInfo apex = createApexInfoForTestPkg(true, true, 37); + when(mApexService.getActivePackages()).thenReturn(new ApexInfo[]{apex}); + + final File backingApexFile = mApexManager.getBackingApexFile( + new File("/apex/com.wrong.apex/apk/App")); + assertThat(backingApexFile).isNull(); + } + + @Test + public void testGetBackingApexFiles_topLevelApexDir_returnsNull() throws Exception { + assertThat(mApexManager.getBackingApexFile(Environment.getApexDirectory())).isNull(); + assertThat(mApexManager.getBackingApexFile(new File("/apex/"))).isNull(); + assertThat(mApexManager.getBackingApexFile(new File("/apex//"))).isNull(); + } + + @Test + public void testGetBackingApexFiles_flattenedApex() throws Exception { + ApexManager flattenedApexManager = new ApexManager.ApexManagerFlattenedApex(); + final File backingApexFile = flattenedApexManager.getBackingApexFile( + new File("/apex/com.android.apex.cts.shim/app/CtsShim/CtsShim.apk")); + assertThat(backingApexFile).isNull(); + } + private ApexInfo createApexInfoForTestPkg(boolean isActive, boolean isFactory, int version) { File apexFile = extractResource(TEST_APEX_PKG, TEST_APEX_FILE_NAME); ApexInfo apexInfo = new ApexInfo();