From 863fe452241f1e55d1ebe3c2988143b16cf4e9fd Mon Sep 17 00:00:00 2001 From: JW Wang Date: Fri, 11 Sep 2020 14:29:56 +0800 Subject: Iterate over parent sessions (1/n) Use #sessionContains(filter) to check if there is overlapping. This allows us to store only parent sessions in #mStagedSessions and move the call to #createSession from PackageInstallerService into StagingManager in the future. Bug: 168268518 Test: atest StagedInstallTest Change-Id: I1d8f71149cd9957f45b6bcaee3e75dd68c8a45d4 --- .../android/server/pm/PackageInstallerSession.java | 2 +- .../java/com/android/server/pm/StagingManager.java | 33 +++++++--------------- 2 files changed, 11 insertions(+), 24 deletions(-) diff --git a/services/core/java/com/android/server/pm/PackageInstallerSession.java b/services/core/java/com/android/server/pm/PackageInstallerSession.java index 2aafe9a6f9a1..fa70936d0e61 100644 --- a/services/core/java/com/android/server/pm/PackageInstallerSession.java +++ b/services/core/java/com/android/server/pm/PackageInstallerSession.java @@ -2250,7 +2250,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { return (params.installFlags & PackageManager.INSTALL_APEX) != 0; } - private boolean sessionContains(Predicate filter) { + boolean sessionContains(Predicate filter) { if (!isMultiPackage()) { return filter.test(this); } diff --git a/services/core/java/com/android/server/pm/StagingManager.java b/services/core/java/com/android/server/pm/StagingManager.java index 35c26d6cd54e..14d62db0b9d8 100644 --- a/services/core/java/com/android/server/pm/StagingManager.java +++ b/services/core/java/com/android/server/pm/StagingManager.java @@ -863,7 +863,9 @@ public class StagingManager { // We cannot say a parent session overlaps until we process its children return; } - if (session.getPackageName() == null) { + + String packageName = session.getPackageName(); + if (packageName == null) { throw new PackageManagerException(PackageManager.INSTALL_FAILED_INVALID_APK, "Cannot stage session " + session.sessionId + " with package name null"); } @@ -876,40 +878,26 @@ public class StagingManager { synchronized (mStagedSessions) { for (int i = 0; i < mStagedSessions.size(); i++) { final PackageInstallerSession stagedSession = mStagedSessions.valueAt(i); - if (!stagedSession.isCommitted() || stagedSession.isStagedAndInTerminalState() + if (stagedSession.hasParentSessionId() || !stagedSession.isCommitted() + || stagedSession.isStagedAndInTerminalState() || stagedSession.isDestroyed()) { continue; } - if (stagedSession.isMultiPackage()) { - // This active parent staged session is useless as it doesn't have a package - // name and the session we are checking is not a parent session either. - continue; - } - // Check if stagedSession has an active parent session or not - if (stagedSession.hasParentSessionId()) { - final int parentId = stagedSession.getParentSessionId(); - final PackageInstallerSession parentSession = mStagedSessions.get(parentId); - if (parentSession == null || parentSession.isStagedAndInTerminalState() - || parentSession.isDestroyed()) { - // Parent session has been abandoned or terminated already - continue; - } - } - // From here on, stagedSession is a non-parent active staged session + // From here on, stagedSession is a parent active staged session // Check if session is one of the active sessions - if (session.sessionId == stagedSession.sessionId) { + if (getSessionIdForParentOrSelf(session) == stagedSession.sessionId) { Slog.w(TAG, "Session " + session.sessionId + " is already staged"); continue; } // New session cannot have same package name as one of the active sessions - if (session.getPackageName().equals(stagedSession.getPackageName())) { + if (stagedSession.sessionContains(s -> s.getPackageName().equals(packageName))) { if (isRollback) { // If the new session is a rollback, then it gets priority. The existing // session is failed to unblock rollback. - final PackageInstallerSession root = getParentSessionOrSelf(stagedSession); + final PackageInstallerSession root = stagedSession; if (!ensureActiveApexSessionIsAborted(root)) { Slog.e(TAG, "Failed to abort apex session " + root.sessionId); // Safe to ignore active apex session abort failure since session @@ -933,8 +921,7 @@ public class StagingManager { // Staging multiple root sessions is not allowed if device doesn't support // checkpoint. If session and stagedSession do not have common ancestor, they are // from two different root sessions. - if (!supportsCheckpoint && getSessionIdForParentOrSelf(session) - != getSessionIdForParentOrSelf(stagedSession)) { + if (!supportsCheckpoint) { throw new PackageManagerException( PackageManager.INSTALL_FAILED_OTHER_STAGED_SESSION_IN_PROGRESS, "Cannot stage multiple sessions without checkpoint support", null); -- cgit v1.2.3-59-g8ed1b From 1b8fc34175338899b9414aee44cd00807420eeff Mon Sep 17 00:00:00 2001 From: JW Wang Date: Fri, 11 Sep 2020 15:14:21 +0800 Subject: Move #checkNonOverlappingWithStagedSessions into pre-reboot verification (2/n) The change breaks CTS. We will fix it in ag/12612516. Bug: 168268518 Test: atest StagedInstallTest Change-Id: I7d526488f09b350ed7753e1995d429174d64d658 --- .../android/server/pm/PackageInstallerSession.java | 7 ------- .../java/com/android/server/pm/StagingManager.java | 21 +++++++++++++++++---- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/services/core/java/com/android/server/pm/PackageInstallerSession.java b/services/core/java/com/android/server/pm/PackageInstallerSession.java index fa70936d0e61..684c35a5365a 100644 --- a/services/core/java/com/android/server/pm/PackageInstallerSession.java +++ b/services/core/java/com/android/server/pm/PackageInstallerSession.java @@ -1600,13 +1600,6 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { validateApkInstallLocked(); } } - } - - if (params.isStaged) { - mStagingManager.checkNonOverlappingWithStagedSessions(this); - } - - synchronized (mLock) { if (mDestroyed) { throw new PackageManagerException(INSTALL_FAILED_INTERNAL_ERROR, "Session destroyed"); diff --git a/services/core/java/com/android/server/pm/StagingManager.java b/services/core/java/com/android/server/pm/StagingManager.java index 14d62db0b9d8..566f285229b7 100644 --- a/services/core/java/com/android/server/pm/StagingManager.java +++ b/services/core/java/com/android/server/pm/StagingManager.java @@ -857,7 +857,7 @@ public class StagingManager { * * @throws PackageManagerException if session fails the check */ - void checkNonOverlappingWithStagedSessions(@NonNull PackageInstallerSession session) + private void checkNonOverlappingWithStagedSessions(@NonNull PackageInstallerSession session) throws PackageManagerException { if (session.isMultiPackage()) { // We cannot say a parent session overlaps until we process its children @@ -866,7 +866,7 @@ public class StagingManager { String packageName = session.getPackageName(); if (packageName == null) { - throw new PackageManagerException(PackageManager.INSTALL_FAILED_INVALID_APK, + throw new PackageManagerException(SessionInfo.STAGED_SESSION_VERIFICATION_FAILED, "Cannot stage session " + session.sessionId + " with package name null"); } @@ -911,7 +911,7 @@ public class StagingManager { + "blocking rollback session: " + session.sessionId); } else { throw new PackageManagerException( - PackageManager.INSTALL_FAILED_OTHER_STAGED_SESSION_IN_PROGRESS, + SessionInfo.STAGED_SESSION_VERIFICATION_FAILED, "Package: " + session.getPackageName() + " in session: " + session.sessionId + " has been staged already by session:" + " " + stagedSession.sessionId, null); @@ -923,7 +923,7 @@ public class StagingManager { // from two different root sessions. if (!supportsCheckpoint) { throw new PackageManagerException( - PackageManager.INSTALL_FAILED_OTHER_STAGED_SESSION_IN_PROGRESS, + SessionInfo.STAGED_SESSION_VERIFICATION_FAILED, "Cannot stage multiple sessions without checkpoint support", null); } } @@ -1266,6 +1266,19 @@ public class StagingManager { * See {@link PreRebootVerificationHandler} to see all nodes of pre reboot verification */ private void handlePreRebootVerification_Start(@NonNull PackageInstallerSession session) { + try { + if (session.isMultiPackage()) { + for (PackageInstallerSession s : session.getChildSessions()) { + checkNonOverlappingWithStagedSessions(s); + } + } else { + checkNonOverlappingWithStagedSessions(session); + } + } catch (PackageManagerException e) { + onPreRebootVerificationFailure(session, e.error, e.getMessage()); + return; + } + int rollbackId = -1; if ((session.params.installFlags & PackageManager.INSTALL_ENABLE_ROLLBACK) != 0) { // If rollback is enabled for this session, we call through to the RollbackManager -- cgit v1.2.3-59-g8ed1b From 0becf9a6c6837a0a774ed01a3bf581ac80366f99 Mon Sep 17 00:00:00 2001 From: JW Wang Date: Fri, 11 Sep 2020 15:33:04 +0800 Subject: Now #mStagedSessions stores only parent sessions (4/n) 1. Don't store sessions until #commitSession or #restoreSession is called. 2. The stored sessions will be used by pre-reboot verification to check overlapping later. Bug: 168268518 Test: atest StagedInstallTest Change-Id: I945d2b92d9194f89493adf616231f8438b1c10b6 --- .../com/android/server/pm/PackageInstallerService.java | 3 --- .../core/java/com/android/server/pm/StagingManager.java | 17 +++++++++-------- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/services/core/java/com/android/server/pm/PackageInstallerService.java b/services/core/java/com/android/server/pm/PackageInstallerService.java index 31ee59717dba..e01ba2c84432 100644 --- a/services/core/java/com/android/server/pm/PackageInstallerService.java +++ b/services/core/java/com/android/server/pm/PackageInstallerService.java @@ -746,9 +746,6 @@ public class PackageInstallerService extends IPackageInstaller.Stub implements synchronized (mSessions) { mSessions.put(sessionId, session); } - if (params.isStaged) { - mStagingManager.createSession(session); - } mCallbacks.notifySessionCreated(session.sessionId, session.userId); diff --git a/services/core/java/com/android/server/pm/StagingManager.java b/services/core/java/com/android/server/pm/StagingManager.java index 566f285229b7..e2ab6a8ae75d 100644 --- a/services/core/java/com/android/server/pm/StagingManager.java +++ b/services/core/java/com/android/server/pm/StagingManager.java @@ -828,6 +828,8 @@ public class StagingManager { } void commitSession(@NonNull PackageInstallerSession session) { + // Store this parent session which will be used to check overlapping later + createSession(session); mPreRebootVerificationHandler.startPreRebootVerification(session); } @@ -930,7 +932,7 @@ public class StagingManager { } } - void createSession(@NonNull PackageInstallerSession sessionInfo) { + private void createSession(@NonNull PackageInstallerSession sessionInfo) { synchronized (mStagedSessions) { mStagedSessions.append(sessionInfo.sessionId, sessionInfo); } @@ -1006,16 +1008,15 @@ public class StagingManager { } void restoreSession(@NonNull PackageInstallerSession session, boolean isDeviceUpgrading) { - PackageInstallerSession sessionToResume = session; - synchronized (mStagedSessions) { - mStagedSessions.append(session.sessionId, session); - if (session.hasParentSessionId()) { - // Only parent sessions can be restored - return; - } + if (session.hasParentSessionId()) { + // Only parent sessions can be restored + return; } + // Store this parent session which will be used to check overlapping later + createSession(session); // The preconditions used during pre-reboot verification might have changed when device // is upgrading. Updated staged sessions to activation failed before we resume the session. + PackageInstallerSession sessionToResume = session; if (isDeviceUpgrading && !sessionToResume.isStagedAndInTerminalState()) { sessionToResume.setStagedSessionFailed(SessionInfo.STAGED_SESSION_ACTIVATION_FAILED, "Build fingerprint has changed"); -- cgit v1.2.3-59-g8ed1b