Be more graceful when terminating keep-alive connections

CHANGES.rst

@@ -1,3 +1,8 @@
+.. scm-version-title:: v8.3.1
+
+- Made terminating keep-alive connections more graceful
+  (:issue:`263` via :pr:`277`).
+
 .. scm-version-title:: v8.3.0
 
 - :cp-issue:`910` via :pr:`243`: Provide TLS-related

cheroot/connections.py

@@ -91,12 +91,6 @@ def expire(self):
         if conn.closeable:
             return
 
-        # Too many connections?
-        ka_limit = self.server.keep_alive_conn_limit
-        if ka_limit is not None and len(self.connections) > ka_limit:
-            conn.closeable = True
-            return
-
         # Connection too old?
         if (conn.last_used + self.server.timeout) < time.time():
             conn.closeable = True
@@ -277,3 +271,9 @@ def close(self):
         for conn in self.connections[:]:
             conn.close()
         self.connections = []
+
+    @property
+    def can_add_keepalive_connection(self):
+        """Flag whether it is allowed to add a new keep-alive connection."""
+        ka_limit = self.server.keep_alive_conn_limit
+        return ka_limit is None or len(self.connections) < ka_limit

cheroot/server.py

@@ -1159,6 +1159,12 @@ def send_headers(self):
                     # Closing the conn is the only way to determine len.
                     self.close_connection = True
 
+        # Override the decision to not close the connection if the connection
+        # manager doesn't have space for it.
+        if not self.close_connection:
+            can_keep = self.server.connections.can_add_keepalive_connection
+            self.close_connection = not can_keep
+
         if b'connection' not in hkeys:
             if self.response_protocol == 'HTTP/1.1':
                 # Both server and client are HTTP/1.1 or better

cheroot/test/test_conn.py

@@ -409,7 +409,7 @@ def connection():
         http_connection.connect()
         return http_connection
 
-    def request(conn):
+    def request(conn, keepalive=True):
         status_line, actual_headers, actual_resp_body = test_client.get(
             '/page3', headers=[('Connection', 'Keep-Alive')],
             http_conn=conn, protocol='HTTP/1.0',
@@ -418,7 +418,10 @@ def request(conn):
         assert actual_status == 200
         assert status_line[4:] == 'OK'
         assert actual_resp_body == pov.encode()
-        assert header_has_value('Connection', 'Keep-Alive', actual_headers)
+        if keepalive:
+            assert header_has_value('Connection', 'Keep-Alive', actual_headers)
+        else:
+            assert not header_exists('Connection', actual_headers)
 
     disconnect_errors = (
         http_client.BadStatusLine,
@@ -437,28 +440,21 @@ def request(conn):
     # Reusing the first connection should still work.
     request(c1)
 
-    # Creating a new connection should still work.
+    # Creating a new connection should still work, but we should
+    # have run out of available connections to keep alive, so the
+    # server should tell us to close.
     c3 = connection()
-    request(c3)
+    request(c3, keepalive=False)
 
-    # Allow a tick.
-    time.sleep(0.2)
-
-    # That's three connections, we should expect the one used less recently
-    # to be expired.
+    # Show that the third connection was closed.
     with pytest.raises(disconnect_errors):
-        request(c2)
-
-    # But the oldest created one should still be valid.
-    # (As well as the newest one).
-    request(c1)
-    request(c3)
+        request(c3)
 
     # Wait for some of our timeout.
-    time.sleep(1.0)
+    time.sleep(1.2)
 
-    # Refresh the third connection.
-    request(c3)
+    # Refresh the second connection.
+    request(c2)
 
     # Wait for the remainder of our timeout, plus one tick.
     time.sleep(1.2)
@@ -467,9 +463,10 @@ def request(conn):
     with pytest.raises(disconnect_errors):
         request(c1)
 
-    # But the third one should still be valid.
-    request(c3)
+    # But the second one should still be valid.
+    request(c2)
 
+    # Restore original timeout.
     test_client.server_instance.timeout = timeout
 
 

cheroot/test/test_wsgi.py

@@ -9,6 +9,10 @@
 from jaraco.context import ExceptionTrap
 
 from cheroot import wsgi
+from cheroot._compat import IS_MACOS, IS_WINDOWS
+
+
+IS_SLOW_ENV = IS_MACOS or IS_WINDOWS
 
 
 @pytest.fixture
@@ -24,13 +28,12 @@ def app(environ, start_response):
 
     host = '::'
     addr = host, port
-    server = wsgi.Server(addr, app)
+    server = wsgi.Server(addr, app, timeout=600 if IS_SLOW_ENV else 20)
     url = 'http://localhost:{port}/'.format(**locals())
     with server._run_in_thread() as thread:
         yield locals()
 
 
-@pytest.mark.xfail(reason='#263')
 def test_connection_keepalive(simple_wsgi_server):
     """Test the connection keepalive works (duh)."""
     session = Session(base_url=simple_wsgi_server['url'])
@@ -45,10 +48,10 @@ def do_request():
             resp.raise_for_status()
         return bool(trap)
 
-    with ThreadPoolExecutor(max_workers=50) as pool:
+    with ThreadPoolExecutor(max_workers=10 if IS_SLOW_ENV else 50) as pool:
         tasks = [
             pool.submit(do_request)
-            for n in range(1000)
+            for n in range(250 if IS_SLOW_ENV else 1000)
         ]
         failures = sum(task.result() for task in tasks)