Tie Runner lifetime to Future's existence

Fixes #1769

diff --git a/tornado/gen.py b/tornado/gen.py
index b308ca7d0b55b9ba6f060293ee7a4eca45e705ce..73f9ba10a36f133a7fd940bb7f7022ca49e97738 100644 (file)
--- a/tornado/gen.py
+++ b/tornado/gen.py
@@ -83,6 +83,7 @@ import os
 import sys
 import textwrap
 import types
+import weakref
 
 from tornado.concurrent import Future, TracebackFuture, is_future, chain_future
 from tornado.ioloop import IOLoop
@@ -244,6 +245,24 @@ 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``.
@@ -294,7 +313,7 @@ def _make_coroutine_wrapper(func, replace_callback):
                 except Exception:
                     future.set_exc_info(sys.exc_info())
                 else:
-                    Runner(result, future, yielded)
+                    _futures_to_runners[future] = Runner(result, future, yielded)
                 try:
                     return future
                 finally:

diff --git a/tornado/test/gen_test.py b/tornado/test/gen_test.py
index 4c873f4b5a580b0d68bfdce4ec7e48148caf56e4..bfaba5678c554ecd5a9c18d09f8126bcf8264cc3 100644 (file)
--- a/tornado/test/gen_test.py
+++ b/tornado/test/gen_test.py
@@ -1,5 +1,6 @@
 from __future__ import absolute_import, division, print_function, with_statement
 
+import gc
 import contextlib
 import datetime
 import functools
@@ -25,7 +26,6 @@ try:
 except ImportError:
     futures = None
 
-
 class GenEngineTest(AsyncTestCase):
     def setUp(self):
         super(GenEngineTest, self).setUp()
@@ -1368,5 +1368,28 @@ class WaitIteratorTest(AsyncTestCase):
                                gen.WaitIterator(gen.sleep(0)).next())
 
 
+class RunnerGCTest(AsyncTestCase):
+    """Github issue 1769: Runner objects can get GCed unexpectedly"""
+    @gen_test
+    def test_gc(self):
+        """Runners shouldn't GC if future is alive"""
+        # Create the weakref
+        weakref_scope = [None]
+        def callback():
+            gc.collect(2)
+            weakref_scope[0]().set_result(123)
+
+        @gen.coroutine
+        def tester():
+            fut = Future()
+            weakref_scope[0] = weakref.ref(fut)
+            self.io_loop.add_callback(callback)
+            yield fut
+
+        yield gen.with_timeout(
+            datetime.timedelta(seconds=0.2),
+            tester()
+        )
+
 if __name__ == '__main__':
     unittest.main()