From 10e844f36fa00117792d43ea34d90a5f9e144b8b Mon Sep 17 00:00:00 2001 From: TAHRI Ahmed R Date: Mon, 6 Nov 2023 19:57:51 +0100 Subject: [PATCH] :bookmark: Release 2.2.904 (#41) 2.2.904 (2023-11-06) ================= - Fixed concurrent/multiplexed request overflow in a full connection pool. - Fixed connection close that had in-flight request (in multiplexed mode), the connection appeared as not idle on clean reuse. --- CHANGES.rst | 6 +++++ src/urllib3/_version.py | 2 +- src/urllib3/backend/_base.py | 4 +++- src/urllib3/backend/hface.py | 6 +++++ src/urllib3/connectionpool.py | 42 +++++++++++++++++++++++++++++++---- src/urllib3/poolmanager.py | 4 +++- 6 files changed, 57 insertions(+), 7 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 67ba593e0c..04d4418aa2 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -1,3 +1,9 @@ +2.2.904 (2023-11-06) +==================== + +- Fixed concurrent/multiplexed request overflow in a full connection pool. +- Fixed connection close that had in-flight request (in multiplexed mode), the connection appeared as not idle on clean reuse. + 2.2.903 (2023-11-06) ==================== diff --git a/src/urllib3/_version.py b/src/urllib3/_version.py index a38f8776a1..dc53ef1ee0 100644 --- a/src/urllib3/_version.py +++ b/src/urllib3/_version.py @@ -1,4 +1,4 @@ # This file is protected via CODEOWNERS from __future__ import annotations -__version__ = "2.2.903" +__version__ = "2.2.904" diff --git a/src/urllib3/backend/_base.py b/src/urllib3/backend/_base.py index dcb089f4ec..ba6e43f568 100644 --- a/src/urllib3/backend/_base.py +++ b/src/urllib3/backend/_base.py @@ -99,7 +99,9 @@ def from_promise(self) -> ResponsePromise | None: @from_promise.setter def from_promise(self, value: ResponsePromise) -> None: if value.stream_id != self._stream_id: - raise ValueError + raise ValueError( + "Trying to assign a ResponsePromise to an unrelated LowLevelResponse" + ) self.__promise = value @property diff --git a/src/urllib3/backend/hface.py b/src/urllib3/backend/hface.py index 6a975fc59b..4de0d44728 100644 --- a/src/urllib3/backend/hface.py +++ b/src/urllib3/backend/hface.py @@ -978,3 +978,9 @@ def close(self) -> None: self._protocol = None self._stream_id = None + self._promises = {} + self._pending_responses = {} + self.__custom_tls_settings = None + self.conn_info = None + self.__expected_body_length = None + self.__remaining_body_length = None diff --git a/src/urllib3/connectionpool.py b/src/urllib3/connectionpool.py index 8584dcccf7..ce9f04a2c2 100644 --- a/src/urllib3/connectionpool.py +++ b/src/urllib3/connectionpool.py @@ -196,6 +196,7 @@ def __init__( self.timeout = timeout self.retries = retries + self._maxsize = maxsize self.pool: queue.LifoQueue[typing.Any] | None = self.QueueCls(maxsize) self.block = block @@ -284,7 +285,7 @@ def _get_conn( pass # Oh well, we'll create a new connection then if no_new and conn is None: - raise ValueError + raise ValueError("Explicitly asked to stop get_conn early with no_new=True") # If this is a persistent connection, check if it got disconnected if conn and is_connection_dropped(conn): @@ -322,7 +323,8 @@ def _put_conn(self, conn: HTTPConnection | None) -> None: except queue.Full: # Connection never got put back into the pool, close it. if conn: - conn.close() + if conn.is_idle: + conn.close() if self.block: # This should never happen if you got the conn from self._get_conn @@ -330,6 +332,20 @@ def _put_conn(self, conn: HTTPConnection | None) -> None: self, "Pool reached maximum size and no more connections are allowed.", ) from None + else: + # multiplexed connection may still have in-flight request not converted into response + # we shall not discard it until responses are consumed. + if conn and conn.is_idle is False: + log.warning( + "Connection pool is full, temporary increase, keeping connection, " + "multiplexed and not idle: %s. Connection pool size: %s", + self.host, + self.pool.qsize(), + ) + + self.pool.maxsize += 1 + + return self._put_conn(conn) log.warning( "Connection pool is full, discarding connection: %s. Connection pool size: %s", @@ -414,8 +430,24 @@ def get_response( continue break + forget_about_connections = [] + + # we exceptionally increased the size (block=False + multiplexed enabled) + if len(connections) > self._maxsize: + expect_drop_count = len(connections) - self._maxsize + + for conn in connections: + if conn.is_idle: + forget_about_connections.append(conn) + if len(forget_about_connections) >= expect_drop_count: + break + for conn in connections: - self._put_conn(conn) + if conn not in forget_about_connections: + self._put_conn(conn) + + for conn in forget_about_connections: + conn.close() if promise is not None and response is None: raise ValueError( @@ -438,7 +470,9 @@ def get_response( from_promise = response._fp.from_promise if from_promise is None: - raise ValueError + raise ValueError( + "Internal: Unable to identify originating ResponsePromise from a LowLevelResponse" + ) # Retrieve request ctx method = typing.cast(str, from_promise.get_parameter("method")) diff --git a/src/urllib3/poolmanager.py b/src/urllib3/poolmanager.py index 37a7d5473d..bb37da1b0e 100644 --- a/src/urllib3/poolmanager.py +++ b/src/urllib3/poolmanager.py @@ -475,7 +475,9 @@ def get_response( from_promise = response._fp.from_promise if from_promise is None: - raise ValueError + raise ValueError( + "Internal: Unable to identify originating ResponsePromise from a LowLevelResponse" + ) # Retrieve request ctx method = typing.cast(str, from_promise.get_parameter("method"))