pytest fixture that uses TaskGroup fails to teardown when pytest-asyncio is installed

[florimondmanca]

Hey,

First, I wanted to thank you @agronholm for this very well crafted library. I decided to use anyio in asgi-lifespan, mostly to validate the idea of distributing packages built on top of anyio, and see how they perform in non-primarily-anyio environments.

In the context of HTTPX though and this issue in particular: encode/httpx#350, I came across an tricky bug that occurs when pytest-asyncio is installed along with anyio.

(Off topic: this setup is very likely to occur for users that use asyncio as well as a library that uses anyio under the hood (e.g. someone building an asyncio-based ASGI web app and using asgi-lifespan as well). Which makes me thinkperhaps pytest-anyio should be a thing?)

Anyway, here's the bug:

I have this setup:

pip install anyio pytest pytest-asyncio

# tests.py
import anyio
import pytest


@pytest.fixture
async def client():
    async with anyio.create_task_group() as tg:
        yield "client"


async def test_something(client):
    pass

If you run $ pytest tests.py, the test will fail during teardown:

$ pytest tests.py
===================================================== test session starts =====================================================
platform darwin -- Python 3.7.3, pytest-5.1.3, py-1.8.0, pluggy-0.13.0
rootdir: /Users/florimond/Desktop/pytest-anyio-proposal
plugins: asyncio-0.10.0, anyio-1.1.0
collected 1 item                                                                                                              

tests.py sE                                                                                                             [100%]

=========================================================== ERRORS ============================================================
_____________________________________________ ERROR at teardown of test_something _____________________________________________

    @pytest.fixture
    async def client():
        async with anyio.create_task_group() as tg:
>           yield "client"

tests.py:8: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
venv/lib/python3.7/site-packages/anyio/_backends/_asyncio.py:398: in __aexit__
    ignore_exception = await self.cancel_scope.__aexit__(exc_type, exc_val, exc_tb)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <anyio._backends._asyncio.CancelScope object at 0x10a818ba8>, exc_type = None, exc_val = None, exc_tb = None

    async def __aexit__(self, exc_type, exc_val, exc_tb):
        self._active = False
        if self._timeout_task:
            self._timeout_task.cancel()
    
        host_task = current_task()
        print(type(host_task), host_task)
        print(host_task in self._tasks)
>       self._tasks.remove(host_task)
E       KeyError: <unprintable KeyError object>

venv/lib/python3.7/site-packages/anyio/_backends/_asyncio.py:232: KeyError
-------------------------------------------------- Captured stdout teardown ---------------------------------------------------
<class '_asyncio.Task'> <Task pending coro=<pytest_fixture_setup.<locals>.wrapper.<locals>.finalizer.<locals>.async_finalizer() running at /Users/florimond/Desktop/pytest-anyio-proposal/venv/lib/python3.7/site-packages/pytest_asyncio/plugin.py:85> cb=[_run_until_complete_cb() at /Users/florimond/.pyenv/versions/3.7.3/lib/python3.7/asyncio/base_events.py:158]>
False
====================================================== warnings summary =======================================================
tests.py::test_something
  /Users/florimond/Desktop/pytest-anyio-proposal/venv/lib/python3.7/site-packages/_pytest/python.py:160: PytestUnhandledCoroutineWarning: async def functions are not natively supported and have been skipped.
  You need to install a suitable plugin for your async framework, for example:
    - pytest-asyncio
    - pytest-trio
    - pytest-tornasync
    warnings.warn(PytestUnhandledCoroutineWarning(msg.format(pyfuncitem.nodeid)))

-- Docs: https://docs.pytest.org/en/latest/warnings.html
=========================================== 1 skipped, 1 warnings, 1 error in 0.15s ===========================================

The print statements in there are mine — it shows that the task group's cancel scope is trying to remove the host task from its set of tasks, but it's not there, leading to a KeyError when calling .remove().

I looked at what exactly pytest-asyncio is doing for async fixtures, and here's what it looks like (code):

            gen_obj = generator(*args, **kwargs)

            async def setup():
                res = await gen_obj.__anext__()
                return res

            def finalizer():
                """Yield again, to finalize."""
                async def async_finalizer():
                    try:
                        await gen_obj.__anext__()
                    except StopAsyncIteration:
                        pass
                    else:
                        msg = "Async generator fixture didn't stop."
                        msg += "Yield only once."
                        raise ValueError(msg)
                asyncio.get_event_loop().run_until_complete(async_finalizer())

            request.addfinalizer(finalizer)
            return asyncio.get_event_loop().run_until_complete(setup())

So indeed, host_task in CancelScope.__aenter__() and CancelScope.__aexit__() doesn't refer to the same coroutine. In one case it's setup(), in the other it's async_finalizer(). This explains the KeyError — we never added async_finalizer() to self._tasks in the first place.

I tried to bypass the issue using:

if host_task in self._tasks:
    self._tasks.remove(host_task)
    _task_states[host_task].cancel_scope = self._parent_scope
...

But this resulted in teardown hanging indefinitely, because of this part in TaskGroup.__aexit__():

while self.cancel_scope._tasks:
        await asyncio.wait(self.cancel_scope._tasks)

Since we're not removing finished tasks from self.cancel_scope._tasks (e.g. setup() in this case), this loops indefinitely. I managed to fix it using:

while self.cancel_scope._tasks:
    done, pending = await asyncio.wait(self.cancel_scope._tasks)
    for task in done:
        self.cancel_scope._tasks.remove(task)

Not sure this won't break anything else, so I'll happily open a PR with this change to see how it performs against the test suite. 👍

[agronholm]

Indeed the thought of __aenter__() being called from a different host task had not occurred to me. I've created a new test that fails at least for the curio backend, but for some mysterious reason, not for the asyncio backend, even though the KeyError is still raised.

But are you not aware that anyio comes with its own pytest plugin? There's a whole section in the documentation dedicated to that. This is also mentioned in the (very brief) README.

[florimondmanca]

Yeah it's a pretty special situation actually — in asyncio it has to be two separate run_until_complete calls for this bug to occur.

I've created a new test that fails at least for the curio backend, but for some mysterious reason, not for the asyncio backend, even though the KeyError is still raised.

FYI the fix in #75 provides a test that was initially failing in the case I reported (asyncio only). Feel free to take a look — if needed I can investigate curio and trio cases as well (e.g. by running the same setup on pytest-trio and pytest-curio).

But are you not aware that anyio comes with its own pytest plugin?

I am aware of this — I actually use the plugin extensively in asgi-lifespan, and it works great. :-) What I meant by "maybe pytest-anyio could be a thing" was that it could be worth extracting it outside of the anyio, and into a pytest-anyio project that users could optionally install. The motivation is that anyio should still work in a context where e.g. pytest-asyncio is all a user needs (e.g. because they don't care about supporting other libraries).