From 16a691f87b8b00fe32f2faf1d7e222d770da7745 Mon Sep 17 00:00:00 2001 From: Martijn Coenen Date: Thu, 24 Sep 2020 19:49:35 +0200 Subject: Don't dump binder proxies with the lock held. There's still a path where the AMS lock is held when we're trying to dump binder proxies; in this case, just don't dump any proxies at all, since there's no safe way to do it with the AMS lock held. Bug: 168353824 Test: adb shell am dump binder-proxies Change-Id: Ia9b23821a953ac73fe6d67cc169998548680083d (cherry picked from commit 7174ebbf55f9fe2dc53dbf17e812298b220d98db) --- .../java/com/android/server/am/ActivityManagerService.java | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java index 491579a5a294..bf92f4e03c46 100644 --- a/services/core/java/com/android/server/am/ActivityManagerService.java +++ b/services/core/java/com/android/server/am/ActivityManagerService.java @@ -10530,7 +10530,7 @@ public class ActivityManagerService extends IActivityManager.Stub private void dumpEverything(FileDescriptor fd, PrintWriter pw, String[] args, int opti, boolean dumpAll, String dumpPackage, boolean dumpClient, boolean dumpNormalPriority, - int dumpAppId) { + int dumpAppId, boolean dumpProxies) { ActiveServices.ServiceDumper sdumper; @@ -10589,7 +10589,7 @@ public class ActivityManagerService extends IActivityManager.Stub } sdumper.dumpWithClient(); } - if (dumpPackage == null) { + if (dumpPackage == null && dumpProxies) { // Intentionally dropping the lock for this, because dumpBinderProxies() will make many // outgoing binder calls to retrieve interface descriptors; while that is system code, // there is nothing preventing an app from overriding this implementation by talking to @@ -10998,13 +10998,14 @@ public class ActivityManagerService extends IActivityManager.Stub // dumpEverything() will take the lock when needed, and momentarily drop // it for dumping client state. dumpEverything(fd, pw, args, opti, dumpAll, dumpPackage, dumpClient, - dumpNormalPriority, dumpAppId); + dumpNormalPriority, dumpAppId, true /* dumpProxies */); } else { // Take the lock here, so we get a consistent state for the entire dump; - // dumpEverything() will take the lock as well, but that is fine. + // dumpEverything() will take the lock as well, which is fine for everything + // except dumping proxies, which can take a long time; exclude them. synchronized(this) { dumpEverything(fd, pw, args, opti, dumpAll, dumpPackage, dumpClient, - dumpNormalPriority, dumpAppId); + dumpNormalPriority, dumpAppId, false /* dumpProxies */); } } } -- cgit v1.2.3-59-g8ed1b