Revert "Avoid wm<->am deadlock when checking uri permission"
This reverts commit d638cd6164bfaede121baff18eeba98265a188e9.
Bug: 157106728
Bug: 115619667
Reason for revert:
Looked into this some with the Camera test case in b/157106728. The wait()/notify strategy isn't going to work here because we have a 3-lock system vs. 2. We have the WM, UGM, and AM locks involved. Both the WM and UGM lock are held in the call to ActivityManagerInternal#checkContentProviderUriPermission and in the subsequence call that ActivityManagerService#checkContentProviderUriPermission AMS calls back into WMS and UGMS for various reasons which requires their locks...wait()/notify strategy allows us to break out of 1 lock, but things get really messed up if we try to use that strategy to break out of 2 locks.
Change-Id: Ic0f30f6c1b24334105a00790488d2758390e85a1
diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java
index dc6714b..2b86d7f 100644
--- a/services/core/java/com/android/server/am/ActivityManagerService.java
+++ b/services/core/java/com/android/server/am/ActivityManagerService.java
@@ -337,7 +337,6 @@
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;
@@ -18764,39 +18763,28 @@
@Override
public int checkContentProviderUriPermission(Uri uri, int userId,
int callingUid, int modeFlags) {
- 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) {
+ // 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.
- }
- return result[0];
- }
- } else {
+ // 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 ActivityManagerService.this.checkContentProviderUriPermission(uri,
userId, callingUid, modeFlags);
- }
- }
-
- 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();
+ } else {
+ final CompletableFuture<Integer> 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);
}
}
}