Skip to content

Finalization of non-exhausted asynchronous generators is deferred #88684

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
serhiy-storchaka opened this issue Jun 27, 2021 · 20 comments
Closed
Labels
3.10 only security fixes 3.11 only security fixes 3.12 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) topic-asyncio type-bug An unexpected behavior, bug, or error

Comments

@serhiy-storchaka
Copy link
Member

BPO 44518
Nosy @gvanrossum, @asvetlov, @markshannon, @serhiy-storchaka, @1st1, @gvanrossum

Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2021-06-27.09:11:32.621>
labels = ['interpreter-core', '3.11', '3.9', '3.10', 'performance']
title = 'Finalization of non-exhausted asynchronous generators is deferred'
updated_at = <Date 2021-06-28.18:33:50.454>
user = 'https://github.com/serhiy-storchaka'

bugs.python.org fields:

activity = <Date 2021-06-28.18:33:50.454>
actor = 'gvanrossum'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Interpreter Core']
creation = <Date 2021-06-27.09:11:32.621>
creator = 'serhiy.storchaka'
dependencies = []
files = []
hgrepos = []
issue_num = 44518
keywords = []
message_count = 6.0
messages = ['396569', '396610', '396611', '396644', '396650', '396659']
nosy_count = 6.0
nosy_names = ['gvanrossum', 'asvetlov', 'Mark.Shannon', 'serhiy.storchaka', 'yselivanov', 'Guido.van.Rossum']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = 'resource usage'
url = 'https://bugs.python.org/issue44518'
versions = ['Python 3.9', 'Python 3.10', 'Python 3.11']

@serhiy-storchaka
Copy link
Member Author

In the following example:

def gen():
    try:
        yield 1
    finally:
        print('finalize inner')

def func():
    try:
        for x in gen():
            break
    finally:
        print('finalize outer')

func()
print('END')

the output in CPython is:

finalize inner
finalize outer
END

But in similar example for asynchronous generator:

async def gen():
    try:
        yield 1
    finally:
        print('finalize inner')

async def func():
    try:
        async for x in gen():
            break
    finally:
        print('finalize outer')

import asyncio
asyncio.run(func())
print('END')

the output in CPython is:

finalize outer
finalize inner
END

There is a strong link somewhere which prevents finalization of the asynchronous generator object until leaving the outer function.

Tested on CPython 3.7-3.11. In PyPy "finalize inner" is not printed at all. Using closing() and aclosing() is the right way to get deterministic finalization, but in any case it would be better to get rid of strong link which prevents finalization of the asynchronous generator object.

@serhiy-storchaka serhiy-storchaka added 3.9 only security fixes 3.10 only security fixes 3.11 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) performance Performance or resource usage labels Jun 27, 2021
@gvanrossum
Copy link
Member

Can you repro this without asyncio?

@serhiy-storchaka
Copy link
Member Author

Sorry, I do not understand what do you mean. The problem is that non-exhausted asynchronous generators are not finalized until asyncio.run() is finished. This differs from synchronous generators which are finalized as early as possible (in CPython).

@gvanrossum
Copy link
Member

I am wondering whether the issue is in asyncio.run, or in the basic async generator implementation. If the latter, you should be able to demonstrate this with a manual "driver" (trampoline?) instead of asyncio.run.

@serhiy-storchaka
Copy link
Member Author

Inlined asyncio.run(func()) is roughly equivalent to the following code:

loop = asyncio.new_event_loop()
loop.run_until_complete(func())
loop.run_until_complete(loop.shutdown_asyncgens())
loop.close()

If comment out loop.run_until_complete(loop.shutdown_asyncgens()) I get the following output:

finalize outer
Task was destroyed but it is pending!
task: <Task pending name='Task-2' coro=<<async_generator_athrow without __name__>()>>
END

No "finalize inner" but a warning about pending task instead.

On other hand, the following code

for _ in func().__await__(): pass

produces the output in expected order:

finalize inner
finalize outer
END

@gvanrossum
Copy link
Member

Okay, that suggests that the extra reference is being held by asyncio, right? I suppose it's in the hands of the asyncio devs then.

@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
@kumaraditya303
Copy link
Contributor

The extra reference is required so that the event loop can finalize the generator in the correct context. The issue is that once the generator is about to be finalized, the event loop creates a new task to finalize it in the context but since there are no awaits after iterating over the generator the task is pending.

The following script finalizes it properly as it add a sleep to switch the tasks and run the finalizer.

async def gen():
    try:
        yield 1
    finally:
        print('finalize inner')

async def func():
    try:
        async for x in gen():
            break
        await asyncio.sleep(0.1) # Switch task and run the finalizer 
    finally:
        print('finalize outer')

import asyncio
asyncio.run(func())
print('END')

Output:

finalize inner
finalize outer
END

@kumaraditya303 kumaraditya303 removed the 3.9 only security fixes label Jun 20, 2022
@kumaraditya303
Copy link
Contributor

The following example demonstrates the number of context switches required to run the finalizer:

