From f712ee0633de9bd5bcfa43edffe9eff0903d8611 Mon Sep 17 00:00:00 2001 From: JW Wang Date: Tue, 8 Sep 2020 11:00:17 +0800 Subject: [PATCH 1/5] Let #startPreRebootVerification take a session object (1/n) Most callers can pass a session object directly. We don't need to call getStagedSession() to remap an id to a session object. Bug: 167996901 Test: atest StagedInstallTest StagedInstallInternalTest Change-Id: If5af54bc15b98a085f55fa4669da437ab9d2506e --- .../java/com/android/server/pm/StagingManager.java | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/services/core/java/com/android/server/pm/StagingManager.java b/services/core/java/com/android/server/pm/StagingManager.java index e5e94482da12b..58713c1efcb3c 100644 --- a/services/core/java/com/android/server/pm/StagingManager.java +++ b/services/core/java/com/android/server/pm/StagingManager.java @@ -558,7 +558,7 @@ public class StagingManager { // failed when not in checkpoint mode, hence it is being processed separately. Slog.d(TAG, "Found pending staged session " + session.sessionId + " still to " + "be verified, resuming pre-reboot verification"); - mPreRebootVerificationHandler.startPreRebootVerification(session.sessionId); + mPreRebootVerificationHandler.startPreRebootVerification(session); return; } } @@ -873,7 +873,7 @@ public class StagingManager { void commitSession(@NonNull PackageInstallerSession session) { updateStoredSession(session); - mPreRebootVerificationHandler.startPreRebootVerification(session.sessionId); + mPreRebootVerificationHandler.startPreRebootVerification(session); } private int getSessionIdForParentOrSelf(PackageInstallerSession session) { @@ -1106,7 +1106,7 @@ public class StagingManager { if (!session.isStagedSessionReady()) { // The framework got restarted before the pre-reboot verification could complete, // restart the verification. - mPreRebootVerificationHandler.startPreRebootVerification(session.sessionId); + mPreRebootVerificationHandler.startPreRebootVerification(session); } else { // Session had already being marked ready. Start the checks to verify if there is any // follow-up work. @@ -1261,14 +1261,16 @@ public class StagingManager { mIsReady = true; if (mPendingSessionIds != null) { for (int i = 0; i < mPendingSessionIds.size(); i++) { - startPreRebootVerification(mPendingSessionIds.get(i)); + PackageInstallerSession session = getStagedSession(mPendingSessionIds.get(i)); + startPreRebootVerification(session); } mPendingSessionIds = null; } } // Method for starting the pre-reboot verification - private synchronized void startPreRebootVerification(int sessionId) { + private synchronized void startPreRebootVerification(PackageInstallerSession session) { + int sessionId = session.sessionId; if (!mIsReady) { if (mPendingSessionIds == null) { mPendingSessionIds = new IntArray(); @@ -1277,7 +1279,6 @@ public class StagingManager { return; } - PackageInstallerSession session = getStagedSession(sessionId); if (session != null && session.notifyStagedStartPreRebootVerification()) { Slog.d(TAG, "Starting preRebootVerification for session " + sessionId); obtainMessage(MSG_PRE_REBOOT_VERIFICATION_START, sessionId, 0).sendToTarget(); From dbe5e57ecf13a2e86c5b45d4d92a67ecf1501662 Mon Sep 17 00:00:00 2001 From: JW Wang Date: Tue, 8 Sep 2020 11:20:06 +0800 Subject: [PATCH 2/5] Pass rollbackId to submitSessionToApexService() (2/n) Do the plumbing to pass rollbackId from handlePreRebootVerification_Start() all the way to submitSessionToApexService(). This allows us to remove mSessionRollbackIds. Bug: 167996901 Test: atest StagedInstallTest StagedInstallInternalTest Change-Id: Icc357561a4743683b3aae796ed7723eec66e3a6e --- .../com/android/server/pm/StagingManager.java | 55 +++++++++---------- 1 file changed, 26 insertions(+), 29 deletions(-) diff --git a/services/core/java/com/android/server/pm/StagingManager.java b/services/core/java/com/android/server/pm/StagingManager.java index 58713c1efcb3c..b00094cb813aa 100644 --- a/services/core/java/com/android/server/pm/StagingManager.java +++ b/services/core/java/com/android/server/pm/StagingManager.java @@ -60,7 +60,6 @@ import android.util.ArraySet; import android.util.IntArray; import android.util.Slog; import android.util.SparseArray; -import android.util.SparseIntArray; import android.util.apk.ApkSignatureVerifier; import com.android.internal.annotations.GuardedBy; @@ -111,9 +110,6 @@ public class StagingManager { @GuardedBy("mStagedSessions") private final SparseArray mStagedSessions = new SparseArray<>(); - @GuardedBy("mStagedSessions") - private final SparseIntArray mSessionRollbackIds = new SparseIntArray(); - @GuardedBy("mFailedPackageNames") private final List mFailedPackageNames = new ArrayList<>(); private String mNativeFailureReason; @@ -236,8 +232,8 @@ public class StagingManager { + " compatible with the one currently installed on device"); } - private List submitSessionToApexService( - @NonNull PackageInstallerSession session) throws PackageManagerException { + private List submitSessionToApexService(@NonNull PackageInstallerSession session, + int rollbackId) throws PackageManagerException { final IntArray childSessionIds = new IntArray(); if (session.isMultiPackage()) { for (PackageInstallerSession s : session.getChildSessions()) { @@ -251,14 +247,11 @@ public class StagingManager { apexSessionParams.childSessionIds = childSessionIds.toArray(); if (session.params.installReason == PackageManager.INSTALL_REASON_ROLLBACK) { apexSessionParams.isRollback = true; - apexSessionParams.rollbackId = retrieveRollbackIdForCommitSession(session.sessionId); + apexSessionParams.rollbackId = rollbackId; } else { - synchronized (mStagedSessions) { - int rollbackId = mSessionRollbackIds.get(session.sessionId, -1); - if (rollbackId != -1) { - apexSessionParams.hasRollbackEnabled = true; - apexSessionParams.rollbackId = rollbackId; - } + if (rollbackId != -1) { + apexSessionParams.hasRollbackEnabled = true; + apexSessionParams.rollbackId = rollbackId; } } // submitStagedSession will throw a PackageManagerException if apexd verification fails, @@ -997,7 +990,6 @@ public class StagingManager { void abortSession(@NonNull PackageInstallerSession session) { synchronized (mStagedSessions) { mStagedSessions.remove(session.sessionId); - mSessionRollbackIds.delete(session.sessionId); } } @@ -1229,6 +1221,7 @@ public class StagingManager { @Override public void handleMessage(Message msg) { final int sessionId = msg.arg1; + final int rollbackId = msg.arg2; final PackageInstallerSession session = getStagedSession(sessionId); if (session == null) { Slog.wtf(TAG, "Session disappeared in the middle of pre-reboot verification: " @@ -1245,7 +1238,7 @@ public class StagingManager { handlePreRebootVerification_Start(session); break; case MSG_PRE_REBOOT_VERIFICATION_APEX: - handlePreRebootVerification_Apex(session); + handlePreRebootVerification_Apex(session, rollbackId); break; case MSG_PRE_REBOOT_VERIFICATION_APK: handlePreRebootVerification_Apk(session); @@ -1281,7 +1274,7 @@ public class StagingManager { if (session != null && session.notifyStagedStartPreRebootVerification()) { Slog.d(TAG, "Starting preRebootVerification for session " + sessionId); - obtainMessage(MSG_PRE_REBOOT_VERIFICATION_START, sessionId, 0).sendToTarget(); + obtainMessage(MSG_PRE_REBOOT_VERIFICATION_START, sessionId, -1).sendToTarget(); } } @@ -1303,16 +1296,16 @@ public class StagingManager { session.notifyStagedEndPreRebootVerification(); } - private void notifyPreRebootVerification_Start_Complete(int sessionId) { - obtainMessage(MSG_PRE_REBOOT_VERIFICATION_APEX, sessionId, 0).sendToTarget(); + private void notifyPreRebootVerification_Start_Complete(int sessionId, int rollbackId) { + obtainMessage(MSG_PRE_REBOOT_VERIFICATION_APEX, sessionId, rollbackId).sendToTarget(); } private void notifyPreRebootVerification_Apex_Complete(int sessionId) { - obtainMessage(MSG_PRE_REBOOT_VERIFICATION_APK, sessionId, 0).sendToTarget(); + obtainMessage(MSG_PRE_REBOOT_VERIFICATION_APK, sessionId, -1).sendToTarget(); } private void notifyPreRebootVerification_Apk_Complete(int sessionId) { - obtainMessage(MSG_PRE_REBOOT_VERIFICATION_END, sessionId, 0).sendToTarget(); + obtainMessage(MSG_PRE_REBOOT_VERIFICATION_END, sessionId, -1).sendToTarget(); } /** @@ -1321,6 +1314,7 @@ public class StagingManager { * See {@link PreRebootVerificationHandler} to see all nodes of pre reboot verification */ private void handlePreRebootVerification_Start(@NonNull PackageInstallerSession session) { + 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 // with the list of sessions it must enable rollback for. Note that @@ -1330,19 +1324,21 @@ public class StagingManager { try { // NOTE: To stay consistent with the non-staged install flow, we don't fail the // entire install if rollbacks can't be enabled. - int rollbackId = rm.notifyStagedSession(session.sessionId); - if (rollbackId != -1) { - synchronized (mStagedSessions) { - mSessionRollbackIds.put(session.sessionId, rollbackId); - } - } + rollbackId = rm.notifyStagedSession(session.sessionId); } catch (RuntimeException re) { Slog.e(TAG, "Failed to notifyStagedSession for session: " + session.sessionId, re); } + } else if (session.params.installReason == PackageManager.INSTALL_REASON_ROLLBACK) { + try { + rollbackId = retrieveRollbackIdForCommitSession(session.sessionId); + } catch (PackageManagerException e) { + onPreRebootVerificationFailure(session, e.error, e.getMessage()); + return; + } } - notifyPreRebootVerification_Start_Complete(session.sessionId); + notifyPreRebootVerification_Start_Complete(session.sessionId, rollbackId); } /** @@ -1353,7 +1349,8 @@ public class StagingManager { *
  • validates signatures of apex files
    • *

    */ - private void handlePreRebootVerification_Apex(@NonNull PackageInstallerSession session) { + private void handlePreRebootVerification_Apex( + @NonNull PackageInstallerSession session, int rollbackId) { final boolean hasApex = sessionContainsApex(session); // APEX checks. For single-package sessions, check if they contain an APEX. For @@ -1361,7 +1358,7 @@ public class StagingManager { if (hasApex) { final List apexPackages; try { - apexPackages = submitSessionToApexService(session); + apexPackages = submitSessionToApexService(session, rollbackId); for (int i = 0, size = apexPackages.size(); i < size; i++) { validateApexSignature(apexPackages.get(i)); } From cba607a5aba529f56906fff7e0dd533794bc7fd3 Mon Sep 17 00:00:00 2001 From: JW Wang Date: Tue, 8 Sep 2020 11:32:27 +0800 Subject: [PATCH 3/5] Let PreRebootVerificationHandler methods take a session object (3/n) So we don't need getStagedSession() to remap an id to the session object. Bug: 167996901 Test: atest StagedInstallTest StagedInstallInternalTest Change-Id: Iaf7444d0fc924d9e2deda66a6ffbd54d8c32b279 --- .../server/pm/PackageInstallerSession.java | 2 +- .../com/android/server/pm/StagingManager.java | 33 +++++++++++-------- 2 files changed, 20 insertions(+), 15 deletions(-) diff --git a/services/core/java/com/android/server/pm/PackageInstallerSession.java b/services/core/java/com/android/server/pm/PackageInstallerSession.java index 73a9e2cb52cde..938ff7dffa65a 100644 --- a/services/core/java/com/android/server/pm/PackageInstallerSession.java +++ b/services/core/java/com/android/server/pm/PackageInstallerSession.java @@ -1998,7 +1998,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { if (isStaged()) { // TODO(b/136257624): Remove this once all verification logic has been transferred out // of StagingManager. - mStagingManager.notifyPreRebootVerification_Apk_Complete(sessionId); + mStagingManager.notifyPreRebootVerification_Apk_Complete(this); // TODO(b/136257624): We also need to destroy internals for verified staged session, // otherwise file descriptors are never closed for verified staged session until reboot return; diff --git a/services/core/java/com/android/server/pm/StagingManager.java b/services/core/java/com/android/server/pm/StagingManager.java index b00094cb813aa..7cb4c46831339 100644 --- a/services/core/java/com/android/server/pm/StagingManager.java +++ b/services/core/java/com/android/server/pm/StagingManager.java @@ -1186,8 +1186,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 notifyPreRebootVerification_Apk_Complete(int sessionId) { - mPreRebootVerificationHandler.notifyPreRebootVerification_Apk_Complete(sessionId); + void notifyPreRebootVerification_Apk_Complete(PackageInstallerSession session) { + mPreRebootVerificationHandler.notifyPreRebootVerification_Apk_Complete(session); } private final class PreRebootVerificationHandler extends Handler { @@ -1222,7 +1222,7 @@ public class StagingManager { public void handleMessage(Message msg) { final int sessionId = msg.arg1; final int rollbackId = msg.arg2; - final PackageInstallerSession session = getStagedSession(sessionId); + final PackageInstallerSession session = (PackageInstallerSession) msg.obj; if (session == null) { Slog.wtf(TAG, "Session disappeared in the middle of pre-reboot verification: " + sessionId); @@ -1274,7 +1274,8 @@ public class StagingManager { if (session != null && session.notifyStagedStartPreRebootVerification()) { Slog.d(TAG, "Starting preRebootVerification for session " + sessionId); - obtainMessage(MSG_PRE_REBOOT_VERIFICATION_START, sessionId, -1).sendToTarget(); + obtainMessage(MSG_PRE_REBOOT_VERIFICATION_START, sessionId, -1, session) + .sendToTarget(); } } @@ -1296,16 +1297,20 @@ public class StagingManager { session.notifyStagedEndPreRebootVerification(); } - private void notifyPreRebootVerification_Start_Complete(int sessionId, int rollbackId) { - obtainMessage(MSG_PRE_REBOOT_VERIFICATION_APEX, sessionId, rollbackId).sendToTarget(); + private void notifyPreRebootVerification_Start_Complete( + PackageInstallerSession session, int rollbackId) { + obtainMessage(MSG_PRE_REBOOT_VERIFICATION_APEX, session.sessionId, rollbackId, session) + .sendToTarget(); } - private void notifyPreRebootVerification_Apex_Complete(int sessionId) { - obtainMessage(MSG_PRE_REBOOT_VERIFICATION_APK, sessionId, -1).sendToTarget(); + private void notifyPreRebootVerification_Apex_Complete(PackageInstallerSession session) { + obtainMessage(MSG_PRE_REBOOT_VERIFICATION_APK, session.sessionId, -1, session) + .sendToTarget(); } - private void notifyPreRebootVerification_Apk_Complete(int sessionId) { - obtainMessage(MSG_PRE_REBOOT_VERIFICATION_END, sessionId, -1).sendToTarget(); + private void notifyPreRebootVerification_Apk_Complete(PackageInstallerSession session) { + obtainMessage(MSG_PRE_REBOOT_VERIFICATION_END, session.sessionId, -1, session) + .sendToTarget(); } /** @@ -1338,7 +1343,7 @@ public class StagingManager { } } - notifyPreRebootVerification_Start_Complete(session.sessionId, rollbackId); + notifyPreRebootVerification_Start_Complete(session, rollbackId); } /** @@ -1372,17 +1377,17 @@ public class StagingManager { packageManagerInternal.pruneCachedApksInApex(apexPackages); } - notifyPreRebootVerification_Apex_Complete(session.sessionId); + notifyPreRebootVerification_Apex_Complete(session); } /** * Pre-reboot verification state for apk files. Session is sent to * {@link PackageManagerService} for verification and it notifies back the result via - * {@link #notifyPreRebootVerification_Apk_Complete(int)} + * {@link #notifyPreRebootVerification_Apk_Complete} */ private void handlePreRebootVerification_Apk(@NonNull PackageInstallerSession session) { if (!sessionContainsApk(session)) { - notifyPreRebootVerification_Apk_Complete(session.sessionId); + notifyPreRebootVerification_Apk_Complete(session); return; } session.verifyStagedSession(); From 921340c284e08f3f855ba397b888dcf32f889206 Mon Sep 17 00:00:00 2001 From: JW Wang Date: Tue, 8 Sep 2020 14:21:06 +0800 Subject: [PATCH 4/5] Store pending sessions as objects instead of ids (4/n) So we don't need getStagedSession() to remap an id to the session object. Bug: 167996901 Test: atest StagedInstallTest StagedInstallInternalTest Change-Id: Ibcba41001b1a8853992bceb427179a9a9296cbce --- .../com/android/server/pm/StagingManager.java | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/services/core/java/com/android/server/pm/StagingManager.java b/services/core/java/com/android/server/pm/StagingManager.java index 7cb4c46831339..527655129d28e 100644 --- a/services/core/java/com/android/server/pm/StagingManager.java +++ b/services/core/java/com/android/server/pm/StagingManager.java @@ -1191,8 +1191,8 @@ public class StagingManager { } private final class PreRebootVerificationHandler extends Handler { - // Hold session ids before handler gets ready to do the verification. - private IntArray mPendingSessionIds; + // Hold sessions before handler gets ready to do the verification. + private List mPendingSessions; private boolean mIsReady; PreRebootVerificationHandler(Looper looper) { @@ -1252,27 +1252,27 @@ public class StagingManager { // Notify the handler that system is ready, and reschedule the pre-reboot verifications. private synchronized void readyToStart() { mIsReady = true; - if (mPendingSessionIds != null) { - for (int i = 0; i < mPendingSessionIds.size(); i++) { - PackageInstallerSession session = getStagedSession(mPendingSessionIds.get(i)); + if (mPendingSessions != null) { + for (int i = 0; i < mPendingSessions.size(); i++) { + PackageInstallerSession session = mPendingSessions.get(i); startPreRebootVerification(session); } - mPendingSessionIds = null; + mPendingSessions = null; } } // Method for starting the pre-reboot verification private synchronized void startPreRebootVerification(PackageInstallerSession session) { - int sessionId = session.sessionId; if (!mIsReady) { - if (mPendingSessionIds == null) { - mPendingSessionIds = new IntArray(); + if (mPendingSessions == null) { + mPendingSessions = new ArrayList<>(); } - mPendingSessionIds.add(sessionId); + mPendingSessions.add(session); return; } if (session != null && session.notifyStagedStartPreRebootVerification()) { + int sessionId = session.sessionId; Slog.d(TAG, "Starting preRebootVerification for session " + sessionId); obtainMessage(MSG_PRE_REBOOT_VERIFICATION_START, sessionId, -1, session) .sendToTarget(); From f8dd1956b4d32932d12f6582302176ed7a305cad Mon Sep 17 00:00:00 2001 From: JW Wang Date: Tue, 8 Sep 2020 15:18:48 +0800 Subject: [PATCH 5/5] Remove null-checks (5/n) 1. Remove the check in handleMessage() since all callers of obtainMessage() pass non-null sessions. 2. Remove the check in startPreRebootVerification() since the passed-in session is non-null. Bug: 167996901 Test: atest StagedInstallTest StagedInstallInternalTest Change-Id: I6c5b5a1357130d3c9f4889b7e6297faf7f2c20a4 --- .../com/android/server/pm/StagingManager.java | 20 +++++++++---------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/services/core/java/com/android/server/pm/StagingManager.java b/services/core/java/com/android/server/pm/StagingManager.java index 527655129d28e..6ce06e087c7c4 100644 --- a/services/core/java/com/android/server/pm/StagingManager.java +++ b/services/core/java/com/android/server/pm/StagingManager.java @@ -1186,7 +1186,7 @@ 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 notifyPreRebootVerification_Apk_Complete(PackageInstallerSession session) { + void notifyPreRebootVerification_Apk_Complete(@NonNull PackageInstallerSession session) { mPreRebootVerificationHandler.notifyPreRebootVerification_Apk_Complete(session); } @@ -1223,11 +1223,6 @@ public class StagingManager { final int sessionId = msg.arg1; final int rollbackId = msg.arg2; final PackageInstallerSession session = (PackageInstallerSession) msg.obj; - if (session == null) { - Slog.wtf(TAG, "Session disappeared in the middle of pre-reboot verification: " - + sessionId); - return; - } if (session.isDestroyed() || session.isStagedSessionFailed()) { // No point in running verification on a destroyed/failed session onPreRebootVerificationComplete(session); @@ -1262,7 +1257,8 @@ public class StagingManager { } // Method for starting the pre-reboot verification - private synchronized void startPreRebootVerification(PackageInstallerSession session) { + private synchronized void startPreRebootVerification( + @NonNull PackageInstallerSession session) { if (!mIsReady) { if (mPendingSessions == null) { mPendingSessions = new ArrayList<>(); @@ -1271,7 +1267,7 @@ public class StagingManager { return; } - if (session != null && session.notifyStagedStartPreRebootVerification()) { + if (session.notifyStagedStartPreRebootVerification()) { int sessionId = session.sessionId; Slog.d(TAG, "Starting preRebootVerification for session " + sessionId); obtainMessage(MSG_PRE_REBOOT_VERIFICATION_START, sessionId, -1, session) @@ -1298,17 +1294,19 @@ public class StagingManager { } private void notifyPreRebootVerification_Start_Complete( - PackageInstallerSession session, int rollbackId) { + @NonNull PackageInstallerSession session, int rollbackId) { obtainMessage(MSG_PRE_REBOOT_VERIFICATION_APEX, session.sessionId, rollbackId, session) .sendToTarget(); } - private void notifyPreRebootVerification_Apex_Complete(PackageInstallerSession session) { + private void notifyPreRebootVerification_Apex_Complete( + @NonNull PackageInstallerSession session) { obtainMessage(MSG_PRE_REBOOT_VERIFICATION_APK, session.sessionId, -1, session) .sendToTarget(); } - private void notifyPreRebootVerification_Apk_Complete(PackageInstallerSession session) { + private void notifyPreRebootVerification_Apk_Complete( + @NonNull PackageInstallerSession session) { obtainMessage(MSG_PRE_REBOOT_VERIFICATION_END, session.sessionId, -1, session) .sendToTarget(); }