Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Revert "Avoid wm<->am deadlock when checking uri permission"

Browse Source 
This reverts commit d638cd6164.

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
This commit is contained in:
Wale Ogunwale
2020-05-21 00:21:08 +00:00
parent d638cd6164
commit f435516c9c
1 changed files with 19 additions and 31 deletions
Download Patch File Download Diff File Expand all files Collapse all files

50
services/core/java/com/android/server/am/ActivityManagerService.java
View File

@@ -337,7 +337,6 @@ 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;
@@ -18764,39 +18763,28 @@ public class ActivityManagerService extends IActivityManager.Stub
@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);
}
}
}