From 13ef57fdd5ee617f5f8d061a7e1bdb6333881be7 Mon Sep 17 00:00:00 2001 From: JW Wang Date: Mon, 3 Feb 2020 12:15:56 +0800 Subject: [PATCH 1/5] Add a flag to facilitate merging 2 rollback collections (8/n) We will move rollbacks from #mNewRollbacks to #mRollbacks in the next CL. To preserve the same semantics for new rollbacks, rollbacks originally managed by #mNewRollbacks will have Rollback#mIsNewRollback==true. Bug: 147400979 Test: atest RollbackTest Change-Id: I7edd3cf75d40ff2a61c485efc0cc4ec7de7d4f99 --- .../com/android/server/rollback/Rollback.java | 21 +++++++++++++++++++ .../rollback/RollbackManagerServiceImpl.java | 2 ++ 2 files changed, 23 insertions(+) diff --git a/services/core/java/com/android/server/rollback/Rollback.java b/services/core/java/com/android/server/rollback/Rollback.java index 4d7af9cc0d44c..b5da1c2ec58ac 100644 --- a/services/core/java/com/android/server/rollback/Rollback.java +++ b/services/core/java/com/android/server/rollback/Rollback.java @@ -181,6 +181,15 @@ class Rollback { @GuardedBy("mLock") private int mNumPackageSessionsWithSuccess; + /** + * A temp flag to facilitate merging of the 2 rollback collections managed by + * RollbackManagerServiceImpl. True if this rollback is in the process of enabling and was + * originally managed by RollbackManagerServiceImpl#mNewRollbacks. + * TODO: remove this flag when merge is completed. + */ + @GuardedBy("mLock") + private boolean mIsNewRollback = false; + /** * Constructs a new, empty Rollback instance. * @@ -829,6 +838,18 @@ class Rollback { } } + void setIsNewRollback(boolean newRollback) { + synchronized (mLock) { + mIsNewRollback = newRollback; + } + } + + boolean isNewRollback() { + synchronized (mLock) { + return mIsNewRollback; + } + } + static String rollbackStateToString(@RollbackState int state) { switch (state) { case Rollback.ROLLBACK_STATE_ENABLING: return "enabling"; diff --git a/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java b/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java index 8bd9533727d65..e2b900f2efb89 100644 --- a/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java +++ b/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java @@ -811,6 +811,7 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { if (newRollback == null) { newRollback = createNewRollbackLocked(parentSession); mNewRollbacks.add(newRollback); + newRollback.setIsNewRollback(true); } } newRollback.addToken(token); @@ -1201,6 +1202,7 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { newRollback = getNewRollbackForPackageSessionLocked(sessionId); if (newRollback != null && newRollback.notifySessionWithSuccess()) { mNewRollbacks.remove(newRollback); + newRollback.setIsNewRollback(false); } else { // Not all child sessions finished with success. // Don't enable the rollback yet. From 70bdd97927ec4692420c02a8ce6cf4324b6d8fc4 Mon Sep 17 00:00:00 2001 From: JW Wang Date: Mon, 3 Feb 2020 12:50:53 +0800 Subject: [PATCH 2/5] Put rollbacks into #mRollbacks (9/n) Now we check Rollback#isNewRollback for rollbacks that were originally in #mNewRollbacks. for (Rollback newRollback : mNewRollbacks) { // Do something with newRollback... } will be replaced by: for (Rollback newRollback : mRollbacks) { if (newRollback.isNewRollback()) { // Do something with newRollback... } } Since mRollbacks includes new rollbacks, be careful not to apply operations not appropriate to new rollbacks when iterating over mRollbacks. Luckily most of the code is future-proof that needs no changes. Note now #mNewRollbacks is always empty. We will remove it in the next CL. Bug: 147400979 Test: atest RollbackTest StagedRollbackTest Change-Id: Ia3a4116b352228adc0b152d42c85920f375beb28 --- .../content/rollback/RollbackManager.java | 4 ++++ .../rollback/RollbackManagerServiceImpl.java | 23 ++++++++++--------- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/core/java/android/content/rollback/RollbackManager.java b/core/java/android/content/rollback/RollbackManager.java index 73b8a48d9153c..7ebeb212b64a3 100644 --- a/core/java/android/content/rollback/RollbackManager.java +++ b/core/java/android/content/rollback/RollbackManager.java @@ -216,6 +216,10 @@ public final class RollbackManager { * across device reboot, by simulating what happens on reboot without * actually rebooting the device. * + * Note rollbacks in the process of enabling will be lost after calling + * this method since they are not persisted yet. Don't call this method + * in the middle of the install process. + * * @throws SecurityException if the caller does not have appropriate permissions. * * @hide diff --git a/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java b/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java index e2b900f2efb89..791d396f9733c 100644 --- a/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java +++ b/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java @@ -241,14 +241,14 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { } synchronized (mLock) { Rollback found = null; - for (Rollback newRollback : mNewRollbacks) { - if (newRollback.hasToken(token)) { - found = newRollback; + for (Rollback rollback : mRollbacks) { + if (rollback.isNewRollback() && rollback.hasToken(token)) { + found = rollback; break; } } if (found != null) { - mNewRollbacks.remove(found); + mRollbacks.remove(found); found.delete(mAppDataRollbackHelper); } } @@ -810,7 +810,7 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { newRollback = getNewRollbackForPackageSessionLocked(packageSession.getSessionId()); if (newRollback == null) { newRollback = createNewRollbackLocked(parentSession); - mNewRollbacks.add(newRollback); + mRollbacks.add(newRollback); newRollback.setIsNewRollback(true); } } @@ -836,7 +836,8 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { Iterator iter = mRollbacks.iterator(); while (iter.hasNext()) { Rollback rollback = iter.next(); - if (rollback.getStagedSessionId() == sessionId) { + if (rollback.getStagedSessionId() == sessionId + || (rollback.isNewRollback() && rollback.containsSessionId(sessionId))) { Slog.w(TAG, "Delete rollback id=" + rollback.info.getRollbackId() + " for session id=" + sessionId); iter.remove(); @@ -1201,7 +1202,7 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { synchronized (mLock) { newRollback = getNewRollbackForPackageSessionLocked(sessionId); if (newRollback != null && newRollback.notifySessionWithSuccess()) { - mNewRollbacks.remove(newRollback); + mRollbacks.remove(newRollback); newRollback.setIsNewRollback(false); } else { // Not all child sessions finished with success. @@ -1385,11 +1386,11 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { @WorkerThread @GuardedBy("mLock") Rollback getNewRollbackForPackageSessionLocked(int packageSessionId) { - // We expect mNewRollbacks to be a very small list; linear search + // We expect mRollbacks to be a very small list; linear search // should be plenty fast. - for (Rollback newRollback: mNewRollbacks) { - if (newRollback.containsSessionId(packageSessionId)) { - return newRollback; + for (Rollback rollback: mRollbacks) { + if (rollback.isNewRollback() && rollback.containsSessionId(packageSessionId)) { + return rollback; } } return null; From 9c6da834019c98d9bafa954452c51ee0d7f051b7 Mon Sep 17 00:00:00 2001 From: JW Wang Date: Mon, 3 Feb 2020 12:58:25 +0800 Subject: [PATCH 3/5] Remove #mNewRollbacks (10/n) mNewRollbacks now is always empty and unsed. We can safely remove it now. Bug: 147400979 Test: atest RollbackTest Change-Id: I31f6b981a9a3eedeb8ef645e93aea1e6d5a7e4e5 --- .../rollback/RollbackManagerServiceImpl.java | 26 ------------------- 1 file changed, 26 deletions(-) diff --git a/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java b/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java index 791d396f9733c..a63f921f65731 100644 --- a/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java +++ b/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java @@ -50,7 +50,6 @@ import android.os.SystemClock; import android.os.UserHandle; import android.os.UserManager; import android.provider.DeviceConfig; -import android.util.ArraySet; import android.util.IntArray; import android.util.Log; import android.util.LongArrayQueue; @@ -121,10 +120,6 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { @GuardedBy("mLock") private final SparseBooleanArray mAllocatedRollbackIds = new SparseBooleanArray(); - // Rollbacks we are in the process of enabling. - @GuardedBy("mLock") - private final Set mNewRollbacks = new ArraySet<>(); - // The list of all rollbacks, including available and committed rollbacks. @GuardedBy("mLock") private final List mRollbacks; @@ -442,15 +437,6 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { rollback.delete(mAppDataRollbackHelper); } } - Iterator iter2 = mNewRollbacks.iterator(); - while (iter2.hasNext()) { - Rollback newRollback = iter2.next(); - if (newRollback.includesPackage(packageName)) { - iter2.remove(); - newRollback.delete(mAppDataRollbackHelper); - } - - } } } @@ -826,13 +812,6 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { } synchronized (mLock) { - Rollback newRollback = getNewRollbackForPackageSessionLocked(sessionId); - if (newRollback != null) { - Slog.w(TAG, "Delete new rollback id=" + newRollback.info.getRollbackId() - + " for session id=" + sessionId); - mNewRollbacks.remove(newRollback); - newRollback.delete(mAppDataRollbackHelper); - } Iterator iter = mRollbacks.iterator(); while (iter.hasNext()) { Rollback rollback = iter.next(); @@ -968,15 +947,10 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { + " users=" + Arrays.toString(userIds)); } synchronized (mLock) { - // staged installs for (int i = 0; i < mRollbacks.size(); i++) { Rollback rollback = mRollbacks.get(i); rollback.snapshotUserData(packageName, userIds, mAppDataRollbackHelper); } - // non-staged installs - for (Rollback rollback : mNewRollbacks) { - rollback.snapshotUserData(packageName, userIds, mAppDataRollbackHelper); - } } } From dc1f92192adcd92a025085da642fc88905f13b33 Mon Sep 17 00:00:00 2001 From: JW Wang Date: Fri, 7 Feb 2020 14:59:51 +0800 Subject: [PATCH 4/5] Rewrite the broadcast receiver for ACTION_CANCEL_ENABLE_ROLLBACK (1/n) It makes sense to remove a rollback when ACTION_CANCEL_ENABLE_ROLLBACK is received no matter it is a new rollback or not. For the sake of defensive programming, we don't want to remove a rollback which is already made available or committed. Bug: 149069841 Test: atest RollbackTest Change-Id: I3d8004916e9438bc03fcb0c71a83617411be7379 --- .../server/rollback/RollbackManagerServiceImpl.java | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java b/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java index a63f921f65731..1188a342793b1 100644 --- a/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java +++ b/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java @@ -235,17 +235,14 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { Slog.v(TAG, "broadcast=ACTION_CANCEL_ENABLE_ROLLBACK token=" + token); } synchronized (mLock) { - Rollback found = null; - for (Rollback rollback : mRollbacks) { - if (rollback.isNewRollback() && rollback.hasToken(token)) { - found = rollback; + for (int i = 0; i < mRollbacks.size(); ++i) { + Rollback rollback = mRollbacks.get(i); + if (rollback.hasToken(token) && rollback.isEnabling()) { + mRollbacks.remove(i); + rollback.delete(mAppDataRollbackHelper); break; } } - if (found != null) { - mRollbacks.remove(found); - found.delete(mAppDataRollbackHelper); - } } } } From 1ac3d11862b73ca43f12e23741a89701f47c9faa Mon Sep 17 00:00:00 2001 From: JW Wang Date: Fri, 7 Feb 2020 15:52:36 +0800 Subject: [PATCH 5/5] Rewrite handling of failed sessions (2/n) We want to delete rollbacks for failed sessions as long as parent or child session id matches. For the sake of defensive programming, we check #isEnabling to ensure we won't delete rollbacks that are already made available or committed. Bug: 149069841 Test: atest RollbackTest StagedRollbackTest Change-Id: Ie95789c9c87f91d2cca84fede621bd17a6a9da18 --- .../rollback/RollbackManagerServiceImpl.java | 52 +++++++++++-------- 1 file changed, 29 insertions(+), 23 deletions(-) diff --git a/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java b/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java index 1188a342793b1..1421258c12f64 100644 --- a/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java +++ b/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java @@ -19,6 +19,7 @@ package com.android.server.rollback; import android.Manifest; import android.annotation.AnyThread; import android.annotation.NonNull; +import android.annotation.Nullable; import android.annotation.UserIdInt; import android.annotation.WorkerThread; import android.app.AppOpsManager; @@ -802,28 +803,6 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { return enableRollbackForPackageSession(newRollback, packageSession); } - @WorkerThread - private void removeRollbackForPackageSessionId(int sessionId) { - if (LOCAL_LOGV) { - Slog.v(TAG, "removeRollbackForPackageSessionId=" + sessionId); - } - - synchronized (mLock) { - Iterator iter = mRollbacks.iterator(); - while (iter.hasNext()) { - Rollback rollback = iter.next(); - if (rollback.getStagedSessionId() == sessionId - || (rollback.isNewRollback() && rollback.containsSessionId(sessionId))) { - Slog.w(TAG, "Delete rollback id=" + rollback.info.getRollbackId() - + " for session id=" + sessionId); - iter.remove(); - rollback.delete(mAppDataRollbackHelper); - break; - } - } - } - } - /** * Do code and userdata backups to enable rollback of the given session. * In case of multiPackage sessions, session should be one of @@ -1189,7 +1168,15 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { } } } else { - removeRollbackForPackageSessionId(sessionId); + synchronized (mLock) { + Rollback rollback = getRollbackForSessionLocked(sessionId); + if (rollback != null && rollback.isEnabling()) { + Slog.w(TAG, "Delete rollback id=" + rollback.info.getRollbackId() + + " for failed session id=" + sessionId); + mRollbacks.remove(rollback); + rollback.delete(mAppDataRollbackHelper); + } + } } } } @@ -1349,6 +1336,25 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { return rollback; } + /** + * Returns the Rollback associated with the given session if parent or child session id matches. + * Returns null if not found. + */ + @WorkerThread + @GuardedBy("mLock") + @Nullable + private Rollback getRollbackForSessionLocked(int sessionId) { + // We expect mRollbacks to be a very small list; linear search should be plenty fast. + for (int i = 0; i < mRollbacks.size(); ++i) { + Rollback rollback = mRollbacks.get(i); + if (rollback.getStagedSessionId() == sessionId + || rollback.containsSessionId(sessionId)) { + return rollback; + } + } + return null; + } + /** * Returns the NewRollback associated with the given package session. * Returns null if no NewRollback is found for the given package