Rev 4733: Cleanup and refactor the server shutdown. in file:///home/vila/src/bzr/bugs/392127-thread-leak/

Vincent Ladeuil v.ladeuil+lp at free.fr
Wed Oct 7 14:31:04 BST 2009


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

------------------------------------------------------------
revno: 4733
revision-id: v.ladeuil+lp at free.fr-20091007133103-bagf11imwtcenmzy
parent: v.ladeuil+lp at free.fr-20091007102404-tqyhafwp5ur19kz0
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: 392127-thread-leak
timestamp: Wed 2009-10-07 15:31:03 +0200
message:
  Cleanup and refactor the server shutdown.
  
  * bzrlib/tests/http_server.py:
  (TestingHTTPServerMixin): Implement a proper server() method able
  to clean after itself and interruptible by calling shutdown() from
  another thread.
  (HttpServer._get_httpd): The server has already got the address
  from the socket, just get it from there.
  (HttpServer._http_start): Delegate the service to the server
  itself.
  (HttpServer.tearDown): Call server.shutdown() since that's more
  appropriate.
-------------- next part --------------
=== modified file 'bzrlib/tests/http_server.py'
--- a/bzrlib/tests/http_server.py	2009-10-07 10:24:04 +0000
+++ b/bzrlib/tests/http_server.py	2009-10-07 13:31:03 +0000
@@ -317,27 +317,55 @@
         # the tests cases.
         self.test_case_server = test_case_server
         self._home_dir = test_case_server._home_dir
-
-    def tearDown(self):
-         """Called to clean-up the server.
-
-         Since the server may be (surely is, even) in a blocking listen, we
-         shutdown its socket before closing it.
-         """
-         # The server is listening for a last connection, let's give it:
-         try:
-             fake_conn = socket.create_connection(self.server_address)
-             fake_conn.close()
-         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
-             # started (when something went wrong during test case setup
-             # leading to self.setUp() *not* being called but self.tearDown()
-             # still being called)
-             pass
-
-
-class TestingHTTPServer(SocketServer.TCPServer, TestingHTTPServerMixin):
+        self.serving = False
+        self.is_shut_down = threading.Event()
+
+    def serve(self):
+        self.serving = True
+        self.is_shut_down.clear()
+        while self.serving:
+            try:
+                # Really a connection but the python framework is generic and
+                # call them requests
+                self.handle_request()
+            except socket.timeout:
+                pass
+            except (socket.error, select.error), e:
+               if e[0] == errno.EBADF:
+                   # Starting with python-2.6, handle_request may raise socket
+                   # or select exceptions when the server is shut down as we
+                   # do.
+                   pass
+               else:
+                   raise
+        # Let's close the listening socket
+        self.server_close()
+        self.is_shut_down.set()
+
+    def shutdown(self):
+        """Stops the serve() loop.
+
+        Blocks until the loop has finished. This must be called while serve()
+        is running in another thread, or it will deadlock.
+        """
+        if not self.serving:
+            return
+        self.serving = False
+        # The server is listening for a last connection, let's give it:
+        try:
+            fake_conn = socket.create_connection(self.server_address)
+            fake_conn.close()
+        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
+            # started (when something went wrong during test case setup
+            # leading to self.setUp() *not* being called but self.tearDown()
+            # still being called)
+            pass
+        self.is_shut_down.wait()
+
+
+class TestingHTTPServer(TestingHTTPServerMixin, SocketServer.TCPServer):
 
     def __init__(self, server_address, request_handler_class,
                  test_case_server):
@@ -346,8 +374,9 @@
                                         request_handler_class)
 
 
-class TestingThreadingHTTPServer(SocketServer.ThreadingTCPServer,
-                                 TestingHTTPServerMixin):
+class TestingThreadingHTTPServer(TestingHTTPServerMixin,
+                                 SocketServer.ThreadingTCPServer,
+                                 ):
     """A threading HTTP test server for HTTP 1.1.
 
     Since tests can initiate several concurrent connections to the same http
@@ -445,18 +474,18 @@
                 raise httplib.UnknownProtocol(proto_vers)
             else:
                 self._httpd = self.create_httpd(serv_cls, rhandler)
-            host, self.port = self._httpd.socket.getsockname()
+            # Ensure we get the right port
+            host, self.port = self._httpd.server_address
         return self._httpd
 
     def _http_start(self):
         """Server thread main entry point. """
-        self._http_running = False
+        server = None
         try:
             try:
-                httpd = self._get_httpd()
+                server = self._get_httpd()
                 self._http_base_url = '%s://%s:%s/' % (self._url_protocol,
                                                        self.host, self.port)
-                self._http_running = True
             except:
                 # Whatever goes wrong, we save the exception for the main
                 # thread. Note that since we are running in a thread, no signal
@@ -469,24 +498,8 @@
 
         # From now on, exceptions are taken care of by the
         # SocketServer.BaseServer or the request handler.
-        while self._http_running:
-            try:
-                # Really an HTTP connection but the python framework is generic
-                # and call them requests
-                httpd.handle_request()
-            except socket.timeout:
-                pass
-            except (socket.error, select.error), e:
-               if e[0] == errno.EBADF:
-                   # Starting with python-2.6, handle_request may raise socket
-                   # or select exceptions when the server is shut down (as we
-                   # do).
-                   pass
-               else:
-                   raise
-        if self._httpd is not None:
-            # Let the server properly close the listening socket
-            self._httpd.server_close()
+        if server is not None:
+            server.serve()
 
     def _get_remote_url(self, path):
         path_parts = path.split(os.path.sep)
@@ -543,7 +556,7 @@
     def tearDown(self):
         """See bzrlib.transport.Server.tearDown."""
         self._http_running = False
-        self._httpd.tearDown()
+        self._httpd.shutdown()
         self._http_thread.join()
 
     def get_url(self):

=== modified file 'bzrlib/tests/test_http.py'
--- a/bzrlib/tests/test_http.py	2009-09-17 11:54:41 +0000
+++ b/bzrlib/tests/test_http.py	2009-10-07 13:31:03 +0000
@@ -325,10 +325,11 @@
         server = http_server.HttpServer()
         server.setUp()
         try:
-            self.assertTrue(server._http_running)
+            self.assertTrue(server._httpd is not None)
+            self.assertTrue(server._httpd.serving)
         finally:
             server.tearDown()
-        self.assertFalse(server._http_running)
+        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