From 3fa0ce99ac29df436d1f69b520492178d255d6c2 Mon Sep 17 00:00:00 2001 From: JW Wang Date: Fri, 20 Dec 2019 14:34:40 +0800 Subject: [PATCH 1/3] Enable a new rollback only when all child sessions succeeded (1/n) Don't enable a new rollback until all its child sessions are notified with success by SessionCallback#onFinished. This change allows us to delete/remove a new rollback if any of its child sessions failed before enabling it. Bug: 134652027 Test: atest RollbackTest Change-Id: Ie7cee8b0998a1df381a83219988a3876629fbcbb --- .../rollback/RollbackManagerServiceImpl.java | 50 +++++++++++++++---- 1 file changed, 39 insertions(+), 11 deletions(-) diff --git a/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java b/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java index 9a65ae6dba4dc..c21c0a99f822d 100644 --- a/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java +++ b/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java @@ -1089,19 +1089,28 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { if (LOCAL_LOGV) { Slog.v(TAG, "SessionCallback.onFinished id=" + sessionId + " success=" + success); } - NewRollback newRollback; - synchronized (mLock) { - newRollback = getNewRollbackForPackageSessionLocked(sessionId); - if (newRollback != null) { - mNewRollbacks.remove(newRollback); - } - } - if (newRollback != null) { - Rollback rollback = completeEnableRollback(newRollback, success); - if (rollback != null && !rollback.isStaged()) { - makeRollbackAvailable(rollback); + if (success) { + NewRollback newRollback; + synchronized (mLock) { + newRollback = getNewRollbackForPackageSessionLocked(sessionId); + if (newRollback != null && newRollback.notifySessionWithSuccess()) { + mNewRollbacks.remove(newRollback); + } else { + // Not all child sessions finished with success. + // Don't enable the rollback yet. + newRollback = null; + } } + + if (newRollback != null) { + Rollback rollback = completeEnableRollback(newRollback, success); + if (rollback != null && !rollback.isStaged()) { + makeRollbackAvailable(rollback); + } + } + } else { + // TODO: delete rollbacks for this failed session } // Clear the queue so it will never be leaked to next tests. @@ -1251,6 +1260,14 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { @GuardedBy("mNewRollbackLock") private boolean mIsCancelled = false; + /** + * The number of sessions in the install which are notified with success by + * {@link PackageInstaller.SessionCallback#onFinished(int, boolean)}. + * This NewRollback will be enabled only after all child sessions finished with success. + */ + @GuardedBy("mNewRollbackLock") + private int mNumPackageSessionsWithSuccess; + private final Object mNewRollbackLock = new Object(); NewRollback(Rollback rollback, int[] packageSessionIds) { @@ -1322,6 +1339,17 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { int getPackageSessionIdCount() { return mPackageSessionIds.length; } + + /** + * Called when a child session finished with success. + * Returns true when all child sessions are notified with success. This NewRollback will be + * enabled only after all child sessions finished with success. + */ + boolean notifySessionWithSuccess() { + synchronized (mNewRollbackLock) { + return ++mNumPackageSessionsWithSuccess == mPackageSessionIds.length; + } + } } @GuardedBy("mLock") From dbf4a8b327bb0e0a8fd9951a4ad13e34779f9fe8 Mon Sep 17 00:00:00 2001 From: JW Wang Date: Thu, 26 Dec 2019 11:27:01 +0800 Subject: [PATCH 2/3] Delete rollbacks immediately when any of child sessions failed (2/n) Currently rollbacks for abandoned sessions will be deleted after reboot. This change deletes rollbacks immediately/proactively when a session is abandoned to be more memory and disk efficient. Bug: 134652027 Test: adb install TestAppAv1.apk adb install --enable-rollback --staged TestAppAv2.apk dumpsys rollback adb shell pm install-abandon dumpsys rollback, confirm rollback is no longer listed Change-Id: I75005b2fd5b9f6035f4817b386e9e54f2b4243e6 --- .../rollback/RollbackManagerServiceImpl.java | 29 ++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java b/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java index c21c0a99f822d..5c83c50b970fa 100644 --- a/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java +++ b/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java @@ -780,6 +780,33 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { return enableRollbackForPackageSession(newRollback.rollback, packageSession); } + private void removeRollbackForPackageSessionId(int sessionId) { + if (LOCAL_LOGV) { + Slog.v(TAG, "removeRollbackForPackageSessionId=" + sessionId); + } + + synchronized (mLock) { + NewRollback newRollback = getNewRollbackForPackageSessionLocked(sessionId); + if (newRollback != null) { + Slog.w(TAG, "Delete new rollback id=" + newRollback.rollback.info.getRollbackId() + + " for session id=" + sessionId); + mNewRollbacks.remove(newRollback); + newRollback.rollback.delete(mAppDataRollbackHelper); + } + Iterator iter = mRollbacks.iterator(); + while (iter.hasNext()) { + Rollback rollback = iter.next(); + if (rollback.getStagedSessionId() == 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 @@ -1110,7 +1137,7 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { } } } else { - // TODO: delete rollbacks for this failed session + removeRollbackForPackageSessionId(sessionId); } // Clear the queue so it will never be leaked to next tests. From 6a70aaf58b3451f51e157a41c7e4efba0d870927 Mon Sep 17 00:00:00 2001 From: JW Wang Date: Thu, 26 Dec 2019 15:02:19 +0800 Subject: [PATCH 3/3] Remove the success parameter from #completeEnableRollback (3/n) Clean up some code since we always pass true to #completeEnableRollback now. Bug: 134652027 Test: atest RollbackTest StagedRollbackTest Change-Id: Ib38bb9acc62798e8455cf302c7038cee3a233704 --- .../rollback/RollbackManagerServiceImpl.java | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java b/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java index 5c83c50b970fa..1475ab9df37b7 100644 --- a/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java +++ b/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java @@ -971,7 +971,7 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { } } - Rollback rollback = completeEnableRollback(newRollback, true); + Rollback rollback = completeEnableRollback(newRollback); if (rollback == null) { result.offer(-1); } else { @@ -1131,7 +1131,7 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { } if (newRollback != null) { - Rollback rollback = completeEnableRollback(newRollback, success); + Rollback rollback = completeEnableRollback(newRollback); if (rollback != null && !rollback.isStaged()) { makeRollbackAvailable(rollback); } @@ -1152,16 +1152,10 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { * @return the Rollback instance for a successfully enable-completed rollback, * or null on error. */ - private Rollback completeEnableRollback(NewRollback newRollback, boolean success) { + private Rollback completeEnableRollback(NewRollback newRollback) { Rollback rollback = newRollback.rollback; if (LOCAL_LOGV) { - Slog.v(TAG, "completeEnableRollback id=" - + rollback.info.getRollbackId() + " success=" + success); - } - if (!success) { - // The install session was aborted, clean up the pending install. - rollback.delete(mAppDataRollbackHelper); - return null; + Slog.v(TAG, "completeEnableRollback id=" + rollback.info.getRollbackId()); } if (newRollback.isCancelled()) {