[tor-commits] [stem/master] Fixing testing thread leak and adding check for it

[atagar at torproject.org]

commit 7271f04a9cdaf6bb6eb633424891c17654b2807f
Author: Damian Johnson <atagar at torproject.org>
Date:   Sun Apr 22 18:38:16 2012 -0700

    Fixing testing thread leak and adding check for it
    
    Python does not gracefully handle lingering threads at shutdown, even if
    they're daemons (... which is dumb since that's the only point of the daemon
    thread). Having threads causes occasional nonsensical stacktraces with things
    like "'NoneType' object has no attribute 'socket'" for module references.
    
    Adding a check after running our integ tests that only the main thread remains,
    and erroring if that's not the case. This also fixes the thread leak that was
    probably the issue in...
    https://trac.torproject.org/projects/tor/ticket/5512
---
 run_tests.py                          |   24 +++++++++++++++++++++---
 stem/control.py                       |    9 ++++++++-
 test/integ/control/base_controller.py |    8 ++++----
 3 files changed, 33 insertions(+), 8 deletions(-)

diff --git a/run_tests.py b/run_tests.py
index dcfcdef..9ee6042 100755
--- a/run_tests.py
+++ b/run_tests.py
@@ -9,6 +9,7 @@ import sys
 import time
 import getopt
 import unittest
+import threading
 import StringIO
 
 import test.output
@@ -204,6 +205,9 @@ def load_user_configuration(test_config):
 if __name__ == '__main__':
   start_time = time.time()
   
+  # override flag to indicate at the end that testing failed somewhere
+  testing_failed = False
+  
   # loads and validates our various configurations
   test_config = stem.util.conf.get_config("test")
   
@@ -320,6 +324,20 @@ if __name__ == '__main__':
           print
           
           test.output.print_logging(logging_buffer)
+        
+        # We should have joined on all threads. If not then that indicates a
+        # leak that could both likely be a bug and disrupt further targets.
+        
+        active_threads = threading.enumerate()
+        
+        if len(active_threads) > 1:
+          test.output.print_line("Threads lingering after test run:", *ERROR_ATTR)
+          
+          for lingering_thread in active_threads:
+            test.output.print_line("  %s" % lingering_thread, *ERROR_ATTR)
+          
+          testing_failed = True
+          break
       except KeyboardInterrupt:
         test.output.print_line("  aborted starting tor: keyboard interrupt\n", *ERROR_ATTR)
         break
@@ -357,11 +375,11 @@ if __name__ == '__main__':
   if runtime < 1: runtime_label = "(%0.1f seconds)" % runtime
   else: runtime_label = "(%i seconds)" % runtime
   
-  if error_tracker.has_error_occured():
-    test.output.print_line("TESTING FAILED %s" % runtime_label, term.Color.RED, term.Attr.BOLD)
+  if testing_failed or error_tracker.has_error_occured():
+    test.output.print_line("TESTING FAILED %s" % runtime_label, *ERROR_ATTR)
     
     for line in error_tracker:
-      test.output.print_line("  %s" % line, term.Color.RED, term.Attr.BOLD)
+      test.output.print_line("  %s" % line, *ERROR_ATTR)
   else:
     test.output.print_line("TESTING PASSED %s" % runtime_label, term.Color.GREEN, term.Attr.BOLD)
     print
diff --git a/stem/control.py b/stem/control.py
index 55c6e7a..0ed4032 100644
--- a/stem/control.py
+++ b/stem/control.py
@@ -15,7 +15,8 @@ BaseController - Base controller class asynchronous message handling.
   |- close - shuts down our connection to the tor process
   |- get_socket - provides the socket used for control communication
   |- add_status_listener - notifies a callback of changes in our status
-  +- remove_status_listener - prevents further notification of status changes
+  |- remove_status_listener - prevents further notification of status changes
+  +- __enter__ / __exit__ - manages socket connection
 """
 
 import time
@@ -269,6 +270,12 @@ class BaseController:
       self._status_listeners = new_listeners
       return is_changed
   
+  def __enter__(self):
+    return self
+  
+  def __exit__(self, exit_type, value, traceback):
+    self.close()
+  
   def _handle_event(self, event_message):
     """
     Callback to be overwritten by subclasses for event listening. This is
diff --git a/test/integ/control/base_controller.py b/test/integ/control/base_controller.py
index 1abab93..940ef99 100644
--- a/test/integ/control/base_controller.py
+++ b/test/integ/control/base_controller.py
@@ -40,8 +40,8 @@ class TestBaseController(unittest.TestCase):
     """
     
     if test.runner.Torrc.PORT in test.runner.get_runner().get_options():
-      controller = stem.control.BaseController.from_port(control_port = test.runner.CONTROL_PORT)
-      self.assertTrue(isinstance(controller, stem.control.BaseController))
+      with stem.control.BaseController.from_port(control_port = test.runner.CONTROL_PORT) as controller:
+        self.assertTrue(isinstance(controller, stem.control.BaseController))
     else:
       self.assertRaises(stem.socket.SocketError, stem.control.BaseController.from_port, "127.0.0.1", test.runner.CONTROL_PORT)
   
@@ -51,8 +51,8 @@ class TestBaseController(unittest.TestCase):
     """
     
     if test.runner.Torrc.SOCKET in test.runner.get_runner().get_options():
-      controller = stem.control.BaseController.from_socket_file(test.runner.CONTROL_SOCKET_PATH)
-      self.assertTrue(isinstance(controller, stem.control.BaseController))
+      with stem.control.BaseController.from_socket_file(socket_path = test.runner.CONTROL_SOCKET_PATH) as controller:
+        self.assertTrue(isinstance(controller, stem.control.BaseController))
     else:
       self.assertRaises(stem.socket.SocketError, stem.control.BaseController.from_socket_file, test.runner.CONTROL_SOCKET_PATH)