From 8cf4a44bd74fefa78b25bc01a508482517adf2d1 Mon Sep 17 00:00:00 2001 From: Brendan Jackman Date: Wed, 20 Sep 2017 13:40:43 +0100 Subject: [PATCH] utils/android: Fix race condition in LogcatMonitor If you call .start then immediately call .stop, the thread may not yet have set ._logcat, resulting in an AttributeError. I initially fixed this by setting _logcat = None in __init__, then putting the `kill` calls inside `if self._logcat`. The problem with this, as pointed out by @valschneider, is that we can then have this sequence: main thread: monitor thread stop() run() if self._logcat: . # False, don't kill process . join() . self._logcat = <...> Therefore, just have the stop() method wait until the process is started before unconditionally killing it. --- devlib/utils/android.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/devlib/utils/android.py b/devlib/utils/android.py index 0123792..fd4c42e 100644 --- a/devlib/utils/android.py +++ b/devlib/utils/android.py @@ -545,6 +545,7 @@ class LogcatMonitor(threading.Thread): self.target = target + self._started = threading.Event() self._stopped = threading.Event() self._match_found = threading.Event() @@ -580,12 +581,17 @@ class LogcatMonitor(threading.Thread): logger.debug('logcat command ="{}"'.format(logcat_cmd)) self._logcat = self.target.background(logcat_cmd) + self._started.set() + while not self._stopped.is_set(): line = self._logcat.stdout.readline(1024) if line: self._add_line(line) def stop(self): + # Make sure we've started before we try to kill anything + self._started.wait() + # Kill the underlying logcat process # This will unblock self._logcat.stdout.readline() host.kill_children(self._logcat.pid)