From d7cd8eda88f063aa9946d7b1586f61d79b789bd9 Mon Sep 17 00:00:00 2001
From: Riddle Hsu <riddlehsu@google.com>
Date: Tue, 16 Jul 2019 18:19:01 +0800
Subject: [PATCH] Don't hold lock when dumping activities of app side

Although TransferPipe has a 5s timeout, there may have
several activities take long time do dump that may lead
to system no response.

Fix: 135770637
Test: Launch an activity which overrides Activity#dump
      and sleeps 5s in the dump method. Enter command:
      "adb shell dumpsys activity all", then check the
      interaction with system UI is still responsive.

Change-Id: Id81463eee0178065a0fb982a6e599bee5f15dd81
---
 .../server/wm/ActivityTaskManagerService.java | 32 +++++++++----------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/services/core/java/com/android/server/wm/ActivityTaskManagerService.java b/services/core/java/com/android/server/wm/ActivityTaskManagerService.java
index 0f96f99c33486..8a9dfc7569dc9 100644
--- a/services/core/java/com/android/server/wm/ActivityTaskManagerService.java
+++ b/services/core/java/com/android/server/wm/ActivityTaskManagerService.java
@@ -4998,6 +4998,9 @@ public class ActivityTaskManagerService extends IActivityTaskManager.Stub {
      *  - the cmd arg isn't the flattened component name of an existing activity:
      *    dump all activity whose component contains the cmd as a substring
      *  - A hex number of the ActivityRecord object instance.
+     * <p>
+     * The caller should not hold lock when calling this method because it will wait for the
+     * activities to complete the dump.
      *
      *  @param dumpVisibleStacksOnly dump activity with {@param name} only if in a visible stack
      *  @param dumpFocusedStackOnly dump activity with {@param name} only if in the focused stack
@@ -5050,29 +5053,28 @@ public class ActivityTaskManagerService extends IActivityTaskManager.Stub {
     private void dumpActivity(String prefix, FileDescriptor fd, PrintWriter pw,
             final ActivityRecord r, String[] args, boolean dumpAll) {
         String innerPrefix = prefix + "  ";
+        IApplicationThread appThread = null;
         synchronized (mGlobalLock) {
             pw.print(prefix); pw.print("ACTIVITY "); pw.print(r.shortComponentName);
             pw.print(" "); pw.print(Integer.toHexString(System.identityHashCode(r)));
             pw.print(" pid=");
-            if (r.hasProcess()) pw.println(r.app.getPid());
-            else pw.println("(not running)");
+            if (r.hasProcess()) {
+                pw.println(r.app.getPid());
+                appThread = r.app.getThread();
+            } else {
+                pw.println("(not running)");
+            }
             if (dumpAll) {
                 r.dump(pw, innerPrefix);
             }
         }
-        if (r.attachedToProcess()) {
+        if (appThread != null) {
             // flush anything that is already in the PrintWriter since the thread is going
             // to write to the file descriptor directly
             pw.flush();
-            try {
-                TransferPipe tp = new TransferPipe();
-                try {
-                    r.app.getThread().dumpActivity(tp.getWriteFd(),
-                            r.appToken, innerPrefix, args);
-                    tp.go(fd);
-                } finally {
-                    tp.kill();
-                }
+            try (TransferPipe tp = new TransferPipe()) {
+                appThread.dumpActivity(tp.getWriteFd(), r.appToken, innerPrefix, args);
+                tp.go(fd);
             } catch (IOException e) {
                 pw.println(innerPrefix + "Failure while dumping the activity: " + e);
             } catch (RemoteException e) {
@@ -7181,10 +7183,8 @@ public class ActivityTaskManagerService extends IActivityTaskManager.Stub {
         public boolean dumpActivity(FileDescriptor fd, PrintWriter pw, String name,
                 String[] args, int opti, boolean dumpAll, boolean dumpVisibleStacksOnly,
                 boolean dumpFocusedStackOnly) {
-            synchronized (mGlobalLock) {
-                return ActivityTaskManagerService.this.dumpActivity(fd, pw, name, args, opti,
-                        dumpAll, dumpVisibleStacksOnly, dumpFocusedStackOnly);
-            }
+            return ActivityTaskManagerService.this.dumpActivity(fd, pw, name, args, opti, dumpAll,
+                    dumpVisibleStacksOnly, dumpFocusedStackOnly);
         }
 
         @Override