[tor-commits] [stem/master] Lingering thread due to recent event unit tests

[atagar at torproject.org]

commit bc06836ee87169da8118ff985fc87ea6880ca219
Author: Damian Johnson <atagar at torproject.org>
Date:   Sun Aug 12 13:56:45 2018 -0700

    Lingering thread due to recent event unit tests
    
    Oops, new tests for #27053 didn't close the threads of the mock controller it
    created. Unit tests passed because we don't check for lingering tests, but if
    ran along with our integ tests the test run concluded with...
    
            Threads lingering after test run:
            <_MainThread(MainThread, started 140086124480256)>
            <Thread(Event Notifier, started daemon 140085629839104)>
            <Thread(Event Notifier, started daemon 140085657794304)>
            <Thread(Event Notifier, started daemon 140085638756096)>
    
    Tracked this down by dumping the internal attributes of those controllers.
    They had mocks from these tests, providing the eureka to get the root cause.
---
 docs/change_log.rst             |  2 +-
 test/unit/control/controller.py | 63 +++++++++++++++++++++++++----------------
 2 files changed, 40 insertions(+), 25 deletions(-)

diff --git a/docs/change_log.rst b/docs/change_log.rst
index 75a25847..9f4049e6 100644
--- a/docs/change_log.rst
+++ b/docs/change_log.rst
@@ -46,8 +46,8 @@ The following are only available within Stem's `git repository
 
  * **Controller**
 
-  * Documented v3 hidden service support (:trac:`25124`, :spec:`6bd0a69`)
   * Listener exceptions and malformed events no longer break further event processing (:trac:`27053`)
+  * Documented v3 hidden service support (:trac:`25124`, :spec:`6bd0a69`)
   * Added the stem.control.MALFORMED_EVENTS event listener constant
   * Added support for limiting the maximum number of streams to :func:`~stem.control.Controller.create_ephemeral_hidden_service` (:spec:`2fcb1c2`)
   * Added a timeout argument to :class:`~stem.control.Controller` methods that could await a response (:trac:`26056`)
diff --git a/test/unit/control/controller.py b/test/unit/control/controller.py
index c5809192..8034abe2 100644
--- a/test/unit/control/controller.py
+++ b/test/unit/control/controller.py
@@ -624,25 +624,30 @@ class TestControl(unittest.TestCase):
     self.assertRaises(InvalidRequest, self.controller.add_event_listener, Mock(), EventType.SIGNAL)
 
   @patch('time.time', Mock(return_value = TEST_TIMESTAMP))
-  @patch('stem.control.Controller.is_alive', Mock(return_value = True))
-  def test_events_get_received(self):
+  @patch('stem.control.Controller.is_alive')
+  def test_events_get_received(self, is_alive_mock):
     """
     Trigger an event, checking that our listeners get notified.
     """
 
+    is_alive_mock.return_value = True
     self.controller._launch_threads()
 
-    self._emit_event(CIRC_EVENT)
-    self.circ_listener.assert_called_once_with(CIRC_EVENT)
-    self.bw_listener.assert_not_called()
-    self.malformed_listener.assert_not_called()
+    try:
+      self._emit_event(CIRC_EVENT)
+      self.circ_listener.assert_called_once_with(CIRC_EVENT)
+      self.bw_listener.assert_not_called()
+      self.malformed_listener.assert_not_called()
 
-    self._emit_event(BW_EVENT)
-    self.bw_listener.assert_called_once_with(BW_EVENT)
+      self._emit_event(BW_EVENT)
+      self.bw_listener.assert_called_once_with(BW_EVENT)
+    finally:
+      is_alive_mock.return_value = False
+      self.controller._close()
 
   @patch('time.time', Mock(return_value = TEST_TIMESTAMP))
-  @patch('stem.control.Controller.is_alive', Mock(return_value = True))
-  def test_event_listing_with_error(self):
+  @patch('stem.control.Controller.is_alive')
+  def test_event_listing_with_error(self, is_alive_mock):
     """
     Raise an exception in an event listener to confirm it doesn't break our
     event thread.
@@ -650,33 +655,43 @@ class TestControl(unittest.TestCase):
 
     self.circ_listener.side_effect = ValueError('boom')
 
+    is_alive_mock.return_value = True
     self.controller._launch_threads()
 
-    self._emit_event(CIRC_EVENT)
-    self.circ_listener.assert_called_once_with(CIRC_EVENT)
-    self.bw_listener.assert_not_called()
-    self.malformed_listener.assert_not_called()
+    try:
+      self._emit_event(CIRC_EVENT)
+      self.circ_listener.assert_called_once_with(CIRC_EVENT)
+      self.bw_listener.assert_not_called()
+      self.malformed_listener.assert_not_called()
 
-    self._emit_event(BW_EVENT)
-    self.bw_listener.assert_called_once_with(BW_EVENT)
+      self._emit_event(BW_EVENT)
+      self.bw_listener.assert_called_once_with(BW_EVENT)
+    finally:
+      is_alive_mock.return_value = False
+      self.controller._close()
 
   @patch('time.time', Mock(return_value = TEST_TIMESTAMP))
-  @patch('stem.control.Controller.is_alive', Mock(return_value = True))
-  def test_event_listing_with_malformed_event(self):
+  @patch('stem.control.Controller.is_alive')
+  def test_event_listing_with_malformed_event(self, is_alive_mock):
     """
     Attempt to parse a malformed event emitted from Tor. It's important this
     doesn't break our event thread.
     """
 
+    is_alive_mock.return_value = True
     self.controller._launch_threads()
 
-    self._emit_event(BAD_EVENT)
-    self.circ_listener.assert_not_called()
-    self.bw_listener.assert_not_called()
-    self.malformed_listener.assert_called_once()
+    try:
+      self._emit_event(BAD_EVENT)
+      self.circ_listener.assert_not_called()
+      self.bw_listener.assert_not_called()
+      self.malformed_listener.assert_called_once()
 
-    self._emit_event(BW_EVENT)
-    self.bw_listener.assert_called_once_with(BW_EVENT)
+      self._emit_event(BW_EVENT)
+      self.bw_listener.assert_called_once_with(BW_EVENT)
+    finally:
+      is_alive_mock.return_value = False
+      self.controller._close()
 
   @patch('stem.control.Controller.get_version', Mock(return_value = stem.version.Version('0.5.0.14')))
   @patch('stem.control.Controller.msg', Mock(return_value = ControlMessage.from_str('250 OK\r\n')))