[tor-commits] [stem/master] Fixing another close() deadlock issue

[atagar at torproject.org]

commit be32e6a5017b220643ee4df4aace3d81a6acdd73
Author: Damian Johnson <atagar at 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