From 957a3ea26131f9f810f652097449de4e7ef0f478 Mon Sep 17 00:00:00 2001 From: Bruce Merry Date: Fri, 6 Dec 2024 10:02:30 +0200 Subject: [PATCH 1/5] Fix infinite callback loop when used with async-solipsism If the keepalive handler is called too soon, it reschedules itself. The test used `now <= close_time`, which means that an exactly on-time notification is treated as "too soon", causing an automatic rescheduling. For real systems the time will eventually advance and break the loop, but with async-solipsism, time doesn't advance until there is some reason to sleep and the loop is infinite. Closes #10149. --- aiohttp/web_protocol.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aiohttp/web_protocol.py b/aiohttp/web_protocol.py index 0bffc849817..3d76fc1f1d7 100644 --- a/aiohttp/web_protocol.py +++ b/aiohttp/web_protocol.py @@ -487,7 +487,7 @@ def _process_keepalive(self) -> None: loop = self._loop now = loop.time() close_time = self._next_keepalive_close_time - if now <= close_time: + if now < close_time: # Keep alive close check fired too early, reschedule self._keepalive_handle = loop.call_at(close_time, self._process_keepalive) return From ec911ed340c6a521199f86df6f1262952fc90d91 Mon Sep 17 00:00:00 2001 From: Bruce Merry Date: Tue, 10 Dec 2024 09:47:39 +0200 Subject: [PATCH 2/5] Add changelog entry for #10149 --- CHANGES/10149.bugfix.rst | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 CHANGES/10149.bugfix.rst diff --git a/CHANGES/10149.bugfix.rst b/CHANGES/10149.bugfix.rst new file mode 100644 index 00000000000..61765a50fcf --- /dev/null +++ b/CHANGES/10149.bugfix.rst @@ -0,0 +1,4 @@ +Fixed an infinite loop that can occur when using aiohttp in combination +with `async-solipsism`_ -- by :user:`bmerry`. + +.. _async-solipsism: https://github.com/bmerry/async-solipsism From 0783b4d561d5e83b343a0761f673c2353c524c36 Mon Sep 17 00:00:00 2001 From: Bruce Merry Date: Tue, 10 Dec 2024 15:51:19 +0200 Subject: [PATCH 3/5] Reclassify #10149 as "packaging" in CHANGES At request of @bdraco --- CHANGES/{10149.bugfix.rst => 10149.packaging.rst} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename CHANGES/{10149.bugfix.rst => 10149.packaging.rst} (100%) diff --git a/CHANGES/10149.bugfix.rst b/CHANGES/10149.packaging.rst similarity index 100% rename from CHANGES/10149.bugfix.rst rename to CHANGES/10149.packaging.rst From c8e502b31d7546d591d769a3ca76b6127d473853 Mon Sep 17 00:00:00 2001 From: Bruce Merry Date: Wed, 11 Dec 2024 08:37:21 +0200 Subject: [PATCH 4/5] Rename 10149.packaging -> 10149.misc --- CHANGES/{10149.packaging.rst => 10149.misc.rst} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename CHANGES/{10149.packaging.rst => 10149.misc.rst} (100%) diff --git a/CHANGES/10149.packaging.rst b/CHANGES/10149.misc.rst similarity index 100% rename from CHANGES/10149.packaging.rst rename to CHANGES/10149.misc.rst From 03f7a3cff74e3cee266b70d48bb9dbe338975624 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Fri, 13 Dec 2024 20:03:02 -0600 Subject: [PATCH 5/5] Add test to ensure keep alive expires on time --- tests/test_web_functional.py | 38 ++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/tests/test_web_functional.py b/tests/test_web_functional.py index 239aea4c25e..8b65d1e08fb 100644 --- a/tests/test_web_functional.py +++ b/tests/test_web_functional.py @@ -2312,3 +2312,41 @@ async def handler(request: web.Request) -> web.Response: # Make 2nd request which will hit the race condition. async with client.get("/") as resp: assert resp.status == 200 + + +async def test_keepalive_expires_on_time(aiohttp_client: AiohttpClient) -> None: + """Test that the keepalive handle expires on time.""" + + async def handler(request: web.Request) -> web.Response: + body = await request.read() + assert b"" == body + return web.Response(body=b"OK") + + app = web.Application() + app.router.add_route("GET", "/", handler) + + connector = aiohttp.TCPConnector(limit=1) + client = await aiohttp_client(app, connector=connector) + + loop = asyncio.get_running_loop() + now = loop.time() + + # Patch loop time so we can control when the keepalive timeout is processed + with mock.patch.object(loop, "time") as loop_time_mock: + loop_time_mock.return_value = now + resp1 = await client.get("/") + await resp1.read() + request_handler = client.server.handler.connections[0] + + # Ensure the keep alive handle is set + assert request_handler._keepalive_handle is not None + + # Set the loop time to exactly the keepalive timeout + loop_time_mock.return_value = request_handler._next_keepalive_close_time + + # sleep twice to ensure the keep alive timeout is processed + await asyncio.sleep(0) + await asyncio.sleep(0) + + # Ensure the keep alive handle expires + assert request_handler._keepalive_handle is None