Skip to content

Commit 9f1b471

Browse files
committed
fix: clear _traces in ClientResponse._cleanup_writer to break circular ref
- add `self._traces = []` in `_cleanup_writer()` after `self._session = None` - this deterministically breaks the ClientResponse->_traces->Trace->_session cycle on every close/release/eof path, avoiding slow RSS growth (#11043) - add tests verifying _traces and _session are cleared via close(), release(), and _response_eof(); add weakref test confirming the cycle is broken
1 parent 33a59da commit 9f1b471

3 files changed

Lines changed: 121 additions & 0 deletions

File tree

CHANGES/11043.bugfix

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fixed a slow memory leak where ``ClientResponse`` retained a circular reference to ``ClientSession`` via ``_traces`` after the response was closed.

aiohttp/client_reqrep.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -593,6 +593,7 @@ def _cleanup_writer(self) -> None:
593593
self._output_size = self._stream_writer.output_size
594594
self._stream_writer = None
595595
self._session = None
596+
self._traces = []
596597

597598
def _notify_content(self) -> None:
598599
content = self.content

tests/test_client_response.py

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import asyncio
44
import gc
55
import sys
6+
import weakref
67
from http.cookies import SimpleCookie
78
from json import JSONDecodeError
89
from unittest import mock
@@ -1834,3 +1835,121 @@ def test_response_cookies_setter_updates_raw_headers(
18341835
response.cookies = empty_cookies
18351836
# Should not set _raw_cookie_headers for empty cookies
18361837
assert response._raw_cookie_headers is None
1838+
1839+
1840+
def test_close_clears_traces_and_session(
1841+
event_loop: asyncio.AbstractEventLoop, session: ClientSession
1842+
) -> None:
1843+
url = URL("http://def-cl-resp.org")
1844+
trace = mock.Mock()
1845+
response = ClientResponse(
1846+
"get",
1847+
url,
1848+
writer=WriterMock(),
1849+
continue100=None,
1850+
timer=TimerNoop(),
1851+
traces=[trace],
1852+
loop=event_loop,
1853+
session=session,
1854+
request_headers=CIMultiDict[str](),
1855+
original_url=url,
1856+
stream_writer=mock.create_autospec(
1857+
AbstractStreamWriter, spec_set=True, instance=True
1858+
),
1859+
)
1860+
response._closed = False
1861+
response._connection = mock.Mock()
1862+
response.close()
1863+
assert response._traces == []
1864+
assert response._session is None
1865+
1866+
1867+
def test_release_clears_traces_and_session(
1868+
event_loop: asyncio.AbstractEventLoop, session: ClientSession
1869+
) -> None:
1870+
url = URL("http://def-cl-resp.org")
1871+
trace = mock.Mock()
1872+
response = ClientResponse(
1873+
"get",
1874+
url,
1875+
writer=WriterMock(),
1876+
continue100=None,
1877+
timer=TimerNoop(),
1878+
traces=[trace],
1879+
loop=event_loop,
1880+
session=session,
1881+
request_headers=CIMultiDict[str](),
1882+
original_url=url,
1883+
stream_writer=mock.create_autospec(
1884+
AbstractStreamWriter, spec_set=True, instance=True
1885+
),
1886+
)
1887+
response._closed = False
1888+
response._connection = mock.Mock()
1889+
response.release()
1890+
assert response._traces == []
1891+
assert response._session is None
1892+
1893+
1894+
def test_response_eof_clears_traces_and_session(
1895+
event_loop: asyncio.AbstractEventLoop, session: ClientSession
1896+
) -> None:
1897+
url = URL("http://def-cl-resp.org")
1898+
trace = mock.Mock()
1899+
response = ClientResponse(
1900+
"get",
1901+
url,
1902+
writer=None,
1903+
continue100=None,
1904+
timer=TimerNoop(),
1905+
traces=[trace],
1906+
loop=event_loop,
1907+
session=session,
1908+
request_headers=CIMultiDict[str](),
1909+
original_url=url,
1910+
stream_writer=mock.create_autospec(
1911+
AbstractStreamWriter, spec_set=True, instance=True
1912+
),
1913+
)
1914+
response._closed = False
1915+
conn = response._connection = mock.Mock()
1916+
conn.protocol.upgraded = False
1917+
1918+
response._response_eof()
1919+
assert response._traces == []
1920+
assert response._session is None
1921+
1922+
1923+
@pytest.mark.skipif(
1924+
sys.implementation.name != "cpython",
1925+
reason="Other implementations have different GC strategies",
1926+
)
1927+
def test_no_circular_ref_after_close(
1928+
event_loop: asyncio.AbstractEventLoop, session: ClientSession
1929+
) -> None:
1930+
url = URL("http://def-cl-resp.org")
1931+
trace = mock.Mock()
1932+
response = ClientResponse(
1933+
"get",
1934+
url,
1935+
writer=WriterMock(),
1936+
continue100=None,
1937+
timer=TimerNoop(),
1938+
traces=[trace],
1939+
loop=event_loop,
1940+
session=session,
1941+
request_headers=CIMultiDict[str](),
1942+
original_url=url,
1943+
stream_writer=mock.create_autospec(
1944+
AbstractStreamWriter, spec_set=True, instance=True
1945+
),
1946+
)
1947+
response._closed = False
1948+
response._connection = mock.Mock()
1949+
1950+
ref = weakref.ref(response)
1951+
response.close()
1952+
del response
1953+
gc.collect()
1954+
1955+
assert ref() is None

0 commit comments

Comments
 (0)