From 1d1e75319a6831615e4fbb3e02e34026f5bae5a2 Mon Sep 17 00:00:00 2001 From: Jeff Sharkey Date: Wed, 3 Jun 2020 13:05:01 -0600 Subject: Relax locking for startActivityInPackage(). Both startActivityInPackage() and startActivitiesInPackage() eventually call through to resolveActivity(), which might need to acquire the AM lock. The remainder of ActivityStarter.execute() already acquires the WM lock when needed, so it should be safe to drop it from the overall method. Add additional guarding to ensure that this doesn't regress in the future. Bug: 115619667, 157863128 Test: atest WmTests:ActivityStarterTests Test: atest FrameworksServicesTests:com.android.server.uri Test: atest CtsAppSecurityHostTestCases:android.appsecurity.cts.AppSecurityTests#testPermissionDiffCert Test: atest CtsWindowManagerDeviceTestCases:CrossAppDragAndDropTests Test: atest CtsWindowManagerDeviceTestCases:ActivityStarterTests Change-Id: Ia1693873a10d11ea61a8a7748e0558beffda8c5b (cherry picked from commit 13a7cf7044a95f105a693d1c3943441de9c3c99a) --- .../com/android/server/wm/ActivityStarter.java | 8 +++++++- .../server/wm/ActivityTaskManagerService.java | 22 +++++++++------------- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/services/core/java/com/android/server/wm/ActivityStarter.java b/services/core/java/com/android/server/wm/ActivityStarter.java index daa97b56ed66..30779973b204 100644 --- a/services/core/java/com/android/server/wm/ActivityStarter.java +++ b/services/core/java/com/android/server/wm/ActivityStarter.java @@ -639,8 +639,14 @@ class ActivityStarter { mRequest.intent, caller); } - // Do not lock the resolving to avoid potential deadlock. + // If the caller hasn't already resolved the activity, we're willing + // to do so here, but because that may require acquiring the AM lock + // as part of calculating the NeededUriGrants, we must never hold + // the WM lock here to avoid deadlocking. if (mRequest.activityInfo == null) { + if (Thread.holdsLock(mService.mGlobalLock)) { + Slog.wtf(TAG, new IllegalStateException("Caller must not hold WM lock")); + } mRequest.resolveActivity(mSupervisor); } diff --git a/services/core/java/com/android/server/wm/ActivityTaskManagerService.java b/services/core/java/com/android/server/wm/ActivityTaskManagerService.java index cf453c7feef8..205523b2a119 100644 --- a/services/core/java/com/android/server/wm/ActivityTaskManagerService.java +++ b/services/core/java/com/android/server/wm/ActivityTaskManagerService.java @@ -6174,12 +6174,10 @@ public class ActivityTaskManagerService extends IActivityTaskManager.Stub { boolean validateIncomingUser, PendingIntentRecord originatingPendingIntent, boolean allowBackgroundActivityStart) { assertPackageMatchesCallingUid(callingPackage); - synchronized (mGlobalLock) { - return getActivityStartController().startActivitiesInPackage(uid, realCallingPid, - realCallingUid, callingPackage, callingFeatureId, intents, resolvedTypes, - resultTo, options, userId, validateIncomingUser, originatingPendingIntent, - allowBackgroundActivityStart); - } + return getActivityStartController().startActivitiesInPackage(uid, realCallingPid, + realCallingUid, callingPackage, callingFeatureId, intents, resolvedTypes, + resultTo, options, userId, validateIncomingUser, originatingPendingIntent, + allowBackgroundActivityStart); } @Override @@ -6190,13 +6188,11 @@ public class ActivityTaskManagerService extends IActivityTaskManager.Stub { boolean validateIncomingUser, PendingIntentRecord originatingPendingIntent, boolean allowBackgroundActivityStart) { assertPackageMatchesCallingUid(callingPackage); - synchronized (mGlobalLock) { - return getActivityStartController().startActivityInPackage(uid, realCallingPid, - realCallingUid, callingPackage, callingFeatureId, intent, resolvedType, - resultTo, resultWho, requestCode, startFlags, options, userId, inTask, - reason, validateIncomingUser, originatingPendingIntent, - allowBackgroundActivityStart); - } + return getActivityStartController().startActivityInPackage(uid, realCallingPid, + realCallingUid, callingPackage, callingFeatureId, intent, resolvedType, + resultTo, resultWho, requestCode, startFlags, options, userId, inTask, + reason, validateIncomingUser, originatingPendingIntent, + allowBackgroundActivityStart); } @Override -- cgit v1.2.3-59-g8ed1b