From d638cd6164bfaede121baff18eeba98265a188e9 Mon Sep 17 00:00:00 2001 From: Wale Ogunwale Date: Sun, 17 May 2020 09:19:53 -0700 Subject: [PATCH] Avoid wm<->am deadlock when checking uri permission Bug: 115619667 Test: N/A Change-Id: I43a868b261eb1546464c401b6ec1417870fa6f9c --- .../server/am/ActivityManagerService.java | 50 ++++++++++++------- 1 file changed, 31 insertions(+), 19 deletions(-) diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java index 2b86d7fe057ed..dc6714b0c2ea4 100644 --- a/services/core/java/com/android/server/am/ActivityManagerService.java +++ b/services/core/java/com/android/server/am/ActivityManagerService.java @@ -337,6 +337,7 @@ import com.android.internal.util.Preconditions; import com.android.internal.util.function.HexFunction; import com.android.internal.util.function.QuadFunction; import com.android.internal.util.function.TriFunction; +import com.android.internal.util.function.pooled.PooledLambda; import com.android.server.AlarmManagerInternal; import com.android.server.AttributeCache; import com.android.server.DeviceIdleInternal; @@ -18763,28 +18764,39 @@ public class ActivityManagerService extends IActivityManager.Stub @Override public int checkContentProviderUriPermission(Uri uri, int userId, int callingUid, int modeFlags) { - // We can find ourselves needing to check Uri permissions while - // already holding the WM lock, which means reaching back here for - // the AM lock would cause an inversion. The WM team has requested - // that we use the strategy below instead of shifting where Uri - // grants are calculated. + final Object wmLock = mActivityTaskManager.getGlobalLock(); + if (Thread.currentThread().holdsLock(wmLock) + && !Thread.currentThread().holdsLock(ActivityManagerService.this)) { + // We can find ourselves needing to check Uri permissions while already holding the + // WM lock, which means reaching back here for the AM lock would cause an inversion. + // The WM team has requested that we use the strategy below instead of shifting + // where Uri grants are calculated. + synchronized (wmLock) { + final int[] result = new int[1]; + final Message msg = PooledLambda.obtainMessage( + LocalService::checkContentProviderUriPermission, + this, uri, userId, callingUid, modeFlags, wmLock, result); + mHandler.sendMessage(msg); + try { + wmLock.wait(); + } catch (InterruptedException ignore) { - // Since we could also arrive here while holding the AM lock, we - // can't always delegate the call through the handler, and we need - // to delicately dance between the deadlocks. - if (Thread.currentThread().holdsLock(ActivityManagerService.this)) { + } + return result[0]; + } + } else { return ActivityManagerService.this.checkContentProviderUriPermission(uri, userId, callingUid, modeFlags); - } else { - final CompletableFuture res = new CompletableFuture<>(); - mHandler.post(() -> { - res.complete(ActivityManagerService.this.checkContentProviderUriPermission(uri, - userId, callingUid, modeFlags)); - }); - try { - return res.get(); - } catch (InterruptedException | ExecutionException e) { - throw new RuntimeException(e); + } + } + + void checkContentProviderUriPermission( + Uri uri, int userId, int callingUid, int modeFlags, Object wmLock, int[] result) { + synchronized (ActivityManagerService.this) { + synchronized (wmLock) { + result[0] = ActivityManagerService.this.checkContentProviderUriPermission( + uri, userId, callingUid, modeFlags); + wmLock.notify(); } } }