[Devil] Add timeout to logcat record thread join.

devil/devil/android/logcat_monitor.py

 # Copyright 2015 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 # pylint: disable=unused-argument
 
 import logging
 import os
 import re
 import shutil
 import tempfile
 import threading
 import time
 
 from devil.android import decorators
 from devil.android import device_errors
 from devil.android.sdk import adb_wrapper
 from devil.utils import reraiser_thread
 
 
 class LogcatMonitor(object):
 
+  _LOGCAT_GENERATOR_TIMEOUT = 5.0
   _WAIT_TIME = 0.2
   _THREADTIME_RE_FORMAT = (
       r'(?P<date>\S*) +(?P<time>\S*) +(?P<proc_id>%s) +(?P<thread_id>%s) +'
       r'(?P<log_level>%s) +(?P<component>%s) *: +(?P<message>%s)$')
 
   def __init__(self, adb, clear=True, filter_specs=None, output_file=None):
     """Create a LogcatMonitor instance.
 
     Args:
       adb: An instance of adb_wrapper.AdbWrapper.
@@ skipping 114 matching lines @@
 
   def _StartRecording(self):
     """Starts recording logcat to file.
 
     Function spawns a thread that records logcat to file and will not die
     until |StopRecording| is called.
     """
     def record_to_file():
       # Write the log with line buffering so the consumer sees each individual
       # line.
+      logcat_generator = self._adb.Logcat(filter_specs=self._filter_specs,
+                                          logcat_format='threadtime')
+      @decorators.WithExplicitTimeoutAndRetries(

[jbudorick, 2016/01/26 23:13:19]

This decorator is here to check a condition (whether the timeout has been hit)
and kill the function's execution if that condition is met. In this case, it
does so s.t. it can check another condition -- whether
self._stop_recording_event is set -- and kill the loop if that condition is met.

Why don't we skip the timeout portion and just have the supervising thread kill
the generator thread if the event is set?

[jbudorick, 2016/01/26 23:32:37]

As for the slowness part, we could potentially add something to reraiser_thread
s.t. we could run a generator in one thread and iterate over it in another. I
think that could keep us from creating another thread on each logcat line.

+          self._LOGCAT_GENERATOR_TIMEOUT, 0)
+      def logcat_next():
+        logcat_generator.next()
+
       with open(self._record_file.name, 'a', 1) as f:
-        for data in self._adb.Logcat(filter_specs=self._filter_specs,
-                                     logcat_format='threadtime'):
+        while True:
+          try:
+            data = logcat_next()
+            f.write(data + '\n')
+          except device_errors.CommandTimeoutError:
+            pass
           if self._stop_recording_event.isSet():
             f.flush()
             return
-          f.write(data + '\n')
 
     self._stop_recording_event.clear()
     if not self._record_thread:
       self._record_thread = reraiser_thread.ReraiserThread(record_to_file)
       self._record_thread.start()
 
   def _StopRecording(self):
     """Finish recording logcat."""
     if self._record_thread:
       self._stop_recording_event.set()
@@ skipping 44 matching lines @@
   def __del__(self):
     """Closes logcat recording file in case |Close| was never called."""
     if self._record_file:
       logging.warning('Need to call |Close| on the logcat monitor when done!')
       self._record_file.close()
 
 
 class LogcatMonitorCommandError(device_errors.CommandFailedError):
   """Exception for errors with logcat monitor commands."""
   pass