diff --git a/services/core/java/com/android/server/pm/PackageInstallerSession.java b/services/core/java/com/android/server/pm/PackageInstallerSession.java index ed62362b04fb0..dea840a6e155f 100644 --- a/services/core/java/com/android/server/pm/PackageInstallerSession.java +++ b/services/core/java/com/android/server/pm/PackageInstallerSession.java @@ -388,6 +388,20 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { @GuardedBy("mLock") private String mStagedSessionErrorMessage; + /** + * The callback to run when pre-reboot verification has ended. Used by {@link #abandonStaged()} + * to delay session clean-up until it is safe to do so. + */ + @GuardedBy("mLock") + @Nullable + private Runnable mPendingAbandonCallback; + /** + * {@code true} if pre-reboot verification is ongoing which means it is not safe for + * {@link #abandon()} to clean up staging directories. + */ + @GuardedBy("mLock") + private boolean mInPreRebootVerification; + /** * Path to the validated base APK for this session, which may point at an * APK inside the session (when the session defines the base), or it may @@ -1570,7 +1584,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { SessionInfo.STAGED_SESSION_VERIFICATION_FAILED, detailMessage); // TODO(b/136257624): Remove this once all verification logic has been transferred out // of StagingManager. - mStagingManager.notifyVerificationComplete(sessionId); + mStagingManager.notifyVerificationComplete(this); } else { // Dispatch message to remove session from PackageInstallerService. dispatchSessionFinished(error, detailMessage, null); @@ -2786,14 +2800,9 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { } private void abandonStaged() { + final Runnable r; synchronized (mLock) { - if (mDestroyed) { - // If a user abandons staged session in an unsafe state, then system will try to - // abandon the destroyed staged session when it is safe on behalf of the user. - assertCallerIsOwnerOrRootOrSystemLocked(); - } else { - assertCallerIsOwnerOrRootLocked(); - } + assertCallerIsOwnerOrRootLocked(); if (isStagedAndInTerminalState()) { // We keep the session in the database if it's in a finalized state. It will be // removed by PackageInstallerService when the last update time is old enough. @@ -2802,17 +2811,25 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { return; } mDestroyed = true; - if (mCommitted) { - if (!mStagingManager.abortCommittedSessionLocked(this)) { - // Do not clean up the staged session from system. It is not safe yet. - mCallback.onStagedSessionChanged(this); - return; + boolean isCommitted = mCommitted; + List childSessions = getChildSessionsLocked(); + r = () -> { + assertNotLocked("abandonStaged"); + if (isCommitted) { + mStagingManager.abortCommittedSession(this); } + cleanStageDir(childSessions); + destroyInternal(); + dispatchSessionFinished(INSTALL_FAILED_ABORTED, "Session was abandoned", null); + }; + if (mInPreRebootVerification) { + // Pre-reboot verification is ongoing. It is not safe to clean up the session yet. + mPendingAbandonCallback = r; + mCallback.onStagedSessionChanged(this); + return; } - cleanStageDir(getChildSessionsLocked()); - destroyInternal(); } - dispatchSessionFinished(INSTALL_FAILED_ABORTED, "Session was abandoned", null); + r.run(); } @Override @@ -2829,6 +2846,50 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { } } + /** + * Notified by the staging manager that pre-reboot verification is about to start. The return + * value should be checked to decide whether it is OK to start pre-reboot verification. In + * the case of a destroyed session, {@code false} is returned and there is no need to start + * pre-reboot verification. + */ + boolean notifyStagedStartPreRebootVerification() { + synchronized (mLock) { + if (mInPreRebootVerification) { + throw new IllegalStateException("Pre-reboot verification has started"); + } + if (mDestroyed) { + return false; + } + mInPreRebootVerification = true; + return true; + } + } + + private void dispatchPendingAbandonCallback() { + final Runnable callback; + synchronized (mLock) { + callback = mPendingAbandonCallback; + mPendingAbandonCallback = null; + } + if (callback != null) { + callback.run(); + } + } + + /** + * Notified by the staging manager that pre-reboot verification has ended. Now it is safe to + * clean up the session if {@link #abandon()} has been called previously. + */ + void notifyStagedEndPreRebootVerification() { + synchronized (mLock) { + if (!mInPreRebootVerification) { + throw new IllegalStateException("Pre-reboot verification not started"); + } + mInPreRebootVerification = false; + } + dispatchPendingAbandonCallback(); + } + @Override public boolean isMultiPackage() { return params.isMultiPackage; diff --git a/services/core/java/com/android/server/pm/StagingManager.java b/services/core/java/com/android/server/pm/StagingManager.java index 0c4eaec32ba52..353459c248e5f 100644 --- a/services/core/java/com/android/server/pm/StagingManager.java +++ b/services/core/java/com/android/server/pm/StagingManager.java @@ -1028,22 +1028,12 @@ public class StagingManager { /** *

Abort committed staged session - * - *

This method must be called while holding {@link PackageInstallerSession#mLock}. - * - *

The method returns {@code false} to indicate it is not safe to clean up the session from - * system yet. When it is safe, the method returns {@code true}. - * - *

When it is safe to clean up, {@link StagingManager} will call - * {@link PackageInstallerSession#abandon()} on the session again. - * - * @return {@code true} if it is safe to cleanup the session resources, otherwise {@code false}. */ - boolean abortCommittedSessionLocked(@NonNull PackageInstallerSession session) { + void abortCommittedSession(@NonNull PackageInstallerSession session) { int sessionId = session.sessionId; - if (session.isStagedSessionApplied()) { - Slog.w(TAG, "Cannot abort applied session : " + sessionId); - return false; + if (session.isStagedAndInTerminalState()) { + Slog.w(TAG, "Cannot abort session in final state: " + sessionId); + return; } if (!session.isDestroyed()) { throw new IllegalStateException("Committed session must be destroyed before aborting it" @@ -1051,15 +1041,7 @@ public class StagingManager { } if (getStagedSession(sessionId) == null) { Slog.w(TAG, "Session " + sessionId + " has been abandoned already"); - return false; - } - - // If pre-reboot verification is running, then return false. StagingManager will call - // abandon again when pre-reboot verification ends. - if (mPreRebootVerificationHandler.isVerificationRunning(sessionId)) { - Slog.w(TAG, "Session " + sessionId + " aborted before pre-reboot " - + "verification completed."); - return false; + return; } // A session could be marked ready once its pre-reboot verification ends @@ -1075,7 +1057,6 @@ public class StagingManager { // Session was successfully aborted from apexd (if required) and pre-reboot verification // is also complete. It is now safe to clean up the session from system. abortSession(session); - return true; } /** @@ -1264,8 +1245,8 @@ public class StagingManager { // TODO(b/136257624): Temporary API to let PMS communicate with StagingManager. When all // verification logic is extracted out of StagingManager into PMS, we can remove // this. - void notifyVerificationComplete(int sessionId) { - mPreRebootVerificationHandler.onPreRebootVerificationComplete(sessionId); + void notifyVerificationComplete(PackageInstallerSession session) { + mPreRebootVerificationHandler.onPreRebootVerificationComplete(session); } // TODO(b/136257624): Temporary API to let PMS communicate with StagingManager. When all @@ -1316,7 +1297,7 @@ public class StagingManager { } if (session.isDestroyed() || session.isStagedSessionFailed()) { // No point in running verification on a destroyed/failed session - onPreRebootVerificationComplete(sessionId); + onPreRebootVerificationComplete(session); return; } switch (msg.what) { @@ -1357,15 +1338,13 @@ public class StagingManager { } PackageInstallerSession session = getStagedSession(sessionId); - synchronized (mVerificationRunning) { - // Do not start verification on a session that has been abandoned - if (session == null || session.isDestroyed()) { - return; + if (session != null && session.notifyStagedStartPreRebootVerification()) { + synchronized (mVerificationRunning) { + Slog.d(TAG, "Starting preRebootVerification for session " + sessionId); + mVerificationRunning.put(sessionId, true); } - Slog.d(TAG, "Starting preRebootVerification for session " + sessionId); - mVerificationRunning.put(sessionId, true); + obtainMessage(MSG_PRE_REBOOT_VERIFICATION_START, sessionId, 0).sendToTarget(); } - obtainMessage(MSG_PRE_REBOOT_VERIFICATION_START, sessionId, 0).sendToTarget(); } private void onPreRebootVerificationFailure(PackageInstallerSession session, @@ -1376,22 +1355,18 @@ public class StagingManager { // failed on next step and staging directory for session will be deleted. } session.setStagedSessionFailed(errorCode, errorMessage); - onPreRebootVerificationComplete(session.sessionId); + onPreRebootVerificationComplete(session); } // Things to do when pre-reboot verification completes for a particular sessionId - private void onPreRebootVerificationComplete(int sessionId) { + private void onPreRebootVerificationComplete(PackageInstallerSession session) { + int sessionId = session.sessionId; // Remove it from mVerificationRunning so that verification is considered complete synchronized (mVerificationRunning) { Slog.d(TAG, "Stopping preRebootVerification for session " + sessionId); mVerificationRunning.delete(sessionId); } - // Check if the session was destroyed while pre-reboot verification was running. If so, - // abandon it again. - PackageInstallerSession session = getStagedSession(sessionId); - if (session != null && session.isDestroyed()) { - session.abandon(); - } + session.notifyStagedEndPreRebootVerification(); } private boolean isVerificationRunning(int sessionId) { @@ -1516,7 +1491,7 @@ public class StagingManager { // or activate its apex, there won't be any files to work with as they will be cleaned // up by the system as part of abandonment. If session is abandoned before this point, // then the session is already destroyed and cannot be marked ready anymore. - onPreRebootVerificationComplete(session.sessionId); + onPreRebootVerificationComplete(session); // Proactively mark session as ready before calling apexd. Although this call order // looks counter-intuitive, this is the easiest way to ensure that session won't end up