Make TimeoutRetryThread's stoppable

build/android/pylib/utils/reraiser_thread.py

Index: build/android/pylib/utils/reraiser_thread.py
diff --git a/build/android/pylib/utils/reraiser_thread.py b/build/android/pylib/utils/reraiser_thread.py
index 2964bef1e8265e3931e2cdda15f626d8f131cc77..f465b43bdc641619f1606ba55f210e8f81835aec 100644
--- a/build/android/pylib/utils/reraiser_thread.py
+++ b/build/android/pylib/utils/reraiser_thread.py
@@ -58,6 +58,7 @@ class ReraiserThread(threading.Thread):
     self._kwargs = kwargs
     self._ret = None
     self._exc_info = None
+    self._stopped = False
 
   def ReraiseIfException(self):
     """Reraise exception if an exception was raised in the thread."""
@@ -69,6 +70,13 @@ class ReraiserThread(threading.Thread):
     self.ReraiseIfException()
     return self._ret
 
+  def StopThread(self):

[jbudorick, 2014/10/24 17:30:51]

I don't think we want to provide this in reraiser_thread unless we can actually
make reraiser_thread stop the thread. Relying on self._func to check Stopped
seems problematic.

I don't have any great suggestions for implementing this otherwise, but I'll
keep thinking about how we could.

[perezju, 2014/10/27 11:07:07]

Looking around, e.g.
http://stackoverflow.com/questions/323972/is-there-any-way-to-kill-a-thread-i...,
asking a thread to stop is perhaps the least problematic approach. (And
definitely better than what we now do, which is... nothing and keep running 2 or
3 threads at the same time during retries).

There seems to be a hack using PyThreadState_SetAsyncExc to make one thread
raise an exception on another, but besides risky also seems quite useless if the
following is true: "If the thread is busy in a system call (time.sleep(),
socket.accept(), ...), the exception is simply ignored.".

[jbudorick, 2014/10/27 19:15:36]

I'm not sure this is better than what we do now. Is there some way that we can
either guarantee that reraiser threads check to see if they should stop or only
make this interface available if the thread supports stopping? If not, this just
seems liable to deceive clients.

We should also rename this to make it clear that this merely asks the thread to
stop.
Yeah, I don't like that here.

[perezju, 2014/10/28 14:31:17]

Just for fun, trying to reboot with a very low timeout.

This is what the TimeoutRetry wrapper currently does:
https://drive.google.com/a/chromium.org/file/d/0B7JfGcdoP-YzTnAzOEJRUXFhNmM/v...

This is what I think it should do:
https://drive.google.com/a/chromium.org/file/d/0B7JfGcdoP-YzQlZIWnEyLTZ1TXM/v...

+    """Request this thread to stop."""
+    self._stopped = True
+
+  def Stopped(self):
+    return self._stopped
+
   #override
   def run(self):
     """Overrides Thread.run() to add support for reraising exceptions."""
@@ -104,7 +112,7 @@ class ReraiserThreadGroup(object):
     for thread in self._threads:
       thread.start()
 
-  def _JoinAll(self, watcher=watchdog_timer.WatchdogTimer(None)):
+  def _JoinAll(self, watcher):
     """Join all threads without stack dumps.
 
     Reraises exceptions raised by the child threads and supports breaking
@@ -127,7 +135,7 @@ class ReraiserThreadGroup(object):
     for thread in self._threads:
       thread.ReraiseIfException()
 
-  def JoinAll(self, watcher=watchdog_timer.WatchdogTimer(None)):
+  def JoinAll(self, watcher=None):
     """Join all threads.
 
     Reraises exceptions raised by the child threads and supports breaking
@@ -137,11 +145,14 @@ class ReraiserThreadGroup(object):
     Args:
       watcher: Watchdog object providing timeout, by default waits forever.
     """
+    if watcher is None:
+      watcher=watchdog_timer.WatchdogTimer(None)

[jbudorick, 2014/10/24 17:30:52]

nice catch.

nit: spaces around =

     try:
       self._JoinAll(watcher)
     except TimeoutError:
       for thread in (t for t in self._threads if t.isAlive()):
         LogThreadStack(thread)
+        thread.StopThread()
       raise
 
   def GetAllReturnValues(self, watcher=watchdog_timer.WatchdogTimer(None)):