Skip to content

Commit

Permalink
Detect blocking calls in coroutines using BlockBuster (aio-libs#10433)
Browse files Browse the repository at this point in the history
  • Loading branch information
cbornet committed Feb 21, 2025
1 parent be1159b commit 20e98d9
Show file tree
Hide file tree
Showing 11 changed files with 113 additions and 66 deletions.
1 change: 1 addition & 0 deletions CHANGES/10433.feature.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Detect blocking calls in coroutines using BlockBuster -- by :user:`cbornet`.
1 change: 1 addition & 0 deletions CONTRIBUTORS.txt
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ Chris AtLee
Chris Laws
Chris Moore
Chris Shucksmith
Christophe Bornet
Christopher Schmitt
Claudiu Popa
Colin Dunklau
Expand Down
9 changes: 6 additions & 3 deletions aiohttp/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -664,11 +664,14 @@ async def _request(
if req_cookies:
all_cookies.load(req_cookies)

proxy_: Optional[URL] = None
if proxy is not None:
proxy = URL(proxy)
proxy_ = URL(proxy)
elif self._trust_env:
with suppress(LookupError):
proxy, proxy_auth = get_env_proxy_for_url(url)
proxy_, proxy_auth = await asyncio.to_thread(
get_env_proxy_for_url, url
)

req = self._request_class(
method,
Expand All @@ -685,7 +688,7 @@ async def _request(
expect100=expect100,
loop=self._loop,
response_class=self._response_class,
proxy=proxy,
proxy=proxy_,
proxy_auth=proxy_auth,
timer=timer,
session=self,
Expand Down
1 change: 1 addition & 0 deletions requirements/lint.in
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
aiodns
blockbuster
freezegun
mypy; implementation_name == "cpython"
pre-commit
Expand Down
4 changes: 4 additions & 0 deletions requirements/lint.txt
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ annotated-types==0.7.0
# via pydantic
async-timeout==5.0.1
# via valkey
blockbuster==1.5.21
# via -r requirements/lint.in
cffi==1.17.1
# via
# cryptography
Expand All @@ -27,6 +29,8 @@ exceptiongroup==1.2.2
# via pytest
filelock==3.17.0
# via virtualenv
forbiddenfruit==0.1.4
# via blockbuster
freezegun==1.5.1
# via -r requirements/lint.in
identify==2.6.7
Expand Down
1 change: 1 addition & 0 deletions requirements/test.in
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
-r base.in

blockbuster
coverage
freezegun
mypy; implementation_name == "cpython"
Expand Down
4 changes: 4 additions & 0 deletions requirements/test.txt
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ async-timeout==5.0.1 ; python_version < "3.11"
# via -r requirements/runtime-deps.in
attrs==25.1.0
# via -r requirements/runtime-deps.in
blockbuster==1.5.21
# via -r requirements/test.in
brotli==1.1.0 ; platform_python_implementation == "CPython"
# via -r requirements/runtime-deps.in
cffi==1.17.1
Expand All @@ -35,6 +37,8 @@ exceptiongroup==1.2.2
# via pytest
execnet==2.1.1
# via pytest-xdist
forbiddenfruit==0.1.4
# via blockbuster
freezegun==1.5.1
# via -r requirements/test.in
frozenlist==1.5.0
Expand Down
31 changes: 31 additions & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
from uuid import uuid4

import pytest
from blockbuster import blockbuster_ctx

from aiohttp.client_proto import ResponseHandler
from aiohttp.http import WS_KEY
Expand All @@ -33,6 +34,36 @@
IS_LINUX = sys.platform.startswith("linux")


@pytest.fixture(autouse=True)
def blockbuster(request):
# No blockbuster for benchmark tests.
node = request.node.parent
while node:
if node.name.startswith("test_benchmarks"):
yield
return
node = node.parent
with blockbuster_ctx(
"aiohttp", excluded_modules=["aiohttp.pytest_plugin", "aiohttp.test_utils"]
) as bb:
# TODO: Fix blocking call in ClientRequest's constructor.
# https://github.com/aio-libs/aiohttp/issues/10435
for func in ["io.TextIOWrapper.read", "os.stat"]:
bb.functions[func].can_block_in("aiohttp/client_reqrep.py", "update_auth")
for func in [
"os.getcwd",
"os.readlink",
"os.stat",
"os.path.abspath",
"os.path.samestat",
]:
bb.functions[func].can_block_in(
"aiohttp/web_urldispatcher.py", "add_static"
)
bb.functions["os.getcwd"].can_block_in("coverage/control.py", "_should_trace")
yield


@pytest.fixture
def tls_certificate_authority():
if not TRUSTME:
Expand Down
97 changes: 49 additions & 48 deletions tests/test_client_functional.py
Original file line number Diff line number Diff line change
Expand Up @@ -477,7 +477,7 @@ async def test_post_data_with_bytesio_file(aiohttp_client) -> None:
async def handler(request):
post_data = await request.post()
assert ["file"] == list(post_data.keys())
assert data == post_data["file"].file.read()
assert data == await asyncio.to_thread(post_data["file"].file.read)
post_data["file"].file.close() # aiohttp < 4 doesn't autoclose files
return web.Response()

Expand Down Expand Up @@ -1595,14 +1595,14 @@ async def handler(request: web.Request) -> web.Response:


async def test_POST_FILES(aiohttp_client, fname) -> None:
content1 = fname.read_bytes()

async def handler(request):
data = await request.post()
assert data["some"].filename == fname.name
with fname.open("rb") as f:
content1 = f.read()
content2 = data["some"].file.read()
assert content1 == content2
assert data["test"].file.read() == b"data"
content2 = await asyncio.to_thread(data["some"].file.read)
assert content2 == content1
assert await asyncio.to_thread(data["test"].file.read) == b"data"
data["some"].file.close()
data["test"].file.close()
return web.Response()
Expand All @@ -1619,14 +1619,14 @@ async def handler(request):


async def test_POST_FILES_DEFLATE(aiohttp_client, fname) -> None:
content1 = fname.read_bytes()

async def handler(request):
data = await request.post()
assert data["some"].filename == fname.name
with fname.open("rb") as f:
content1 = f.read()
content2 = data["some"].file.read()
content2 = await asyncio.to_thread(data["some"].file.read)
data["some"].file.close()
assert content1 == content2
assert content2 == content1
return web.Response()

app = web.Application()
Expand Down Expand Up @@ -1676,12 +1676,12 @@ async def handler(request):


async def test_POST_FILES_STR(aiohttp_client, fname) -> None:
content1 = fname.read_bytes().decode()

async def handler(request):
data = await request.post()
with fname.open("rb") as f:
content1 = f.read().decode()
content2 = data["some"]
assert content1 == content2
assert content2 == content1
return web.Response()

app = web.Application()
Expand All @@ -1694,11 +1694,11 @@ async def handler(request):


async def test_POST_FILES_STR_SIMPLE(aiohttp_client, fname) -> None:
content = fname.read_bytes()

async def handler(request):
data = await request.read()
with fname.open("rb") as f:
content = f.read()
assert content == data
assert data == content
return web.Response()

app = web.Application()
Expand All @@ -1711,12 +1711,12 @@ async def handler(request):


async def test_POST_FILES_LIST(aiohttp_client, fname) -> None:
content = fname.read_bytes()

async def handler(request):
data = await request.post()
assert fname.name == data["some"].filename
with fname.open("rb") as f:
content = f.read()
assert content == data["some"].file.read()
assert await asyncio.to_thread(data["some"].file.read) == content
data["some"].file.close()
return web.Response()

Expand All @@ -1730,13 +1730,13 @@ async def handler(request):


async def test_POST_FILES_CT(aiohttp_client, fname) -> None:
content = fname.read_bytes()

async def handler(request):
data = await request.post()
assert fname.name == data["some"].filename
assert "text/plain" == data["some"].content_type
with fname.open("rb") as f:
content = f.read()
assert content == data["some"].file.read()
assert await asyncio.to_thread(data["some"].file.read) == content
data["some"].file.close()
return web.Response()

Expand All @@ -1752,11 +1752,11 @@ async def handler(request):


async def test_POST_FILES_SINGLE(aiohttp_client, fname) -> None:
content = fname.read_bytes().decode()

async def handler(request):
data = await request.text()
with fname.open("rb") as f:
content = f.read().decode()
assert content == data
assert data == content
# if system cannot determine 'text/x-python' MIME type
# then use 'application/octet-stream' default
assert request.content_type in [
Expand All @@ -1778,11 +1778,11 @@ async def handler(request):


async def test_POST_FILES_SINGLE_content_disposition(aiohttp_client, fname) -> None:
content = fname.read_bytes().decode()

async def handler(request):
data = await request.text()
with fname.open("rb") as f:
content = f.read().decode()
assert content == data
assert data == content
# if system cannot determine 'application/pgp-keys' MIME type
# then use 'application/octet-stream' default
assert request.content_type in [
Expand All @@ -1808,11 +1808,11 @@ async def handler(request):


async def test_POST_FILES_SINGLE_BINARY(aiohttp_client, fname) -> None:
content = fname.read_bytes()

async def handler(request):
data = await request.read()
with fname.open("rb") as f:
content = f.read()
assert content == data
assert data == content
# if system cannot determine 'application/pgp-keys' MIME type
# then use 'application/octet-stream' default
assert request.content_type in [
Expand All @@ -1835,7 +1835,7 @@ async def handler(request):
async def test_POST_FILES_IO(aiohttp_client) -> None:
async def handler(request):
data = await request.post()
assert b"data" == data["unknown"].file.read()
assert b"data" == await asyncio.to_thread(data["unknown"].file.read)
assert data["unknown"].content_type == "application/octet-stream"
assert data["unknown"].filename == "unknown"
data["unknown"].file.close()
Expand All @@ -1856,7 +1856,7 @@ async def handler(request):
assert data["test"] == "true"
assert data["unknown"].content_type == "application/octet-stream"
assert data["unknown"].filename == "unknown"
assert data["unknown"].file.read() == b"data"
assert await asyncio.to_thread(data["unknown"].file.read) == b"data"
data["unknown"].file.close()
assert data.getall("q") == ["t1", "t2"]

Expand All @@ -1875,6 +1875,8 @@ async def handler(request):


async def test_POST_FILES_WITH_DATA(aiohttp_client, fname) -> None:
content = fname.read_bytes()

async def handler(request):
data = await request.post()
assert data["test"] == "true"
Expand All @@ -1884,9 +1886,8 @@ async def handler(request):
"application/octet-stream",
]
assert data["some"].filename == fname.name
with fname.open("rb") as f:
assert data["some"].file.read() == f.read()
data["some"].file.close()
assert await asyncio.to_thread(data["some"].file.read) == content
data["some"].file.close()

return web.Response()

Expand All @@ -1900,13 +1901,13 @@ async def handler(request):


async def test_POST_STREAM_DATA(aiohttp_client, fname) -> None:
expected = fname.read_bytes()

async def handler(request):
assert request.content_type == "application/octet-stream"
content = await request.read()
with fname.open("rb") as f:
expected = f.read()
assert request.content_length == len(expected)
assert content == expected
assert request.content_length == len(expected)
assert content == expected

return web.Response()

Expand All @@ -1922,10 +1923,10 @@ async def handler(request):
@aiohttp.streamer
async def stream(writer, fname):
with fname.open("rb") as f:
data = f.read(100)
data = await asyncio.to_thread(f.read, 100)
while data:
await writer.write(data)
data = f.read(100)
data = await asyncio.to_thread(f.read, 100)

async with client.post(
"/", data=stream(fname), headers={"Content-Length": str(data_size)}
Expand All @@ -1934,13 +1935,13 @@ async def stream(writer, fname):


async def test_POST_STREAM_DATA_no_params(aiohttp_client, fname) -> None:
expected = fname.read_bytes()

async def handler(request):
assert request.content_type == "application/octet-stream"
content = await request.read()
with fname.open("rb") as f:
expected = f.read()
assert request.content_length == len(expected)
assert content == expected
assert request.content_length == len(expected)
assert content == expected

return web.Response()

Expand All @@ -1956,10 +1957,10 @@ async def handler(request):
@aiohttp.streamer
async def stream(writer):
with fname.open("rb") as f:
data = f.read(100)
data = await asyncio.to_thread(f.read, 100)
while data:
await writer.write(data)
data = f.read(100)
data = await asyncio.to_thread(f.read, 100)

async with client.post(
"/", data=stream, headers={"Content-Length": str(data_size)}
Expand Down
2 changes: 1 addition & 1 deletion tests/test_cookiejar.py
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ async def test_constructor_with_expired(
assert jar._loop is loop


async def test_save_load(tmp_path, loop, cookies_to_send, cookies_to_receive) -> None:
def test_save_load(tmp_path, loop, cookies_to_send, cookies_to_receive) -> None:
file_path = pathlib.Path(str(tmp_path)) / "aiohttp.test.cookie"

# export cookie jar
Expand Down
Loading

0 comments on commit 20e98d9

Please sign in to comment.