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

Task finalizers run outside of event loop #878

Open
CGamesPlay opened this issue Jul 15, 2024 · 3 comments
Open

Task finalizers run outside of event loop #878

CGamesPlay opened this issue Jul 15, 2024 · 3 comments
Labels
Milestone

Comments

@CGamesPlay
Copy link

CGamesPlay commented Jul 15, 2024

Something about how pytest-asyncio is handling finalizers isn't working right. When a task is abandoned by a test, the task's finalizers get run outside of the event loop, meaning any async cleanup will not be run. The following test fails with pytest-asyncio, but works fine if you switch to using asyncio.run.

import asyncio
import pytest

async def func():
    try:
        await asyncio.sleep(0.1)
    finally:
        assert asyncio.current_task() is not None

@pytest.mark.asyncio
async def test_it():
    asyncio.create_task(func())

I encountered this issue because my cleanup code was calling ContextVar.reset, and it would emit a warning (shown below), and I believe this is the root cause.

ValueError: <Token var=<ContextVar name='var' at 0x106a40a40> at 0x106a74240> was created in a different Context

Python 3.11.9
Pytest 8.2.2
pytest-asyncio 0.23.7

@seifertm seifertm added the bug label Jul 15, 2024
@seifertm seifertm added this to the v1.0 milestone Jul 15, 2024
@seifertm
Copy link
Contributor

Thanks for the report and the reproducer!

asyncio.run has dedicated logic to get rid of all remaining tasks when the asyncio.Runner is closed (see https://github.com/python/cpython/blob/3.12/Lib/asyncio/runners.py). Although we plan to do this in the future, pytest-asyncio doesn't use asyncio.Runner or asyncio.run at the moment. The logic to clean up the remaining tasks is not accessible via public API, either (see #309).

I don't think we can address this issue anytime soon.

@CGamesPlay
Copy link
Author

CGamesPlay commented Jul 16, 2024

Thanks for the response!

Honestly, up until I discovered this bug, I just assumed that pytest-asyncio was a simple wrapper around asyncio.run. Now I understand the internals a bit better I see that it isn't, but my question is: why isn't it? What does a user gain by the additional complexity, and in what circumstances would a user be wiser to just wrap their tests in asyncio.run? It might be helpful to add the answers to this question in the documentation somewhere as well.

@seifertm
Copy link
Contributor

Thanks for the response!

Honestly, up until I discovered this bug, I just assumed that pytest-asyncio was a simple wrapper around asyncio.run. Now I understand the internals a bit better I see that it isn't, but my question is: why isn't it? What does a user gain by the additional complexity, and in what circumstances would a user be wiser to just wrap their tests in asyncio.run? It might be helpful to add the answers to this question in the documentation somewhere as well.

I think the straightforward reason is legacy. Pytest-asyncio has been around for some time and the asyncio API is evolving. A lot of the low-level asyncio calls used by pytest-asyncio have been (partially) deprecated.

The current plan for pytest-asyncio is to move to using asyncio.Runner internally. This should replace the deprecated calls and open up new possibilities, such as correct handling of contextvars.

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

No branches or pull requests

2 participants