Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 16a691f8 authored by Martijn Coenen's avatar Martijn Coenen Committed by android-build-team Robot
Browse files

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 7174ebbf)
parent 6cb400d3

services/core/java/com/android/server/am/ActivityManagerService.java

+6 −5
Original line number Diff line number Diff line
@@ -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 */);















                }















            }















        }