From 53141f320a856545e0c1dc71ff03001afe6be92c Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Mon, 19 Jun 2017 16:05:26 +0300 Subject: [PATCH 1/2] Fix #1985: Keep a reference to ClientSession in response object --- CHANGES.rst | 2 + aiohttp/client.py | 10 +--- aiohttp/client_reqrep.py | 10 ++-- tests/test_client_proto.py | 2 +- tests/test_client_request.py | 2 +- tests/test_client_response.py | 101 ++++++++++++++++++---------------- 6 files changed, 65 insertions(+), 62 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 71c8722c22c..0f687864152 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -36,6 +36,8 @@ Changes - Fix BadStatusLine caused by extra `CRLF` after `POST` data #1792 +- Keep a reference to ClientSession in response object #1985 + 2.1.0 (2017-05-26) ------------------ diff --git a/aiohttp/client.py b/aiohttp/client.py index 9b588746e6e..031602acfa8 100644 --- a/aiohttp/client.py +++ b/aiohttp/client.py @@ -222,7 +222,8 @@ def _request(self, method, url, *, compress=compress, chunked=chunked, expect100=expect100, loop=self._loop, response_class=self._response_class, - proxy=proxy, proxy_auth=proxy_auth, timer=timer) + proxy=proxy, proxy_auth=proxy_auth, timer=timer, + session=self) # connection timeout try: @@ -688,13 +689,6 @@ def __await__(self): self._session.close() raise - def __del__(self): - # in case of "resp = aiohttp.request(...)" - # _SessionRequestContextManager get destroyed before resp get processed - # and connection has to stay alive during this time - # ClientSession.detach just cleans up connector attribute - self._session.detach() - def request(method, url, *, params=None, diff --git a/aiohttp/client_reqrep.py b/aiohttp/client_reqrep.py index 58bec10374f..b98a00f3032 100644 --- a/aiohttp/client_reqrep.py +++ b/aiohttp/client_reqrep.py @@ -66,14 +66,14 @@ def __init__(self, method, url, *, chunked=None, expect100=False, loop=None, response_class=None, proxy=None, proxy_auth=None, proxy_from_env=False, - timer=None): + timer=None, session=None): if loop is None: loop = asyncio.get_event_loop() assert isinstance(url, URL), url assert isinstance(proxy, (URL, type(None))), proxy - + self._session = session if params: q = MultiDict(url.query) url2 = url.with_query(params) @@ -409,7 +409,7 @@ def send(self, conn): request_info=self.request_info ) - self.response._post_init(self.loop) + self.response._post_init(self.loop, self._session) return self.response @asyncio.coroutine @@ -446,6 +446,7 @@ class ClientResponse(HeadersMixin): # post-init stage allows to not change ctor signature _loop = None _closed = True # to allow __del__ for non-initialized properly response + _session = None def __init__(self, method, url, *, writer=None, continue100=None, timer=None, @@ -487,8 +488,9 @@ def _headers(self): def request_info(self): return self._request_info - def _post_init(self, loop): + def _post_init(self, loop, session): self._loop = loop + self._session = session # store a reference to session #1985 if loop.get_debug(): self._source_traceback = traceback.extract_stack(sys._getframe(1)) diff --git a/tests/test_client_proto.py b/tests/test_client_proto.py index a36f2afbeef..bdef41af466 100644 --- a/tests/test_client_proto.py +++ b/tests/test_client_proto.py @@ -72,7 +72,7 @@ def test_client_protocol_readuntil_eof(loop): proto.data_received(b'HTTP/1.1 200 Ok\r\n\r\n') response = ClientResponse('get', URL('http://def-cl-resp.org')) - response._post_init(loop) + response._post_init(loop, mock.Mock()) yield from response.start(conn, read_until_eof=True) assert not response.content.is_eof() diff --git a/tests/test_client_request.py b/tests/test_client_request.py index 698fe08256f..2a086b00c2f 100644 --- a/tests/test_client_request.py +++ b/tests/test_client_request.py @@ -1104,7 +1104,7 @@ def send(self, conn): self.url, writer=self._writer, continue100=self._continue) - resp._post_init(self.loop) + resp._post_init(self.loop, mock.Mock()) self.response = resp nonlocal called called = True diff --git a/tests/test_client_response.py b/tests/test_client_response.py index ec20a199f98..5188730ae25 100644 --- a/tests/test_client_response.py +++ b/tests/test_client_response.py @@ -13,13 +13,18 @@ from aiohttp.client_reqrep import ClientResponse, RequestInfo +@pytest.fixture +def session(): + return mock.Mock() + + @asyncio.coroutine -def test_http_processing_error(): +def test_http_processing_error(session): loop = mock.Mock() request_info = mock.Mock() response = ClientResponse( 'get', URL('http://del-cl-resp.org'), request_info=request_info) - response._post_init(loop) + response._post_init(loop, session) loop.get_debug = mock.Mock() loop.get_debug.return_value = True @@ -34,10 +39,10 @@ def test_http_processing_error(): assert info.value.request_info is request_info -def test_del(): +def test_del(session): loop = mock.Mock() response = ClientResponse('get', URL('http://del-cl-resp.org')) - response._post_init(loop) + response._post_init(loop, session) loop.get_debug = mock.Mock() loop.get_debug.return_value = True @@ -53,9 +58,9 @@ def test_del(): connection.release.assert_called_with() -def test_close(loop): +def test_close(loop, session): response = ClientResponse('get', URL('http://def-cl-resp.org')) - response._post_init(loop) + response._post_init(loop, session) response._closed = False response._connection = mock.Mock() response.close() @@ -64,25 +69,25 @@ def test_close(loop): response.close() -def test_wait_for_100_1(loop): +def test_wait_for_100_1(loop, session): response = ClientResponse( 'get', URL('http://python.org'), continue100=object()) - response._post_init(loop) + response._post_init(loop, session) assert response._continue is not None response.close() -def test_wait_for_100_2(loop): +def test_wait_for_100_2(loop, session): response = ClientResponse( 'get', URL('http://python.org')) - response._post_init(loop) + response._post_init(loop, session) assert response._continue is None response.close() -def test_repr(loop): +def test_repr(loop, session): response = ClientResponse('get', URL('http://def-cl-resp.org')) - response._post_init(loop) + response._post_init(loop, session) response.status = 200 response.reason = 'Ok' assert ''\ @@ -109,9 +114,9 @@ def test_url_obj_deprecated(): @asyncio.coroutine -def test_read_and_release_connection(loop): +def test_read_and_release_connection(loop, session): response = ClientResponse('get', URL('http://def-cl-resp.org')) - response._post_init(loop) + response._post_init(loop, session) def side_effect(*args, **kwargs): fut = helpers.create_future(loop) @@ -126,9 +131,9 @@ def side_effect(*args, **kwargs): @asyncio.coroutine -def test_read_and_release_connection_with_error(loop): +def test_read_and_release_connection_with_error(loop, session): response = ClientResponse('get', URL('http://def-cl-resp.org')) - response._post_init(loop) + response._post_init(loop, session) content = response.content = mock.Mock() content.read.return_value = helpers.create_future(loop) content.read.return_value.set_exception(ValueError) @@ -139,9 +144,9 @@ def test_read_and_release_connection_with_error(loop): @asyncio.coroutine -def test_release(loop): +def test_release(loop, session): response = ClientResponse('get', URL('http://def-cl-resp.org')) - response._post_init(loop) + response._post_init(loop, session) fut = helpers.create_future(loop) fut.set_result(b'') content = response.content = mock.Mock() @@ -152,13 +157,13 @@ def test_release(loop): @asyncio.coroutine -def test_release_on_del(loop): +def test_release_on_del(loop, session): connection = mock.Mock() connection.protocol.upgraded = False def run(conn): response = ClientResponse('get', URL('http://def-cl-resp.org')) - response._post_init(loop) + response._post_init(loop, session) response._closed = False response._connection = conn @@ -168,9 +173,9 @@ def run(conn): @asyncio.coroutine -def test_response_eof(loop): +def test_response_eof(loop, session): response = ClientResponse('get', URL('http://def-cl-resp.org')) - response._post_init(loop) + response._post_init(loop, session) response._closed = False conn = response._connection = mock.Mock() conn.protocol.upgraded = False @@ -181,9 +186,9 @@ def test_response_eof(loop): @asyncio.coroutine -def test_response_eof_upgraded(loop): +def test_response_eof_upgraded(loop, session): response = ClientResponse('get', URL('http://def-cl-resp.org')) - response._post_init(loop) + response._post_init(loop, session) conn = response._connection = mock.Mock() conn.protocol.upgraded = True @@ -194,9 +199,9 @@ def test_response_eof_upgraded(loop): @asyncio.coroutine -def test_response_eof_after_connection_detach(loop): +def test_response_eof_after_connection_detach(loop, session): response = ClientResponse('get', URL('http://def-cl-resp.org')) - response._post_init(loop) + response._post_init(loop, session) response._closed = False conn = response._connection = mock.Mock() conn.protocol = None @@ -207,9 +212,9 @@ def test_response_eof_after_connection_detach(loop): @asyncio.coroutine -def test_text(loop): +def test_text(loop, session): response = ClientResponse('get', URL('http://def-cl-resp.org')) - response._post_init(loop) + response._post_init(loop, session) def side_effect(*args, **kwargs): fut = helpers.create_future(loop) @@ -227,9 +232,9 @@ def side_effect(*args, **kwargs): @asyncio.coroutine -def test_text_bad_encoding(loop): +def test_text_bad_encoding(loop, session): response = ClientResponse('get', URL('http://def-cl-resp.org')) - response._post_init(loop) + response._post_init(loop, session) def side_effect(*args, **kwargs): fut = helpers.create_future(loop) @@ -250,9 +255,9 @@ def side_effect(*args, **kwargs): @asyncio.coroutine -def test_text_custom_encoding(loop): +def test_text_custom_encoding(loop, session): response = ClientResponse('get', URL('http://def-cl-resp.org')) - response._post_init(loop) + response._post_init(loop, session) def side_effect(*args, **kwargs): fut = helpers.create_future(loop) @@ -272,9 +277,9 @@ def side_effect(*args, **kwargs): @asyncio.coroutine -def test_text_detect_encoding(loop): +def test_text_detect_encoding(loop, session): response = ClientResponse('get', URL('http://def-cl-resp.org')) - response._post_init(loop) + response._post_init(loop, session) def side_effect(*args, **kwargs): fut = helpers.create_future(loop) @@ -292,9 +297,9 @@ def side_effect(*args, **kwargs): @asyncio.coroutine -def test_text_after_read(loop): +def test_text_after_read(loop, session): response = ClientResponse('get', URL('http://def-cl-resp.org')) - response._post_init(loop) + response._post_init(loop, session) def side_effect(*args, **kwargs): fut = helpers.create_future(loop) @@ -312,9 +317,9 @@ def side_effect(*args, **kwargs): @asyncio.coroutine -def test_json(loop): +def test_json(loop, session): response = ClientResponse('get', URL('http://def-cl-resp.org')) - response._post_init(loop) + response._post_init(loop, session) def side_effect(*args, **kwargs): fut = helpers.create_future(loop) @@ -332,9 +337,9 @@ def side_effect(*args, **kwargs): @asyncio.coroutine -def test_json_custom_loader(loop): +def test_json_custom_loader(loop, session): response = ClientResponse('get', URL('http://def-cl-resp.org')) - response._post_init(loop) + response._post_init(loop, session) response.headers = { 'Content-Type': 'application/json;charset=cp1251'} response._content = b'data' @@ -347,9 +352,9 @@ def custom(content): @asyncio.coroutine -def test_json_no_content(loop): +def test_json_no_content(loop, session): response = ClientResponse('get', URL('http://def-cl-resp.org')) - response._post_init(loop) + response._post_init(loop, session) response.headers = { 'Content-Type': 'data/octet-stream'} response._content = b'' @@ -364,9 +369,9 @@ def test_json_no_content(loop): @asyncio.coroutine -def test_json_override_encoding(loop): +def test_json_override_encoding(loop, session): response = ClientResponse('get', URL('http://def-cl-resp.org')) - response._post_init(loop) + response._post_init(loop, session) def side_effect(*args, **kwargs): fut = helpers.create_future(loop) @@ -386,19 +391,19 @@ def side_effect(*args, **kwargs): @pytest.mark.xfail -def test_override_flow_control(loop): +def test_override_flow_control(loop, session): class MyResponse(ClientResponse): flow_control_class = aiohttp.StreamReader response = MyResponse('get', URL('http://my-cl-resp.org')) - response._post_init(loop) + response._post_init(loop, session) response._connection = mock.Mock() assert isinstance(response.content, aiohttp.StreamReader) response.close() -def test_get_encoding_unknown(loop): +def test_get_encoding_unknown(loop, session): response = ClientResponse('get', URL('http://def-cl-resp.org')) - response._post_init(loop) + response._post_init(loop, session) response.headers = {'Content-Type': 'application/json'} with mock.patch('aiohttp.client_reqrep.chardet') as m_chardet: From c32af56bb166cde818e0a810d65753ad2d35216a Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Tue, 20 Jun 2017 10:26:11 +0300 Subject: [PATCH 2/2] Reset a reference to session in ClientResponse._cleanup_writer --- aiohttp/client_reqrep.py | 1 + 1 file changed, 1 insertion(+) diff --git a/aiohttp/client_reqrep.py b/aiohttp/client_reqrep.py index b98a00f3032..b1a9939aec7 100644 --- a/aiohttp/client_reqrep.py +++ b/aiohttp/client_reqrep.py @@ -656,6 +656,7 @@ def _cleanup_writer(self): if self._writer is not None and not self._writer.done(): self._writer.cancel() self._writer = None + self._session = None def _notify_content(self): content = self.content