Rev 6337: (vila) Properly synchronize connection thread start with test server main in file:///srv/pqm.bazaar-vcs.org/archives/thelove/bzr/%2Btrunk/

Patch Queue Manager pqm at pqm.ubuntu.com
Fri Dec 2 12:02:55 UTC 2011


At file:///srv/pqm.bazaar-vcs.org/archives/thelove/bzr/%2Btrunk/

------------------------------------------------------------
revno: 6337 [merge]
revision-id: pqm at pqm.ubuntu.com-20111202120254-kccrfj6buuqt1iui
parent: pqm at pqm.ubuntu.com-20111201125000-n57mg7g9plq10hfu
parent: v.ladeuil+lp at free.fr-20111124154829-avowjpsxdl8yp2vz
committer: Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Fri 2011-12-02 12:02:54 +0000
message:
  (vila) Properly synchronize connection thread start with test server main
   thread. (Vincent Ladeuil)
modified:
  bzrlib/tests/test_server.py    test_server.py-20100209163834-im1ozfuenfmqaa2m-1
  bzrlib/tests/test_test_server.py test_test_server.py-20100601152414-r8rln0ok7514pcoz-1
=== modified file 'bzrlib/tests/test_server.py'
--- a/bzrlib/tests/test_server.py	2011-10-10 13:59:22 +0000
+++ b/bzrlib/tests/test_server.py	2011-11-24 15:48:29 +0000
@@ -432,12 +432,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)
@@ -446,12 +449,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))
+            target = self.process_request_thread,
+            args = (started, detached, stopped, request, client_address))
         # Update the client description
         self.clients.pop()
         self.clients.append((request, client_address, t))
@@ -460,12 +464,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 13:59:22 +0000
+++ b/bzrlib/tests/test_test_server.py	2011-11-24 15:48:29 +0000
@@ -212,10 +212,12 @@
 
         class FailingDuringResponseHandler(TCPConnectionHandler):
 
+            # We use 'request' instead of 'self' below because the test matters
+            # more and we need a container to properly set connection_thread.
             def handle_connection(request):
                 req = request.readline()
                 # Capture the thread and make it use 'caught' so we can wait on
-                # the even that will be set when the exception is caught. We
+                # the event that will be set when the exception is caught. We
                 # also capture the thread to know where to look.
                 self.connection_thread = threading.currentThread()
                 self.connection_thread.set_sync_event(caught)
@@ -232,20 +234,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 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
@@ -260,9 +254,11 @@
 
         class FailingWhileServingConnectionHandler(TCPConnectionHandler):
 
+            # We use 'request' instead of 'self' below because the test matters
+            # more and we need a container to properly set connection_thread.
             def handle(request):
                 # Capture the thread and make it use 'caught' so we can wait on
-                # the even that will be set when the exception is caught. We
+                # the event that will be set when the exception is caught. We
                 # also capture the thread to know where to look.
                 self.connection_thread = threading.currentThread()
                 self.connection_thread.set_sync_event(caught)
@@ -270,6 +266,7 @@
 
         server = self.get_server(
             connection_handler_class=FailingWhileServingConnectionHandler)
+        self.assertEquals(True, server.server.serving)
         # Install the exception swallower
         server.set_ignored_exceptions(CantServe)
         client = self.get_client()
@@ -284,8 +281,8 @@
         # 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()
+        self.assertIs(None, self.connection_thread.pending_exception())
+        self.assertIs(None, server.pending_exception())
 
     def test_handle_request_closes_if_it_doesnt_process(self):
         server = self.get_server()




More information about the bazaar-commits mailing list