Finalize async generators in the correct context #150
Conversation
oremanj
force-pushed
the
asyncgens
branch
3 times, most recently
from
3efdbf1
to
eabd970
Compare
tests/test_trio_asyncio.py
Outdated
| for _ in range(5): | ||
| gc.collect() |
There was a problem hiding this comment.
is there a significance of running this 5 times?
There was a problem hiding this comment.
There was a problem hiding this comment.
thanks! it's unfortunate that we need to duplicate util functions between trio and trio-asyncio 😞
|
|
||
| def firstiter(self, agen): | ||
| if sniffio_library.name == "asyncio": | ||
| loop = asyncio.get_running_loop() |
There was a problem hiding this comment.
do we need to handle the case where there's no running loop / if the loop is no longer running?
There was a problem hiding this comment.
If there's no running loop, then sniffio should not be reporting that we're in asyncio mode.
There was a problem hiding this comment.
you're right, what about in finalizer? do we need to check if it's still running?
There was a problem hiding this comment.
Python remembers the finalizer hook that was active when an async generator was firstiter'ed, and calls it even if it is no longer the active finalizer hook. Thus, finalizer hooks have to be prepared to be called after the event loop has stopped. This can happen even without our intervention (ie with pure asyncio) so I don't think we need to do anything special. The enqueued task will run if the event loop is resumed.
oremanj@shainfra10:/usr/scratch/oremanj$ python3.12
Python 3.12.2 (main, Feb 12 2024, 10:12:13) [GCC 11.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import asyncio
>>> async def agen(): yield
...
>>> async def main():
... global ag
... ag = agen()
... await ag.asend(None)
...
>>> loop = asyncio.new_event_loop()
>>> loop.run_until_complete(main())
>>>
# now we have a stopped event loop and a half-finished async generator:
>>> loop._asyncgens
{<weakref at 0x7f7004348b80; to 'async_generator' at 0x7f700472eda0 (agen)>}
>>> loop._ready
deque([])
# if we destroy the async generator, asyncio will schedule its finalizer:
>>> del ag
>>> loop._asyncgens
set()
>>> loop._ready
deque([<Handle BaseEventLoop.create_task(<async_genera...x7f700432fc00>)>])
|
|
||
| def firstiter(self, agen): | ||
| if sniffio_library.name == "asyncio": | ||
| loop = asyncio.get_running_loop() |
There was a problem hiding this comment.
you're right, what about in finalizer? do we need to check if it's still running?
tests/test_trio_asyncio.py
Outdated
| for _ in range(5): | ||
| gc.collect() |
There was a problem hiding this comment.
thanks! it's unfortunate that we need to duplicate util functions between trio and trio-asyncio 😞
tests/test_trio_asyncio.py
Outdated
| async with trio_asyncio.open_loop() as loop2: | ||
| async with trio.open_nursery() as nursery: | ||
| # Make sure the iterate_one aio tasks don't get | ||
| # cancelled before they start: |
There was a problem hiding this comment.
i would add an assertion that the dispatcher object is the same for both loops and that the refcount is 2
There was a problem hiding this comment.
I disagree. The test is testing observable public behavior: that each async generator is finalized in the context where it was first iterated. If we later change trio-asyncio to ensure that in some other way, the test should still work.
I'll add a test that once both trio-asyncio loops are closed, the global async generator hooks go back to what they were before the loops were opened.
Install custom async generator hooks that finalize each asyncgen in the same context (trio vs asyncio mode, and the correct trio-asyncio loop if asyncio mode) as it was first iterated. Fixes #92.