Skip to content

Commit 771d203

Browse files
[PR #10556/9d4e1161 backport][3.11] Break cyclic references at connection close when there was a traceback (#10566)
**This is a backport of PR #10556 as merged into master (9d4e116).** <!-- Thank you for your contribution! --> ## What do these changes do? Clears the exception on the `DataQueue` and `WebSocketDataQueue` when the connection is closed to break cyclic references. ## Are there changes in behavior for the user? bugfix ## Is it a substantial burden for the maintainers to support this? no ## Related issue number fixes #10535 ## Checklist - [ ] I think the code is well written - [ ] Unit tests for the changes exist - [ ] Documentation reflects the changes - [ ] If you provide code modification, please add yourself to `CONTRIBUTORS.txt` * The format is &lt;Name&gt; &lt;Surname&gt;. * Please keep alphabetical order, the file is sorted by names. - [ ] Add a new news fragment into the `CHANGES/` folder * name it `<issue_or_pr_num>.<type>.rst` (e.g. `588.bugfix.rst`) * if you don't have an issue number, change it to the pull request number after creating the PR * `.bugfix`: A bug fix for something the maintainers deemed an improper undesired behavior that got corrected to match pre-agreed expectations. * `.feature`: A new behavior, public APIs. That sort of stuff. * `.deprecation`: A declaration of future API removals and breaking changes in behavior. * `.breaking`: When something public is removed in a breaking way. Could be deprecated in an earlier release. * `.doc`: Notable updates to the documentation structure or build process. * `.packaging`: Notes for downstreams about unobvious side effects and tooling. Changes in the test invocation considerations and runtime assumptions. * `.contrib`: Stuff that affects the contributor experience. e.g. Running tests, building the docs, setting up the development environment. * `.misc`: Changes that are hard to assign to any of the above categories. * Make sure to use full sentences with correct case and punctuation, for example: ```rst Fixed issue with non-ascii contents in doctest text files -- by :user:`contributor-gh-handle`. ``` Use the past tense or the present tense a non-imperative mood, referring to what's changed compared to the last released version of this project. Co-authored-by: J. Nick Koston <[email protected]>
1 parent 6357c05 commit 771d203

File tree

5 files changed

+77
-0
lines changed

5 files changed

+77
-0
lines changed

CHANGES/10556.bugfix.rst

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Break cyclic references at connection close when there was a traceback -- by :user:`bdraco`.
2+
3+
Special thanks to :user:`availov` for reporting the issue.

aiohttp/_websocket/reader_py.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@ def _release_waiter(self) -> None:
9393
def feed_eof(self) -> None:
9494
self._eof = True
9595
self._release_waiter()
96+
self._exception = None # Break cyclic references
9697

9798
def feed_data(self, data: "WSMessage", size: "int_") -> None:
9899
self._size += size

aiohttp/client_proto.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ def force_close(self) -> None:
6464
self._should_close = True
6565

6666
def close(self) -> None:
67+
self._exception = None # Break cyclic references
6768
transport = self.transport
6869
if transport is not None:
6970
transport.close()
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
import asyncio
2+
import contextlib
3+
import gc
4+
import sys
5+
6+
from aiohttp import ClientError, ClientSession, web
7+
from aiohttp.test_utils import get_unused_port_socket
8+
9+
gc.set_debug(gc.DEBUG_LEAK)
10+
11+
12+
async def main() -> None:
13+
app = web.Application()
14+
15+
async def stream_handler(request: web.Request) -> web.Response:
16+
assert request.transport is not None
17+
request.transport.close() # Forcefully closing connection
18+
return web.Response()
19+
20+
app.router.add_get("/stream", stream_handler)
21+
sock = get_unused_port_socket("127.0.0.1")
22+
port = sock.getsockname()[1]
23+
24+
runner = web.AppRunner(app)
25+
await runner.setup()
26+
site = web.SockSite(runner, sock)
27+
await site.start()
28+
29+
session = ClientSession()
30+
31+
async def fetch_stream(url: str) -> None:
32+
"""Fetch a stream and read a few bytes from it."""
33+
with contextlib.suppress(ClientError):
34+
await session.get(url)
35+
36+
client_task = asyncio.create_task(fetch_stream(f"http://localhost:{port}/stream"))
37+
await client_task
38+
gc.collect()
39+
client_response_present = any(
40+
type(obj).__name__ == "ClientResponse" for obj in gc.garbage
41+
)
42+
await session.close()
43+
await runner.cleanup()
44+
sys.exit(1 if client_response_present else 0)
45+
46+
47+
asyncio.run(main())

tests/test_leaks.py

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
import pathlib
2+
import platform
3+
import subprocess
4+
import sys
5+
6+
import pytest
7+
8+
IS_PYPY = platform.python_implementation() == "PyPy"
9+
10+
11+
@pytest.mark.skipif(IS_PYPY, reason="gc.DEBUG_LEAK not available on PyPy")
12+
def test_client_response_does_not_leak_on_server_disconnected_error() -> None:
13+
"""Test that ClientResponse is collected after server disconnects.
14+
15+
https://github.com/aio-libs/aiohttp/issues/10535
16+
"""
17+
leak_test_script = pathlib.Path(__file__).parent.joinpath(
18+
"isolated", "check_for_client_response_leak.py"
19+
)
20+
21+
with subprocess.Popen(
22+
[sys.executable, "-u", str(leak_test_script)],
23+
stdout=subprocess.PIPE,
24+
) as proc:
25+
assert proc.wait() == 0, "ClientResponse leaked"

0 commit comments

Comments
 (0)