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 #875

Merged
merged 11 commits into from
Mar 12, 2025

Conversation

cbornet
Copy link
Contributor

@cbornet cbornet commented Feb 21, 2025

Changes

This PR uses the blockbuster library to detect blocking calls made in the asyncio event loop during unit tests.
Avoiding blocking calls is hard as these can be deeply buried in the code or made in 3rd party libraries.
Blockbuster makes it easier to detect them by raising an exception when a call is made to a known blocking function (eg: time.sleep).

Checklist

If this is a user-facing code change, like a bugfix or a new feature, please ensure that
you've fulfilled the following conditions (where applicable):

  • You've added tests (in tests/) added which would fail without your patch
  • You've updated the documentation (in docs/, in case of behavior changes or new
    features)
  • You've added a new changelog entry (in docs/versionhistory.rst).

If there's no issue linked, just link to your pull request instead by updating the
changelog after you've created the PR.

Sorry, something went wrong.

@cbornet
Copy link
Contributor Author

cbornet commented Feb 21, 2025

@agronholm about your question if it would have detected the iterdir issue: it would have.
I did on my local with Python 3.13 venv

git revert 38885f9
pytest "tests/test_fileio.py::TestPath::test_iterdir[asyncio]"
=================================================================== test session starts ====================================================================
platform darwin -- Python 3.13.2, pytest-8.3.4, pluggy-1.5.0
rootdir: /Users/christophe.bornet/workspace/anyio
configfile: pyproject.toml
plugins: anyio-0.0.post1087+dirty, hypothesis-6.126.0
collected 1 item

tests/test_fileio.py F                                                                                                                               [100%]

========================================================================= FAILURES =========================================================================
______________________________________________________________ TestPath.test_iterdir[asyncio] ______________________________________________________________
tests/test_fileio.py:409: in test_iterdir
    async for path in Path(populated_tmpdir).iterdir():
src/anyio/_core/_fileio.py:548: in iterdir
    gen = self._path.iterdir()
../../.pyenv/versions/3.13.2/lib/python3.13/pathlib/_local.py:576: in iterdir
    paths = [entry.path for entry in scandir_it]
.venv/lib/python3.13/site-packages/blockbuster/blockbuster.py:109: in wrapper
    raise BlockingError(func_name)
E   blockbuster.blockbuster.BlockingError: Blocking call to ScandirIterator.__next__

@agronholm
Copy link
Owner

There are test failures still. I'll take another look after you've fixed them.

@agronholm
Copy link
Owner

There are test failures still. I'll take another look after you've fixed them.

Oh, sorry, these are caused by the calls you asked about. Never mind.

@cbornet cbornet force-pushed the blockbuster branch 2 times, most recently from 12c0579 to b44ce08 Compare February 24, 2025 11:18
@agronholm
Copy link
Owner

By the way, I suggest you refrain from force-pushing. It makes it harder to track the incremental changes you're making.

@cbornet cbornet force-pushed the blockbuster branch 2 times, most recently from bd34b7d to 53dc8e8 Compare February 24, 2025 12:34
@agronholm
Copy link
Owner

Are you sure you're not adding exemptions for legit errors reported by this tool?

@cbornet
Copy link
Contributor Author

cbornet commented Feb 24, 2025

By the way, I suggest you refrain from force-pushing. It makes it harder to track the incremental changes you're making.

Sorry about that. Some commit/pushs I have done were to figure out how to fix. I was using the CI to test because I don't have a Windows machine to test locally.

@cbornet
Copy link
Contributor Author

cbornet commented Feb 24, 2025

Are you sure you're not adding exemptions for legit errors reported by this tool?

Yes. These errors are maybe legit reports. I've put them as exemptions for now so we can discuss how to fix them.

agronholm and others added 4 commits March 1, 2025 03:09

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@agronholm agronholm merged commit d228020 into agronholm:master Mar 12, 2025
17 checks passed
@agronholm
Copy link
Owner

Thanks!

@cbornet cbornet deleted the blockbuster branch March 12, 2025 16:38
@cbornet
Copy link
Contributor Author

cbornet commented Mar 12, 2025

Awesome, thanks !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants