commit 7271f04a9cdaf6bb6eb633424891c17654b2807f Author: Damian Johnson atagar@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)