diff --git a/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java b/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java index fe362b042644e..d4c7ec355c197 100644 --- a/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java +++ b/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java @@ -50,6 +50,7 @@ import android.os.UserManager; // out of order to avoid merge conflict import android.os.SystemClock; import android.os.UserHandle; import android.provider.DeviceConfig; +import android.util.ArraySet; import android.util.IntArray; import android.util.Log; import android.util.SparseBooleanArray; @@ -69,11 +70,9 @@ import java.security.SecureRandom; import java.time.Instant; import java.time.temporal.ChronoUnit; import java.util.ArrayList; -import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; import java.util.List; -import java.util.Map; import java.util.Random; import java.util.Set; import java.util.concurrent.LinkedBlockingQueue; @@ -107,15 +106,9 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { @GuardedBy("mLock") private final SparseBooleanArray mAllocatedRollbackIds = new SparseBooleanArray(); - // Package rollback data for rollback-enabled installs that have not yet - // been committed. Maps from sessionId to rollback data. + // Package rollback data for rollbacks we are in the process of enabling. @GuardedBy("mLock") - private final Map mPendingRollbacks = new HashMap<>(); - - // Map from child session id's for enabled rollbacks to their - // corresponding parent session ids. - @GuardedBy("mLock") - private final Map mChildSessions = new HashMap<>(); + private final Set mNewRollbacks = new ArraySet<>(); // The list of all rollbacks, including available and committed rollbacks. // This list is null until the rollback data has been loaded. @@ -136,7 +129,6 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { // No need for guarding with lock because value is only accessed in handler thread. private long mRelativeBootTime = calculateRelativeBootTime(); - RollbackManagerServiceImpl(Context context) { mContext = context; // Note that we're calling onStart here because this object is only constructed on @@ -824,7 +816,6 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { // TODO: It would be nice if package manager or package installer told // us the session directly, rather than have to search for it // ourselves. - PackageInstaller.SessionInfo session = null; // getAllSessions only returns sessions for the associated user. // Create a context with the right user so we can find the matching @@ -835,7 +826,8 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { return false; } - int parentSessionId = -1; + PackageInstaller.SessionInfo parentSession = null; + PackageInstaller.SessionInfo packageSession = null; PackageInstaller installer = context.getPackageManager().getPackageInstaller(); for (PackageInstaller.SessionInfo info : installer.getAllSessions()) { if (info.isMultiPackage()) { @@ -843,21 +835,21 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { PackageInstaller.SessionInfo child = installer.getSessionInfo(childId); if (sessionMatchesForEnableRollback(child, installFlags, newPackageCodePath)) { // TODO: Check we only have one matching session? - parentSessionId = info.getSessionId(); - session = child; + parentSession = info; + packageSession = child; break; } } } else if (sessionMatchesForEnableRollback(info, installFlags, newPackageCodePath)) { // TODO: Check we only have one matching session? - parentSessionId = info.getSessionId(); - session = info; + parentSession = info; + packageSession = info; break; } } - if (session == null) { - Log.e(TAG, "Unable to find session id for enabled rollback."); + if (parentSession == null || packageSession == null) { + Log.e(TAG, "Unable to find session for enabled rollback."); return false; } @@ -869,7 +861,7 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { ensureRollbackDataLoadedLocked(); for (int i = 0; i < mRollbacks.size(); ++i) { RollbackData data = mRollbacks.get(i); - if (data.apkSessionId == parentSessionId) { + if (data.apkSessionId == parentSession.getSessionId()) { rd = data; break; } @@ -882,7 +874,7 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { PackageParser.PackageLite newPackage = null; try { newPackage = PackageParser.parsePackageLite( - new File(session.resolvedBaseCodePath), 0); + new File(packageSession.resolvedBaseCodePath), 0); } catch (PackageParser.PackageParserException e) { Log.e(TAG, "Unable to parse new package", e); return false; @@ -900,16 +892,32 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { return false; } - return enableRollbackForSession(session, installedUsers, true); + NewRollback newRollback; + synchronized (mLock) { + // See if we already have a NewRollback that contains this package + // session. If not, create a NewRollback for the parent session + // that we will use for all the packages in the session. + newRollback = getNewRollbackForPackageSessionLocked(packageSession.getSessionId()); + if (newRollback == null) { + newRollback = createNewRollbackLocked(parentSession); + mNewRollbacks.add(newRollback); + } + } + + return enableRollbackForPackageSession(newRollback.data, packageSession, + installedUsers, /* snapshotUserData*/ true); } /** * Do code and userdata backups to enable rollback of the given session. * In case of multiPackage sessions, session should be one of * the child sessions, not the parent session. + * + * @return true on success, false on failure. */ - private boolean enableRollbackForSession(PackageInstaller.SessionInfo session, - @NonNull int[] installedUsers, boolean snapshotUserData) { + private boolean enableRollbackForPackageSession(RollbackData data, + PackageInstaller.SessionInfo session, @NonNull int[] installedUsers, + boolean snapshotUserData) { // TODO: Don't attempt to enable rollback for split installs. final int installFlags = session.installFlags; if ((installFlags & PackageManager.INSTALL_ENABLE_ROLLBACK) == 0) { @@ -964,41 +972,14 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { VersionedPackage installedVersion = new VersionedPackage(packageName, pkgInfo.getLongVersionCode()); - PackageRollbackInfo info = new PackageRollbackInfo(newVersion, installedVersion, + PackageRollbackInfo packageRollbackInfo = new PackageRollbackInfo( + newVersion, installedVersion, new IntArray() /* pendingBackups */, new ArrayList<>() /* pendingRestores */, isApex, IntArray.wrap(installedUsers), new SparseLongArray() /* ceSnapshotInodes */); - RollbackData data; - try { - int childSessionId = session.getSessionId(); - int parentSessionId = session.getParentSessionId(); - if (parentSessionId == PackageInstaller.SessionInfo.INVALID_ID) { - parentSessionId = childSessionId; - } - - synchronized (mLock) { - // TODO: no need to add to mChildSessions if childSessionId is - // the same as parentSessionId. - mChildSessions.put(childSessionId, parentSessionId); - data = mPendingRollbacks.get(parentSessionId); - if (data == null) { - int rollbackId = allocateRollbackIdLocked(); - if (session.isStaged()) { - data = mRollbackStore.createStagedRollback(rollbackId, parentSessionId); - } else { - data = mRollbackStore.createNonStagedRollback(rollbackId); - } - mPendingRollbacks.put(parentSessionId, data); - } - data.info.getPackages().add(info); - } - } catch (IOException e) { - Log.e(TAG, "Unable to create rollback for " + packageName, e); - return false; - } if (snapshotUserData && !isApex) { - mAppDataRollbackHelper.snapshotAppData(data.info.getRollbackId(), info); + mAppDataRollbackHelper.snapshotAppData(data.info.getRollbackId(), packageRollbackInfo); } try { @@ -1013,6 +994,10 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { Log.e(TAG, "Unable to copy package for rollback for " + packageName, e); return false; } + + synchronized (mLock) { + data.info.getPackages().add(packageRollbackInfo); + } return true; } @@ -1083,8 +1068,14 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { return; } + NewRollback newRollback; + synchronized (mLock) { + newRollback = createNewRollbackLocked(session); + } + if (!session.isMultiPackage()) { - if (!enableRollbackForSession(session, new int[0], false)) { + if (!enableRollbackForPackageSession(newRollback.data, session, + new int[0], /* snapshotUserData */ false)) { Log.e(TAG, "Unable to enable rollback for session: " + sessionId); result.offer(false); return; @@ -1098,7 +1089,8 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { result.offer(false); return; } - if (!enableRollbackForSession(childSession, new int[0], false)) { + if (!enableRollbackForPackageSession(newRollback.data, childSession, + new int[0], /* snapshotUserData */ false)) { Log.e(TAG, "Unable to enable rollback for session: " + sessionId); result.offer(false); return; @@ -1106,8 +1098,7 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { } } - completeEnableRollback(sessionId, true); - result.offer(true); + result.offer(completeEnableRollback(newRollback, true) != null); }); try { @@ -1238,9 +1229,19 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { @Override public void onFinished(int sessionId, boolean success) { - RollbackData rollback = completeEnableRollback(sessionId, success); - if (rollback != null && !rollback.isStaged()) { - makeRollbackAvailable(rollback); + NewRollback newRollback; + synchronized (mLock) { + newRollback = getNewRollbackForPackageSessionLocked(sessionId); + if (newRollback != null) { + mNewRollbacks.remove(newRollback); + } + } + + if (newRollback != null) { + RollbackData rollback = completeEnableRollback(newRollback, success); + if (rollback != null && !rollback.isStaged()) { + makeRollbackAvailable(rollback); + } } } } @@ -1250,25 +1251,22 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { * This should be called after rollback has been enabled for all packages * in the rollback. It does not make the rollback available yet. * - * @return the rollback data for a successfully enable-completed rollback. + * @return the rollback data for a successfully enable-completed rollback, + * or null on error. */ - private RollbackData completeEnableRollback(int sessionId, boolean success) { - RollbackData data = null; - synchronized (mLock) { - Integer parentSessionId = mChildSessions.remove(sessionId); - if (parentSessionId != null) { - sessionId = parentSessionId; - } - - data = mPendingRollbacks.remove(sessionId); - } - - if (data == null) { + private RollbackData completeEnableRollback(NewRollback newRollback, boolean success) { + RollbackData data = newRollback.data; + if (!success) { + // The install session was aborted, clean up the pending install. + deleteRollback(data); return null; } - if (!success) { - // The install session was aborted, clean up the pending install. + // It's safe to access data.info outside a synchronized block because + // this is running on the handler thread and all changes to the + // data.info occur on the handler thread. + if (data.info.getPackages().size() != newRollback.packageSessionIds.length) { + Log.e(TAG, "Failed to enable rollback for all packages in session."); deleteRollback(data); return null; } @@ -1352,7 +1350,7 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { } @GuardedBy("mLock") - private int allocateRollbackIdLocked() throws IOException { + private int allocateRollbackIdLocked() { int n = 0; int rollbackId; do { @@ -1363,7 +1361,7 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { } } while (n++ < 32); - throw new IOException("Failed to allocate rollback ID"); + throw new IllegalStateException("Failed to allocate rollback ID"); } private void deleteRollback(RollbackData rollbackData) { @@ -1438,4 +1436,60 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { + Manifest.permission.TEST_MANAGE_ROLLBACKS); } } + + private static class NewRollback { + public final RollbackData data; + + /** + * Session ids for all packages in the install. + * For multi-package sessions, this is the list of child session ids. + * For normal sessions, this list is a single element with the normal + * session id. + */ + public final int[] packageSessionIds; + + NewRollback(RollbackData data, int[] packageSessionIds) { + this.data = data; + this.packageSessionIds = packageSessionIds; + } + } + + NewRollback createNewRollbackLocked(PackageInstaller.SessionInfo parentSession) { + int rollbackId = allocateRollbackIdLocked(); + final RollbackData data; + int parentSessionId = parentSession.getSessionId(); + + if (parentSession.isStaged()) { + data = mRollbackStore.createStagedRollback(rollbackId, parentSessionId); + } else { + data = mRollbackStore.createNonStagedRollback(rollbackId); + } + + int[] packageSessionIds; + if (parentSession.isMultiPackage()) { + packageSessionIds = parentSession.getChildSessionIds(); + } else { + packageSessionIds = new int[]{parentSessionId}; + } + + return new NewRollback(data, packageSessionIds); + } + + /** + * Returns the NewRollback associated with the given package session. + * Returns null if no NewRollback is found for the given package + * session. + */ + NewRollback getNewRollbackForPackageSessionLocked(int packageSessionId) { + // We expect mNewRollbacks to be a very small list; linear search + // should be plenty fast. + for (NewRollback newRollbackData : mNewRollbacks) { + for (int id : newRollbackData.packageSessionIds) { + if (id == packageSessionId) { + return newRollbackData; + } + } + } + return null; + } } diff --git a/services/core/java/com/android/server/rollback/RollbackStore.java b/services/core/java/com/android/server/rollback/RollbackStore.java index 2cfa465328344..8a26368c3e133 100644 --- a/services/core/java/com/android/server/rollback/RollbackStore.java +++ b/services/core/java/com/android/server/rollback/RollbackStore.java @@ -194,7 +194,7 @@ class RollbackStore { * Creates a new RollbackData instance for a non-staged rollback with * backupDir assigned. */ - RollbackData createNonStagedRollback(int rollbackId) throws IOException { + RollbackData createNonStagedRollback(int rollbackId) { File backupDir = new File(mRollbackDataDir, Integer.toString(rollbackId)); return new RollbackData(rollbackId, backupDir, -1); } @@ -203,8 +203,7 @@ class RollbackStore { * Creates a new RollbackData instance for a staged rollback with * backupDir assigned. */ - RollbackData createStagedRollback(int rollbackId, int stagedSessionId) - throws IOException { + RollbackData createStagedRollback(int rollbackId, int stagedSessionId) { File backupDir = new File(mRollbackDataDir, Integer.toString(rollbackId)); return new RollbackData(rollbackId, backupDir, stagedSessionId); } diff --git a/tests/RollbackTest/RollbackTest/src/com/android/tests/rollback/RollbackTest.java b/tests/RollbackTest/RollbackTest/src/com/android/tests/rollback/RollbackTest.java index 28c371c034b82..13b5b9ac104e9 100644 --- a/tests/RollbackTest/RollbackTest/src/com/android/tests/rollback/RollbackTest.java +++ b/tests/RollbackTest/RollbackTest/src/com/android/tests/rollback/RollbackTest.java @@ -775,6 +775,42 @@ public class RollbackTest { } } + /** + * Test failure to enable rollback for multi-package installs. + * If any one of the packages fail to enable rollback, we shouldn't enable + * rollback for any package. + */ + @Test + public void testMultiPackageEnableFail() throws Exception { + try { + RollbackTestUtils.adoptShellPermissionIdentity( + Manifest.permission.INSTALL_PACKAGES, + Manifest.permission.DELETE_PACKAGES, + Manifest.permission.TEST_MANAGE_ROLLBACKS); + RollbackManager rm = RollbackTestUtils.getRollbackManager(); + + RollbackTestUtils.uninstall(TEST_APP_A); + RollbackTestUtils.uninstall(TEST_APP_B); + RollbackTestUtils.install("RollbackTestAppAv1.apk", false); + + // We should fail to enable rollback here because TestApp B is not + // already installed. + RollbackTestUtils.installMultiPackage(true, + "RollbackTestAppAv2.apk", + "RollbackTestAppBv2.apk"); + + assertEquals(2, RollbackTestUtils.getInstalledVersion(TEST_APP_A)); + assertEquals(2, RollbackTestUtils.getInstalledVersion(TEST_APP_B)); + + assertNull(getUniqueRollbackInfoForPackage( + rm.getAvailableRollbacks(), TEST_APP_A)); + assertNull(getUniqueRollbackInfoForPackage( + rm.getAvailableRollbacks(), TEST_APP_B)); + } finally { + RollbackTestUtils.dropShellPermissionIdentity(); + } + } + @Test @Ignore("b/120200473") /**