Fix bug when entering/exiting task group as part of different asyncio tasks

[florimondmanca]

Fixes #74

[agronholm]

I do not expect I will merge this, as I have my own fix underway which also tests all backends, not just asyncio.

[agronholm]

This brings to light a fairly big issue: say task A enters the cancel scope and then hits a yield. Then another task resumes the generator. Next, a spawned task crashes. The host task must now be cancelled, but how do we get the host task now?

[agronholm]

I tested this on trio and it simply crashes with a Cancelled exception when there is no host task to receive the exception.

[agronholm]

Although anyio currently trips on __aexit__() every time, even if that is fixed, any tasks spawned from fixtures will cause undefined behavior if they crash.

[agronholm]

This has been a problem for quite a while: https://discuss.python.org/t/preventing-yield-inside-certain-context-managers/1091/13

[florimondmanca]

Oh, that was interesting, thanks. (The correct link is: https://discuss.python.org/t/preventing-yield-inside-certain-context-managers/1091/13)

any tasks spawned from fixtures will cause undefined behavior if they crash.

It seems it doesn't have to be that way. Along the way I came across these docs on pytest-trio handling of fixtures via @pytest_trio.trio_fixture. What it does is setup fixtures inside the trio.run() call made by the test, instead of outside as pytest-asyncio does. In theory this is a better situation than what I showed in #74, since the task group spawned by the yield fixture would now share the same host task (the same trio.run() call) both on enter and exit.

If I uninstall pytest-asyncio and add pytest-trio and trio instead, and run:

import anyio
import pytest_trio


@pytest_trio.trio_fixture
async def client():
    async with anyio.create_task_group() as tg:
        yield "client"


async def test_something(client):
    pass

Then I get an exception (RuntimeError: Trio fixtures can only be used by Trio tests, which is pytest-trio telling me to add @pytest.mark.trio on top of the test) but exitting the task group went fine.

But as soon as I add pytest-asyncio again, the KeyError problem with host_task comes up again because of how pytest-asyncio eagerly loads fixtures. I think this is all a bug in pytest-asyncio actually — they ought to take into account the issues that come with executing steps of a generator without proper cleanup. I'll probably open an issue there or try to provide a fix myself.

[agronholm]

There is already a bug open against pytest-asyncio about this issue: pytest-dev/pytest-asyncio#124
I may have missed this before, but is there a specific reason you're running the test suite with pytest-asyncio installed?

[smurfix]

Well, I'm not @florimondmanca but on my system I have a heap of normally-benign modules installed so that I can test various stuff while I develop (and thus break) things. Requiring the removal (or reinstallation) of pytest-asyncio is no fun. Worse, I can't test that the module I'm writing works in a "pure" asyncio environment and in one that's running under trio, asyncio+anyio, and/or whatever.

[agronholm]

That is unfortunate, but there is little I can do about the invasive behavior of pytest-asyncio. You can of course use AnyIO's pytest plugin to test your pure async stuff, as it only acts as a light wrapper around asyncio.run().

[florimondmanca]

There is already a bug open against pytest-asyncio about this issue: pytest-dev/pytest-asyncio#124

Nice, thanks. There’s even a pending PR attempting to solve it: pytest-dev/pytest-asyncio#125

I may have missed this before, but is there a specific reason you're running the test suite with pytest-asyncio installed?

I was providing a code snippet to someone who runs on pytest-asyncio, and one of the components in that snippet uses anyio under the hood. I would assume this will be a common situation unless we propagate the need to know about anyio (use the anyio plugin) up to anyone who depends on anything that uses anyio. The compatibility layer at the run function level is looking good already, this is just one edge case that I think needs fixing on the pytest-asyncio side.

[agronholm]

Fixing #74 will at least let people use task groups inside async fixtures. But the handling of exceptions from tasks spawned from those task groups is still a tricky problem.

[agronholm]

There is also the problem of async fixtures with the autouse=True flag set. They may not be bound to any specific test so the pytest plugin may not recognize them as fixtures it needs to handle.

[agronholm]

I now understand the reason why the asyncio test run did not show as failing. This is because asyncio.run() shuts down async generators on exit...including the fixture.

[florimondmanca]

@agronholm Exactly — the fixture gets torn down by running the second part of the async generator, but that fails and so the test is marked as errorred, when it would have ended up being marked as skipped otherwise.

[agronholm]

No, I think you misunderstood. The reason why my test passed on asyncio (even though it shouldn't have) is that the cancel scope is both entered and exited during test setup (due to the semantics of asyncio.run()). The StopIteration / StopAsyncIteration exceptions are ignored.

[agronholm]

I need to look at both trio and curio codebases to see how they handle this.

[agronholm]

I don't think trio shuts down async generators at the end, and neither does curio.
I experimented by calling loop.run_until_complete() directly instead of asyncio.run(). This change allowed the KeyError to pass through as expected.

[florimondmanca]

Is there any branch or PR where the tests you’re mentioning are available to look at? Would help me better follow the discussion here. I thought you were mentioning the original issue in #74. :)

[agronholm]

Closing in favor of #76.