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