python-trio / trio-asyncio

a re-implementation of the asyncio mainloop on top of Trio
Other
189 stars 38 forks source link

Detect when nested nursery and asyncio loop end up in silent jamming #34

Closed touilleMan closed 6 years ago

touilleMan commented 6 years ago

Considering the following code:

@pytest.fixture
async def nursery2():
    async with trio.open_nursery() as nursery:
        yield nursery

@pytest.fixture
async def asyncio_loop2(nursery2):
    async with trio_asyncio.open_loop() as loop:
        yield loop

@pytest.mark.trio
async def test_wrong_fixture_teardown_order(
        nursery2, asyncio_loop2, postgresql_connection_specs
):

    async def db_runner(*, task_status=trio.TASK_STATUS_IGNORED):
        async with triopg.connect(**postgresql_connection_specs):
            task_status.started()

    await nursery2.start(db_runner)

Running this test will hang forever (dosn't even react to ^C)

The trouble is asyncio_loop2 will be teardown before nursery2 given it depends on it. But given nursery2 contains a coroutine that will call trio-asyncio it end up in a weird jammed state.

This issue also occurs with when nesting nursery and asyncio_loop in the wrong order, though there is less magic involve so it's much easier to realize the issue than with pytest trio-flavored fixtures:

@pytest.mark.trio
async def test_wrong_context_manager_order(
        postgresql_connection_specs
):
    async def db_runner(*, task_status=trio.TASK_STATUS_IGNORED):
        async with triopg.connect(**postgresql_connection_specs):
            task_status.started()

    async with trio.open_nursery() as nursery:
        async with trio_asyncio.open_loop():
            await nursery.start(db_runner)

I'm wondering if we could come up with a solution to easily detect this, for instance using a context var as a canary to detect if there is no longer any trio-asyncio loop available in the current context.

smurfix commented 6 years ago

The root problem is that there's still code around to handle stopped-but-not-closed loops (you need that stuff for loop.run_until_complete and friends).

I should remove that nonsense and declare a stopped loop to be closed, which will raise errors whenever somebody tries to queue something to it, which will (I hope …) cause a huge nasty traceback instead of hanging.

touilleMan commented 6 years ago

which will (I hope …) cause a huge nasty traceback instead of hanging.

That would be much better indeed ^^

Is there something I can do to help on this ?

smurfix commented 6 years ago

Yes. Create at testcase that hangs – ideally one that sends SIGINT to itself, instead of requiring the user to press ^C.

touilleMan commented 6 years ago

@smurfix I've created a PR with two tests hanging forever (not sure where to put them though, so there are in test_misc.py for the moment...)

smurfix commented 6 years ago

Should be solved in https://github.com/python-trio/trio-asyncio/pull/38