Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Detect blocking calls in coroutines using BlockBuster #10433

Merged
merged 9 commits into from
Feb 20, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -82,6 +82,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 @@ -607,11 +607,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 @@ -628,7 +631,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 @@ -14,6 +14,8 @@ annotated-types==0.7.0
# via pydantic
async-timeout==5.0.1 ; python_version < "3.11"
# 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 @@ -33,6 +35,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: pytest.FixtureRequest) -> Iterator[None]:
# 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() -> trustme.CA:
if not TRUSTME:
Expand Down
85 changes: 43 additions & 42 deletions tests/test_client_functional.py
Original file line number Diff line number Diff line change
Expand Up @@ -507,7 +507,7 @@ async def handler(request: web.Request) -> web.Response:
assert ["file"] == list(post_data.keys())
file_field = post_data["file"]
assert isinstance(file_field, web.FileField)
assert data == file_field.file.read()
assert data == await asyncio.to_thread(file_field.file.read)
return web.Response()

app = web.Application()
Expand Down Expand Up @@ -1633,16 +1633,16 @@ async def handler(request: web.Request) -> web.Response:


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

async def handler(request: web.Request) -> web.Response:
data = await request.post()
assert isinstance(data["some"], web.FileField)
assert data["some"].filename == fname.name
with fname.open("rb") as f:
content1 = f.read()
content2 = data["some"].file.read()
assert content1 == content2
content2 = await asyncio.to_thread(data["some"].file.read)
assert content2 == content1
assert isinstance(data["test"], web.FileField)
assert data["test"].file.read() == b"data"
assert await asyncio.to_thread(data["test"].file.read) == b"data"
assert isinstance(data["some"], web.FileField)
data["some"].file.close()
data["test"].file.close()
Expand All @@ -1662,15 +1662,15 @@ async def handler(request: web.Request) -> web.Response:
async def test_POST_FILES_DEFLATE(
aiohttp_client: AiohttpClient, fname: pathlib.Path
) -> None:
content1 = fname.read_bytes()

async def handler(request: web.Request) -> web.Response:
data = await request.post()
assert isinstance(data["some"], web.FileField)
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 @@ -1722,12 +1722,12 @@ async def handler(request: web.Request) -> web.Response:
async def test_POST_FILES_STR(
aiohttp_client: AiohttpClient, fname: pathlib.Path
) -> None:
content1 = fname.read_bytes().decode()

async def handler(request: web.Request) -> web.Response:
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 @@ -1742,11 +1742,11 @@ async def handler(request: web.Request) -> web.Response:
async def test_POST_FILES_STR_SIMPLE(
aiohttp_client: AiohttpClient, fname: pathlib.Path
) -> None:
content = fname.read_bytes()

async def handler(request: web.Request) -> web.Response:
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 @@ -1761,13 +1761,13 @@ async def handler(request: web.Request) -> web.Response:
async def test_POST_FILES_LIST(
aiohttp_client: AiohttpClient, fname: pathlib.Path
) -> None:
content = fname.read_bytes()

async def handler(request: web.Request) -> web.Response:
data = await request.post()
assert isinstance(data["some"], web.FileField)
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 @@ -1783,14 +1783,14 @@ async def handler(request: web.Request) -> web.Response:
async def test_POST_FILES_CT(
aiohttp_client: AiohttpClient, fname: pathlib.Path
) -> None:
content = fname.read_bytes()

async def handler(request: web.Request) -> web.Response:
data = await request.post()
assert isinstance(data["some"], web.FileField)
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 @@ -1808,11 +1808,11 @@ async def handler(request: web.Request) -> web.Response:
async def test_POST_FILES_SINGLE(
aiohttp_client: AiohttpClient, fname: pathlib.Path
) -> None:
content = fname.read_bytes().decode()

async def handler(request: web.Request) -> web.Response:
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 @@ -1836,11 +1836,11 @@ async def handler(request: web.Request) -> web.Response:
async def test_POST_FILES_SINGLE_content_disposition(
aiohttp_client: AiohttpClient, fname: pathlib.Path
) -> None:
content = fname.read_bytes().decode()

async def handler(request: web.Request) -> web.Response:
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 Down Expand Up @@ -1868,11 +1868,11 @@ async def handler(request: web.Request) -> web.Response:
async def test_POST_FILES_SINGLE_BINARY(
aiohttp_client: AiohttpClient, fname: pathlib.Path
) -> None:
content = fname.read_bytes()

async def handler(request: web.Request) -> web.Response:
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 @@ -1896,7 +1896,7 @@ async def test_POST_FILES_IO(aiohttp_client: AiohttpClient) -> None:
async def handler(request: web.Request) -> web.Response:
data = await request.post()
assert isinstance(data["unknown"], web.FileField)
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 @@ -1918,7 +1918,7 @@ async def handler(request: web.Request) -> web.Response:
assert isinstance(data["unknown"], web.FileField)
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 @@ -1939,6 +1939,8 @@ async def handler(request: web.Request) -> web.Response:
async def test_POST_FILES_WITH_DATA(
aiohttp_client: AiohttpClient, fname: pathlib.Path
) -> None:
content = fname.read_bytes()

async def handler(request: web.Request) -> web.Response:
data = await request.post()
assert data["test"] == "true"
Expand All @@ -1949,9 +1951,8 @@ async def handler(request: web.Request) -> web.Response:
"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 @@ -1967,13 +1968,13 @@ async def handler(request: web.Request) -> web.Response:
async def test_POST_STREAM_DATA(
aiohttp_client: AiohttpClient, fname: pathlib.Path
) -> None:
expected = fname.read_bytes()

async def handler(request: web.Request) -> web.Response:
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 @@ -1986,10 +1987,10 @@ async def handler(request: web.Request) -> web.Response:

async def gen(fname: pathlib.Path) -> AsyncIterator[bytes]:
with fname.open("rb") as f:
data = f.read(100)
data = await asyncio.to_thread(f.read, 100)
while data:
yield data
data = f.read(100)
data = await asyncio.to_thread(f.read, 100)

async with client.post(
"/", data=gen(fname), 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 @@ -185,7 +185,7 @@ async def test_constructor_with_expired(
assert jar_cookies != expected_cookies


async def test_save_load(
def test_save_load(
tmp_path: Path,
loop: asyncio.AbstractEventLoop,
cookies_to_send: SimpleCookie,
Expand Down
Loading
Loading