From a49b7c6867fe27f313b231caa660e04a662d7ad3 Mon Sep 17 00:00:00 2001 From: JW Wang Date: Mon, 17 Jan 2022 18:24:02 +0800 Subject: [PATCH] Apply APPLIED/FAILED to non-staged sessions (8/n) 1. installNonStaged() now returns a future so we can do cleanup and state changes when the whole install phase is completed. This makes it easy to apply APPLIED/FAILED to both staged and non-staged sessions. 2. Following (1), dispatchSessionFinished() is no longer needed inside onPackageInstalled(). It is now called in install() when the install phase is completed. 3. Following (2), child sessions don't need mRemoteStatusReceiver anymore. The parent's mRemoteStatusReceiver will be used when needing user actions. 4. StagedSession#installSession() is always called for apex-only sessions. Now we have a single code path to do cleanup and state changes for all staged/non-staged apk/apex/mix sessions. Bug: 210359798 Test: atest StagingManagerTest \ CtsAtomicInstallTestCases \ GtsStagedInstallHostTestCases \ CtsRootPackageInstallerHostTestCases \ CtsStagedInstallHostTestCases \ CtsInstallHostTestCases \ StagedInstallInternalTest Change-Id: I5b37185256a11fa3c534f4cbe0b56154f4816a82 --- .../server/pm/PackageInstallerService.java | 5 +- .../server/pm/PackageInstallerSession.java | 151 ++++++++++-------- .../com/android/server/pm/StagingManager.java | 34 ++-- .../android/server/pm/StagingManagerTest.java | 4 +- 4 files changed, 102 insertions(+), 92 deletions(-) diff --git a/services/core/java/com/android/server/pm/PackageInstallerService.java b/services/core/java/com/android/server/pm/PackageInstallerService.java index 1f10d77086d3a..fccd0319fc399 100644 --- a/services/core/java/com/android/server/pm/PackageInstallerService.java +++ b/services/core/java/com/android/server/pm/PackageInstallerService.java @@ -1641,7 +1641,10 @@ public class PackageInstallerService extends IPackageInstaller.Stub implements public void onSessionChanged(PackageInstallerSession session) { session.markUpdated(); mSettingsWriteRequest.schedule(); - if (mOkToSendBroadcasts && !session.isDestroyed()) { + // TODO(b/210359798): Remove the session.isStaged() check. Some apps assume this + // broadcast is sent by only staged sessions and call isStagedSessionApplied() without + // checking if it is a staged session or not and cause exception. + if (mOkToSendBroadcasts && !session.isDestroyed() && session.isStaged()) { // we don't scrub the data here as this is sent only to the installer several // privileged system packages sendSessionUpdatedBroadcast( diff --git a/services/core/java/com/android/server/pm/PackageInstallerSession.java b/services/core/java/com/android/server/pm/PackageInstallerSession.java index 4d0d5532399ec..17ce33f708183 100644 --- a/services/core/java/com/android/server/pm/PackageInstallerSession.java +++ b/services/core/java/com/android/server/pm/PackageInstallerSession.java @@ -179,6 +179,7 @@ import java.util.Collections; import java.util.List; import java.util.Objects; import java.util.Set; +import java.util.concurrent.CompletableFuture; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Predicate; @@ -546,18 +547,15 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { /** * Installs apks of staged session while skipping the verification process for a committed * and ready session. + * + * @return a CompletableFuture that will be completed when installation completes. */ @Override - public void installSession(IntentSender statusReceiver) { + public CompletableFuture installSession() { assertCallerIsOwnerOrRootOrSystem(); assertNotChild("StagedSession#installSession"); Preconditions.checkArgument(isCommitted() && isSessionReady()); - - // Since staged sessions are installed during boot, the original reference to status - // receiver from the owner has already been lost. We can safely replace it with a - // status receiver from the system without effecting the flow. - updateRemoteStatusReceiver(statusReceiver); - install(); + return install(); } private void updateRemoteStatusReceiver(IntentSender remoteStatusReceiver) { @@ -1691,15 +1689,11 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { } if (isMultiPackage()) { synchronized (mLock) { - final IntentSender childIntentSender = - new ChildStatusIntentReceiver(mChildSessions.clone(), statusReceiver) - .getIntentSender(); boolean sealFailed = false; for (int i = mChildSessions.size() - 1; i >= 0; --i) { // seal all children, regardless if any of them fail; we'll throw/return // as appropriate once all children have been processed - if (!mChildSessions.valueAt(i) - .markAsSealed(childIntentSender, forTransfer)) { + if (!mChildSessions.valueAt(i).markAsSealed(null, forTransfer)) { sealFailed = true; } } @@ -1887,8 +1881,9 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { * This method may be called multiple times to update the status receiver validate caller * permissions. */ - private boolean markAsSealed(@NonNull IntentSender statusReceiver, boolean forTransfer) { - Objects.requireNonNull(statusReceiver); + private boolean markAsSealed(@Nullable IntentSender statusReceiver, boolean forTransfer) { + Preconditions.checkState(statusReceiver != null || hasParentSessionId(), + "statusReceiver can't be null for the root session"); assertCallerIsOwnerOrRoot(); synchronized (mLock) { @@ -2188,7 +2183,8 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { } @WorkerThread - private static boolean checkUserActionRequirement(PackageInstallerSession session) { + private static boolean checkUserActionRequirement( + PackageInstallerSession session, IntentSender target) { if (session.isMultiPackage()) { return false; } @@ -2197,7 +2193,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { // TODO(b/159331446): Move this to makeSessionActiveForInstall and update javadoc userActionRequirement = session.computeUserActionRequirement(); if (userActionRequirement == USER_ACTION_REQUIRED) { - session.sendPendingUserActionIntent(); + session.sendPendingUserActionIntent(target); return true; } @@ -2209,7 +2205,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { if (validatedTargetSdk != INVALID_TARGET_SDK_VERSION && validatedTargetSdk < Build.VERSION_CODES.Q) { - session.sendPendingUserActionIntent(); + session.sendPendingUserActionIntent(target); return true; } @@ -2218,7 +2214,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { session.getInstallerPackageName(), session.getPackageName())) { // Fall back to the non-silent update if a repeated installation is invoked // within the throttle time. - session.sendPendingUserActionIntent(); + session.sendPendingUserActionIntent(target); return true; } session.mSilentUpdatePolicy.track(session.getInstallerPackageName(), @@ -2238,7 +2234,8 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { private boolean sendPendingUserActionIntentIfNeeded() { assertNotChild("PackageInstallerSession#sendPendingUserActionIntentIfNeeded"); - return sessionContains(PackageInstallerSession::checkUserActionRequirement); + final IntentSender statusReceiver = getRemoteStatusReceiver(); + return sessionContains(s -> checkUserActionRequirement(s, statusReceiver)); } @WorkerThread @@ -2436,53 +2433,69 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { }); } - private void install() { - try { - installNonStaged(); - } catch (PackageManagerException e) { - final String completeMsg = ExceptionUtils.getCompleteMessage(e); - onSessionInstallationFailure(e.error, completeMsg); - } + /** + * Stages installs and do cleanup accordingly depending on whether the installation is + * successful or not. + * + * @return a future that will be completed when the whole process is completed. + */ + private CompletableFuture install() { + return installNonStaged().whenComplete((r, t) -> { + if (t == null) { + setSessionApplied(); + dispatchSessionFinished(INSTALL_SUCCEEDED, "Session installed", null); + maybeFinishChildSessions(INSTALL_SUCCEEDED, "Session installed"); + } else { + PackageManagerException e = (PackageManagerException) t.getCause(); + setSessionFailed(SessionInfo.SESSION_ACTIVATION_FAILED, + PackageManager.installStatusToString(e.error, e.getMessage())); + dispatchSessionFinished(e.error, e.getMessage(), null); + maybeFinishChildSessions(e.error, e.getMessage()); + } + }); } - private void installNonStaged() - throws PackageManagerException { - final InstallParams installingSession = makeInstallParams(); - if (installingSession == null) { - throw new PackageManagerException(INSTALL_FAILED_INTERNAL_ERROR, - "Session should contain at least one apk session for installation"); - } - if (isMultiPackage()) { - final List childSessions = getChildSessions(); - List installingChildSessions = new ArrayList<>(childSessions.size()); - boolean success = true; - PackageManagerException failure = null; - for (int i = 0; i < childSessions.size(); ++i) { - final PackageInstallerSession session = childSessions.get(i); - try { - final InstallParams installingChildSession = - session.makeInstallParams(); + /** + * Stages sessions (including child sessions if any) for install. + * + * @return a future that will be completed when the whole session is completed (could be + * success or failure). + */ + private CompletableFuture installNonStaged() { + try { + List> futures = new ArrayList<>(); + CompletableFuture future = new CompletableFuture<>(); + futures.add(future); + final InstallParams installingSession = makeInstallParams(future); + if (isMultiPackage()) { + final List childSessions = getChildSessions(); + List installingChildSessions = new ArrayList<>(childSessions.size()); + for (int i = 0; i < childSessions.size(); ++i) { + final PackageInstallerSession session = childSessions.get(i); + future = new CompletableFuture<>(); + futures.add(future); + final InstallParams installingChildSession = session.makeInstallParams(future); if (installingChildSession != null) { installingChildSessions.add(installingChildSession); } - } catch (PackageManagerException e) { - failure = e; - success = false; } + if (!installingChildSessions.isEmpty()) { + installingSession.installStage(installingChildSessions); + } + } else if (installingSession != null) { + installingSession.installStage(); } - if (!success) { - sendOnPackageInstalled(mContext, getRemoteStatusReceiver(), sessionId, - isInstallerDeviceOwnerOrAffiliatedProfileOwner(), userId, null, - failure.error, failure.getLocalizedMessage(), null); - return; - } - installingSession.installStage(installingChildSessions); - } else { - installingSession.installStage(); + + CompletableFuture[] arr = new CompletableFuture[futures.size()]; + return CompletableFuture.allOf(futures.toArray(arr)); + } catch (PackageManagerException e) { + CompletableFuture future = new CompletableFuture<>(); + future.completeExceptionally(e); + return future; } } - private void sendPendingUserActionIntent() { + private void sendPendingUserActionIntent(IntentSender target) { // User needs to confirm installation; // give installer an intent they can use to involve // user. @@ -2490,7 +2503,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { intent.setPackage(mPm.getPackageInstallerPackageName()); intent.putExtra(PackageInstaller.EXTRA_SESSION_ID, sessionId); - sendOnUserActionRequired(mContext, getRemoteStatusReceiver(), sessionId, intent); + sendOnUserActionRequired(mContext, target, sessionId, intent); // Commit was keeping session marked as active until now; release // that extra refcount so session appears idle. @@ -2522,9 +2535,11 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { /** * Stages this session for install and returns a * {@link InstallParams} representing this new staged state. + * + * @param future a future that will be completed when this session is completed. */ @Nullable - private InstallParams makeInstallParams() + private InstallParams makeInstallParams(CompletableFuture future) throws PackageManagerException { synchronized (mLock) { if (mDestroyed) { @@ -2537,11 +2552,12 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { } } - // Do not try to install staged apex session. Parent session will have at least one apk - // session. - if (!isMultiPackage() && isApexSession() && params.isStaged) { - dispatchSessionFinished(INSTALL_SUCCEEDED, - "Apex package should have been installed by apexd", null); + if (isMultiPackage()) { + // Always treat parent session as success for it has nothing to install + future.complete(null); + } else if (isApexSession() && params.isStaged) { + // Staged apex sessions have been handled by apexd + future.complete(null); return null; } @@ -2554,12 +2570,11 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { @Override public void onPackageInstalled(String basePackageName, int returnCode, String msg, Bundle extras) { - if (!isStaged()) { - // We've reached point of no return; call into PMS to install the stage. - // Regardless of success or failure we always destroy session. - destroyInternal(); + if (returnCode == INSTALL_SUCCEEDED) { + future.complete(null); + } else { + future.completeExceptionally(new PackageManagerException(returnCode, msg)); } - dispatchSessionFinished(returnCode, msg, extras); } }; diff --git a/services/core/java/com/android/server/pm/StagingManager.java b/services/core/java/com/android/server/pm/StagingManager.java index 29de5551cb27a..804e5ec00b041 100644 --- a/services/core/java/com/android/server/pm/StagingManager.java +++ b/services/core/java/com/android/server/pm/StagingManager.java @@ -77,6 +77,7 @@ import java.util.List; import java.util.Map; import java.util.Set; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutionException; import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.TimeUnit; import java.util.function.Predicate; @@ -131,7 +132,7 @@ public class StagingManager { void setSessionReady(); void setSessionFailed(@SessionErrorCode int errorCode, String errorMessage); void setSessionApplied(); - void installSession(IntentSender statusReceiver); + CompletableFuture installSession(); boolean hasParentSessionId(); long getCommittedMillis(); void abandon(); @@ -417,8 +418,6 @@ public class StagingManager { installApksInSession(session); t.traceEnd(); - Slog.d(TAG, "Marking session " + session.sessionId() + " as applied"); - session.setSessionApplied(); if (hasApex) { if (supportsCheckpoint) { // Store the session ID, which will be marked as successful by ApexManager upon @@ -494,24 +493,17 @@ public class StagingManager { } } - private void installApksInSession(StagedSession session) - throws PackageManagerException { - if (!session.containsApkSession()) { - return; - } - - final LocalIntentReceiverSync receiver = new LocalIntentReceiverSync(); - session.installSession(receiver.getIntentSender()); - final Intent result = receiver.getResult(); - final int status = result.getIntExtra(PackageInstaller.EXTRA_STATUS, - PackageInstaller.STATUS_FAILURE); - if (status != PackageInstaller.STATUS_SUCCESS) { - final String errorMessage = result.getStringExtra( - PackageInstaller.EXTRA_STATUS_MESSAGE); - Slog.e(TAG, "Failure to install APK staged session " - + session.sessionId() + " [" + errorMessage + "]"); - throw new PackageManagerException( - SessionInfo.SESSION_ACTIVATION_FAILED, errorMessage); + private void installApksInSession(StagedSession session) throws PackageManagerException { + try { + // Blocking wait for installation to complete + session.installSession().get(); + } catch (InterruptedException e) { + // Should be impossible + throw new RuntimeException(e); + } catch (ExecutionException ee) { + PackageManagerException e = (PackageManagerException) ee.getCause(); + final String errorMsg = PackageManager.installStatusToString(e.error, e.getMessage()); + throw new PackageManagerException(SessionInfo.SESSION_ACTIVATION_FAILED, errorMsg); } } diff --git a/services/tests/mockingservicestests/src/com/android/server/pm/StagingManagerTest.java b/services/tests/mockingservicestests/src/com/android/server/pm/StagingManagerTest.java index bdfdf7723c026..e2dcc26b37b52 100644 --- a/services/tests/mockingservicestests/src/com/android/server/pm/StagingManagerTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/pm/StagingManagerTest.java @@ -36,7 +36,6 @@ import android.apex.ApexInfo; import android.apex.ApexSessionInfo; import android.apex.ApexSessionParams; import android.content.Context; -import android.content.IntentSender; import android.content.pm.ApexStagedEvent; import android.content.pm.IStagedApexObserver; import android.content.pm.PackageInstaller; @@ -75,6 +74,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; import java.util.Map; +import java.util.concurrent.CompletableFuture; import java.util.function.Predicate; @Presubmit @@ -953,7 +953,7 @@ public class StagingManagerTest { } @Override - public void installSession(IntentSender statusReceiver) { + public CompletableFuture installSession() { throw new UnsupportedOperationException(); }