Detect blocking calls in coroutines using BlockBuster (#10433)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

CHANGES/10433.feature.rst

@@ -0,0 +1 @@
+Detect blocking calls in coroutines using BlockBuster -- by :user:`cbornet`.

CONTRIBUTORS.txt

@@ -82,6 +82,7 @@ Chris AtLee
 Chris Laws
 Chris Moore
 Chris Shucksmith
+Christophe Bornet
 Christopher Schmitt
 Claudiu Popa
 Colin Dunklau

aiohttp/client.py

@@ -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,
@@ -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,

requirements/lint.in

@@ -1,4 +1,5 @@
 aiodns
+blockbuster
 freezegun
 mypy; implementation_name == "cpython"
 pre-commit

requirements/lint.txt

@@ -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
@@ -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

requirements/test.in

@@ -1,5 +1,6 @@
 -r base.in
 
+blockbuster
 coverage
 freezegun
 mypy; implementation_name == "cpython"

requirements/test.txt

@@ -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
@@ -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

tests/conftest.py

@@ -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
@@ -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:

tests/test_client_functional.py

@@ -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()
@@ -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()
@@ -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()
@@ -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()
@@ -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()
@@ -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()
 
@@ -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()
 
@@ -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 [
@@ -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 [
@@ -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 [
@@ -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()
@@ -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"]
 
@@ -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"
@@ -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()
 
@@ -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()
 
@@ -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)}

tests/test_cookiejar.py

@@ -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,