async def gen():
    try:
        yield 1
    finally:
        print('finalize inner')

async def func():
    try:
        async for x in gen():
            break
        # Schedules callback to create a task on next loop iteration 
    finally:
        await asyncio.sleep(0) # Creates a task to finalize gen
        await asyncio.sleep(0) # Run the task to finalize gen
        print('finalize outer')

import asyncio
asyncio.run(func())
print('END')

Output:

finalize inner
finalize outer
END

@gvanrossum
Copy link
Member

Hopefully @serhiy-storchaka can do something with your research!

@kumaraditya303
Copy link
Contributor

This would probably require something like https://peps.python.org/pep-0533/

@gvanrossum
Copy link
Member

Could you implement a prototype? That might help the PEP move forward. @njs @1st1

@graingert
Copy link
Contributor

Can you repro this without asyncio?

async def gen():
    try:
        yield 1
    finally:
        print('finalize inner')

async def func():
    try:
        async for x in gen():
            break
    finally:
        print('finalize outer')

import anyio
anyio.run(func, backend="trio")
print('END')

this happens with trio too:

python -W error demo.py
Exception ignored in: <async_generator object gen at 0x7fd98b9db040>
Traceback (most recent call last):
  File "/home/graingert/.virtualenvs/testing39/lib/python3.9/site-packages/trio/_core/_asyncgens.py", line 79, in finalizer
    warnings.warn(
ResourceWarning: Async generator '__main__.gen' was garbage collected before it had been exhausted. Surround its use in 'async with aclosing(...):' to ensure that it gets cleaned up as soon as you're done using it.
finalize outer
finalize inner
END

@graingert
Copy link
Contributor

running it with plain coroutines results in:

import threading

async def gen():
    try:
        yield 1
    finally:
        print('finalize inner')

async def func():
    try:
        async for x in gen():
            break
    finally:
        print('finalize outer')

import threading

coro = func()
coro.send(None)
finalize inner
finalize outer
Traceback (most recent call last):
  File "/home/graingert/projects/cpython/demo.py", line 19, in <module>
    coro.send(None)
StopIteration

@gvanrossum gvanrossum added the 3.12 only security fixes label Jul 19, 2022
@gvanrossum
Copy link
Member

That's a rather literalist interpretation of my question. :-) I meant whether you can come up with a repro without an event loop framework, since that would presumably make reasoning about the root cause of the problem easier.

@graingert
Copy link
Contributor

graingert commented Jul 19, 2022

@gvanrossum
Copy link
Member

Ah, sorry. The whole point of the bug seems to be that the event loop hangs on to an extra reference, and there is a reason.

I should really recuse myself from this issue since I don't understand async generators.

@graingert
Copy link
Contributor

In PyPy "finalize inner" is not printed at all.

This looks like it could be an asyncio bug - asyncio should use the firstiter hook to keep a reference to generators that need shutting down

@graingert
Copy link
Contributor

In PyPy "finalize inner" is not printed at all.

seems that got fixed:

import threading
import trio
import asyncio

async def gen(runner):
    try:
        yield 1
    finally:
        print(f'{runner=} finalize inner')

async def func(runner):
    try:
        async for x in gen(runner):
            break
    finally:
        print(f'{runner=} finalize outer')

print(f"{trio.run(func, 'trio')=}")
print(f"{asyncio.run(func('asyncio'))=}")
print(f"{func('manual').send(None)=}")  # on pypy3 this doesn't print finalize inner

prints:

$ pypy3.8 demo.py
runner='trio' finalize outer
runner='trio' finalize inner
trio.run(func, 'trio')=None
runner='asyncio' finalize outer
runner='asyncio' finalize inner
asyncio.run(func('asyncio'))=None
runner='manual' finalize outer
Traceback (most recent call last):
  File "demo.py", line 20, in <module>
    print(f"{func('manual').send(None)=}")
StopIteration

@ezio-melotti ezio-melotti moved this to Todo in asyncio Oct 29, 2022
@iritkatriel iritkatriel added type-bug An unexpected behavior, bug, or error and removed performance Performance or resource usage labels Oct 29, 2022
@kumaraditya303
Copy link
Contributor

The behaviour cannot be changed without significant changes to the interpreter. The documentation will be handled for this as part of #100108 so closing.

@kumaraditya303 kumaraditya303 closed this as not planned Won't fix, can't repro, duplicate, stale Apr 16, 2023
@github-project-automation github-project-automation bot moved this from Todo to Done in asyncio Apr 16, 2023
@kumaraditya303
Copy link
Contributor

For anyone who comes here for a possible fix which always works:

import contextlib

async def gen():
    try:
        yield 1
    finally:
        print('finalize inner')

async def func():
    try:
        async with contextlib.aclosing(gen()) as g:
            async for x in g:
                break
    finally:
        print('finalize outer')

import asyncio
asyncio.run(func())
print('END')

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.10 only security fixes 3.11 only security fixes 3.12 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) topic-asyncio type-bug An unexpected behavior, bug, or error
Projects
Status: Done
Development

No branches or pull requests

5 participants