Skip to content
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

Unexpected RuntimeError: partially-exhausted async_generator garbage collected #13

Closed
pquentin opened this issue Feb 11, 2018 · 4 comments

Comments

@pquentin
Copy link
Member

async_generator is really useful! But I see one surprising behavior. When I run this code:

import trio
from async_generator import async_generator, yield_


@async_generator
async def items():
    for i in range(10):
        await yield_(i)


async def main():
    async for i in items():
        if i > 4:
            break
        print(i)


if __name__ == '__main__':
    trio.run(main)

I get the following exception:

Exception ignored in: <bound method AsyncGenerator.__del__ of <async_generator._impl.AsyncGenerator object at 0x110a9b908>>
Traceback (most recent call last):
  File ".../python3.5/site-packages/async_generator/_impl.py", line 324, in __del__
    .format(self._coroutine.cr_frame.f_code.co_name)
RuntimeError: partially-exhausted async_generator 'items' garbage collected

Is this expected? The same code using the Python 3.6 syntax does not print any exception. Thanks!

@pmav99
Copy link

pmav99 commented Feb 14, 2018

Hmm.... I get the same error while running trio's test suite.

On Python 3.5 it happens rather seldomly. E.g. after 20 runs I got it only once:

feanor at valinor in ~/Prog/venvs/p35/trio (master●●)
(p35)$ py.test -v trio/tests/test_util.py
=============================================================================================== test session starts ===============================================================================================
platform linux -- Python 3.5.4, pytest-3.4.0, py-1.5.2, pluggy-0.6.0 -- /home/feanor/.virtualenvs/p35/bin/python3.5
cachedir: .pytest_cache
rootdir: /home/feanor/Prog/venvs/p35/trio, inifile:
plugins: faulthandler-1.4.1, cov-2.5.1
collected 11 items                                                                                                                                                                                                

trio/tests/test_util.py::test_signal_raise PASSED                                                                                                                                                           [  9%]
trio/tests/test_util.py::test_ConflictDetector PASSED                                                                                                                                                       [ 18%]
trio/tests/test_util.py::test_contextmanager_do_not_unchain_non_stopiteration_exceptions PASSED                                                                                                             [ 27%]
trio/tests/test_util.py::test_native_contextmanager_do_not_unchain_non_stopiteration_exceptions SKIPPED                                                                                                     [ 36%]
trio/tests/test_util.py::test_acontextmanager_exception_passthrough PASSED                                                                                                                                  [ 45%]
trio/tests/test_util.py::test_acontextmanager_catches_exception PASSED                                                                                                                                      [ 54%]
trio/tests/test_util.py::test_acontextmanager_no_yield PASSED                                                                                                                                               [ 63%]
trio/tests/test_util.py::test_acontextmanager_too_many_yields PASSED                                                                                                                                        [ 72%]
trio/tests/test_util.py::test_acontextmanager_requires_asyncgenfunction PASSED                                                                                                                              [ 81%]
trio/tests/test_util.py::test_module_metadata_is_fixed_up PASSED                                                                                                                                            [ 90%]
trio/tests/test_util.py::test_fspath PASSED                                                                                                                                                                 [100%]

====================================================================================== 10 passed, 1 skipped in 0.02 seconds =======================================================================================
Exception ignored in: <bound method AsyncGenerator.__del__ of <async_generator._impl.AsyncGenerator object at 0x7fb68d4cbc50>>
Traceback (most recent call last):
  File "/home/feanor/.virtualenvs/p35/lib/python3.5/site-packages/async_generator/_impl.py", line 324, in __del__
    .format(self._coroutine.cr_frame.f_code.co_name)
RuntimeError: partially-exhausted async_generator 'doubleyield' garbage collected

On python 3.6 though, this is way more common, but it doesn't happen every time! E.g. on two consecutive runs:

feanor at valinor in ~/Prog/venvs/p35/trio (master●●)
(p36)$ py.test -v trio/tests/test_util.py
=============================================================================================== test session starts ===============================================================================================
platform linux -- Python 3.6.4, pytest-3.3.1, py-1.5.2, pluggy-0.6.0 -- /home/feanor/.virtualenvs/p36/bin/python
cachedir: .cache
rootdir: /home/feanor/Prog/venvs/p35/trio, inifile:
plugins: mock-1.6.3, faulthandler-1.4.1, cov-2.5.1
collected 11 items                                                                                                                                                                                                

trio/tests/test_util.py::test_signal_raise PASSED                                                                                                                                                           [  9%]
trio/tests/test_util.py::test_ConflictDetector PASSED                                                                                                                                                       [ 18%]
trio/tests/test_util.py::test_contextmanager_do_not_unchain_non_stopiteration_exceptions PASSED                                                                                                             [ 27%]
trio/tests/test_util.py::test_native_contextmanager_do_not_unchain_non_stopiteration_exceptions PASSED                                                                                                      [ 36%]
trio/tests/test_util.py::test_acontextmanager_exception_passthrough PASSED                                                                                                                                  [ 45%]
trio/tests/test_util.py::test_acontextmanager_catches_exception PASSED                                                                                                                                      [ 54%]
trio/tests/test_util.py::test_acontextmanager_no_yield PASSED                                                                                                                                               [ 63%]
trio/tests/test_util.py::test_acontextmanager_too_many_yields PASSED                                                                                                                                        [ 72%]
trio/tests/test_util.py::test_acontextmanager_requires_asyncgenfunction PASSED                                                                                                                              [ 81%]
trio/tests/test_util.py::test_module_metadata_is_fixed_up PASSED                                                                                                                                            [ 90%]
trio/tests/test_util.py::test_fspath PASSED                                                                                                                                                                 [100%]

