From: Aaron Opfer <aaron.opfer@akunacapital.com>
Date: Tue, 26 Jul 2016 13:45:08 +0000 (-0500)
Subject: Tie Runner lifetime to Future's existence
X-Git-Tag: v4.5.0~75^2
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=refs%2Fpull%2F1782%2Fhead;p=thirdparty%2Ftornado.git

Tie Runner lifetime to Future's existence

Fixes #1769
---

diff --git a/tornado/gen.py b/tornado/gen.py
index b308ca7d0..73f9ba10a 100644
--- 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 4c873f4b5..bfaba5678 100644
--- 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()