Skip to content

Commit 0aa2ee1

Browse files
committed
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.
1 parent 973cbdd commit 0aa2ee1

File tree

4 files changed

+28
-18
lines changed

4 files changed

+28
-18
lines changed

httpcore/_async/connection_pool.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -291,7 +291,7 @@ def _assign_requests_to_connections(self) -> list[AsyncConnectionInterface]:
291291
closing_connections.append(connection)
292292
elif (
293293
connection.is_idle()
294-
and len([connection.is_idle() for connection in self._connections])
294+
and sum(connection.is_idle() for connection in self._connections)
295295
> self._max_keepalive_connections
296296
):
297297
# log: "closing idle connection"

httpcore/_sync/connection_pool.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -291,7 +291,7 @@ def _assign_requests_to_connections(self) -> list[ConnectionInterface]:
291291
closing_connections.append(connection)
292292
elif (
293293
connection.is_idle()
294-
and len([connection.is_idle() for connection in self._connections])
294+
and sum(connection.is_idle() for connection in self._connections)
295295
> self._max_keepalive_connections
296296
):
297297
# log: "closing idle connection"

tests/_async/test_connection_pool.py

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ async def test_connection_pool_with_keepalive():
3030
)
3131

3232
async with httpcore.AsyncConnectionPool(
33-
network_backend=network_backend,
33+
network_backend=network_backend, max_keepalive_connections=1
3434
) as pool:
3535
# Sending an intial request, which once complete will return to the pool, IDLE.
3636
async with pool.stream("GET", "https://example.com/") as response:
@@ -79,28 +79,33 @@ async def test_connection_pool_with_keepalive():
7979
)
8080

8181
# Sending a request to a different origin will not reuse the existing IDLE connection.
82-
async with pool.stream("GET", "http://example.com/") as response:
82+
async with pool.stream("GET", "http://example.com/") as response_1, pool.stream(
83+
"GET", "http://example.com/"
84+
) as response_2:
8385
info = [repr(c) for c in pool.connections]
8486
assert info == [
8587
"<AsyncHTTPConnection ['https://example.com:443', HTTP/1.1, IDLE, Request Count: 2]>",
8688
"<AsyncHTTPConnection ['http://example.com:80', HTTP/1.1, ACTIVE, Request Count: 1]>",
89+
"<AsyncHTTPConnection ['http://example.com:80', HTTP/1.1, ACTIVE, Request Count: 1]>",
8790
]
8891
assert (
8992
repr(pool)
90-
== "<AsyncConnectionPool [Requests: 1 active, 0 queued | Connections: 1 active, 1 idle]>"
93+
== "<AsyncConnectionPool [Requests: 2 active, 0 queued | Connections: 2 active, 1 idle]>"
9194
)
92-
await response.aread()
95+
await response_1.aread()
96+
await response_2.aread()
9397

94-
assert response.status == 200
95-
assert response.content == b"Hello, world!"
98+
assert response_1.status == 200
99+
assert response_1.content == b"Hello, world!"
100+
assert response_2.status == 200
101+
assert response_2.content == b"Hello, world!"
96102
info = [repr(c) for c in pool.connections]
97103
assert info == [
98-
"<AsyncHTTPConnection ['https://example.com:443', HTTP/1.1, IDLE, Request Count: 2]>",
99104
"<AsyncHTTPConnection ['http://example.com:80', HTTP/1.1, IDLE, Request Count: 1]>",
100105
]
101106
assert (
102107
repr(pool)
103-
== "<AsyncConnectionPool [Requests: 0 active, 0 queued | Connections: 0 active, 2 idle]>"
108+
== "<AsyncConnectionPool [Requests: 0 active, 0 queued | Connections: 0 active, 1 idle]>"
104109
)
105110

106111

tests/_sync/test_connection_pool.py

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ def test_connection_pool_with_keepalive():
3030
)
3131

3232
with httpcore.ConnectionPool(
33-
network_backend=network_backend,
33+
network_backend=network_backend, max_keepalive_connections=1
3434
) as pool:
3535
# Sending an intial request, which once complete will return to the pool, IDLE.
3636
with pool.stream("GET", "https://example.com/") as response:
@@ -79,28 +79,33 @@ def test_connection_pool_with_keepalive():
7979
)
8080

8181
# Sending a request to a different origin will not reuse the existing IDLE connection.
82-
with pool.stream("GET", "http://example.com/") as response:
82+
with pool.stream("GET", "http://example.com/") as response_1, pool.stream(
83+
"GET", "http://example.com/"
84+
) as response_2:
8385
info = [repr(c) for c in pool.connections]
8486
assert info == [
8587
"<HTTPConnection ['https://example.com:443', HTTP/1.1, IDLE, Request Count: 2]>",
8688
"<HTTPConnection ['http://example.com:80', HTTP/1.1, ACTIVE, Request Count: 1]>",
89+
"<HTTPConnection ['http://example.com:80', HTTP/1.1, ACTIVE, Request Count: 1]>",
8790
]
8891
assert (
8992
repr(pool)
90-
== "<ConnectionPool [Requests: 1 active, 0 queued | Connections: 1 active, 1 idle]>"
93+
== "<ConnectionPool [Requests: 2 active, 0 queued | Connections: 2 active, 1 idle]>"
9194
)
92-
response.read()
95+
response_1.read()
96+
response_2.read()
9397

94-
assert response.status == 200
95-
assert response.content == b"Hello, world!"
98+
assert response_1.status == 200
99+
assert response_1.content == b"Hello, world!"
100+
assert response_2.status == 200
101+
assert response_2.content == b"Hello, world!"
96102
info = [repr(c) for c in pool.connections]
97103
assert info == [
98-
"<HTTPConnection ['https://example.com:443', HTTP/1.1, IDLE, Request Count: 2]>",
99104
"<HTTPConnection ['http://example.com:80', HTTP/1.1, IDLE, Request Count: 1]>",
100105
]
101106
assert (
102107
repr(pool)
103-
== "<ConnectionPool [Requests: 0 active, 0 queued | Connections: 0 active, 2 idle]>"
108+
== "<ConnectionPool [Requests: 0 active, 0 queued | Connections: 0 active, 1 idle]>"
104109
)
105110

106111

0 commit comments

Comments
 (0)