============================================================================================ 11 passed in 0.02 seconds ============================================================================================
Exception ignored in: <bound method AsyncGenerator.__del__ of <async_generator._impl.AsyncGenerator object at 0x7f919b3b5a20>>
Traceback (most recent call last):
  File "/home/feanor/.virtualenvs/p36/lib/python3.6/site-packages/async_generator/_impl.py", line 324, in __del__
    .format(self._coroutine.cr_frame.f_code.co_name)
RuntimeError: partially-exhausted async_generator 'doubleyield' garbage collected

feanor at valinor in ~/Prog/venvs/p35/trio (master●●)
(p36)$ py.test -v trio/tests/test_util.py
=============================================================================================== test session starts ===============================================================================================
platform linux -- Python 3.6.4, pytest-3.3.1, py-1.5.2, pluggy-0.6.0 -- /home/feanor/.virtualenvs/p36/bin/python
cachedir: .cache
rootdir: /home/feanor/Prog/venvs/p35/trio, inifile:
plugins: mock-1.6.3, faulthandler-1.4.1, cov-2.5.1
collected 11 items                                                                                                                                                                                                

trio/tests/test_util.py::test_signal_raise PASSED                                                                                                                                                           [  9%]
trio/tests/test_util.py::test_ConflictDetector PASSED                                                                                                                                                       [ 18%]
trio/tests/test_util.py::test_contextmanager_do_not_unchain_non_stopiteration_exceptions PASSED                                                                                                             [ 27%]
trio/tests/test_util.py::test_native_contextmanager_do_not_unchain_non_stopiteration_exceptions PASSED                                                                                                      [ 36%]
trio/tests/test_util.py::test_acontextmanager_exception_passthrough PASSED                                                                                                                                  [ 45%]
trio/tests/test_util.py::test_acontextmanager_catches_exception PASSED                                                                                                                                      [ 54%]
trio/tests/test_util.py::test_acontextmanager_no_yield PASSED                                                                                                                                               [ 63%]
trio/tests/test_util.py::test_acontextmanager_too_many_yields PASSED                                                                                                                                        [ 72%]
trio/tests/test_util.py::test_acontextmanager_requires_asyncgenfunction PASSED                                                                                                                              [ 81%]
trio/tests/test_util.py::test_module_metadata_is_fixed_up PASSED                                                                                                                                            [ 90%]
trio/tests/test_util.py::test_fspath PASSED                                                                                                                                                                 [100%]

============================================================================================ 11 passed in 0.02 seconds ============================================================================================

Using ArchLinux x64.

@njsmith
Copy link
Member

njsmith commented Feb 14, 2018

The one in trio's test suite is harmless; it's a mild bug in trio's version of @acontextmanager, that's fixed in async_generator.asynccontextmanager. So the fix there is python-trio/trio#418 :-)

The broader problem here is that there's no really decent way to handle garbage collection for async generators. For normal generators, if they're garbage collected, then the interpreter throws in an exception to run any finally blocks. (That's part of PEP 342; before this, it simply wasn't allowed to yield inside a try or with block.) For async generators, the interpreter wants to do the same thing... But if you want to throw an exception into an async generator, you need to call athrow, which is async, so you can't call it from there garbage collector. Only the coroutine runner can actually do this.

PEP 525 tried to work around this by adding some hooks that a coroutine runner can register to get notified when the GC wants to kill an async generator. It's kind of messy, and in particular is problematic for trio, since there's no way to make the cleanup happen in the right task context (python-trio/trio#265). This is particularly bad because there are some common with blocks that must be cleaned up in the right context, ie nurseries – though the problems with generators and trio's nurseries and cancel scopes to beyond just cleanup (python-trio/trio#264).

PEP 533 is an alternative proposal I wrote, but it hasn't managed to get any momentum so far.

There are also wackier options. For example, Curio analyses the byte code of async generators to figure out whether they actually have any of the tricky constructs like a yield inside a try, and tries to only apply special handling to the tricky generators, while leaving simple cases like your example alone.

Anyway, it's a mess. So far async_generator has tried to stay out of this mess by using the simplest possible rule: tell users they should close their async generators explicitly (e.g. using the aclosing context manager we helpfully provide), and issue a warning if they don't.

It's possible we should grit our teeth and implement the gc dance specified in PEP 525 (at least for now), to match native async generators. (Fun fact: curio's bytecode inspection will give the wrong results if we do this, because it runs via the PEP 525 hooks and can't recognize that await yield_() is a yield. I'm less worried about curio compatibility than I once was though...)

@oremanj
Copy link
Member

oremanj commented Apr 17, 2018

PEP 525-style cleanup has been implemented in #15. If you don't install a finalizer hook, async_generator now behaves like native async generators: it'll throw in GeneratorExit and complain if anything tries to await while unwinding. (So, for your generator above it would unwind successfully and not warn.) Sane finalization semantics for async generators that do await while unwinding will get added to trio at some point after the async_generator change makes it into a release. I'm going to track that at python-trio/trio#265, and close this issue.

@oremanj oremanj closed this as completed Apr 17, 2018
@pquentin
Copy link
Member Author

Thank you @oremanj!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants