Avoid redundant response release-connection calls

Author: rodrigo.nogueira

CHANGES/10089.bugfix.rst

@@ -0,0 +1,2 @@
+Reduced redundant ``ClientResponse._release_connection()`` invocations so
+successful requests now trigger a single release-path call per response.

aiohttp/client_reqrep.py

@@ -510,7 +510,8 @@ def release(self) -> None:
         self._closed = True
 
         self._cleanup_writer()
-        self._release_connection()
+        if self._connection is not None:
+            self._release_connection()
 
     @property
     def ok(self) -> bool:
@@ -558,7 +559,8 @@ async def _wait_released(self) -> None:
                     and task.cancelling()
                 ):
                     raise
-        self._release_connection()
+        if self._connection is not None:
+            self._release_connection()
 
     def _cleanup_writer(self) -> None:
         if self.__writer is not None:
@@ -583,7 +585,8 @@ async def wait_for_close(self) -> None:
                     and task.cancelling()
                 ):
                     raise
-        self.release()
+        if not self._released:
+            self.release()
 
     async def read(self) -> bytes:
         """Read response payload."""

tests/test_client_functional.py

@@ -370,6 +370,26 @@ async def handler(request: web.Request) -> web.Response:
     assert 1 == len(client._session.connector._conns)
 
 
+async def test_release_connection_called_once_per_request(
+    aiohttp_client: AiohttpClient, mocker: MockerFixture
+) -> None:
+    async def handler(request: web.Request) -> web.Response:
+        await request.read()
+        return web.Response(body=b"OK")
+
+    app = web.Application()
+    app.router.add_route("GET", "/", handler)
+    client = await aiohttp_client(app)
+
+    spy = mocker.spy(aiohttp.client_reqrep.ClientResponse, "_release_connection")
+    for _ in range(3):
+        async with client.get("/") as resp:
+            await resp.read()
+    await asyncio.sleep(0)
+
+    assert spy.call_count == 3
+
+
 async def test_HTTP_304(aiohttp_client: AiohttpClient) -> None:
     async def handler(request: web.Request) -> web.Response:
         body = await request.read()

tests/test_client_response.py

@@ -286,6 +286,78 @@ async def test_release(loop: asyncio.AbstractEventLoop, session: ClientSession)
     assert response._connection is None
 
 
+async def test_wait_released_releases_connection(
+    loop: asyncio.AbstractEventLoop, session: ClientSession
+) -> None:
+    url = URL("http://def-cl-resp.org")
+    response = ClientResponse(
+        "get",
+        url,
+        writer=None,
+        continue100=None,
+        timer=TimerNoop(),
+        traces=[],
+        loop=loop,
+        session=session,
+        request_headers=CIMultiDict[str](),
+        original_url=url,
+    )
+    conn = response._connection = mock.Mock()
+
+    await response._wait_released()
+
+    conn.release.assert_called_once_with()
+    assert response._connection is None
+
+
+async def test_wait_released_skips_release_without_connection(
+    loop: asyncio.AbstractEventLoop, session: ClientSession
+) -> None:
+    url = URL("http://def-cl-resp.org")
+    response = ClientResponse(
+        "get",
+        url,
+        writer=None,
+        continue100=None,
+        timer=TimerNoop(),
+        traces=[],
+        loop=loop,
+        session=session,
+        request_headers=CIMultiDict[str](),
+        original_url=url,
+    )
+    response._connection = None
+
+    with mock.patch.object(response, "_release_connection") as release_connection:
+        await response._wait_released()
+
+    release_connection.assert_not_called()
+
+
+async def test_wait_for_close_skips_release_if_already_released(
+    loop: asyncio.AbstractEventLoop, session: ClientSession
+) -> None:
+    url = URL("http://def-cl-resp.org")
+    response = ClientResponse(
+        "get",
+        url,
+        writer=None,
+        continue100=None,
+        timer=TimerNoop(),
+        traces=[],
+        loop=loop,
+        session=session,
+        request_headers=CIMultiDict[str](),
+        original_url=url,
+    )
+    response._released = True
+
+    with mock.patch.object(response, "release") as release:
+        await response.wait_for_close()
+
+    release.assert_not_called()
+
+
 @pytest.mark.skipif(
     sys.implementation.name != "cpython",
     reason="Other implementations has different GC strategies",