Merge lp:~vila/bzr/405745-http-hangs into lp:bzr

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):...

+ def connect_socket(self):
+ msg = "getaddrinfo returns an empty list"
+ for res in socket.getaddrinfo(*self.server_address):
[...]
+ except socket.error, msg:
+ if sock is not None:
+ sock.close()
+ raise socket.error, msg
+

"except socket.error, msg" is bogus, especially when "msg" was a string earlier. It's also a bit arbitrary to that if no addresses work that this will raise an error from the last address, rather than the first, or some sort of aggregation.

I'd probably write this as:
    err = socket.error("getaddrinfo returned an empty list")
    for ...
        except socket.error, err:
            # 'err' is now the most recent error
            if sock is not None:
                sock.close()
    raise err

Otherwise this seems ok. bb:tweak.