From fe5e684a8f81f3a8db69a8c063641b7d1da94f61 Mon Sep 17 00:00:00 2001 From: Sviatoslav Sydorenko Date: Wed, 24 Feb 2021 12:25:25 +0100 Subject: [PATCH] Merge branch 'bugfixes/release-resources-pytest' This change makes sure to release most of the hanging resources in the lib and tests. They were detected by pytest v6.2+. PR #5494 (cherry picked from commit 8c82ba11b9e38851d75476d261a1442402cc7592) --- CHANGES/5494.bugfix | 4 +++ CHANGES/5494.misc | 3 ++ aiohttp/web_request.py | 1 + requirements/dev.txt | 2 +- requirements/lint.txt | 2 +- requirements/test.txt | 2 +- setup.cfg | 9 ++++++ tests/test_client_request.py | 2 ++ tests/test_client_response.py | 1 + tests/test_client_session.py | 18 +++++++++--- tests/test_connector.py | 8 +++++- tests/test_proxy.py | 4 +++ tests/test_run_app.py | 34 ++++++++++++----------- tests/test_web_functional.py | 49 +++++++++++++++++++++++++-------- tests/test_web_urldispatcher.py | 1 + 15 files changed, 104 insertions(+), 36 deletions(-) create mode 100644 CHANGES/5494.bugfix create mode 100644 CHANGES/5494.misc diff --git a/CHANGES/5494.bugfix b/CHANGES/5494.bugfix new file mode 100644 index 00000000000..449b6bdf3d6 --- /dev/null +++ b/CHANGES/5494.bugfix @@ -0,0 +1,4 @@ +Fixed the multipart POST requests processing to always release file +descriptors for the ``tempfile.Temporaryfile``-created +``_io.BufferedRandom`` instances of files sent within multipart request +bodies via HTTP POST requests. diff --git a/CHANGES/5494.misc b/CHANGES/5494.misc new file mode 100644 index 00000000000..3d83a77a033 --- /dev/null +++ b/CHANGES/5494.misc @@ -0,0 +1,3 @@ +Made sure to always close most of file descriptors and release other +resouces in tests. Started ignoring ``ResourceWarning``s in pytest for +warnings that are hard to track. diff --git a/aiohttp/web_request.py b/aiohttp/web_request.py index a7f2ee66412..b0d1e3587b4 100644 --- a/aiohttp/web_request.py +++ b/aiohttp/web_request.py @@ -665,6 +665,7 @@ async def post(self) -> "MultiDictProxy[Union[str, bytes, FileField]]": tmp.write(chunk) size += len(chunk) if 0 < max_size < size: + tmp.close() raise HTTPRequestEntityTooLarge( max_size=max_size, actual_size=size ) diff --git a/requirements/dev.txt b/requirements/dev.txt index 6c3e3f33d68..30021369936 100644 --- a/requirements/dev.txt +++ b/requirements/dev.txt @@ -160,7 +160,7 @@ pytest-cov==2.10.1 # via -r requirements/test.txt pytest-mock==3.5.1 # via -r requirements/test.txt -pytest==6.1.2 +pytest==6.2.2 # via # -r requirements/lint.txt # -r requirements/test.txt diff --git a/requirements/lint.txt b/requirements/lint.txt index 71ddfd1f8b0..c180c150f0c 100644 --- a/requirements/lint.txt +++ b/requirements/lint.txt @@ -4,4 +4,4 @@ flake8-pyi==20.10.0 isort==5.6.4 mypy==0.790; implementation_name=="cpython" pre-commit==2.9.3 -pytest==6.1.2 +pytest==6.2.2 diff --git a/requirements/test.txt b/requirements/test.txt index 285636ecce9..360d7aade7a 100644 --- a/requirements/test.txt +++ b/requirements/test.txt @@ -5,7 +5,7 @@ cryptography==3.3.1; platform_machine!="i686" and python_version<"3.9" # no 32-b freezegun==1.0.0 mypy==0.790; implementation_name=="cpython" mypy-extensions==0.4.3; implementation_name=="cpython" -pytest==6.1.2 +pytest==6.2.2 pytest-cov==2.10.1 pytest-mock==3.5.1 re-assert==1.1.0 diff --git a/setup.cfg b/setup.cfg index a2420b14d15..91d8bdef461 100644 --- a/setup.cfg +++ b/setup.cfg @@ -53,6 +53,15 @@ addopts = filterwarnings = error ignore:module 'ssl' has no attribute 'OP_NO_COMPRESSION'. The Python interpreter is compiled against OpenSSL < 1.0.0. Ref. https.//docs.python.org/3/library/ssl.html#ssl.OP_NO_COMPRESSION:UserWarning + ignore:Exception ignored in. :pytest.PytestUnraisableExceptionWarning:_pytest.unraisableexception + + # aiohttp 3.x: + ignore:Exception ignored in. :pytest.PytestUnraisableExceptionWarning:_pytest.unraisableexception + ignore:Exception ignored in. None: resp = await req.send(conn) assert "CONTENT-TYPE" not in req.headers resp.close() + await req.close() async def test_content_type_auto_header_form(loop, conn) -> None: @@ -715,6 +716,7 @@ async def test_pass_falsy_data_file(loop, tmpdir) -> None: ) assert req.headers.get("CONTENT-LENGTH", None) is not None await req.close() + testfile.close() # Elasticsearch API requires to send request body with GET-requests diff --git a/tests/test_client_response.py b/tests/test_client_response.py index 1798c5b5d16..f8bee42be49 100644 --- a/tests/test_client_response.py +++ b/tests/test_client_response.py @@ -46,6 +46,7 @@ async def test_http_processing_error(session) -> None: await response.start(connection) assert info.value.request_info is request_info + response.close() def test_del(session) -> None: diff --git a/tests/test_client_session.py b/tests/test_client_session.py index ab51d068797..937b75ff2ee 100644 --- a/tests/test_client_session.py +++ b/tests/test_client_session.py @@ -5,6 +5,7 @@ import sys from http.cookies import SimpleCookie from io import BytesIO +from typing import Any from unittest import mock import pytest @@ -30,7 +31,7 @@ async def make_conn(): proto = mock.Mock() conn._conns["a"] = [(proto, 123)] yield conn - conn.close() + loop.run_until_complete(conn.close()) @pytest.fixture @@ -292,7 +293,7 @@ async def test_connector(create_session, loop, mocker) -> None: await session.close() assert connector.close.called - connector.close() + await connector.close() async def test_create_connector(create_session, loop, mocker) -> None: @@ -327,7 +328,7 @@ async def make_sess(): ) -def test_detach(session) -> None: +def test_detach(loop: Any, session: Any) -> None: conn = session.connector try: assert not conn.closed @@ -336,7 +337,7 @@ def test_detach(session) -> None: assert session.closed assert not conn.closed finally: - conn.close() + loop.run_until_complete(conn.close()) async def test_request_closed_session(session) -> None: @@ -514,6 +515,7 @@ async def handler(request): async def test_session_default_version(loop) -> None: session = aiohttp.ClientSession(loop=loop) assert session.version == aiohttp.HttpVersion11 + await session.close() async def test_session_loop(loop) -> None: @@ -639,6 +641,8 @@ async def test_request_tracing_exception() -> None: ) assert not on_request_end.called + await session.close() + async def test_request_tracing_interpose_headers(loop, aiohttp_client) -> None: async def handler(request): @@ -681,6 +685,7 @@ async def test_client_session_custom_attr(loop) -> None: session = ClientSession(loop=loop) with pytest.warns(DeprecationWarning): session.custom = None + await session.close() async def test_client_session_timeout_args(loop) -> None: @@ -705,21 +710,25 @@ async def test_client_session_timeout_args(loop) -> None: async def test_client_session_timeout_default_args(loop) -> None: session1 = ClientSession() assert session1.timeout == client.DEFAULT_TIMEOUT + await session1.close() async def test_client_session_timeout_argument() -> None: session = ClientSession(timeout=500) assert session.timeout == 500 + await session.close() async def test_requote_redirect_url_default() -> None: session = ClientSession() assert session.requote_redirect_url + await session.close() async def test_requote_redirect_url_default_disable() -> None: session = ClientSession(requote_redirect_url=False) assert not session.requote_redirect_url + await session.close() async def test_requote_redirect_setter() -> None: @@ -728,3 +737,4 @@ async def test_requote_redirect_setter() -> None: with pytest.warns(DeprecationWarning): session.requote_redirect_url = False assert not session.requote_redirect_url + await session.close() diff --git a/tests/test_connector.py b/tests/test_connector.py index 09841923e16..f9034f6dba7 100644 --- a/tests/test_connector.py +++ b/tests/test_connector.py @@ -657,7 +657,7 @@ def get_extra_info(param): conn._loop.create_connection = create_connection - await conn.connect(req, [], ClientTimeout()) + established_connection = await conn.connect(req, [], ClientTimeout()) assert ips == ips_tried assert os_error @@ -666,6 +666,8 @@ def get_extra_info(param): assert fingerprint_error assert connected + established_connection.close() + async def test_tcp_connector_resolve_host(loop) -> None: conn = aiohttp.TCPConnector(loop=loop, use_dns_cache=True) @@ -1595,6 +1597,8 @@ async def test_connect_with_limit_cancelled(loop) -> None: await asyncio.wait_for(conn.connect(req, None, ClientTimeout()), 0.01) connection.close() + await conn.close() + async def test_connect_with_capacity_release_waiters(loop) -> None: def check_with_exc(err): @@ -2260,3 +2264,5 @@ async def allow_connection_and_add_dummy_waiter(): await_connection_and_check_waiters(), allow_connection_and_add_dummy_waiter(), ) + + await connector.close() diff --git a/tests/test_proxy.py b/tests/test_proxy.py index 3b1bf0c052a..541783153de 100644 --- a/tests/test_proxy.py +++ b/tests/test_proxy.py @@ -72,6 +72,8 @@ async def make_conn(): ssl=None, ) + conn.close() + @mock.patch("aiohttp.connector.ClientRequest") def test_proxy_headers(self, ClientRequestMock) -> None: req = ClientRequest( @@ -112,6 +114,8 @@ async def make_conn(): ssl=None, ) + conn.close() + def test_proxy_auth(self) -> None: with self.assertRaises(ValueError) as ctx: ClientRequest( diff --git a/tests/test_run_app.py b/tests/test_run_app.py index 74e951cd11a..7fa04100b39 100644 --- a/tests/test_run_app.py +++ b/tests/test_run_app.py @@ -631,27 +631,29 @@ def test_run_app_multiple_preexisting_sockets(patched_loop) -> None: def test_sigint() -> None: skip_if_on_windows() - proc = subprocess.Popen( - [sys.executable, "-u", "-c", _script_test_signal], stdout=subprocess.PIPE - ) - for line in proc.stdout: - if line.startswith(b"======== Running on"): - break - proc.send_signal(signal.SIGINT) - assert proc.wait() == 0 + with subprocess.Popen( + [sys.executable, "-u", "-c", _script_test_signal], + stdout=subprocess.PIPE, + ) as proc: + for line in proc.stdout: + if line.startswith(b"======== Running on"): + break + proc.send_signal(signal.SIGINT) + assert proc.wait() == 0 def test_sigterm() -> None: skip_if_on_windows() - proc = subprocess.Popen( - [sys.executable, "-u", "-c", _script_test_signal], stdout=subprocess.PIPE - ) - for line in proc.stdout: - if line.startswith(b"======== Running on"): - break - proc.terminate() - assert proc.wait() == 0 + with subprocess.Popen( + [sys.executable, "-u", "-c", _script_test_signal], + stdout=subprocess.PIPE, + ) as proc: + for line in proc.stdout: + if line.startswith(b"======== Running on"): + break + proc.terminate() + assert proc.wait() == 0 def test_startup_cleanup_signals_even_on_failure(patched_loop) -> None: diff --git a/tests/test_web_functional.py b/tests/test_web_functional.py index 8c4ff103298..adf1528893f 100644 --- a/tests/test_web_functional.py +++ b/tests/test_web_functional.py @@ -4,6 +4,7 @@ import pathlib import socket import zlib +from typing import Any from unittest import mock import pytest @@ -324,7 +325,8 @@ async def handler(request): fname = here / "data.unknown_mime_type" - resp = await client.post("/", data=[fname.open("rb")]) + with fname.open("rb") as fd: + resp = await client.post("/", data=[fd]) assert 200 == resp.status @@ -874,13 +876,16 @@ async def handler(request): assert resp.headers.get("Content-Length") == str(len(resp_data)) -async def test_response_with_file(aiohttp_client, fname) -> None: +async def test_response_with_file(aiohttp_client: Any, fname: Any) -> None: + outer_file_descriptor = None with fname.open("rb") as f: data = f.read() async def handler(request): - return web.Response(body=fname.open("rb")) + nonlocal outer_file_descriptor + outer_file_descriptor = fname.open("rb") + return web.Response(body=outer_file_descriptor) app = web.Application() app.router.add_get("/", handler) @@ -901,15 +906,21 @@ async def handler(request): assert resp.headers.get("Content-Length") == str(len(resp_data)) assert resp.headers.get("Content-Disposition") == expected_content_disposition + outer_file_descriptor.close() -async def test_response_with_file_ctype(aiohttp_client, fname) -> None: + +async def test_response_with_file_ctype(aiohttp_client: Any, fname: Any) -> None: + outer_file_descriptor = None with fname.open("rb") as f: data = f.read() async def handler(request): + nonlocal outer_file_descriptor + outer_file_descriptor = fname.open("rb") + return web.Response( - body=fname.open("rb"), headers={"content-type": "text/binary"} + body=outer_file_descriptor, headers={"content-type": "text/binary"} ) app = web.Application() @@ -927,14 +938,19 @@ async def handler(request): assert resp.headers.get("Content-Length") == str(len(resp_data)) assert resp.headers.get("Content-Disposition") == expected_content_disposition + outer_file_descriptor.close() + -async def test_response_with_payload_disp(aiohttp_client, fname) -> None: +async def test_response_with_payload_disp(aiohttp_client: Any, fname: Any) -> None: + outer_file_descriptor = None with fname.open("rb") as f: data = f.read() async def handler(request): - pl = aiohttp.get_payload(fname.open("rb")) + nonlocal outer_file_descriptor + outer_file_descriptor = fname.open("rb") + pl = aiohttp.get_payload(outer_file_descriptor) pl.set_content_disposition("inline", filename="test.txt") return web.Response(body=pl, headers={"content-type": "text/binary"}) @@ -953,6 +969,8 @@ async def handler(request): == "inline; filename=\"test.txt\"; filename*=utf-8''test.txt" ) + outer_file_descriptor.close() + async def test_response_with_payload_stringio(aiohttp_client, fname) -> None: async def handler(request): @@ -1565,6 +1583,7 @@ async def handler(request): assert ( "Maximum request body size 10 exceeded, " "actual body size 1024" in resp_text ) + data["file"].close() async def test_post_max_client_size_for_file(aiohttp_client) -> None: @@ -1618,11 +1637,12 @@ async def handler(request): f = tmpdir.join("foobar.txt") f.write_text("test", encoding="utf8") - data = {"file": open(str(f), "rb")} - resp = await client.post("/", data=data) + with open(str(f), "rb") as fd: + data = {"file": fd} + resp = await client.post("/", data=data) - assert 200 == resp.status - body = await resp.read() + assert 200 == resp.status + body = await resp.read() assert body == b"test" disp = multipart.parse_content_disposition(resp.headers["content-disposition"]) @@ -1700,12 +1720,15 @@ async def handler(request): app = web.Application() app.router.add_route("GET", "/", handler) server = await aiohttp_server(app) - resp = await aiohttp.ClientSession().get(server.make_url("/")) + session = aiohttp.ClientSession() + resp = await session.get(server.make_url("/")) async with resp: assert resp.status == 200 assert resp.connection is None assert resp.connection is None + await session.close() + async def test_response_context_manager_error(aiohttp_server) -> None: async def handler(request): @@ -1726,6 +1749,8 @@ async def handler(request): assert len(session._connector._conns) == 1 + await session.close() + async def aiohttp_client_api_context_manager(aiohttp_server): async def handler(request): diff --git a/tests/test_web_urldispatcher.py b/tests/test_web_urldispatcher.py index 0ba2e7c2034..b45a27bd38d 100644 --- a/tests/test_web_urldispatcher.py +++ b/tests/test_web_urldispatcher.py @@ -152,6 +152,7 @@ async def test_access_to_the_file_with_spaces( r = await client.get(url) assert r.status == 200 assert (await r.text()) == data + await r.release() async def test_access_non_existing_resource(tmp_dir_path, aiohttp_client) -> None: