From b5c50ce152cfe376a524bf2269d21cb07bb5e732 Mon Sep 17 00:00:00 2001 From: Joshua Oreman Date: Thu, 5 Apr 2018 14:43:52 -0400 Subject: [PATCH 1/5] Add support for PEP 525-style garbage collection hooks --- async_generator/__init__.py | 4 + async_generator/_impl.py | 137 +++++++-- .../_tests/test_async_generator.py | 271 ++++++++++++++++-- docs/source/reference.rst | 37 ++- 4 files changed, 398 insertions(+), 51 deletions(-) diff --git a/async_generator/__init__.py b/async_generator/__init__.py index ced7770..e81a8fe 100644 --- a/async_generator/__init__.py +++ b/async_generator/__init__.py @@ -5,6 +5,8 @@ yield_from_, isasyncgen, isasyncgenfunction, + get_asyncgen_hooks, + set_asyncgen_hooks, ) from ._util import aclosing, asynccontextmanager @@ -16,4 +18,6 @@ "isasyncgen", "isasyncgenfunction", "asynccontextmanager", + "get_asyncgen_hooks", + "set_asyncgen_hooks", ] diff --git a/async_generator/_impl.py b/async_generator/_impl.py index cf2c783..e798cae 100644 --- a/async_generator/_impl.py +++ b/async_generator/_impl.py @@ -225,11 +225,58 @@ def _invoke(self, fn, *args): return result +UNSPECIFIED = object() +try: + from sys import get_asyncgen_hooks, set_asyncgen_hooks + +except ImportError: + import threading + + asyncgen_hooks = collections.namedtuple( + "asyncgen_hooks", ("firstiter", "finalizer") + ) + + class _hooks_storage(threading.local): + def __init__(self): + self.firstiter = None + self.finalizer = None + + _hooks = _hooks_storage() + + def get_asyncgen_hooks(): + return asyncgen_hooks( + firstiter=_hooks.firstiter, finalizer=_hooks.finalizer + ) + + def set_asyncgen_hooks(firstiter=UNSPECIFIED, finalizer=UNSPECIFIED): + if firstiter is not UNSPECIFIED: + if firstiter is None or callable(firstiter): + _hooks.firstiter = firstiter + else: + raise TypeError( + "callable firstiter expected, got {}".format( + type(firstiter).__name__ + ) + ) + + if finalizer is not UNSPECIFIED: + if finalizer is None or callable(finalizer): + _hooks.finalizer = finalizer + else: + raise TypeError( + "callable finalizer expected, got {}".format( + type(finalizer).__name__ + ) + ) + + class AsyncGenerator: def __init__(self, coroutine): self._coroutine = coroutine self._it = coroutine.__await__() self.ag_running = False + self._finalizer = None + self._closed = False # On python 3.5.0 and 3.5.1, __aiter__ must be awaitable. # Starting in 3.5.2, it should not be awaitable, and if it is, then it @@ -263,33 +310,44 @@ def ag_frame(self): # Core functionality ################################################################ - # We make these async functions and use await, rather than just regular - # functions that pass back awaitables, in order to get more useful - # tracebacks when debugging. + # These need to return awaitables, rather than being async functions, + # to match the native behavior where the firstiter hook is called + # immediately on asend()/etc, even if the coroutine that asend() + # produces isn't awaited for a bit. + + def __anext__(self): + return self._do_it(self._it.__next__) - async def __anext__(self): - return await self._do_it(self._it.__next__) + def asend(self, value): + return self._do_it(self._it.send, value) - async def asend(self, value): - return await self._do_it(self._it.send, value) + def athrow(self, type, value=None, traceback=None): + return self._do_it(self._it.throw, type, value, traceback) - async def athrow(self, type, value=None, traceback=None): - return await self._do_it(self._it.throw, type, value, traceback) + def _do_it(self, start_fn, *args): + coro_state = getcoroutinestate(self._coroutine) + if coro_state is CORO_CREATED: + (firstiter, self._finalizer) = get_asyncgen_hooks() + if firstiter is not None: + firstiter(self) - async def _do_it(self, start_fn, *args): # On CPython 3.5.2 (but not 3.5.0), coroutines get cranky if you try # to iterate them after they're exhausted. Generators OTOH just raise # StopIteration. We want to convert the one into the other, so we need # to avoid iterating stopped coroutines. if getcoroutinestate(self._coroutine) is CORO_CLOSED: raise StopAsyncIteration() - if self.ag_running: - raise ValueError("async generator already executing") - try: - self.ag_running = True - return await ANextIter(self._it, start_fn, *args) - finally: - self.ag_running = False + + async def step(): + if self.ag_running: + raise ValueError("async generator already executing") + try: + self.ag_running = True + return await ANextIter(self._it, start_fn, *args) + finally: + self.ag_running = False + + return step() ################################################################ # Cleanup @@ -297,13 +355,14 @@ async def _do_it(self, start_fn, *args): async def aclose(self): state = getcoroutinestate(self._coroutine) + if state is CORO_CLOSED or self._closed: + return + self._closed = True if state is CORO_CREATED: # Make sure that aclose() on an unstarted generator returns # successfully and prevents future iteration. self._it.close() return - elif state is CORO_CLOSED: - return try: await self.athrow(GeneratorExit) except (GeneratorExit, StopAsyncIteration): @@ -316,13 +375,39 @@ def __del__(self): # Never started, nothing to clean up, just suppress the "coroutine # never awaited" message. self._coroutine.close() - if getcoroutinestate(self._coroutine) is CORO_SUSPENDED: - # This exception will get swallowed because this is __del__, but - # it's an easy way to trigger the print-to-console logic - raise RuntimeError( - "partially-exhausted async_generator {!r} garbage collected" - .format(self._coroutine.cr_frame.f_code.co_name) - ) + if getcoroutinestate(self._coroutine + ) is CORO_SUSPENDED and not self._closed: + if sys.implementation.name == "pypy": + # pypy segfaults if we resume the coroutine from our __del__ + # and it executes any more 'await' statements, so we use the + # old async_generator behavior of "don't even try to finalize + # correctly". https://bitbucket.org/pypy/pypy/issues/2786/ + raise RuntimeError( + "partially-exhausted async_generator {!r} garbage collected" + .format(self.ag_code.co_name) + ) + elif self._finalizer is not None: + self._finalizer(self) + else: + # Mimic the behavior of native generators on GC with no finalizer: + # throw in GeneratorExit, run for one turn, and complain if it didn't + # finish. + thrower = self.athrow(GeneratorExit) + try: + thrower.send(None) + except (GeneratorExit, StopAsyncIteration): + pass + except StopIteration: + raise RuntimeError("async_generator ignored GeneratorExit") + else: + raise RuntimeError( + "async_generator {!r} awaited during finalization; install " + "a finalization hook to support this, or wrap it in " + "'async with aclosing(...):'" + .format(self.ag_code.co_name) + ) + finally: + thrower.close() if hasattr(collections.abc, "AsyncGenerator"): diff --git a/async_generator/_tests/test_async_generator.py b/async_generator/_tests/test_async_generator.py index eaf7c92..e3b16e6 100644 --- a/async_generator/_tests/test_async_generator.py +++ b/async_generator/_tests/test_async_generator.py @@ -13,6 +13,8 @@ yield_from_, isasyncgen, isasyncgenfunction, + get_asyncgen_hooks, + set_asyncgen_hooks, ) @@ -190,6 +192,22 @@ async def recurse(): pass # pragma: no cover +async def test_reentrance_forbidden_simultaneous_asends(): + @async_generator + async def f(): + await mock_sleep() + + ag = f() + sender1 = ag.asend(None) + sender2 = ag.asend(None) + assert sender1.send(None) == "mock_sleep" + with pytest.raises(ValueError): + sender2.send(None) + with pytest.raises(StopAsyncIteration): + sender1.send(None) + await ag.aclose() + + # https://bugs.python.org/issue32526 async def test_reentrance_forbidden_while_suspended_in_coroutine_runner(): @async_generator @@ -601,31 +619,83 @@ async def yield_from_catch(): ################################################################ -async def test___del__(): - gen = async_range(10) +async def test___del__(capfd): + completions = 0 + + @async_generator + async def awaits_when_unwinding(): + await yield_(1) + try: + await yield_(2) + finally: + await mock_sleep() + try: + await yield_(3) + finally: + nonlocal completions + completions += 1 + + gen = awaits_when_unwinding() # Hasn't started yet, so no problem gen.__del__() - gen = async_range(10) - await collect(gen) + gen = awaits_when_unwinding() + assert await collect(gen) == [1, 2, 3] # Exhausted, so no problem gen.__del__() - gen = async_range(10) - await gen.aclose() - # Closed, so no problem - gen.__del__() + for turns in (1, 2, 3): + gen = awaits_when_unwinding() + for turn in range(1, turns + 1): + assert await gen.__anext__() == turn + await gen.aclose() + # Closed, so no problem + gen.__del__() - gen = async_range(10) - await gen.__anext__() - await gen.aclose() - # Closed, so no problem - gen.__del__() + for turns in (1, 2, 3): + gen = awaits_when_unwinding() + for turn in range(1, turns + 1): + assert await gen.__anext__() == turn + + if sys.implementation.name == "pypy": + # pypy can't do the full finalization dance yet: + # https://bitbucket.org/pypy/pypy/issues/2786/. + # Also, pypy suppresses exceptions on explicit __del__ calls, + # not just implicit ones. + with pytest.raises(RuntimeError) as info: + gen.__del__() + assert "partially-exhausted async_generator" in str(info.value) + if turns == 3: + # We didn't increment completions, because we didn't finalize + # the generator. Increment it now so the check below (which is + # calibrated for the correct/CPython behavior) doesn't fire; + # we know about the pypy bug. + completions += 1 + + elif turns == 2: + # Stopped in the middle of a try/finally that awaits in the finally, + # so __del__ can't cleanup. + with pytest.raises(RuntimeError) as info: + gen.__del__() + assert "awaited during finalization; install a finalization hook" in str( + info.value + ) + else: + # Can clean up without awaiting, so __del__ is fine + gen.__del__() - gen = async_range(10) - await gen.__anext__() - # Started, but not exhausted or closed -- big problem - with pytest.raises(RuntimeError): + assert completions == 3 + + @async_generator + async def yields_when_unwinding(): + try: + await yield_(1) + finally: + await yield_(2) + + gen = yields_when_unwinding() + assert await gen.__anext__() == 1 + with pytest.raises(RuntimeError) as info: gen.__del__() @@ -779,3 +849,170 @@ async def test_no_spurious_unawaited_coroutine_warning(recwarn): for msg in recwarn: # pragma: no cover print(msg) assert not issubclass(msg.category, RuntimeWarning) + + +################################################################ +# +# GC hooks +# +################################################################ + + +@pytest.fixture +def local_asyncgen_hooks(): + old_hooks = get_asyncgen_hooks() + yield + set_asyncgen_hooks(*old_hooks) + + +def test_gc_hooks_interface(local_asyncgen_hooks): + def one(agen): # pragma: no cover + pass + + def two(agen): # pragma: no cover + pass + + set_asyncgen_hooks(None, None) + assert get_asyncgen_hooks() == (None, None) + set_asyncgen_hooks(finalizer=two) + assert get_asyncgen_hooks() == (None, two) + set_asyncgen_hooks(firstiter=one) + assert get_asyncgen_hooks() == (one, two) + set_asyncgen_hooks(finalizer=None, firstiter=two) + assert get_asyncgen_hooks() == (two, None) + set_asyncgen_hooks(None, one) + assert get_asyncgen_hooks() == (None, one) + tup = (one, two) + set_asyncgen_hooks(*tup) + assert get_asyncgen_hooks() == tup + + with pytest.raises(TypeError): + set_asyncgen_hooks(firstiter=42) + + with pytest.raises(TypeError): + set_asyncgen_hooks(finalizer=False) + + def in_thread(results=[]): + results.append(get_asyncgen_hooks()) + set_asyncgen_hooks(two, one) + results.append(get_asyncgen_hooks()) + + from threading import Thread + results = [] + thread = Thread(target=in_thread, args=(results,)) + thread.start() + thread.join() + assert results == [(None, None), (two, one)] + assert get_asyncgen_hooks() == (one, two) + + +async def test_gc_hooks_behavior(local_asyncgen_hooks): + events = [] + to_finalize = [] + + def firstiter(agen): + events.append("firstiter {}".format(agen.ag_frame.f_locals["ident"])) + + def finalizer(agen): + events.append("finalizer {}".format(agen.ag_frame.f_locals["ident"])) + to_finalize.append(agen) + + @async_generator + async def agen(ident): + events.append("yield 1 {}".format(ident)) + await yield_(1) + try: + events.append("yield 2 {}".format(ident)) + await yield_(2) + events.append("after yield 2 {}".format(ident)) + finally: + events.append("mock_sleep {}".format(ident)) + await mock_sleep() + try: + events.append("yield 3 {}".format(ident)) + await yield_(3) + finally: + events.append("unwind 3 {}".format(ident)) + # this one is included to make sure we _don't_ execute it + events.append("done {}".format(ident)) # pragma: no cover + + async def anext_verbosely(iter, ident): + events.append("before asend {}".format(ident)) + sender = iter.asend(None) + events.append("before send {}".format(ident)) + await sender + events.append("after asend {}".format(ident)) + + # Ensure that firstiter is called immediately on asend(), + # before the first turn of the coroutine that asend() returns, + # to match the behavior of native generators. + # Ensure that the firstiter that gets used is the one in effect + # at the time of that first call, rather than at the time of iteration. + iterA = agen("A") + iterB = agen("B") + await anext_verbosely(iterA, "A") + set_asyncgen_hooks(firstiter, finalizer) + await anext_verbosely(iterB, "B") + iterC = agen("C") + await anext_verbosely(iterC, "C") + + assert events == [ + "before asend A", "before send A", "yield 1 A", "after asend A", + "before asend B", "firstiter B", "before send B", "yield 1 B", + "after asend B", "before asend C", "firstiter C", "before send C", + "yield 1 C", "after asend C" + ] + del events[:] + + if sys.implementation.name == "pypy": + # pypy segfaults if an async generator's __del__ is called (even if it resurrects!) + # and then the underlying coroutine encounters another await: + # https://bitbucket.org/pypy/pypy/issues/2786/ + return + + from weakref import ref + refA, refB, refC = map(ref, (iterA, iterB, iterC)) + + # iterA uses the finalizer that was in effect when it started, i.e. no finalizer + await iterA.__anext__() + await iterA.__anext__() + del iterA + gc.collect() + assert refA() is None + assert events == [ + "yield 2 A", "after yield 2 A", "mock_sleep A", "yield 3 A", + "unwind 3 A" + ] + assert not to_finalize + del events[:] + + # iterB and iterC do use our finalizer + await iterC.__anext__() + await iterB.__anext__() + await iterC.__anext__() + idB, idC = id(iterB), id(iterC) + del iterB + gc.collect() + del iterC + gc.collect() + assert events == [ + "yield 2 C", "yield 2 B", "after yield 2 C", "mock_sleep C", + "yield 3 C", "finalizer B", "finalizer C" + ] + del events[:] + + # finalizer invokes aclose() is not called again once the revived reference drops + assert list(map(id, to_finalize)) == [idB, idC] + events.append("before aclose B") + await to_finalize[0].aclose() + events.append("before aclose C") + await to_finalize[1].aclose() + events.append("after aclose both") + del to_finalize[:] + gc.collect() + assert refB() is None and refC() is None + + assert events == [ + "before aclose B", "mock_sleep B", "before aclose C", "unwind 3 C", + "after aclose both" + ] diff --git a/docs/source/reference.rst b/docs/source/reference.rst index c2a0bbb..d6169f5 100644 --- a/docs/source/reference.rst +++ b/docs/source/reference.rst @@ -107,18 +107,39 @@ Semantics This library generally tries hard to match the semantics of Python 3.6's native async generators in every detail (`PEP 525 -`__), except that it adds -``yield from`` support, and it doesn't currently support the -``sys.{get,set}_asyncgen_hooks`` garbage collection API. There are two -main reasons for this: (a) it doesn't exist on Python 3.5, and (b) -even on 3.6, only built-in generators are supposed to use that API, -and that's not us. In any case, you probably shouldn't be relying on -garbage collection for async generators – see `this discussion +`__), with additional +support for ``yield from`` and for returning non-None values from +an async generator (under the theory that these may well be added +to native async generators one day). + + +Garbage collection hooks +~~~~~~~~~~~~~~~~~~~~~~~~ + +This library fully supports the native async generator +`finalization semantics `__, +including the per-thread ``firstiter`` and ``finalizer`` hooks. +You can use ``async_generator.set_asyncgen_hooks()`` exactly +like you would use ``sys.set_asyncgen_hooks()`` with native +generators. On Python 3.6+, the former is an alias for the latter, +so libraries that use the native mechanism should work seamlessly +with ``@async_generator`` functions. + +While finishing cleanup of an async generator is better than dropping +it on the floor at the first ``await``, it's still not a perfect solution; +in addition to the unpredictability of GC timing, the ``finalizer`` hook +has no practical way to determine the context in which the generator was +being iterated, so an exception thrown from the generator during ``aclose()`` +must either crash the program or get discarded. It's much better to close +your generators explicitly when you're done with them, perhaps using the +:ref:`aclosing context manager `. See `this discussion `__ and `PEP 533 `__ for more details. +.. _contextmanagers: + Context managers ---------------- @@ -135,7 +156,7 @@ module, but does ``await obj.aclose()`` instead of async for json_obj in agen: ... -Or if you want to write your own async context managers, we got you +Or if you want to write your own async context managers, we've got you covered: .. function:: asynccontextmanager From 0a49fd93cf3adc9b4d45c77aa14268a7c0f0c657 Mon Sep 17 00:00:00 2001 From: Joshua Oreman Date: Thu, 5 Apr 2018 16:31:13 -0400 Subject: [PATCH 2/5] Add newsfragment for GC hooks --- newsfragments/15.feature.rst | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 newsfragments/15.feature.rst diff --git a/newsfragments/15.feature.rst b/newsfragments/15.feature.rst new file mode 100644 index 0000000..de7fb49 --- /dev/null +++ b/newsfragments/15.feature.rst @@ -0,0 +1,5 @@ +Add support for PEP 525-style finalization hooks via ``set_asyncgen_hooks()`` +and ``get_asyncgen_hooks()`` functions, which mimic the behavior of the ones +in ``sys`` if we're running on a Python version that doesn't have them natively. +``@async_generator`` generators behave the same way as native ones with respect +to these hooks. From cca57d99ef09567498f6e416c9befc78cbf9dd2e Mon Sep 17 00:00:00 2001 From: Joshua Oreman Date: Fri, 6 Apr 2018 11:53:58 -0400 Subject: [PATCH 3/5] Cleanups after code review --- async_generator/_impl.py | 8 +++-- .../_tests/test_async_generator.py | 34 +++++++++++++------ docs/source/reference.rst | 6 +++- 3 files changed, 34 insertions(+), 14 deletions(-) diff --git a/async_generator/_impl.py b/async_generator/_impl.py index e798cae..9b648e8 100644 --- a/async_generator/_impl.py +++ b/async_generator/_impl.py @@ -277,6 +277,7 @@ def __init__(self, coroutine): self.ag_running = False self._finalizer = None self._closed = False + self._hooks_inited = False # On python 3.5.0 and 3.5.1, __aiter__ must be awaitable. # Starting in 3.5.2, it should not be awaitable, and if it is, then it @@ -325,8 +326,8 @@ def athrow(self, type, value=None, traceback=None): return self._do_it(self._it.throw, type, value, traceback) def _do_it(self, start_fn, *args): - coro_state = getcoroutinestate(self._coroutine) - if coro_state is CORO_CREATED: + if not self._hooks_inited: + self._hooks_inited = True (firstiter, self._finalizer) = get_asyncgen_hooks() if firstiter is not None: firstiter(self) @@ -357,6 +358,9 @@ async def aclose(self): state = getcoroutinestate(self._coroutine) if state is CORO_CLOSED or self._closed: return + # Make sure that even if we raise "async_generator ignored + # GeneratorExit", and thus fail to exhaust the coroutine, + # __del__ doesn't complain again. self._closed = True if state is CORO_CREATED: # Make sure that aclose() on an unstarted generator returns diff --git a/async_generator/_tests/test_async_generator.py b/async_generator/_tests/test_async_generator.py index e3b16e6..73b364b 100644 --- a/async_generator/_tests/test_async_generator.py +++ b/async_generator/_tests/test_async_generator.py @@ -624,13 +624,13 @@ async def test___del__(capfd): @async_generator async def awaits_when_unwinding(): - await yield_(1) + await yield_(0) try: - await yield_(2) + await yield_(1) finally: await mock_sleep() try: - await yield_(3) + await yield_(2) finally: nonlocal completions completions += 1 @@ -640,21 +640,21 @@ async def awaits_when_unwinding(): gen.__del__() gen = awaits_when_unwinding() - assert await collect(gen) == [1, 2, 3] + assert await collect(gen) == [0, 1, 2] # Exhausted, so no problem gen.__del__() - for turns in (1, 2, 3): + for stop_after_turn in (1, 2, 3): gen = awaits_when_unwinding() - for turn in range(1, turns + 1): + for turn in range(stop_after_turn): assert await gen.__anext__() == turn await gen.aclose() # Closed, so no problem gen.__del__() - for turns in (1, 2, 3): + for stop_after_turn in (1, 2, 3): gen = awaits_when_unwinding() - for turn in range(1, turns + 1): + for turn in range(stop_after_turn): assert await gen.__anext__() == turn if sys.implementation.name == "pypy": @@ -665,14 +665,14 @@ async def awaits_when_unwinding(): with pytest.raises(RuntimeError) as info: gen.__del__() assert "partially-exhausted async_generator" in str(info.value) - if turns == 3: + if stop_after_turn == 3: # We didn't increment completions, because we didn't finalize # the generator. Increment it now so the check below (which is # calibrated for the correct/CPython behavior) doesn't fire; # we know about the pypy bug. completions += 1 - elif turns == 2: + elif stop_after_turn == 2: # Stopped in the middle of a try/finally that awaits in the finally, # so __del__ can't cleanup. with pytest.raises(RuntimeError) as info: @@ -892,7 +892,7 @@ def two(agen): # pragma: no cover with pytest.raises(TypeError): set_asyncgen_hooks(finalizer=False) - def in_thread(results=[]): + def in_thread(results): results.append(get_asyncgen_hooks()) set_asyncgen_hooks(two, one) results.append(get_asyncgen_hooks()) @@ -964,6 +964,18 @@ async def anext_verbosely(iter, ident): ] del events[:] + # Ensure that firstiter is only called once, even if we create + # two asend() coroutines before iterating either of them. + iterX = agen("X") + sender1 = iterX.asend(None) + sender2 = iterX.asend(None) + events.append("before close") + sender1.close() + sender2.close() + await iterX.aclose() + assert events == ["firstiter X", "before close"] + del events[:] + if sys.implementation.name == "pypy": # pypy segfaults if an async generator's __del__ is called (even if it resurrects!) # and then the underlying coroutine encounters another await: diff --git a/docs/source/reference.rst b/docs/source/reference.rst index d6169f5..3f53032 100644 --- a/docs/source/reference.rst +++ b/docs/source/reference.rst @@ -123,7 +123,11 @@ You can use ``async_generator.set_asyncgen_hooks()`` exactly like you would use ``sys.set_asyncgen_hooks()`` with native generators. On Python 3.6+, the former is an alias for the latter, so libraries that use the native mechanism should work seamlessly -with ``@async_generator`` functions. +with ``@async_generator`` functions. On Python 3.5, where there is +no ``sys.set_asyncgen_hooks()``, most libraries probably *won't* know +about ``async_generator.set_asyncgen_hooks()``, so you'll need +to exercise more care with explicit cleanup, or install appropriate +hooks yourself. While finishing cleanup of an async generator is better than dropping it on the floor at the first ``await``, it's still not a perfect solution; From c9aa69aa43119f56391a8cd0b1b8e4c9d469c078 Mon Sep 17 00:00:00 2001 From: Joshua Oreman Date: Fri, 13 Apr 2018 00:27:53 -0400 Subject: [PATCH 4/5] Proper workaround for PyPy issue 2786, restoring full GC hooks functionality --- async_generator/_impl.py | 36 +++++++++++++------ .../_tests/test_async_generator.py | 33 ++++------------- 2 files changed, 32 insertions(+), 37 deletions(-) diff --git a/async_generator/_impl.py b/async_generator/_impl.py index 9b648e8..0b87421 100644 --- a/async_generator/_impl.py +++ b/async_generator/_impl.py @@ -271,6 +271,23 @@ def set_asyncgen_hooks(firstiter=UNSPECIFIED, finalizer=UNSPECIFIED): class AsyncGenerator: + # https://bitbucket.org/pypy/pypy/issues/2786: + # PyPy implements 'await' in a way that requires the frame object + # used to execute a coroutine to keep a weakref to that coroutine. + # During a GC pass, weakrefs to all doomed objects are broken + # before any of the doomed objects' finalizers are invoked. + # If an AsyncGenerator is unreachable, its _coroutine probably + # is too, and the weakref from ag._coroutine.cr_frame to + # ag._coroutine will be broken before ag.__del__ can do its + # one-turn close attempt or can schedule a full aclose() using + # the registered finalization hook. It doesn't look like the + # underlying issue is likely to be fully fixed anytime soon, + # so we work around it by preventing an AsyncGenerator and + # its _coroutine from being considered newly unreachable at + # the same time if the AsyncGenerator's finalizer might want + # to iterate the coroutine some more. + _pypy_issue2786_workaround = set() + def __init__(self, coroutine): self._coroutine = coroutine self._it = coroutine.__await__() @@ -331,6 +348,8 @@ def _do_it(self, start_fn, *args): (firstiter, self._finalizer) = get_asyncgen_hooks() if firstiter is not None: firstiter(self) + if sys.implementation.name == "pypy": + self._pypy_issue2786_workaround.add(self._coroutine) # On CPython 3.5.2 (but not 3.5.0), coroutines get cranky if you try # to iterate them after they're exhausted. Generators OTOH just raise @@ -345,6 +364,9 @@ async def step(): try: self.ag_running = True return await ANextIter(self._it, start_fn, *args) + except StopAsyncIteration: + self._pypy_issue2786_workaround.discard(self._coroutine) + raise finally: self.ag_running = False @@ -370,27 +392,19 @@ async def aclose(self): try: await self.athrow(GeneratorExit) except (GeneratorExit, StopAsyncIteration): - pass + self._pypy_issue2786_workaround.discard(self._coroutine) else: raise RuntimeError("async_generator ignored GeneratorExit") def __del__(self): + self._pypy_issue2786_workaround.discard(self._coroutine) if getcoroutinestate(self._coroutine) is CORO_CREATED: # Never started, nothing to clean up, just suppress the "coroutine # never awaited" message. self._coroutine.close() if getcoroutinestate(self._coroutine ) is CORO_SUSPENDED and not self._closed: - if sys.implementation.name == "pypy": - # pypy segfaults if we resume the coroutine from our __del__ - # and it executes any more 'await' statements, so we use the - # old async_generator behavior of "don't even try to finalize - # correctly". https://bitbucket.org/pypy/pypy/issues/2786/ - raise RuntimeError( - "partially-exhausted async_generator {!r} garbage collected" - .format(self.ag_code.co_name) - ) - elif self._finalizer is not None: + if self._finalizer is not None: self._finalizer(self) else: # Mimic the behavior of native generators on GC with no finalizer: diff --git a/async_generator/_tests/test_async_generator.py b/async_generator/_tests/test_async_generator.py index 73b364b..03f623d 100644 --- a/async_generator/_tests/test_async_generator.py +++ b/async_generator/_tests/test_async_generator.py @@ -657,22 +657,7 @@ async def awaits_when_unwinding(): for turn in range(stop_after_turn): assert await gen.__anext__() == turn - if sys.implementation.name == "pypy": - # pypy can't do the full finalization dance yet: - # https://bitbucket.org/pypy/pypy/issues/2786/. - # Also, pypy suppresses exceptions on explicit __del__ calls, - # not just implicit ones. - with pytest.raises(RuntimeError) as info: - gen.__del__() - assert "partially-exhausted async_generator" in str(info.value) - if stop_after_turn == 3: - # We didn't increment completions, because we didn't finalize - # the generator. Increment it now so the check below (which is - # calibrated for the correct/CPython behavior) doesn't fire; - # we know about the pypy bug. - completions += 1 - - elif stop_after_turn == 2: + if stop_after_turn == 2: # Stopped in the middle of a try/finally that awaits in the finally, # so __del__ can't cleanup. with pytest.raises(RuntimeError) as info: @@ -976,12 +961,6 @@ async def anext_verbosely(iter, ident): assert events == ["firstiter X", "before close"] del events[:] - if sys.implementation.name == "pypy": - # pypy segfaults if an async generator's __del__ is called (even if it resurrects!) - # and then the underlying coroutine encounters another await: - # https://bitbucket.org/pypy/pypy/issues/2786/ - return - from weakref import ref refA, refB, refC = map(ref, (iterA, iterB, iterC)) @@ -989,7 +968,9 @@ async def anext_verbosely(iter, ident): await iterA.__anext__() await iterA.__anext__() del iterA - gc.collect() + # Do multiple GC passes since we're deliberately shielding the + # coroutine objects from the first pass due to PyPy issue 2786. + for _ in range(4): gc.collect() assert refA() is None assert events == [ "yield 2 A", "after yield 2 A", "mock_sleep A", "yield 3 A", @@ -1004,9 +985,9 @@ async def anext_verbosely(iter, ident): await iterC.__anext__() idB, idC = id(iterB), id(iterC) del iterB - gc.collect() + for _ in range(4): gc.collect() del iterC - gc.collect() + for _ in range(4): gc.collect() assert events == [ "yield 2 C", "yield 2 B", "after yield 2 C", "mock_sleep C", "yield 3 C", "finalizer B", "finalizer C" @@ -1021,7 +1002,7 @@ async def anext_verbosely(iter, ident): await to_finalize[1].aclose() events.append("after aclose both") del to_finalize[:] - gc.collect() + for _ in range(4): gc.collect() assert refB() is None and refC() is None assert events == [ From c9168c1cb03d72a459e6b8a3ce1f888e7a4f7c19 Mon Sep 17 00:00:00 2001 From: Joshua Oreman Date: Fri, 13 Apr 2018 00:36:24 -0400 Subject: [PATCH 5/5] yapfify --- async_generator/_tests/test_async_generator.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/async_generator/_tests/test_async_generator.py b/async_generator/_tests/test_async_generator.py index 03f623d..7463663 100644 --- a/async_generator/_tests/test_async_generator.py +++ b/async_generator/_tests/test_async_generator.py @@ -970,7 +970,8 @@ async def anext_verbosely(iter, ident): del iterA # Do multiple GC passes since we're deliberately shielding the # coroutine objects from the first pass due to PyPy issue 2786. - for _ in range(4): gc.collect() + for _ in range(4): + gc.collect() assert refA() is None assert events == [ "yield 2 A", "after yield 2 A", "mock_sleep A", "yield 3 A", @@ -985,9 +986,11 @@ async def anext_verbosely(iter, ident): await iterC.__anext__() idB, idC = id(iterB), id(iterC) del iterB - for _ in range(4): gc.collect() + for _ in range(4): + gc.collect() del iterC - for _ in range(4): gc.collect() + for _ in range(4): + gc.collect() assert events == [ "yield 2 C", "yield 2 B", "after yield 2 C", "mock_sleep C", "yield 3 C", "finalizer B", "finalizer C" @@ -1002,7 +1005,8 @@ async def anext_verbosely(iter, ident): await to_finalize[1].aclose() events.append("after aclose both") del to_finalize[:] - for _ in range(4): gc.collect() + for _ in range(4): + gc.collect() assert refB() is None and refC() is None assert events == [