python-trio / trio-asyncio

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

Cancel trio_as_aio tasks before stopping the trio-asyncio loop #96

Closed oremanj closed 3 years ago

oremanj commented 3 years ago

trio-asyncio creates a Trio nursery with one task that runs the asyncio event loop, and more tasks that run any Trio tasks started from asyncio context using loop.run_trio() or trio_asyncio.trio_as_aio(). Previously, it would first stop the asyncio event loop and then cancel the Trio tasks in the nursery. This created problems in some cases, because the Trio tasks run inside a wrapper that propagates their result to an asyncio future, and that propagation stops working when the asyncio event loop stops running. Depending on the exact circumstances, this could lead to an "Event loop is closed" error (#95) or a deadlock (#89).

This PR resolves the issue by cancelling the relevant Trio tasks and waiting for them to exit before shutting down the trio-asyncio loop.

Fixes #56 Fixes #89 Fixes #95

shamrin commented 3 years ago

Regarding loop= test failures on nightly python. Cherry-picked @smurfix commits do not fix them because they come from PR #94, which removed a lot of tests, including loop=-using tests. I think the best way the proceed is to revert those commits from here and live with nightly python failures a little bit more (we've had them before). PR #94 will fix those failures anyhow.

Update: 👇 I took the liberty of reverting those commits myself 👇 (please let me if it was a mistake 😬).

oremanj commented 3 years ago

I've temporarily made the Travis checks not be required to merge PRs. We can reenable them once we remove the loop= args. or just stop using Travis :-)