Skip to content

Commit

Permalink
Merge branch 'bugfixes/release-resources-pytest'
Browse files Browse the repository at this point in the history
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
  • Loading branch information
webknjaz committed Feb 24, 2021
2 parents 1ec360f + 3c5aa75 commit 8c82ba1
Show file tree
Hide file tree
Showing 13 changed files with 106 additions and 30 deletions.
4 changes: 4 additions & 0 deletions CHANGES/5494.bugfix
Original file line number Diff line number Diff line change
@@ -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.
3 changes: 3 additions & 0 deletions CHANGES/5494.misc
Original file line number Diff line number Diff line change
@@ -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.
14 changes: 14 additions & 0 deletions aiohttp/web_request.py
Original file line number Diff line number Diff line change
Expand Up @@ -728,6 +728,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
)
Expand Down Expand Up @@ -818,6 +819,19 @@ def _finish(self) -> None:
for fut in self._disconnection_waiters:
fut.cancel()

if self._post is None or self.content_type != "multipart/form-data":
return

# NOTE: Release file descriptors for the
# NOTE: `tempfile.Temporaryfile`-created `_io.BufferedRandom`
# NOTE: instances of files sent within multipart request body
# NOTE: via HTTP POST request.
for file_name, file_field_object in self._post.items():
if not isinstance(file_field_object, FileField):
continue

file_field_object.file.close()

async def wait_for_disconnection(self) -> None:
loop = asyncio.get_event_loop()
fut = loop.create_future() # type: asyncio.Future[None]
Expand Down
5 changes: 5 additions & 0 deletions setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,11 @@ 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. <function _SSLProtocolTransport.__del__ at.:pytest.PytestUnraisableExceptionWarning:_pytest.unraisableexception
ignore:Exception ignored in. <coroutine object BaseConnector.close at 0x.:pytest.PytestUnraisableExceptionWarning:_pytest.unraisableexception
ignore:Exception ignored in. <coroutine object ClientSession._request at 0x.:pytest.PytestUnraisableExceptionWarning:_pytest.unraisableexception
ignore:Exception ignored in. <function ClientSession.__del__ at 0x.:pytest.PytestUnraisableExceptionWarning:_pytest.unraisableexception
ignore:Exception ignored in. <_io.FileIO .closed.>:pytest.PytestUnraisableExceptionWarning:_pytest.unraisableexception
junit_suite_name = aiohttp_test_suite
norecursedirs = dist docs build .tox .eggs
minversion = 3.8.2
Expand Down
2 changes: 2 additions & 0 deletions tests/test_client_request.py
Original file line number Diff line number Diff line change
Expand Up @@ -580,6 +580,7 @@ async def test_content_type_auto_header_get(loop: Any, conn: Any) -> 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: Any, conn: Any) -> None:
Expand Down Expand Up @@ -686,6 +687,7 @@ async def test_pass_falsy_data_file(loop: Any, tmp_path: Any) -> 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
Expand Down
1 change: 1 addition & 0 deletions tests/test_client_response.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ async def test_http_processing_error(session: Any) -> None:
await response.start(connection)

assert info.value.request_info is request_info
response.close()


def test_del(session: Any) -> None:
Expand Down
16 changes: 12 additions & 4 deletions tests/test_client_session.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ async def make_conn():
proto = create_mocked_conn()
conn._conns["a"] = [(proto, 123)]
yield conn
conn.close()
loop.run_until_complete(conn.close())


@pytest.fixture
Expand Down Expand Up @@ -293,7 +293,7 @@ async def test_connector(create_session: Any, loop: Any, mocker: Any) -> None:

await session.close()
assert connector.close.called
connector.close()
await connector.close()


async def test_create_connector(create_session: Any, loop: Any, mocker: Any) -> None:
Expand Down Expand Up @@ -327,7 +327,7 @@ async def make_sess():
)


def test_detach(session: Any) -> None:
def test_detach(loop: Any, session: Any) -> None:
conn = session.connector
try:
assert not conn.closed
Expand All @@ -336,7 +336,7 @@ def test_detach(session: Any) -> None:
assert session.closed
assert not conn.closed
finally:
conn.close()
loop.run_until_complete(conn.close())


