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

@@ -583,7 +583,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,28 @@ async def handler(request: web.Request) -> web.Response:
     assert 1 == len(client._session.connector._conns)
 
 
+async def test_release_connection_called_three_times_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)
+
+    # One request with response body read in a context manager currently hits:
+    # _response_eof(), _wait_released(), and release() from __aexit__().
+    assert spy.call_count == 9
+
+
 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_calls_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_called_once_with()
+
+
+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",