commit be32e6a5017b220643ee4df4aace3d81a6acdd73 Author: Damian Johnson atagar@torproject.org Date: Sun Feb 19 16:33:42 2012 -0800

Fixing another close() deadlock issue

The previous fix narrowed the window where close() / recv() calls could trigger deadlock, but it didn't eliminate it. I'm adding another test that reliably triggered deadlock in that case and narrowing the window even more (which fixed the issue).

I'm a little worried that this doesn't completely eliminate the issue since there is a theoretical race if recv() calls close after someone else calls close() but before they set the boolean flag. That said, I'm not sure if this is really an issue in practice. --- stem/socket.py | 3 +- test/integ/control/base_controller.py | 34 +++++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletions(-)

diff --git a/stem/socket.py b/stem/socket.py index afea436..7f15df6 100644 --- a/stem/socket.py +++ b/stem/socket.py @@ -202,12 +202,13 @@ class ControlSocket: Shuts down the socket. If it's already closed then this is a no-op. """

+ self._handling_close = True + with self._send_lock: # Function is idempotent with one exception: we notify _close() if this # is causing our is_alive() state to change.

is_change = self.is_alive() - self._handling_close = True

if self._socket: # if we haven't yet established a connection then this raises an error diff --git a/test/integ/control/base_controller.py b/test/integ/control/base_controller.py index 4c6260d..da4cc08 100644 --- a/test/integ/control/base_controller.py +++ b/test/integ/control/base_controller.py @@ -4,6 +4,7 @@ Integration tests for the stem.control.BaseController class.

import time import unittest +import threading

import stem.control import stem.socket @@ -101,6 +102,39 @@ class TestBaseController(unittest.TestCase): response = controller.msg("GETINFO blarg") self.assertEquals('Unrecognized key "blarg"', str(response))

+ def test_msg_repeatedly(self): + """ + Connects, sends a burst of messages, and closes the socket repeatedly. This + is a simple attempt to trigger concurrency issues. + """ + + with test.runner.get_runner().get_tor_socket() as control_socket: + controller = stem.control.BaseController(control_socket) + + def run_getinfo(): + for i in xrange(150): + try: + controller.msg("GETINFO version") + controller.msg("GETINFO blarg") + controller.msg("blarg") + except stem.socket.ControllerError: + pass + + message_threads = [] + + for i in xrange(5): + msg_thread = threading.Thread(target = run_getinfo) + message_threads.append(msg_thread) + msg_thread.setDaemon(True) + msg_thread.start() + + for i in xrange(100): + controller.connect() + controller.close() + + for msg_thread in message_threads: + msg_thread.join() + def test_status_notifications(self): """ Checks basic functionality of the add_status_listener() and