From 6414fd8855787aaf2e29e49785f4afa08ac6600b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20B=C5=82a=C5=BCewicz?= Date: Wed, 26 Mar 2025 15:20:47 +0100 Subject: [PATCH] Fix max_keepalive_connections This commit fixes a bug in both sync and async connection pools where idle connections were dropped from the pool even when max_keepalive_connections limit has not been reached. This happened because the check compared this number to the total number of connections, not only the idle ones. --- CHANGELOG.md | 4 ++++ httpcore/_async/connection_pool.py | 2 +- httpcore/_sync/connection_pool.py | 2 +- tests/_async/test_connection_pool.py | 21 +++++++++++++-------- tests/_sync/test_connection_pool.py | 21 +++++++++++++-------- 5 files changed, 32 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0b46c6c02..a348ab84d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). +## Unreleased + +- Fix `max_keepalive_connections` not being properly handled. (#1000) + ## Version 1.0.7 (November 15th, 2024) - Support `proxy=…` configuration on `ConnectionPool()`. (#974) diff --git a/httpcore/_async/connection_pool.py b/httpcore/_async/connection_pool.py index 96e973d0c..5ef74e649 100644 --- a/httpcore/_async/connection_pool.py +++ b/httpcore/_async/connection_pool.py @@ -291,7 +291,7 @@ def _assign_requests_to_connections(self) -> list[AsyncConnectionInterface]: closing_connections.append(connection) elif ( connection.is_idle() - and len([connection.is_idle() for connection in self._connections]) + and sum(connection.is_idle() for connection in self._connections) > self._max_keepalive_connections ): # log: "closing idle connection" diff --git a/httpcore/_sync/connection_pool.py b/httpcore/_sync/connection_pool.py index 9ccfa53e5..4b26f9c63 100644 --- a/httpcore/_sync/connection_pool.py +++ b/httpcore/_sync/connection_pool.py @@ -291,7 +291,7 @@ def _assign_requests_to_connections(self) -> list[ConnectionInterface]: closing_connections.append(connection) elif ( connection.is_idle() - and len([connection.is_idle() for connection in self._connections]) + and sum(connection.is_idle() for connection in self._connections) > self._max_keepalive_connections ): # log: "closing idle connection" diff --git a/tests/_async/test_connection_pool.py b/tests/_async/test_connection_pool.py index 2fc272049..bc4b251e3 100644 --- a/tests/_async/test_connection_pool.py +++ b/tests/_async/test_connection_pool.py @@ -30,7 +30,7 @@ async def test_connection_pool_with_keepalive(): ) async with httpcore.AsyncConnectionPool( - network_backend=network_backend, + network_backend=network_backend, max_keepalive_connections=1 ) as pool: # Sending an intial request, which once complete will return to the pool, IDLE. async with pool.stream("GET", "https://example.com/") as response: @@ -79,28 +79,33 @@ async def test_connection_pool_with_keepalive(): ) # Sending a request to a different origin will not reuse the existing IDLE connection. - async with pool.stream("GET", "http://example.com/") as response: + async with pool.stream("GET", "http://example.com/") as response_1, pool.stream( + "GET", "http://example.com/" + ) as response_2: info = [repr(c) for c in pool.connections] assert info == [ "", "", + "", ] assert ( repr(pool) - == "" + == "" ) - await response.aread() + await response_1.aread() + await response_2.aread() - assert response.status == 200 - assert response.content == b"Hello, world!" + assert response_1.status == 200 + assert response_1.content == b"Hello, world!" + assert response_2.status == 200 + assert response_2.content == b"Hello, world!" info = [repr(c) for c in pool.connections] assert info == [ - "", "", ] assert ( repr(pool) - == "" + == "" ) diff --git a/tests/_sync/test_connection_pool.py b/tests/_sync/test_connection_pool.py index ee303e5cf..7adc3f5c8 100644 --- a/tests/_sync/test_connection_pool.py +++ b/tests/_sync/test_connection_pool.py @@ -30,7 +30,7 @@ def test_connection_pool_with_keepalive(): ) with httpcore.ConnectionPool( - network_backend=network_backend, + network_backend=network_backend, max_keepalive_connections=1 ) as pool: # Sending an intial request, which once complete will return to the pool, IDLE. with pool.stream("GET", "https://example.com/") as response: @@ -79,28 +79,33 @@ def test_connection_pool_with_keepalive(): ) # Sending a request to a different origin will not reuse the existing IDLE connection. - with pool.stream("GET", "http://example.com/") as response: + with pool.stream("GET", "http://example.com/") as response_1, pool.stream( + "GET", "http://example.com/" + ) as response_2: info = [repr(c) for c in pool.connections] assert info == [ "", "", + "", ] assert ( repr(pool) - == "" + == "" ) - response.read() + response_1.read() + response_2.read() - assert response.status == 200 - assert response.content == b"Hello, world!" + assert response_1.status == 200 + assert response_1.content == b"Hello, world!" + assert response_2.status == 200 + assert response_2.content == b"Hello, world!" info = [repr(c) for c in pool.connections] assert info == [ - "", "", ] assert ( repr(pool) - == "" + == "" )