Rev 4744: We're back at full test suite passing under all circumstances. in file:///home/vila/src/bzr/bugs/392127-thread-leak/

Vincent Ladeuil v.ladeuil+lp at free.fr
Mon Oct 12 12:55:56 BST 2009


At file:///home/vila/src/bzr/bugs/392127-thread-leak/

------------------------------------------------------------
revno: 4744
revision-id: v.ladeuil+lp at free.fr-20091012115555-swx60t4o8jg8kekt
parent: v.ladeuil+lp at free.fr-20091009231522-09cs7tqrzi2splv7
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: 392127-thread-leak
timestamp: Mon 2009-10-12 13:55:55 +0200
message:
  We're back at full test suite passing under all circumstances.
  
  * bzrlib/tests/test_http.py:
  (TestHTTPServer.test_server_start_and_stop): Adapt test to new
  implementation.
  
  * bzrlib/tests/http_server.py:
  (TestingHTTPServerMixin.serve): We need to tell caller when we are
  started exactly.
  (TestingHTTPServerMixin.shutdown): Closing the last connection
  socket should be delayed, since we can initiate other actions in
  the mean time.
  (HttpServer._http_start): Remove some try/finally and share the
  'started' definition with serve().
-------------- next part --------------
=== modified file 'bzrlib/tests/http_server.py'
--- a/bzrlib/tests/http_server.py	2009-10-09 23:15:22 +0000
+++ b/bzrlib/tests/http_server.py	2009-10-12 11:55:55 +0000
@@ -369,7 +369,7 @@
         if sys.version < (2, 5):
             self.server_address = self.socket.getsockname()
 
-    def serve(self):
+    def serve(self, started):
         self.serving  = threading.Event()
         self.serving.set()
         self.is_shut_down.clear()
@@ -377,8 +377,12 @@
         self.socket.settimeout(1)
         if 'threads' in tests.selftest_debug_flags:
             print 'Starting %r' % (self.server_address,)
+        # We are listening and ready to accept connections
+        started.set()
         while self.serving.isSet():
             try:
+                if 'threads' in tests.selftest_debug_flags:
+                    print 'Accepting on %r' % (self.server_address,)
                 # Really a connection but the python framework is generic and
                 # call them requests
                 self.handle_request()
@@ -430,12 +434,9 @@
         # one to get out of the blocking listen.
         self.serving.clear()
         # The server is listening for a last connection, let's give it:
+        last_conn = None
         try:
-            fake_conn = self.connect_socket()
-            # But close it immediately without trying to use it. The server
-            # will not process a single byte on that socket to avoid
-            # complications (SSL starts with a handshake for example).
-            fake_conn.close()
+            last__conn = self.connect_socket()
         except socket.error, e:
             # But ignore connection errors as the point is to unblock the
             # server thread, it may happen that it's not blocked or even not
@@ -450,6 +451,11 @@
         self.clients = []
         # Now we wait for the thread running serve() to finish
         self.is_shut_down.wait()
+        if last_conn is not None:
+            # Close the last connection without trying to use it. The server
+            # will not process a single byte on that socket to avoid
+            # complications (SSL starts with a handshake for example).
+            last_conn.close()
 
     def shutdown_client(self, client):
         sock, addr = client[:2]
@@ -517,6 +523,8 @@
         return SocketServer.ThreadingTCPServer.get_request(self)
 
     def process_request_thread(self, started, request, client_address):
+        if 'threads' in tests.selftest_debug_flags:
+            print 'Processing: %s' % (threading.currentThread().name,)
         started.set()
         SocketServer.ThreadingTCPServer.process_request_thread(
             self, request, client_address)
@@ -546,6 +554,8 @@
             # is shut down.
             sock, addr, thread = client
             if thread.isAlive():
+                if 'threads' in tests.selftest_debug_flags:
+                    print 'Try    joining: %s' % (thread.name,)
                 self.join_thread(thread)
 
     def server_bind(self):
@@ -623,25 +633,23 @@
         """Server thread main entry point. """
         server = None
         try:
-            try:
-                server = self._get_httpd()
-                self._http_base_url = '%s://%s:%s/' % (self._url_protocol,
-                                                       self.host, self.port)
-            except:
-                # Whatever goes wrong, we save the exception for the main
-                # thread. Note that since we are running in a thread, no signal
-                # can be received, so we don't care about KeyboardInterrupt.
-                self._http_exception = sys.exc_info()
-        finally:
-            # Release the lock or the main thread will block and the whole
-            # process will hang.
+            server = self._get_httpd()
+            self._http_base_url = '%s://%s:%s/' % (self._url_protocol,
+                                                   self.host, self.port)
+        except:
+            # Whatever goes wrong, we save the exception for the main
+            # thread. Note that since we are running in a thread, no signal
+            # can be received, so we don't care about KeyboardInterrupt.
+            self._http_exception = sys.exc_info()
+
+        if server is not None:
+            # From now on, exceptions are taken care of by the
+            # SocketServer.BaseServer or the request handler.
+            server.serve(started)
+        if not started.isSet():
+            # Hmm, something went wrong, but we can release the caller anyway
             started.set()
 
-        # From now on, exceptions are taken care of by the
-        # SocketServer.BaseServer or the request handler.
-        if server is not None:
-            server.serve()
-
     def _get_remote_url(self, path):
         path_parts = path.split(os.path.sep)
         if os.path.isabs(path):
@@ -699,6 +707,8 @@
     def tearDown(self):
         """See bzrlib.transport.Server.tearDown."""
         self._httpd.shutdown()
+        if 'threads' in tests.selftest_debug_flags:
+            print 'Try    joining: %s' % (self._http_thread.name,)
         self._httpd.join_thread(self._http_thread)
         if 'threads' in tests.selftest_debug_flags:
             print 'Thread  joined: %s' % (self._http_thread.name,)

=== modified file 'bzrlib/tests/test_http.py'
--- a/bzrlib/tests/test_http.py	2009-10-08 17:19:38 +0000
+++ b/bzrlib/tests/test_http.py	2009-10-12 11:55:55 +0000
@@ -354,10 +354,10 @@
         server.setUp()
         try:
             self.assertTrue(server._httpd is not None)
-            self.assertTrue(server._httpd.serving)
+            self.assertTrue(server._httpd.serving is not None)
+            self.assertTrue(server._httpd.serving.isSet())
         finally:
             server.tearDown()
-        self.assertFalse(server._httpd.serving)
 
     def test_create_http_server_one_zero(self):
         class RequestHandlerOneZero(http_server.TestingHTTPRequestHandler):



More information about the bazaar-commits mailing list