From edeb2b7a4ef068128a855520ea01d5dd1c09998e Mon Sep 17 00:00:00 2001
From: "J. Nick Koston" <nick@koston.org>
Date: Sun, 16 Mar 2025 11:38:28 -1000
Subject: [PATCH] Break cyclic references when there is an exception handling a
 request (#10569)

<!-- Thank you for your contribution! -->

## What do these changes do?

This is a partial fix for #10548

- There is still another case for `SystemRoute`s that needs to be
addressed. No reproducer available yet.
- There is also another case on the client side on connection refused
that still needs to be addressed
https://github.com/aio-libs/aiohttp/issues/10548#issuecomment-2727643763

## Are there changes in behavior for the user?

fixes memory leak

## Is it a substantial burden for the maintainers to support this?
no

(cherry picked from commit dfbf782ba4ea3eabfe052e6224727cf83efdffb5)
---
 CHANGES/10569.bugfix.rst                 |  1 +
 aiohttp/web_protocol.py                  | 14 ++++++--
 tests/isolated/check_for_request_leak.py | 41 ++++++++++++++++++++++++
 tests/test_leaks.py                      | 17 ++++++++++
 4 files changed, 70 insertions(+), 3 deletions(-)
 create mode 100644 CHANGES/10569.bugfix.rst
 create mode 100644 tests/isolated/check_for_request_leak.py

diff --git a/CHANGES/10569.bugfix.rst b/CHANGES/10569.bugfix.rst
new file mode 100644
index 00000000000..7d817e867d4
--- /dev/null
+++ b/CHANGES/10569.bugfix.rst
@@ -0,0 +1 @@
+Break cyclic references when there is an exception handling a request -- by :user:`bdraco`.
diff --git a/aiohttp/web_protocol.py b/aiohttp/web_protocol.py
index e4c347e5a9e..1dba9606ea0 100644
--- a/aiohttp/web_protocol.py
+++ b/aiohttp/web_protocol.py
@@ -520,8 +520,6 @@ async def start(self) -> None:
         keep_alive(True) specified.
         """
         loop = self._loop
-        handler = asyncio.current_task(loop)
-        assert handler is not None
         manager = self._manager
         assert manager is not None
         keepalive_timeout = self._keepalive_timeout
@@ -551,7 +549,16 @@ async def start(self) -> None:
             else:
                 request_handler = self._request_handler
 
-            request = self._request_factory(message, payload, self, writer, handler)
+            # Important don't hold a reference to the current task
+            # as on traceback it will prevent the task from being
+            # collected and will cause a memory leak.
+            request = self._request_factory(
+                message,
+                payload,
+                self,
+                writer,
+                self._task_handler or asyncio.current_task(loop),  # type: ignore[arg-type]
+            )
             try:
                 # a new task is used for copy context vars (#3406)
                 coro = self._handle_request(request, start, request_handler)
@@ -617,6 +624,7 @@ async def start(self) -> None:
                 self.force_close()
                 raise
             finally:
+                request._task = None  # type: ignore[assignment] # Break reference cycle in case of exception
                 if self.transport is None and resp is not None:
                     self.log_debug("Ignored premature client disconnection.")
 
diff --git a/tests/isolated/check_for_request_leak.py b/tests/isolated/check_for_request_leak.py
new file mode 100644
index 00000000000..6f340a05277
--- /dev/null
+++ b/tests/isolated/check_for_request_leak.py
@@ -0,0 +1,41 @@
+import asyncio
+import gc
+import sys
+from typing import NoReturn
+
+from aiohttp import ClientSession, web
+from aiohttp.test_utils import get_unused_port_socket
+
+gc.set_debug(gc.DEBUG_LEAK)
+
+
+async def main() -> None:
+    app = web.Application()
+
+    async def handler(request: web.Request) -> NoReturn:
+        await request.json()
+        assert False
+
+    app.router.add_route("GET", "/json", handler)
+    sock = get_unused_port_socket("127.0.0.1")
+    port = sock.getsockname()[1]
+
+    runner = web.AppRunner(app)
+    await runner.setup()
+    site = web.SockSite(runner, sock)
+    await site.start()
+
+    async with ClientSession() as session:
+        async with session.get(f"http://127.0.0.1:{port}/json") as resp:
+            await resp.read()
+
+    # Give time for the cancelled task to be collected
+    await asyncio.sleep(0.5)
+    gc.collect()
+    request_present = any(type(obj).__name__ == "Request" for obj in gc.garbage)
+    await session.close()
+    await runner.cleanup()
+    sys.exit(1 if request_present else 0)
+
+
+asyncio.run(main())
diff --git a/tests/test_leaks.py b/tests/test_leaks.py
index a3b6b624346..f527ce18cae 100644
--- a/tests/test_leaks.py
+++ b/tests/test_leaks.py
@@ -23,3 +23,20 @@ def test_client_response_does_not_leak_on_server_disconnected_error() -> None:
         stdout=subprocess.PIPE,
     ) as proc:
         assert proc.wait() == 0, "ClientResponse leaked"
+
+
+@pytest.mark.skipif(IS_PYPY, reason="gc.DEBUG_LEAK not available on PyPy")
+def test_request_does_not_leak_when_request_handler_raises() -> None:
+    """Test that the Request object is collected when the handler raises.
+
+    https://github.com/aio-libs/aiohttp/issues/10548
+    """
+    leak_test_script = pathlib.Path(__file__).parent.joinpath(
+        "isolated", "check_for_request_leak.py"
+    )
+
+    with subprocess.Popen(
+        [sys.executable, "-u", str(leak_test_script)],
+        stdout=subprocess.PIPE,
+    ) as proc:
+        assert proc.wait() == 0, "Request leaked"