Rev 6058: Ensures that the connection thread is detached from the server thread before handling the connection. 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: 6058
revision-id: v.ladeuil+lp at free.fr-20111109160552-qqa88x55f70o4yh8
parent: v.ladeuil+lp at free.fr-20111010102716-b8vgjpvzg3mx8w7a
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: 869366-test-server-race
timestamp: Wed 2011-11-09 17:05:52 +0100
message:
  Ensures that the connection thread is detached from the server thread before handling the connection.
-------------- next part --------------
=== modified file 'bzrlib/tests/test_server.py'
--- a/bzrlib/tests/test_server.py	2011-10-07 14:48:59 +0000
+++ b/bzrlib/tests/test_server.py	2011-11-09 16:05:52 +0000
@@ -421,12 +421,15 @@
     def get_request (self):
         """Get the request and client address from the socket."""
         sock, addr = TestingTCPServerMixin.get_request(self)
-        # The thread is not create yet, it will be updated in process_request
+        # The thread is not created yet, it will be updated in process_request
         self.clients.append((sock, addr, None))
         return sock, addr
 
-    def process_request_thread(self, started, stopped, request, client_address):
+    def process_request_thread(self, started, detached, stopped,
+                               request, client_address):
         started.set()
+        # We will be on our own once the server tells us we're detached
+        detached.wait()
         SocketServer.ThreadingTCPServer.process_request_thread(
             self, request, client_address)
         self.close_request(request)
@@ -435,12 +438,13 @@
     def process_request(self, request, client_address):
         """Start a new thread to process the request."""
         started = threading.Event()
+        detached = threading.Event()
         stopped = threading.Event()
         t = TestThread(
             sync_event=stopped,
             name='%s -> %s' % (client_address, self.server_address),
             target = self.process_request_thread,
-            args = (started, stopped, request, client_address))
+            args = (started, detached, stopped, request, client_address))
         # Update the client description
         self.clients.pop()
         self.clients.append((request, client_address, t))
@@ -449,12 +453,12 @@
         t.set_ignored_exceptions(self.ignored_exceptions)
         t.start()
         started.wait()
-        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()
+        if debug_threads():
+            sys.stderr.write('Client thread %s started\n' % (t.name,))
+        # Tell the thread, it's now on its own for exception handling.
+        detached.set()
 
     # 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-10 10:27:16 +0000
+++ b/bzrlib/tests/test_test_server.py	2011-11-09 16:05:52 +0000
@@ -203,20 +203,12 @@
         # 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 (or not).
-        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's one. 
-            server.server.stopped.wait()
-            # The exception is available now
-            self.assertRaises(FailToRespond, server.pending_exception)
+        # same thread than the server one but was racy for
+        # TestingThreadingTCPServer. Since we the connection thread detaches
+        # itself before handling the request, we are guaranteed that the
+        # exception won't leak into the server thread anymore.
+        self.assertRaises(FailToRespond,
+                          self.connection_thread.pending_exception)
 
     def test_exception_swallowed_while_serving(self):
         # We need to ensure the exception has been caught