Merge "Fail staged install if any apk-in-apex fails to install" into rvc-dev

4 files changed, 141 insertions, 33 deletions

diff --git a/services/core/java/com/android/server/pm/ApexManager.java b/services/core/java/com/android/server/pm/ApexManager.java
index 1c41c2eb4702..4872b66ff1b4 100644
--- a/services/core/java/com/android/server/pm/ApexManager.java
+++ b/services/core/java/com/android/server/pm/ApexManager.java
@@ -60,7 +60,6 @@ import java.lang.annotation.RetentionPolicy;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashSet;
-import java.util.Iterator;
 import java.util.List;
 import java.util.Objects;
 import java.util.Set;
@@ -290,6 +289,21 @@ public abstract class ApexManager {
     abstract void registerApkInApex(AndroidPackage pkg);
 
     /**
+     * Reports error raised during installation of apk-in-apex.
+     *
+     * @param scanDir the directory of the apex inside which apk-in-apex resides.
+     */
+    abstract void reportErrorWithApkInApex(String scanDirPath);
+
+    /**
+     * Returns true if there were no errors when installing apk-in-apex inside
+     * {@param apexPackageName}, otherwise false.
+     *
+     * @param apexPackageName Package name of the apk container of apex
+     */
+    abstract boolean isApkInApexInstallSuccess(String apexPackageName);
+
+    /**
      * Returns list of {@code packageName} of apks inside the given apex.
      * @param apexPackageName Package name of the apk container of apex
      */
@@ -368,6 +382,13 @@ public abstract class ApexManager {
         @GuardedBy("mLock")
         private ArrayMap<String, List<String>> mApksInApex = new ArrayMap<>();
 
+        /**
+         * Contains the list of {@code Exception}s that were raised when installing apk-in-apex
+         * inside {@code apexModuleName}.
+         */
+        @GuardedBy("mLock")
+        private Set<String> mErrorWithApkInApex = new ArraySet<>();
+
         @GuardedBy("mLock")
         private List<PackageInfo> mAllPackagesCache;
 
@@ -733,9 +754,7 @@ public abstract class ApexManager {
         @Override
         void registerApkInApex(AndroidPackage pkg) {
             synchronized (mLock) {
-                final Iterator<ActiveApexInfo> it = mActiveApexInfosCache.iterator();
-                while (it.hasNext()) {
-                    final ActiveApexInfo aai = it.next();
+                for (ActiveApexInfo aai : mActiveApexInfosCache) {
                     if (pkg.getBaseCodePath().startsWith(aai.apexDirectory.getAbsolutePath())) {
                         List<String> apks = mApksInApex.get(aai.apexModuleName);
                         if (apks == null) {
@@ -749,6 +768,30 @@ public abstract class ApexManager {
         }
 
         @Override
+        void reportErrorWithApkInApex(String scanDirPath) {
+            synchronized (mLock) {
+                for (ActiveApexInfo aai : mActiveApexInfosCache) {
+                    if (scanDirPath.startsWith(aai.apexDirectory.getAbsolutePath())) {
+                        mErrorWithApkInApex.add(aai.apexModuleName);
+                    }
+                }
+            }
+        }
+
+        @Override
+        boolean isApkInApexInstallSuccess(String apexPackageName) {
+            synchronized (mLock) {
+                Preconditions.checkState(mPackageNameToApexModuleName != null,
+                        "APEX packages have not been scanned");
+                String moduleName = mPackageNameToApexModuleName.get(apexPackageName);
+                if (moduleName == null) {
+                    return false;
+                }
+                return !mErrorWithApkInApex.contains(moduleName);
+            }
+        }
+
+        @Override
         List<String> getApksInApex(String apexPackageName) {
             synchronized (mLock) {
                 Preconditions.checkState(mPackageNameToApexModuleName != null,
@@ -1040,6 +1083,16 @@ public abstract class ApexManager {
         }
 
         @Override
+        void reportErrorWithApkInApex(String scanDirPath) {
+            // No-op
+        }
+
+        @Override
+        boolean isApkInApexInstallSuccess(String apexPackageName) {
+            return true;
+        }
+
+        @Override
         List<String> getApksInApex(String apexPackageName) {
             return Collections.emptyList();
         }
diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java
index 1e247f206d64..74acf42ae307 100644
--- a/services/core/java/com/android/server/pm/PackageManagerService.java
+++ b/services/core/java/com/android/server/pm/PackageManagerService.java
@@ -8985,6 +8985,10 @@ public class PackageManagerService extends IPackageManager.Stub
                         + parseResult.scanFile, throwable);
             }
 
+            if ((scanFlags & SCAN_AS_APK_IN_APEX) != 0 && errorCode != INSTALL_SUCCEEDED) {
+                mApexManager.reportErrorWithApkInApex(scanDir.getAbsolutePath());
+            }
+
             // Delete invalid userdata apps
             if ((scanFlags & SCAN_AS_SYSTEM) == 0
                     && errorCode != PackageManager.INSTALL_SUCCEEDED) {
diff --git a/services/core/java/com/android/server/pm/StagingManager.java b/services/core/java/com/android/server/pm/StagingManager.java
index 1c1e64d70bbd..9a297d601a6b 100644
--- a/services/core/java/com/android/server/pm/StagingManager.java
+++ b/services/core/java/com/android/server/pm/StagingManager.java
@@ -370,24 +370,9 @@ public class StagingManager {
     }
 
     /**
-     * Perform snapshot and restore as required both for APEXes themselves and for apks in APEX.
-     * Apks inside apex are not installed using apk-install flow. They are scanned from the system
-     * directory directly by PackageManager, as such, RollbackManager need to handle their data
-     * separately here.
+     * Utility function for extracting apex sessions out of multi-package/single session.
      */
-    private void snapshotAndRestoreForApexSession(PackageInstallerSession session) {
-        if (!sessionContainsApex(session)) {
-            return;
-        }
-
-        boolean doSnapshotOrRestore =
-                (session.params.installFlags & PackageManager.INSTALL_ENABLE_ROLLBACK) != 0
-                || session.params.installReason == PackageManager.INSTALL_REASON_ROLLBACK;
-        if (!doSnapshotOrRestore) {
-            return;
-        }
-
-        // Find all the apex sessions that needs processing
+    private List<PackageInstallerSession> extractApexSessions(PackageInstallerSession session) {
         List<PackageInstallerSession> apexSessions = new ArrayList<>();
         if (session.isMultiPackage()) {
             List<PackageInstallerSession> childrenSessions = new ArrayList<>();
@@ -408,6 +393,50 @@ public class StagingManager {
         } else {
             apexSessions.add(session);
         }
+        return apexSessions;
+    }
+
+    /**
+     * Checks if all apk-in-apex were installed without errors for all of the apex sessions. Throws
+     * error for any apk-in-apex failed to install.
+     *
+     * @throws PackageManagerException if any apk-in-apex failed to install
+     */
+    private void checkInstallationOfApkInApexSuccessful(PackageInstallerSession session)
+            throws PackageManagerException {
+        final List<PackageInstallerSession> apexSessions = extractApexSessions(session);
+        if (apexSessions.isEmpty()) {
+            return;
+        }
+
+        for (PackageInstallerSession apexSession : apexSessions) {
+            String packageName = apexSession.getPackageName();
+            if (!mApexManager.isApkInApexInstallSuccess(packageName)) {
+                throw new PackageManagerException(SessionInfo.STAGED_SESSION_ACTIVATION_FAILED,
+                        "Failed to install apk-in-apex of " + packageName);
+            }
+        }
+    }
+
+    /**
+     * Perform snapshot and restore as required both for APEXes themselves and for apks in APEX.
+     * Apks inside apex are not installed using apk-install flow. They are scanned from the system
+     * directory directly by PackageManager, as such, RollbackManager need to handle their data
+     * separately here.
+     */
+    private void snapshotAndRestoreForApexSession(PackageInstallerSession session) {
+        boolean doSnapshotOrRestore =
+                (session.params.installFlags & PackageManager.INSTALL_ENABLE_ROLLBACK) != 0
+                || session.params.installReason == PackageManager.INSTALL_REASON_ROLLBACK;
+        if (!doSnapshotOrRestore) {
+            return;
+        }
+
+        // Find all the apex sessions that needs processing
+        final List<PackageInstallerSession> apexSessions = extractApexSessions(session);
+        if (apexSessions.isEmpty()) {
+            return;
+        }
 
         final UserManagerInternal um = LocalServices.getService(UserManagerInternal.class);
         final int[] allUsers = um.getUserIds();
@@ -545,18 +574,19 @@ public class StagingManager {
             return;
         }
 
+        // Check if apex packages in the session failed to activate
         if (hasApex) {
             if (apexSessionInfo == null) {
-                String errorMsg = "apexd did not know anything about a staged session supposed to"
-                        + " be activated";
+                final String errorMsg = "apexd did not know anything about a staged session "
+                        + "supposed to be activated";
                 session.setStagedSessionFailed(SessionInfo.STAGED_SESSION_ACTIVATION_FAILED,
                         errorMsg);
                 abortCheckpoint(errorMsg);
                 return;
             }
             if (isApexSessionFailed(apexSessionInfo)) {
-                String errorMsg = "APEX activation failed. Check logcat messages from apexd for "
-                        + "more information.";
+                String errorMsg = "APEX activation failed. Check logcat messages from apexd "
+                        + "for more information.";
                 if (!TextUtils.isEmpty(mNativeFailureReason)) {
                     errorMsg = "Session reverted due to crashing native process: "
                             + mNativeFailureReason;
@@ -567,21 +597,26 @@ public class StagingManager {
                 return;
             }
             if (!apexSessionInfo.isActivated && !apexSessionInfo.isSuccess) {
-                // Apexd did not apply the session for some unknown reason. There is no guarantee
-                // that apexd will install it next time. Safer to proactively mark as failed.
-                String errorMsg = "Staged session " + session.sessionId + "at boot didn't "
-                        + "activate nor fail. Marking it as failed anyway.";
+                // Apexd did not apply the session for some unknown reason. There is no
+                // guarantee that apexd will install it next time. Safer to proactively mark
+                // it as failed.
+                final String errorMsg = "Staged session " + session.sessionId + "at boot "
+                        + "didn't activate nor fail. Marking it as failed anyway.";
                 session.setStagedSessionFailed(SessionInfo.STAGED_SESSION_ACTIVATION_FAILED,
                         errorMsg);
                 abortCheckpoint(errorMsg);
                 return;
             }
-            snapshotAndRestoreForApexSession(session);
-            Slog.i(TAG, "APEX packages in session " + session.sessionId
-                    + " were successfully activated. Proceeding with APK packages, if any");
         }
-        // The APEX part of the session is activated, proceed with the installation of APKs.
+        // Handle apk and apk-in-apex installation
         try {
+            if (hasApex) {
+                checkInstallationOfApkInApexSuccessful(session);
+                snapshotAndRestoreForApexSession(session);
+                Slog.i(TAG, "APEX packages in session " + session.sessionId
+                        + " were successfully activated. Proceeding with APK packages, if any");
+            }
+            // The APEX part of the session is activated, proceed with the installation of APKs.
             Slog.d(TAG, "Installing APK packages in session " + session.sessionId);
             installApksInSession(session);
         } catch (PackageManagerException e) {
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 0fbd1b64c379..72afca0300cd 100644
--- a/services/tests/servicestests/src/com/android/server/pm/ApexManagerTest.java
+++ b/services/tests/servicestests/src/com/android/server/pm/ApexManagerTest.java
@@ -273,6 +273,21 @@ public class ApexManagerTest {
         assertThat(mApexManager.uninstallApex(TEST_APEX_PKG)).isFalse();
     }
 
+    @Test
+    public void testReportErrorWithApkInApex() throws RemoteException {
+        when(mApexService.getActivePackages()).thenReturn(createApexInfo(true, true));
+        final ApexManager.ActiveApexInfo activeApex = mApexManager.getActiveApexInfos().get(0);
+        assertThat(activeApex.apexModuleName).isEqualTo(TEST_APEX_PKG);
+
+        when(mApexService.getAllPackages()).thenReturn(createApexInfo(true, true));
+        mApexManager.scanApexPackagesTraced(mPackageParser2,
+                ParallelPackageParser.makeExecutorService());
+
+        assertThat(mApexManager.isApkInApexInstallSuccess(activeApex.apexModuleName)).isTrue();
+        mApexManager.reportErrorWithApkInApex(activeApex.apexDirectory.getAbsolutePath());
+        assertThat(mApexManager.isApkInApexInstallSuccess(activeApex.apexModuleName)).isFalse();
+    }
+
     private ApexInfo[] createApexInfo(boolean isActive, boolean isFactory) {
         File apexFile = extractResource(TEST_APEX_PKG,  TEST_APEX_FILE_NAME);
         ApexInfo apexInfo = new ApexInfo();
@@ -281,6 +296,7 @@ public class ApexManagerTest {
         apexInfo.moduleName = TEST_APEX_PKG;
         apexInfo.modulePath = apexFile.getPath();
         apexInfo.versionCode = 191000070;
+        apexInfo.preinstalledModulePath = apexFile.getPath();
 
         return new ApexInfo[]{apexInfo};
     }