async def test_request_closed_session(session: Any) -> None:
Expand Down Expand Up @@ -510,6 +510,7 @@ async def handler(request):
async def test_session_default_version(loop: Any) -> None:
session = aiohttp.ClientSession()
assert session.version == aiohttp.HttpVersion11
await session.close()


def test_proxy_str(session: Any, params: Any) -> None:
Expand Down Expand Up @@ -627,6 +628,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: Any, aiohttp_client: Any
Expand Down Expand Up @@ -669,23 +672,28 @@ async def test_client_session_custom_attr() -> None:
session = ClientSession()
with pytest.raises(AttributeError):
session.custom = None
await session.close()


async def test_client_session_timeout_default_args(loop: Any) -> 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()
8 changes: 7 additions & 1 deletion tests/test_connector.py
Original file line number Diff line number Diff line change
Expand Up @@ -646,7 +646,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
Expand All @@ -655,6 +655,8 @@ def get_extra_info(param):
assert fingerprint_error
assert connected

established_connection.close()


async def test_tcp_connector_resolve_host(loop: Any) -> None:
conn = aiohttp.TCPConnector(use_dns_cache=True)
Expand Down Expand Up @@ -1599,6 +1601,8 @@ async def test_connect_with_limit_cancelled(loop: Any) -> 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: Any) -> None:
def check_with_exc(err):
Expand Down Expand Up @@ -2262,3 +2266,5 @@ async def allow_connection_and_add_dummy_waiter() -> None:
await_connection_and_check_waiters(),
allow_connection_and_add_dummy_waiter(),
)

await connector.close()
4 changes: 4 additions & 0 deletions tests/test_proxy.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,8 @@ async def make_conn():
ssl=None,
)

conn.close()

@mock.patch("aiohttp.connector.ClientRequest")
def test_proxy_headers(self, ClientRequestMock: Any) -> None:
req = ClientRequest(
Expand Down Expand Up @@ -115,6 +117,8 @@ async def make_conn():
ssl=None,
)

conn.close()

def test_proxy_auth(self) -> None:
with self.assertRaises(ValueError) as ctx:
ClientRequest(
Expand Down
34 changes: 18 additions & 16 deletions tests/test_run_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -636,27 +636,29 @@ def test_run_app_multiple_preexisting_sockets(patched_loop: Any) -> 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: Any) -> None:
Expand Down
42 changes: 33 additions & 9 deletions tests/test_web_functional.py
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,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


Expand Down Expand Up @@ -813,12 +814,15 @@ async def handler(request):


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)
Expand All @@ -837,15 +841,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: 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()
Expand All @@ -861,14 +871,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: 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"})

Expand All @@ -884,6 +899,8 @@ async def handler(request):
assert resp.headers.get("Content-Length") == str(len(resp_data))
assert resp.headers.get("Content-Disposition") == 'inline; filename="test.txt"'

outer_file_descriptor.close()


async def test_response_with_payload_stringio(aiohttp_client: Any, fname: Any) -> None:
async def handler(request):
Expand Down Expand Up @@ -1489,6 +1506,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: Any) -> None:
Expand Down Expand Up @@ -1539,11 +1557,12 @@ async def handler(request):

f = tmp_path / "foobar.txt"
f.write_text("test", encoding="utf8")
data = {"file": f.open("rb")}
resp = await client.post("/", data=data)
with f.open("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"])
Expand Down Expand Up @@ -1618,12 +1637,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: Any) -> None:
async def handler(request):
Expand All @@ -1644,6 +1666,8 @@ async def handler(request):

assert len(session._connector._conns) == 1

await session.close()


async def aiohttp_client_api_context_manager(aiohttp_server: Any):
async def handler(request):
Expand Down
2 changes: 2 additions & 0 deletions tests/test_web_request.py
Original file line number Diff line number Diff line change
Expand Up @@ -649,6 +649,8 @@ async def test_multipart_formdata_file(protocol: Any) -> None:
content = result["a_file"].file.read()
assert content == b"\ff"

req._finish()


async def test_make_too_big_request_limit_None(protocol: Any) -> None:
payload = StreamReader(protocol, 2 ** 16, loop=asyncio.get_event_loop())
Expand Down
1 change: 1 addition & 0 deletions tests/test_web_urldispatcher.py
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,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_path: Any, aiohttp_client: Any) -> None:
Expand Down

0 comments on commit 8c82ba1

Please sign in to comment.