Rev 6180: Suppress ConnectionTimeout as a server-side exception. in http://bazaar.launchpad.net/~jameinel/bzr/2.5-no-hanging-teardown

[John Arbash Meinel]

At http://bazaar.launchpad.net/~jameinel/bzr/2.5-no-hanging-teardown

------------------------------------------------------------
revno: 6180
revision-id: john at arbash-meinel.com-20111003081539-ondt7hnvoec1jg8o
parent: john at arbash-meinel.com-20111003071117-bbysp7zj0l5ow9mi
committer: John Arbash Meinel <john at arbash-meinel.com>
branch nick: 2.5-no-hanging-teardown
timestamp: Mon 2011-10-03 10:15:39 +0200
message:
  Suppress ConnectionTimeout as a server-side exception.
  
  Make sure that if we get a 'shutdown' request while we are waiting on an idle
  connection, this won't generate an exception as part of handling the request.
-------------- next part --------------
=== modified file 'bzrlib/smart/medium.py'
--- a/bzrlib/smart/medium.py	2011-09-26 07:56:05 +0000
+++ b/bzrlib/smart/medium.py	2011-10-03 08:15:39 +0000
@@ -230,11 +230,6 @@
         try:
             while not self.finished:
                 server_protocol = self._build_protocol()
-                # TODO: This seems inelegant:
-                if server_protocol is None:
-                    # We could 'continue' only to notice that self.finished is
-                    # True...
-                    break
                 self._serve_one_request(server_protocol)
         except errors.ConnectionTimeout, e:
             trace.note('%s' % (e,))
@@ -326,6 +321,8 @@
 
         :param protocol: a SmartServerRequestProtocol.
         """
+        if protocol is None:
+            return
         try:
             self._serve_one_request_unguarded(protocol)
         except KeyboardInterrupt:

=== modified file 'bzrlib/tests/test_server.py'
--- a/bzrlib/tests/test_server.py	2011-10-03 06:56:19 +0000
+++ b/bzrlib/tests/test_server.py	2011-10-03 08:15:39 +0000
@@ -24,6 +24,7 @@
 
 from bzrlib import (
     cethread,
+    errors,
     osutils,
     transport,
     urlutils,
@@ -605,9 +606,13 @@
                                                  server)
 
     def handle(self):
-        while not self.finished:
-            server_protocol = self._build_protocol()
-            self._serve_one_request(server_protocol)
+        try:
+            while not self.finished:
+                server_protocol = self._build_protocol()
+                self._serve_one_request(server_protocol)
+        except errors.ConnectionTimeout:
+            # idle connections aren't considered a failure of the server
+            return
 
 
 _DEFAULT_TESTING_CLIENT_TIMEOUT = 4.0

=== modified file 'bzrlib/tests/test_test_server.py'
--- a/bzrlib/tests/test_test_server.py	2011-10-03 07:11:17 +0000
+++ b/bzrlib/tests/test_test_server.py	2011-10-03 08:15:39 +0000
@@ -31,6 +31,21 @@
 load_tests = load_tests_apply_scenarios
 
 
+def portable_socket_pair():
+    """Return a pair of TCP sockets connected to each other.
+
+    Unlike socket.socketpair, this should work on Windows.
+    """
+    listen_sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
+    listen_sock.bind(('127.0.0.1', 0))
+    listen_sock.listen(1)
+    client_sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
+    client_sock.connect(listen_sock.getsockname())
+    server_sock, addr = listen_sock.accept()
+    listen_sock.close()
+    return server_sock, client_sock
+
+
 class TCPClient(object):
 
     def __init__(self):
@@ -255,3 +270,34 @@
         h = server._make_handler(sock)
         self.assertEqual(test_server._DEFAULT_TESTING_CLIENT_TIMEOUT,
                          h._client_timeout)
+
+
+class FakeServer(object):
+    """Minimal implementation to pass to TestingSmartConnectionHandler"""
+    backing_transport = None
+    root_client_path = '/'
+
+
+class TestTestingSmartConnectionHandler(tests.TestCase):
+
+    def test_connection_timeout_suppressed(self):
+        self.overrideAttr(test_server, '_DEFAULT_TESTING_CLIENT_TIMEOUT', 0.01)
+        s = FakeServer()
+        server_sock, client_sock = portable_socket_pair()
+        # This should timeout quickly, but not generate an exception.
+        handler = test_server.TestingSmartConnectionHandler(server_sock,
+            server_sock.getpeername(), s)
+
+    def test_connection_shutdown_while_serving_no_error(self):
+        s = FakeServer()
+        server_sock, client_sock = portable_socket_pair()
+        class ShutdownConnectionHandler(
+            test_server.TestingSmartConnectionHandler):
+
+            def _build_protocol(self):
+                self.finished = True
+                return super(ShutdownConnectionHandler, self)._build_protocol()
+        # This should trigger shutdown after the entering _build_protocol, and
+        # we should exit cleanly, without raising an exception.
+        handler = ShutdownConnectionHandler(server_sock,
+            server_sock.getpeername(), s)