From 06650e7326efe5c67d989f3d2bc49cbd86c79324 Mon Sep 17 00:00:00 2001 From: JW Wang Date: Fri, 7 Feb 2020 16:03:35 +0800 Subject: [PATCH 1/4] Use #getRollbackForSessionLocked to search for the rollback (3/n) Each session is allocated with a unique id. No matter it is a parent session or a child one, a staged session or a non-staged one. For a given session id, we can correctly locate the matching rollback without checking #isNewRollback. In the end, we will be able to remove #getNewRollbackForPackageSessionLocked when the refactoring is done. Bug: 149069841 Test: atest RollbackTest Change-Id: Id7166def31d53ab33dc5212b038a135076c6cb8c --- .../android/server/rollback/RollbackManagerServiceImpl.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java b/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java index 1421258c12f64..e2cf9ad0a2ffe 100644 --- a/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java +++ b/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java @@ -788,10 +788,10 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { Rollback newRollback; synchronized (mLock) { - // See if we already have a NewRollback that contains this package - // session. If not, create a NewRollback for the parent session + // See if we already have a Rollback that contains this package + // session. If not, create a new Rollback for the parent session // that we will use for all the packages in the session. - newRollback = getNewRollbackForPackageSessionLocked(packageSession.getSessionId()); + newRollback = getRollbackForSessionLocked(packageSession.getSessionId()); if (newRollback == null) { newRollback = createNewRollbackLocked(parentSession); mRollbacks.add(newRollback); From 7e5eaadc271043c89db71993907710852f2135d6 Mon Sep 17 00:00:00 2001 From: JW Wang Date: Fri, 7 Feb 2020 16:25:53 +0800 Subject: [PATCH 2/4] Rewrite the handling of session finished with success (4/n) We only want to handle non-staged rollbacks here. Staged rollbacks are handled elsewhere. Checks for new rollbacks can be replaced by |!rollback.isStaged() && rollback.isEnabling()|. Bug: 149069841 Test: atest RollbackTest StagedRollbackTest Change-Id: I9488f94a76fdd4cedbfc494f9378d0baa054e057 --- .../rollback/RollbackManagerServiceImpl.java | 30 +++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java b/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java index e2cf9ad0a2ffe..e48fcf4080560 100644 --- a/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java +++ b/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java @@ -1148,24 +1148,24 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { } if (success) { - Rollback newRollback; + Rollback rollback; synchronized (mLock) { - newRollback = getNewRollbackForPackageSessionLocked(sessionId); - if (newRollback != null && newRollback.notifySessionWithSuccess()) { - mRollbacks.remove(newRollback); - newRollback.setIsNewRollback(false); - } else { - // Not all child sessions finished with success. - // Don't enable the rollback yet. - newRollback = null; + rollback = getRollbackForSessionLocked(sessionId); + if (rollback == null || rollback.isStaged() || !rollback.isEnabling() + || !rollback.notifySessionWithSuccess()) { + return; } + // All child sessions finished with success. We can enable this rollback now. + // TODO: refactor #completeEnableRollback so we won't remove 'rollback' from + // mRollbacks here and add it back in #completeEnableRollback later. + mRollbacks.remove(rollback); + rollback.setIsNewRollback(false); } - - if (newRollback != null) { - Rollback rollback = completeEnableRollback(newRollback); - if (rollback != null && !rollback.isStaged()) { - makeRollbackAvailable(rollback); - } + // TODO: Now #completeEnableRollback returns the same rollback object as the + // parameter on success. It would be more readable to return a boolean to indicate + // success or failure. + if (completeEnableRollback(rollback) != null) { + makeRollbackAvailable(rollback); } } else { synchronized (mLock) { From d1c18006cd25127d25ab0613f49f7b40f03d5ecc Mon Sep 17 00:00:00 2001 From: JW Wang Date: Fri, 7 Feb 2020 16:30:59 +0800 Subject: [PATCH 3/4] Remove unused code (5/n) Bug: 149069841 Test: atest RollbackTest StagedRollbackTest Change-Id: Ib9f3db7863d10d46ee639b6dc4d0b9441755502f --- .../com/android/server/rollback/Rollback.java | 21 ------------------- .../rollback/RollbackManagerServiceImpl.java | 20 ------------------ 2 files changed, 41 deletions(-) diff --git a/services/core/java/com/android/server/rollback/Rollback.java b/services/core/java/com/android/server/rollback/Rollback.java index b5da1c2ec58ac..4d7af9cc0d44c 100644 --- a/services/core/java/com/android/server/rollback/Rollback.java +++ b/services/core/java/com/android/server/rollback/Rollback.java @@ -181,15 +181,6 @@ 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. * @@ -838,18 +829,6 @@ 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 e48fcf4080560..59d93e16bae3a 100644 --- a/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java +++ b/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java @@ -795,7 +795,6 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { if (newRollback == null) { newRollback = createNewRollbackLocked(parentSession); mRollbacks.add(newRollback); - newRollback.setIsNewRollback(true); } } newRollback.addToken(token); @@ -1159,7 +1158,6 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { // TODO: refactor #completeEnableRollback so we won't remove 'rollback' from // mRollbacks here and add it back in #completeEnableRollback later. mRollbacks.remove(rollback); - rollback.setIsNewRollback(false); } // TODO: Now #completeEnableRollback returns the same rollback object as the // parameter on success. It would be more readable to return a boolean to indicate @@ -1354,22 +1352,4 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { } return null; } - - /** - * Returns the NewRollback associated with the given package session. - * Returns null if no NewRollback is found for the given package - * session. - */ - @WorkerThread - @GuardedBy("mLock") - Rollback getNewRollbackForPackageSessionLocked(int packageSessionId) { - // We expect mRollbacks to be a very small list; linear search - // should be plenty fast. - for (Rollback rollback: mRollbacks) { - if (rollback.isNewRollback() && rollback.containsSessionId(packageSessionId)) { - return rollback; - } - } - return null; - } } From 529d3aa46b3905733d226c4e2ed9b25a410bd3dd Mon Sep 17 00:00:00 2001 From: JW Wang Date: Mon, 10 Feb 2020 16:22:40 +0800 Subject: [PATCH 4/4] Delete rollbacks when build fingerprint has changed (1/n) Bug: 148688328 Test: atest CtsRollbackManagerHostTestCases Change-Id: Ia2ec05d421b91f8576510b838afe560dea58d5b6 --- .../server/rollback/RollbackManagerServiceImpl.java | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java b/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java index 59d93e16bae3a..91e7cc981b892 100644 --- a/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java +++ b/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java @@ -164,8 +164,16 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { // Load rollback data from device storage. synchronized (mLock) { mRollbacks = mRollbackStore.loadRollbacks(); - for (Rollback rollback : mRollbacks) { - mAllocatedRollbackIds.put(rollback.info.getRollbackId(), true); + if (!context.getPackageManager().isDeviceUpgrading()) { + for (Rollback rollback : mRollbacks) { + mAllocatedRollbackIds.put(rollback.info.getRollbackId(), true); + } + } else { + // Delete rollbacks when build fingerprint has changed. + for (Rollback rollback : mRollbacks) { + rollback.delete(mAppDataRollbackHelper); + } + mRollbacks.clear(); } }