From 00348e6609c7d4b8b1a4739ac191539d5a205a58 Mon Sep 17 00:00:00 2001
From: Jing Ji <jji@google.com>
Date: Thu, 8 Apr 2021 12:10:29 -0700
Subject: [PATCH] Dump stack trace without holding the global lock in Watchdog

Since the dumping could last quite long.

Bug: 182937137
Test: adb shell am hang & check the log
Change-Id: I60c0b4aa5fab2f7e7ed6c9bea0365956b446967c
---
 .../java/com/android/server/Watchdog.java     | 44 ++++++++++++-------
 1 file changed, 27 insertions(+), 17 deletions(-)

diff --git a/services/core/java/com/android/server/Watchdog.java b/services/core/java/com/android/server/Watchdog.java
index 211999f431831..9835a9adb7ac8 100644
--- a/services/core/java/com/android/server/Watchdog.java
+++ b/services/core/java/com/android/server/Watchdog.java
@@ -56,6 +56,7 @@ import java.io.IOException;
 import java.io.StringWriter;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
 import java.util.concurrent.TimeUnit;
@@ -585,10 +586,12 @@ public class Watchdog {
     private void run() {
         boolean waitedHalf = false;
         while (true) {
-            final List<HandlerChecker> blockedCheckers;
-            final String subject;
-            final boolean allowRestart;
+            List<HandlerChecker> blockedCheckers = Collections.emptyList();
+            String subject = "";
+            boolean allowRestart = true;
             int debuggerWasConnected = 0;
+            boolean doWaitedHalfDump = false;
+            final ArrayList<Integer> pids;
             synchronized (mLock) {
                 long timeout = CHECK_INTERVAL;
                 // Make sure we (re)spin the checkers that have become idle within
@@ -634,20 +637,28 @@ public class Watchdog {
                 } else if (waitState == WAITED_HALF) {
                     if (!waitedHalf) {
                         Slog.i(TAG, "WAITED_HALF");
-                        // We've waited half the deadlock-detection interval.  Pull a stack
-                        // trace and wait another half.
-                        ArrayList<Integer> pids = new ArrayList<>(mInterestingJavaPids);
-                        ActivityManagerService.dumpStackTraces(pids, null, null,
-                                getInterestingNativePids(), null);
                         waitedHalf = true;
+                        // We've waited half, but we'd need to do the stack trace dump w/o the lock.
+                        pids = new ArrayList<>(mInterestingJavaPids);
+                        doWaitedHalfDump = true;
+                    } else {
+                        continue;
                     }
-                    continue;
+                } else {
+                    // something is overdue!
+                    blockedCheckers = getBlockedCheckersLocked();
+                    subject = describeCheckersLocked(blockedCheckers);
+                    allowRestart = mAllowRestart;
+                    pids = new ArrayList<>(mInterestingJavaPids);
                 }
+            } // END synchronized (mLock)
 
-                // something is overdue!
-                blockedCheckers = getBlockedCheckersLocked();
-                subject = describeCheckersLocked(blockedCheckers);
-                allowRestart = mAllowRestart;
+            if (doWaitedHalfDump) {
+                // We've waited half the deadlock-detection interval.  Pull a stack
+                // trace and wait another half.
+                ActivityManagerService.dumpStackTraces(pids, null, null,
+                        getInterestingNativePids(), null);
+                continue;
             }
 
             // If we got here, that means that the system is most likely hung.
@@ -655,8 +666,6 @@ public class Watchdog {
             // Then kill this process so that the system will restart.
             EventLog.writeEvent(EventLogTags.WATCHDOG, subject);
 
-            ArrayList<Integer> pids = new ArrayList<>(mInterestingJavaPids);
-
             long anrTime = SystemClock.uptimeMillis();
             StringBuilder report = new StringBuilder();
             report.append(MemoryPressureUtil.currentPsiState());
@@ -681,6 +690,7 @@ public class Watchdog {
             // Try to add the error to the dropbox, but assuming that the ActivityManager
             // itself may be deadlocked.  (which has happened, causing this statement to
             // deadlock and the watchdog as a whole to be ineffective)
+            final String localSubject = subject;
             Thread dropboxThread = new Thread("watchdogWriteToDropbox") {
                     public void run() {
                         // If a watched thread hangs before init() is called, we don't have a
@@ -688,10 +698,10 @@ public class Watchdog {
                         if (mActivity != null) {
                             mActivity.addErrorToDropBox(
                                     "watchdog", null, "system_server", null, null, null,
-                                    subject, report.toString(), stack, null, null, null);
+                                    localSubject, report.toString(), stack, null, null, null);
                         }
                         FrameworkStatsLog.write(FrameworkStatsLog.SYSTEM_SERVER_WATCHDOG_OCCURRED,
-                                subject);
+                                localSubject);
                     }
                 };
             dropboxThread.start();