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

3 files changed, 40 insertions, 14 deletions

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();
         }
diff --git a/tests/wifitests/src/com/android/server/wifi/WifiDiagnosticsTest.java b/tests/wifitests/src/com/android/server/wifi/WifiDiagnosticsTest.java
index 6b235b9f7..c89fe675a 100644
--- a/tests/wifitests/src/com/android/server/wifi/WifiDiagnosticsTest.java
+++ b/tests/wifitests/src/com/android/server/wifi/WifiDiagnosticsTest.java
@@ -26,6 +26,7 @@ import static org.mockito.Matchers.anyString;
 import static org.mockito.Matchers.contains;
 import static org.mockito.Mockito.anyInt;
 import static org.mockito.Mockito.anyObject;
+import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.eq;
 import static org.mockito.Mockito.never;
@@ -72,6 +73,7 @@ public class WifiDiagnosticsTest extends WifiBaseTest {
     @Mock WifiMetrics mWifiMetrics;
     @Mock Clock mClock;
     @Mock BugreportManager mBugreportManager;
+    private long mBootTimeMs = 0L;
     MockResources mResources;
     WifiDiagnostics mWifiDiagnostics;
 
@@ -135,6 +137,13 @@ public class WifiDiagnosticsTest extends WifiBaseTest {
         when(mWifiInjector.getWifiMetrics()).thenReturn(mWifiMetrics);
         when(mWifiInjector.getDeviceConfigFacade()).thenReturn(mDeviceConfigFacade);
         when(mDeviceConfigFacade.getBugReportMinWindowMs()).thenReturn(BUG_REPORT_MIN_WINDOW_MS);
+        // needed to for the loop in WifiDiagnostics.readLogcatStreamLinesWithTimeout().
+        doAnswer(new AnswerWithArguments() {
+            public long answer() throws Exception {
+                mBootTimeMs += WifiDiagnostics.LOGCAT_READ_TIMEOUT_MILLIS / 2;
+                return mBootTimeMs;
+            }
+        }).when(mClock).getElapsedSinceBootMillis();
         mWifiDiagnostics = new WifiDiagnostics(
                 mContext, mWifiInjector, mWifiNative, mBuildProperties, mLastMileLogger, mClock);
         mWifiNative.enableVerboseLogging(0);