From a4fc5402d0a52aff7092160b7e73d071a53ef4a4 Mon Sep 17 00:00:00 2001
From: Roshan Pius <rpius@google.com>
Date: Fri, 29 May 2020 15:47:43 -0700
Subject: WifiDiagnostics: Remove blocking call to readLine()

Changes in the CL:
a) Add a check for isReady() before invoking blocking readLine().
b) Add a timeout in the loop to ensure we're not looping forever.

Note: Even with this, there is a theoretical chance that we will get
stuck in readLine() since isReady() will indicate some bytes available,
but not necessarily EOL. But, the logcat daemon outputs a line of data
at a time.

Bug: 157062041
Test: Manual verification of wifi dumpsys after a connection failure.
Change-Id: I6bbdf5f2fe7ed6afc9dd65d95fcb9e938366fdad
---
 service/java/com/android/server/wifi/Clock.java    |  6 ++++
 .../com/android/server/wifi/WifiDiagnostics.java   | 39 ++++++++++++++--------
 2 files changed, 31 insertions(+), 14 deletions(-)

(limited to 'service')

diff --git a/service/java/com/android/server/wifi/Clock.java b/service/java/com/android/server/wifi/Clock.java
index bb2f21e2d..b1d45961b 100644
--- a/service/java/com/android/server/wifi/Clock.java
+++ b/service/java/com/android/server/wifi/Clock.java
@@ -58,4 +58,10 @@ public class Clock {
         return SystemClock.uptimeMillis();
     }
 
+    /**
+     * Waits a given number of milliseconds (of uptimeMillis) before returning.
+     */
+    public void sleep(long ms) {
+        SystemClock.sleep(ms);
+    }
 }
diff --git a/service/java/com/android/server/wifi/WifiDiagnostics.java b/service/java/com/android/server/wifi/WifiDiagnostics.java
index 2ba90c968..a2328cd26 100644
--- a/service/java/com/android/server/wifi/WifiDiagnostics.java
+++ b/service/java/com/android/server/wifi/WifiDiagnostics.java
@@ -43,6 +43,7 @@ import java.util.Calendar;
 import java.util.Collections;
 import java.util.Comparator;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Set;
 import java.util.concurrent.TimeUnit;
 import java.util.stream.Collectors;
@@ -108,8 +109,11 @@ class WifiDiagnostics extends BaseWifiDiagnostics {
     /** Minimum dump period with same error code */
     public static final long MIN_DUMP_TIME_WINDOW_MILLIS = 10 * 60 * 1000; // 10 mins
 
-    // Timeout for logcat
-    private static final int LOGCAT_TIMEOUT_MILLIS = 500;
+    // Timeout for logcat process termination
+    private static final long LOGCAT_PROC_TIMEOUT_MILLIS = 500;
+    // Timeout for logcat read from input/error stream each.
+    @VisibleForTesting
+    public static final long LOGCAT_READ_TIMEOUT_MILLIS = 500;
 
     private long mLastBugReportTime;
 
@@ -708,23 +712,30 @@ class WifiDiagnostics extends BaseWifiDiagnostics {
         return result;
     }
 
+    private void readLogcatStreamLinesWithTimeout(
+            BufferedReader inReader, List<String> outLinesList) throws IOException {
+        long startTimeMs = mClock.getElapsedSinceBootMillis();
+        while (mClock.getElapsedSinceBootMillis() < startTimeMs + LOGCAT_READ_TIMEOUT_MILLIS) {
+            // If there is a burst of data, continue reading without checking for timeout.
+            while (inReader.ready()) {
+                String line = inReader.readLine();
+                if (line == null) return; // end of stream.
+                outLinesList.add(line);
+            }
+            mClock.sleep(LOGCAT_READ_TIMEOUT_MILLIS / 10);
+        }
+    }
+
     private ArrayList<String> getLogcat(String logcatSections, int maxLines) {
         ArrayList<String> lines = new ArrayList<>(maxLines);
         try {
             Process process = mJavaRuntime.exec(
                     String.format("logcat -b %s -t %d", logcatSections, maxLines));
-            BufferedReader reader = new BufferedReader(
-                    new InputStreamReader(process.getInputStream()));
-            String line;
-            while ((line = reader.readLine()) != null) {
-                lines.add(line);
-            }
-            reader = new BufferedReader(
-                    new InputStreamReader(process.getErrorStream()));
-            while ((line = reader.readLine()) != null) {
-                lines.add(line);
-            }
-            process.waitFor(LOGCAT_TIMEOUT_MILLIS, TimeUnit.MILLISECONDS);
+            readLogcatStreamLinesWithTimeout(
+                    new BufferedReader(new InputStreamReader(process.getInputStream())), lines);
+            readLogcatStreamLinesWithTimeout(
+                    new BufferedReader(new InputStreamReader(process.getErrorStream())), lines);
+            process.waitFor(LOGCAT_PROC_TIMEOUT_MILLIS, TimeUnit.MILLISECONDS);
         } catch (InterruptedException|IOException e) {
             mLog.dump("Exception while capturing logcat: %").c(e.toString()).flush();
         }
-- 
cgit v1.2.3