I had to merge some refactoring done for bug #392127 so that the 2.4 band-aids stay smaller. I changed the server.tearDown() into server.shutdown() as that better matches the SocketServer.py methods names. I'm not particularly happy with having to override server_bind() twice, but that will be solved with #392127 where the inheritance scheme is a bit different. I may need to make connect_socket() a bit more reusable but I also plan to make more test_server bits reusable so that could wait too. Here is the differential diff (but it's ~200 lines while the full one is ~250): === modified file 'bzrlib/tests/http_server.py' --- bzrlib/tests/http_server.py 2009-10-07 10:24:04 +0000 +++ bzrlib/tests/http_server.py 2009-10-08 08:25:53 +0000 @@ -317,27 +317,70 @@ # 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 connect_socket(self): + msg = "getaddrinfo returns an empty list" + for res in socket.getaddrinfo(*self.server_address): + af, socktype, proto, canonname, sa = res + sock = None + try: + sock = socket.socket(af, socktype, proto) + sock.connect(sa) + return sock + + except socket.error, msg: + if sock is not None: + sock.close() + raise socket.error, msg + + 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 = self.connect_socket() + 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): @@ -345,9 +388,15 @@ SocketServer.TCPServer.__init__(self, server_address, request_handler_class) - -class TestingThreadingHTTPServer(SocketServer.ThreadingTCPServer, - TestingHTTPServerMixin): + def server_bind(self): + SocketServer.TCPServer.server_bind(self) + if sys.version < (2, 5): + self.server_address = self.socket.getsockname() + + +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 @@ -382,6 +431,11 @@ else: raise + def server_bind(self): + SocketServer.ThreadingTCPServer.server_bind(self) + if sys.version < (2, 5): + self.server_address = self.socket.getsockname() + class HttpServer(transport.Server): """A test server for http transports. @@ -445,18 +499,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 +523,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 +581,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' --- bzrlib/tests/test_http.py 2009-09-17 11:54:41 +0000 +++ bzrlib/tests/test_http.py 2009-10-08 07:25:52 +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):