Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGES/11043.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixed a slow memory leak where ``ClientResponse`` retained a circular reference to ``ClientSession`` via ``_traces`` after the response was closed.
2 changes: 2 additions & 0 deletions aiohttp/client_reqrep.py
Original file line number Diff line number Diff line change
Expand Up @@ -525,6 +525,7 @@ def close(self) -> None:
return

self._cleanup_writer()
self._traces = []
if self._connection is not None:
self._connection.close()
self._connection = None
Expand All @@ -536,6 +537,7 @@ def release(self) -> None:
self._closed = True

self._cleanup_writer()
self._traces = []
self._release_connection()

@property
Expand Down
122 changes: 122 additions & 0 deletions tests/test_client_response.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import asyncio
import gc
import sys
import weakref
from http.cookies import SimpleCookie
from json import JSONDecodeError
from unittest import mock
Expand Down Expand Up @@ -1834,3 +1835,124 @@ def test_response_cookies_setter_updates_raw_headers(
response.cookies = empty_cookies
# Should not set _raw_cookie_headers for empty cookies
assert response._raw_cookie_headers is None


def test_close_clears_traces_and_session(
event_loop: asyncio.AbstractEventLoop, session: ClientSession
) -> None:
url = URL("http://def-cl-resp.org")
trace = mock.Mock()
response = ClientResponse(
"get",
url,
writer=WriterMock(),
continue100=None,
timer=TimerNoop(),
traces=[trace],
loop=event_loop,
session=session,
request_headers=CIMultiDict[str](),
original_url=url,
stream_writer=mock.create_autospec(
AbstractStreamWriter, spec_set=True, instance=True
),
)
response._closed = False
response._connection = mock.Mock()
response.close()
assert response._traces == []
assert response._session is None


def test_release_clears_traces_and_session(
event_loop: asyncio.AbstractEventLoop, session: ClientSession
) -> None:
url = URL("http://def-cl-resp.org")
trace = mock.Mock()
response = ClientResponse(
"get",
url,
writer=WriterMock(),
continue100=None,
timer=TimerNoop(),
traces=[trace],
loop=event_loop,
session=session,
request_headers=CIMultiDict[str](),
original_url=url,
stream_writer=mock.create_autospec(
AbstractStreamWriter, spec_set=True, instance=True
),
)
response._closed = False
response._connection = mock.Mock()
response.release()
assert response._traces == []
assert response._session is None


def test_response_eof_clears_session_but_not_traces(
event_loop: asyncio.AbstractEventLoop, session: ClientSession
) -> None:
# _response_eof() must NOT clear _traces: it fires during payload EOF which
# happens inside read(), before read() iterates _traces to send callbacks.
# Clearing _traces here would silently drop on_response_chunk_received hooks.
url = URL("http://def-cl-resp.org")
trace = mock.Mock()
response = ClientResponse(
"get",
url,
writer=None,
continue100=None,
timer=TimerNoop(),
traces=[trace],
loop=event_loop,
session=session,
request_headers=CIMultiDict[str](),
original_url=url,
stream_writer=mock.create_autospec(
AbstractStreamWriter, spec_set=True, instance=True
),
)
response._closed = False
conn = response._connection = mock.Mock()
conn.protocol.upgraded = False

response._response_eof()
assert response._traces == [trace] # preserved so read() callbacks can still fire
assert response._session is None


@pytest.mark.skipif(
sys.implementation.name != "cpython",
reason="Other implementations have different GC strategies",
)
def test_no_circular_ref_after_close(
event_loop: asyncio.AbstractEventLoop, session: ClientSession
) -> None:
url = URL("http://def-cl-resp.org")
trace = mock.Mock()
response = ClientResponse(
"get",
url,
writer=WriterMock(),
continue100=None,
timer=TimerNoop(),
traces=[trace],
loop=event_loop,
session=session,
request_headers=CIMultiDict[str](),
original_url=url,
stream_writer=mock.create_autospec(
AbstractStreamWriter, spec_set=True, instance=True
),
)
response._closed = False
response._connection = mock.Mock()

ref = weakref.ref(response)
response.close()
del response
gc.collect()

assert ref() is None