From b3ea9a61d4e847496c90708ab53655fed97ccfb7 Mon Sep 17 00:00:00 2001 From: JW Wang Date: Wed, 12 Feb 2020 10:53:28 +0800 Subject: Don't return early on error (1/n) Continue the call flow even when #enableRollbackForPackageSession fails so #completeEnableRollback has a chance to delete a rollback when any of the child sessions fails to enable the rollback for the package. Without this CL, the rollback data won't be deleted until next reboot. Since storage cost might be heavy for the rollback data, we want to delete it as soon as possible without waiting for reboot which might happen much later. Bug: 149352598 Test: test RollbackTest StagedRollbackTest Change-Id: I4aeda1f2447e1b12353d2e3866408aab67a637eb --- .../android/server/rollback/RollbackManagerServiceImpl.java | 11 +++-------- 1 file changed, 3 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 91e7cc981b89..47a1b0e0eca0 100644 --- a/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java +++ b/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java @@ -982,8 +982,6 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { if (!session.isMultiPackage()) { if (!enableRollbackForPackageSession(newRollback, session)) { Slog.e(TAG, "Unable to enable rollback for session: " + sessionId); - result.offer(-1); - return; } } else { for (int childSessionId : session.getChildSessionIds()) { @@ -991,13 +989,11 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { installer.getSessionInfo(childSessionId); if (childSession == null) { Slog.e(TAG, "No matching child install session for: " + childSessionId); - result.offer(-1); - return; + break; } if (!enableRollbackForPackageSession(newRollback, childSession)) { Slog.e(TAG, "Unable to enable rollback for session: " + sessionId); - result.offer(-1); - return; + break; } } } @@ -1188,8 +1184,7 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { } /** - * Add a rollback to the list of rollbacks. This should be called after rollback has been - * enabled for all packages in the rollback. It does not make the rollback available yet. + * Add a rollback to the list of rollbacks. It does not make the rollback available yet. * * @return the Rollback instance for a successfully enable-completed rollback, * or null on error. -- cgit v1.2.3-59-g8ed1b From 7fcc730e8d5fc00d0945783e6a649997432b9c1c Mon Sep 17 00:00:00 2001 From: JW Wang Date: Wed, 12 Feb 2020 11:30:18 +0800 Subject: Always enable apk-in-apex before the apex (2/n) To keep a rollback object in a consistent state, an apex shouldn't be enabled (for rollback) until all its embedded apk-in-apex are enabled successfully. Bug: 149352598 Test: test RollbackTest StagedRollbackTest Change-Id: I4d0208118050f948675093f4ed67de8ae148f41b --- .../rollback/RollbackManagerServiceImpl.java | 28 ++++++++++++---------- 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java b/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java index 47a1b0e0eca0..9e150fd6a8b3 100644 --- a/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java +++ b/services/core/java/com/android/server/rollback/RollbackManagerServiceImpl.java @@ -869,14 +869,6 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { return false; } - ApplicationInfo appInfo = pkgInfo.applicationInfo; - boolean success = rollback.enableForPackage(packageName, newPackage.versionCode, - pkgInfo.getLongVersionCode(), isApex, appInfo.sourceDir, - appInfo.splitSourceDirs, session.rollbackDataPolicy); - if (!success) { - return success; - } - if (isApex) { // Check if this apex contains apks inside it. If true, then they should be added as // a RollbackPackageInfo into this rollback @@ -894,12 +886,24 @@ class RollbackManagerServiceImpl extends IRollbackManager.Stub { Slog.e(TAG, apkInApex + " is not installed"); return false; } - success = rollback.enableForPackageInApex( - apkInApex, apkPkgInfo.getLongVersionCode(), session.rollbackDataPolicy); - if (!success) return success; + if (!rollback.enableForPackageInApex( + apkInApex, apkPkgInfo.getLongVersionCode(), session.rollbackDataPolicy)) { + return false; + } } } - return true; + + /** + * The order is important here! Always enable the embedded apk-in-apex (if any) before + * enabling the embedding apex. Otherwise the rollback object might be in an inconsistent + * state where an embedding apex is successfully enabled while one of its embedded + * apk-in-apex failed. Note {@link Rollback#allPackagesEnabled()} won't behave correctly if + * a rollback object is inconsistent because it doesn't count apk-in-apex. + */ + ApplicationInfo appInfo = pkgInfo.applicationInfo; + return rollback.enableForPackage(packageName, newPackage.versionCode, + pkgInfo.getLongVersionCode(), isApex, appInfo.sourceDir, + appInfo.splitSourceDirs, session.rollbackDataPolicy); } @Override -- cgit v1.2.3-59-g8ed1b