Rev 6054: Fix a naughty race in test_server_crash_while_responding in file:///home/vila/src/bzr/bugs/869366-test-server-race/

[Vincent Ladeuil]

At file:///home/vila/src/bzr/bugs/869366-test-server-race/

------------------------------------------------------------
revno: 6054
revision-id: v.ladeuil+lp at free.fr-20111007141534-jmwit7px0qoz1bbm
parent: v.ladeuil+lp at free.fr-20111007134112-seo2r1u8y0pf97a7
fixes bug: https://launchpad.net/bugs/869366
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: 869366-test-server-race
timestamp: Fri 2011-10-07 16:15:34 +0200
message:
  Fix a naughty race in test_server_crash_while_responding
-------------- next part --------------
=== modified file 'bzrlib/cethread.py'
--- a/bzrlib/cethread.py	2011-05-19 09:32:38 +0000
+++ b/bzrlib/cethread.py	2011-10-07 14:15:34 +0000
@@ -126,6 +126,8 @@
                 super(CatchingExceptionThread, self).run()
             except:
                 self.exception = sys.exc_info()
+                print '\nthread %r, sync: %r see %r' % (
+                    self, self.sync_event, self.exception[0],)
         finally:
             # Make sure the calling thread is released
             self.sync_event.set()

=== modified file 'bzrlib/tests/test_test_server.py'
--- a/bzrlib/tests/test_test_server.py	2011-10-07 13:41:12 +0000
+++ b/bzrlib/tests/test_test_server.py	2011-10-07 14:15:34 +0000
@@ -169,22 +169,27 @@
         self.assertRaises(CantConnect, server.stop_server)
 
     def test_server_crash_while_responding(self):
-        sync = threading.Event()
-        sync.clear()
+        # We need to ensure the exception will be raised
+        will_raise = threading.Event()
+        will_raise.clear()
+        # We need to ensure the exception has been caught
+        caught = threading.Event()
+        caught.clear()
+        print 'will_raise: %r, caught: %r\n' % (will_raise, caught)
         # The thread that will serve the client
         self.connection_thread = None
-        test = self # So the handler can set connection_thread
         class FailToRespond(Exception):
             pass
 
         class FailingDuringResponseHandler(TCPConnectionHandler):
 
-            def handle_connection(self):
-                req = self.rfile.readline()
-                # Capture the thread and make it use 'sync' so we can wait on
+            def handle_connection(request):
+                req = request.rfile.readline()
+                # Capture the thread and make it use 'caught' so we can wait on
                 # the even set when the exception is caught
-                test.connection_thread = threading.currentThread()
-                test.connection_thread.set_sync_event(sync)
+                self.connection_thread = threading.currentThread()
+                self.connection_thread.set_sync_event(caught)
+                will_raise.set()
                 raise FailToRespond()
 
         server = self.get_server(
@@ -192,7 +197,10 @@
         client = self.get_client()
         client.connect((server.host, server.port))
         client.write('ping\n')
-        sync.wait()
+        # Wait for the connection to succeed and be processed
+        will_raise.wait()
+        # Wait for the exception to be caught
+        caught.wait()
         # Check that the connection thread did catch the exception,
         # http://pad.lv/869366 was wrongly checking the server thread which
         # works for TestingTCPServer where the connection is handled in the
@@ -203,21 +211,25 @@
                           self.connection_thread.pending_exception)
 
     def test_exception_swallowed_while_serving(self):
-        sync = threading.Event()
-        sync.clear()
+        # We need to ensure the exception will be raised
+        will_raise = threading.Event()
+        will_raise.clear()
+        # We need to ensure the exception has been caught
+        caught = threading.Event()
+        caught.clear()
         # The thread that will serve the client
         self.connection_thread = None
-        test = self # So the handler can set connection_thread
         class CantServe(Exception):
             pass
 
         class FailingWhileServingConnectionHandler(TCPConnectionHandler):
 
             def handle(request):
-                # Capture the thread and make it use 'sync' so we can wait on
+                # Capture the thread and make it use 'caught' so we can wait on
                 # the even set when the exception is caught
-                test.connection_thread = threading.currentThread()
-                test.connection_thread.set_sync_event(sync)
+                self.connection_thread = threading.currentThread()
+                self.connection_thread.set_sync_event(caught)
+                will_raise.set()
                 raise CantServe()
 
         server = self.get_server(
@@ -227,10 +239,14 @@
         client = self.get_client()
         # Connect to the server so the exception is raised there
         client.connect((server.host, server.port))
-        # Wait for the exception to propagate.
-        sync.wait()
+        # Wait for the connection to succeed and be processed
+        will_raise.wait()
+        # Wait for the exception to be caught
+        caught.wait()
         # The connection wasn't served properly but the exception should have
         # been swallowed (see test_server_crash_while_responding remark about
         # http://pad.lv/869366 explaining why we can't check the server thread
-        # here).
+        # here). More precisely, the exception *has* been caught and captured
+        # but it is cleared when joining the thread (or trying to acquire the
+        # exception) and as such won't propagate to the server thread.
         self.connection_thread.pending_exception()