python-trio / async_generator

Making it easy to write async iterators in Python 3.5
Other
95 stars 24 forks source link

Unexpected RuntimeError: partially-exhausted async_generator garbage collected #13

Closed pquentin closed 6 years ago

pquentin commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 https://github.com/python-trio/trio/issues/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 (https://github.com/python-trio/trio/issues/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 (https://github.com/python-trio/trio/issues/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 commented 6 years ago

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 https://github.com/python-trio/trio/issues/265, and close this issue.

pquentin commented 6 years ago

Thank you @oremanj!