From ce8bd1e37c802daac5dcab204c8b6465c9d7b638 Mon Sep 17 00:00:00 2001 From: JW Wang Date: Wed, 3 Nov 2021 14:47:56 +0800 Subject: [PATCH 1/2] Remove dead code (8/n) Bug: 161121612 Test: atest StagingManagerTest Change-Id: I9790714766ecf784e9b38821e7ef65871a58797a --- .../com/android/server/pm/StagingManager.java | 20 ------- .../android/server/pm/StagingManagerTest.java | 60 ------------------- 2 files changed, 80 deletions(-) diff --git a/services/core/java/com/android/server/pm/StagingManager.java b/services/core/java/com/android/server/pm/StagingManager.java index d121ad5020e58..5a09dbaad8982 100644 --- a/services/core/java/com/android/server/pm/StagingManager.java +++ b/services/core/java/com/android/server/pm/StagingManager.java @@ -525,26 +525,6 @@ public class StagingManager { } } - /** - * Returns id of a committed and non-finalized stated session that contains same - * {@code packageName}, or {@code -1} if no sessions have this {@code packageName} staged. - */ - int getSessionIdByPackageName(@NonNull String packageName) { - synchronized (mStagedSessions) { - for (int i = 0; i < mStagedSessions.size(); i++) { - StagedSession stagedSession = mStagedSessions.valueAt(i); - if (!stagedSession.isCommitted() || stagedSession.isDestroyed() - || stagedSession.isInTerminalState()) { - continue; - } - if (stagedSession.getPackageName().equals(packageName)) { - return stagedSession.sessionId(); - } - } - } - return -1; - } - @VisibleForTesting void createSession(@NonNull StagedSession sessionInfo) { synchronized (mStagedSessions) { 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 b72f05fee0feb..663bb2bd2d676 100644 --- a/services/tests/mockingservicestests/src/com/android/server/pm/StagingManagerTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/pm/StagingManagerTest.java @@ -453,66 +453,6 @@ public class StagingManagerTest { assertThat(apkSession.getErrorMessage()).isEqualTo("Another apex session failed"); } - @Test - public void getSessionIdByPackageName() throws Exception { - FakeStagedSession session = new FakeStagedSession(239); - session.setCommitted(true); - session.setSessionReady(); - session.setPackageName("com.foo"); - - mStagingManager.createSession(session); - assertThat(mStagingManager.getSessionIdByPackageName("com.foo")).isEqualTo(239); - } - - @Test - public void getSessionIdByPackageName_appliedSession_ignores() throws Exception { - FakeStagedSession session = new FakeStagedSession(37); - session.setCommitted(true); - session.setSessionApplied(); - session.setPackageName("com.foo"); - - mStagingManager.createSession(session); - assertThat(mStagingManager.getSessionIdByPackageName("com.foo")).isEqualTo(-1); - } - - @Test - public void getSessionIdByPackageName_failedSession_ignores() throws Exception { - FakeStagedSession session = new FakeStagedSession(73); - session.setCommitted(true); - session.setSessionFailed(1, "whatevs"); - session.setPackageName("com.foo"); - - mStagingManager.createSession(session); - assertThat(mStagingManager.getSessionIdByPackageName("com.foo")).isEqualTo(-1); - } - - @Test - public void getSessionIdByPackageName_destroyedSession_ignores() throws Exception { - FakeStagedSession session = new FakeStagedSession(23); - session.setCommitted(true); - session.setDestroyed(true); - session.setPackageName("com.foo"); - - mStagingManager.createSession(session); - assertThat(mStagingManager.getSessionIdByPackageName("com.foo")).isEqualTo(-1); - } - - @Test - public void getSessionIdByPackageName_noSessions() throws Exception { - assertThat(mStagingManager.getSessionIdByPackageName("com.foo")).isEqualTo(-1); - } - - @Test - public void getSessionIdByPackageName_noSessionHasThisPackage() throws Exception { - FakeStagedSession session = new FakeStagedSession(37); - session.setCommitted(true); - session.setSessionApplied(); - session.setPackageName("com.foo"); - - mStagingManager.createSession(session); - assertThat(mStagingManager.getSessionIdByPackageName("com.bar")).isEqualTo(-1); - } - @Test public void getStagedApexInfos_validatePreConditions() throws Exception { // Invalid session: null session From 5814ae95fe8d5c90bbb6d608ab4e5c0b2d534421 Mon Sep 17 00:00:00 2001 From: JW Wang Date: Wed, 3 Nov 2021 16:56:22 +0800 Subject: [PATCH 2/2] Fix a data race * mPendingVerification should only be accessed in the handler thread. * Add comments to note the thread invariants of the fields. Bug: 204963930 Test: m Change-Id: I322ff9d35bb2a603bc34a1dd3536cb0d1edeb9a7 --- .../server/pm/PackageManagerService.java | 55 ++++++++++++------- 1 file changed, 35 insertions(+), 20 deletions(-) diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index 7f1ac15f2ba73..c66d6c4eaa416 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -806,10 +806,16 @@ public class PackageManagerService extends IPackageManager.Stub @GuardedBy("mProtectedBroadcasts") final ArraySet mProtectedBroadcasts = new ArraySet<>(); - /** List of packages waiting for verification. */ + /** + * List of packages waiting for verification. + * Handler thread only! + */ final SparseArray mPendingVerification = new SparseArray<>(); - /** List of packages waiting for rollback to be enabled. */ + /** + * List of packages waiting for rollback to be enabled. + * Handler thread only! + */ final SparseArray mPendingEnableRollback = new SparseArray<>(); final PackageInstallerService mInstallerService; @@ -830,10 +836,16 @@ public class PackageManagerService extends IPackageManager.Stub // Cache of users who need badging. private final SparseBooleanArray mUserNeedsBadging = new SparseBooleanArray(); - /** Token for keys in mPendingVerification. */ + /** + * Token for keys in mPendingVerification. + * Handler thread only! + */ int mPendingVerificationToken = 0; - /** Token for keys in mPendingEnableRollback. */ + /** + * Token for keys in mPendingEnableRollback. + * Handler thread only! + */ int mPendingEnableRollbackToken = 0; @Watched(manual = true) @@ -6357,25 +6369,28 @@ public class PackageManagerService extends IPackageManager.Stub android.Manifest.permission.PACKAGE_VERIFICATION_AGENT, "Only package verification agents can extend verification timeouts"); - final PackageVerificationState state = mPendingVerification.get(id); - final PackageVerificationResponse response = new PackageVerificationResponse( - verificationCodeAtTimeout, Binder.getCallingUid()); + mHandler.post(() -> { + final PackageVerificationState state = mPendingVerification.get(id); + final PackageVerificationResponse response = new PackageVerificationResponse( + verificationCodeAtTimeout, Binder.getCallingUid()); - if (millisecondsToDelay > PackageManager.MAXIMUM_VERIFICATION_TIMEOUT) { - millisecondsToDelay = PackageManager.MAXIMUM_VERIFICATION_TIMEOUT; - } - if (millisecondsToDelay < 0) { - millisecondsToDelay = 0; - } + long delay = millisecondsToDelay; + if (delay > PackageManager.MAXIMUM_VERIFICATION_TIMEOUT) { + delay = PackageManager.MAXIMUM_VERIFICATION_TIMEOUT; + } + if (delay < 0) { + delay = 0; + } - if ((state != null) && !state.timeoutExtended()) { - state.extendTimeout(); + if ((state != null) && !state.timeoutExtended()) { + state.extendTimeout(); - final Message msg = mHandler.obtainMessage(PACKAGE_VERIFIED); - msg.arg1 = id; - msg.obj = response; - mHandler.sendMessageDelayed(msg, millisecondsToDelay); - } + final Message msg = mHandler.obtainMessage(PACKAGE_VERIFIED); + msg.arg1 = id; + msg.obj = response; + mHandler.sendMessageDelayed(msg, delay); + } + }); } private void setEnableRollbackCode(int token, int enableRollbackCode) {