Rev 6054: One race can hide another... the exception may pass from the connection thread to the server thread so both need to be checked, with care. in file:///home/vila/src/bzr/bugs/869366-test-server-race/

Vincent Ladeuil v.ladeuil+lp at free.fr
Fri Oct 7 14:49:00 UTC 2011


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

------------------------------------------------------------
revno: 6054
revision-id: v.ladeuil+lp at free.fr-20111007144859-oqoe6td0sc2gim43
parent: v.ladeuil+lp at free.fr-20111007134112-seo2r1u8y0pf97a7
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: 869366-test-server-race
timestamp: Fri 2011-10-07 16:48:59 +0200
message:
  One race can hide another... the exception may pass from the connection thread to the server thread so both need to be checked, with care.
-------------- 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:48:59 +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_server.py'
--- a/bzrlib/tests/test_server.py	2011-10-07 13:41:12 +0000
+++ b/bzrlib/tests/test_server.py	2011-10-07 14:48:59 +0000
@@ -452,6 +452,8 @@
         if debug_threads():
             sys.stderr.write('Client thread %s started\n' % (t.name,))
         # If an exception occured during the thread start, it will get raised.
+        # In rare cases, an exception raised during the request processing may
+        # also get caught here (see http://pad.lv/869366)
         t.pending_exception()
 
     # The following methods are called by the main thread

=== 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:48:59 +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,32 +197,48 @@
         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
         # same thread than the server one but is racy for
         # TestingThreadingTCPServer where the server thread may be in a
         # blocking accept() call.
-        self.assertRaises(FailToRespond,
-                          self.connection_thread.pending_exception)
+        try:
+            self.connection_thread.pending_exception()
+        except FailToRespond:
+            # Great, the test succeeded
+            pass
+        else:
+            # If the exception is not in the connection thread anymore, it's in
+            # the server. 
+            server.server.stopped.wait()
+            # The exception is available now
+            self.assertRaises(FailToRespond, server.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 +248,15 @@
         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()
+        server.pending_exception()



More information about the bazaar-commits mailing list