From: Antoine Pitrou Date: Fri, 29 Dec 2017 17:13:12 +0000 (+0100) Subject: Issue #2229: allow GCing of suspended coroutines X-Git-Tag: v5.0.0~16^2~2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=8c513117fcae553aa938427159de029da4f56edf;p=thirdparty%2Ftornado.git Issue #2229: allow GCing of suspended coroutines A suspended coroutine should be GCed if the underlying loop is closed and no other outside reference exists to it. However, a suspended coroutine with a refcycle would be kept alive by the _futures_to_runners mapping. Instead use a private attribute on the future. --- diff --git a/tornado/gen.py b/tornado/gen.py index f6b786add..183b28f17 100644 --- a/tornado/gen.py +++ b/tornado/gen.py @@ -82,7 +82,6 @@ import itertools import os import sys import types -import weakref from tornado.concurrent import (Future, is_future, chain_future, future_set_exc_info, future_add_done_callback, future_set_result_unless_cancelled) @@ -257,26 +256,6 @@ def coroutine(func, replace_callback=True): return _make_coroutine_wrapper(func, replace_callback=True) -# Ties lifetime of runners to their result futures. Github Issue #1769 -# Generators, like any object in Python, must be strong referenced -# in order to not be cleaned up by the garbage collector. When using -# coroutines, the Runner object is what strong-refs the inner -# generator. However, the only item that strong-reffed the Runner -# was the last Future that the inner generator yielded (via the -# Future's internal done_callback list). Usually this is enough, but -# it is also possible for this Future to not have any strong references -# other than other objects referenced by the Runner object (usually -# when using other callback patterns and/or weakrefs). In this -# situation, if a garbage collection ran, a cycle would be detected and -# Runner objects could be destroyed along with their inner generators -# and everything in their local scope. -# This map provides strong references to Runner objects as long as -# their result future objects also have strong references (typically -# from the parent coroutine's Runner). This keeps the coroutine's -# Runner alive. -_futures_to_runners = weakref.WeakKeyDictionary() - - def _make_coroutine_wrapper(func, replace_callback): """The inner workings of ``@gen.coroutine`` and ``@gen.engine``. @@ -331,7 +310,16 @@ def _make_coroutine_wrapper(func, replace_callback): except Exception: future_set_exc_info(future, sys.exc_info()) else: - _futures_to_runners[future] = Runner(result, future, yielded) + # Provide strong references to Runner objects as long + # as their result future objects also have strong + # references (typically from the parent coroutine's + # Runner). This keeps the coroutine's Runner alive. + # We do this by exploiting the public API + # add_done_callback() instead of putting a private + # attribute on the Future. + # (Github issues #1769, #2229). + runner = Runner(result, future, yielded) + future.add_done_callback(lambda _: runner) yielded = None try: return future diff --git a/tornado/test/concurrent_test.py b/tornado/test/concurrent_test.py index 3c7d435fc..1b6f8ee96 100644 --- a/tornado/test/concurrent_test.py +++ b/tornado/test/concurrent_test.py @@ -213,6 +213,9 @@ class ReturnFutureTest(AsyncTestCase): ".*ZeroDivisionError"): yield gen.moment yield gen.moment + # For some reason, TwistedIOLoop and pypy3 need a third iteration + # in order to drain references to the future + yield gen.moment del g gc.collect() # for PyPy @@ -425,3 +428,7 @@ class RunOnExecutorTest(AsyncTestCase): o = Object() answer = yield o.f() self.assertEqual(answer, 42) + + +if __name__ == '__main__': + unittest.main() diff --git a/tornado/test/gen_test.py b/tornado/test/gen_test.py index eee8655bc..ff7d9770a 100644 --- a/tornado/test/gen_test.py +++ b/tornado/test/gen_test.py @@ -4,6 +4,7 @@ import gc import contextlib import datetime import functools +import platform import sys import textwrap import time @@ -1478,12 +1479,14 @@ class WaitIteratorTest(AsyncTestCase): class RunnerGCTest(AsyncTestCase): - """Github issue 1769: Runner objects can get GCed unexpectedly""" - @skipOnTravis + def is_pypy3(self): + return (platform.python_implementation() == 'PyPy' and + sys.version_info > (3,)) + @gen_test def test_gc(self): - """Runners shouldn't GC if future is alive""" - # Create the weakref + # Github issue 1769: Runner objects can get GCed unexpectedly + # while their future is alive. weakref_scope = [None] def callback(): @@ -1502,6 +1505,85 @@ class RunnerGCTest(AsyncTestCase): tester() ) + def test_gc_infinite_coro(self): + # Github issue 2229: suspended coroutines should be GCed when + # their loop is closed, even if they're involved in a reference + # cycle. + if IOLoop.configured_class().__name__.endswith('TwistedIOLoop'): + raise unittest.SkipTest("Test may fail on TwistedIOLoop") + + loop = self.get_new_ioloop() + result = [] + wfut = [] + + @gen.coroutine + def infinite_coro(): + try: + while True: + yield gen.sleep(1e-3) + result.append(True) + finally: + # coroutine finalizer + result.append(None) + + @gen.coroutine + def do_something(): + fut = infinite_coro() + fut._refcycle = fut + wfut.append(weakref.ref(fut)) + yield gen.sleep(0.2) + + loop.run_sync(do_something) + loop.close() + gc.collect() + # Future was collected + self.assertIs(wfut[0](), None) + # At least one wakeup + self.assertGreaterEqual(len(result), 2) + if not self.is_pypy3(): + # coroutine finalizer was called (not on PyPy3 apparently) + self.assertIs(result[-1], None) + + @skipBefore35 + def test_gc_infinite_async_await(self): + # Same as test_gc_infinite_coro, but with a `async def` function + import asyncio + + namespace = exec_test(globals(), locals(), """ + async def infinite_coro(result): + try: + while True: + await gen.sleep(1e-3) + result.append(True) + finally: + # coroutine finalizer + result.append(None) + """) + + infinite_coro = namespace['infinite_coro'] + loop = self.get_new_ioloop() + result = [] + wfut = [] + + @gen.coroutine + def do_something(): + fut = asyncio.get_event_loop().create_task(infinite_coro(result)) + fut._refcycle = fut + wfut.append(weakref.ref(fut)) + yield gen.sleep(0.2) + + loop.run_sync(do_something) + with ExpectLog('asyncio', "Task was destroyed but it is pending"): + loop.close() + gc.collect() + # Future was collected + self.assertIs(wfut[0](), None) + # At least one wakeup and one finally + self.assertGreaterEqual(len(result), 2) + if not self.is_pypy3(): + # coroutine finalizer was called (not on PyPy3 apparently) + self.assertIs(result[-1], None) + if __name__ == '__main__': unittest.main()