asyncio: Fix a leak when event loops are created and destroyed

The WeakKeyDictionary in IOLoop wasn't doing its job because of
reference cycles. This was easiest to see with the synchronous
HTTPClient.

Fixes #2321

diff --git a/tornado/ioloop.py b/tornado/ioloop.py
index 839e7ee54ef0431ff961e360f7537a008856aed6..f6ec177bd8c009b1879a01d0da56468b47735fd9 100644 (file)
--- a/tornado/ioloop.py
+++ b/tornado/ioloop.py
@@ -47,7 +47,6 @@ import threading
 import time
 import traceback
 import math
-import weakref
 
 from tornado.concurrent import Future, is_future, chain_future, future_set_exc_info, future_add_done_callback  # noqa: E501
 from tornado.log import app_log, gen_log
@@ -185,7 +184,7 @@ class IOLoop(Configurable):
     _current = threading.local()
 
     # In Python 3, _ioloop_for_asyncio maps from asyncio loops to IOLoops.
-    _ioloop_for_asyncio = weakref.WeakKeyDictionary()
+    _ioloop_for_asyncio = dict()
 
     @classmethod
     def configure(cls, impl, **kwargs):

diff --git a/tornado/platform/asyncio.py b/tornado/platform/asyncio.py
index b2ad9fe6ef030193689ebef8e719e60495bb5839..b6a490afac5c76727d5dbb280177cc0604418d54 100644 (file)
--- a/tornado/platform/asyncio.py
+++ b/tornado/platform/asyncio.py
@@ -38,6 +38,20 @@ class BaseAsyncIOLoop(IOLoop):
         self.readers = set()
         self.writers = set()
         self.closing = False
+        # If an asyncio loop was closed through an asyncio interface
+        # instead of IOLoop.close(), we'd never hear about it and may
+        # have left a dangling reference in our map. In case an
+        # application (or, more likely, a test suite) creates and
+        # destroys a lot of event loops in this way, check here to
+        # ensure that we don't have a lot of dead loops building up in
+        # the map.
+        #
+        # TODO(bdarnell): consider making self.asyncio_loop a weakref
+        # for AsyncIOMainLoop and make _ioloop_for_asyncio a
+        # WeakKeyDictionary.
+        for loop in list(IOLoop._ioloop_for_asyncio):
+            if loop.is_closed():
+                del IOLoop._ioloop_for_asyncio[loop]
         IOLoop._ioloop_for_asyncio[asyncio_loop] = self
         super(BaseAsyncIOLoop, self).initialize(**kwargs)
 
@@ -49,6 +63,7 @@ class BaseAsyncIOLoop(IOLoop):
             if all_fds:
                 self.close_fd(fileobj)
         self.asyncio_loop.close()
+        del IOLoop._ioloop_for_asyncio[self.asyncio_loop]
 
     def add_handler(self, fd, handler, events):
         fd, fileobj = self.split_fd(fd)

diff --git a/tornado/test/asyncio_test.py b/tornado/test/asyncio_test.py
index 37f2f6e1b4d3e18efa3299c34f5a6dfd8b5af3f7..a7c7564964b2b97521413c3a15915d56293101ab 100644 (file)
--- a/tornado/test/asyncio_test.py
+++ b/tornado/test/asyncio_test.py
@@ -122,6 +122,44 @@ class AsyncIOLoopTest(AsyncTestCase):
             42)
 
 
+@unittest.skipIf(asyncio is None, "asyncio module not present")
+class LeakTest(unittest.TestCase):
+    def setUp(self):
+        # Trigger a cleanup of the mapping so we start with a clean slate.
+        AsyncIOLoop().close()
+        # If we don't clean up after ourselves other tests may fail on
+        # py34.
+        self.orig_policy = asyncio.get_event_loop_policy()
+        asyncio.set_event_loop_policy(asyncio.DefaultEventLoopPolicy())
+
+    def tearDown(self):
+        asyncio.get_event_loop().close()
+        asyncio.set_event_loop_policy(self.orig_policy)
+
+    def test_ioloop_close_leak(self):
+        orig_count = len(IOLoop._ioloop_for_asyncio)
+        for i in range(10):
+            # Create and close an AsyncIOLoop using Tornado interfaces.
+            loop = AsyncIOLoop()
+            loop.close()
+        new_count = len(IOLoop._ioloop_for_asyncio) - orig_count
+        self.assertEqual(new_count, 0)
+
+    def test_asyncio_close_leak(self):
+        orig_count = len(IOLoop._ioloop_for_asyncio)
+        for i in range(10):
+            # Create and close an AsyncIOMainLoop using asyncio interfaces.
+            loop = asyncio.new_event_loop()
+            loop.call_soon(IOLoop.current)
+            loop.call_soon(loop.stop)
+            loop.run_forever()
+            loop.close()
+        new_count = len(IOLoop._ioloop_for_asyncio) - orig_count
+        # Because the cleanup is run on new loop creation, we have one
+        # dangling entry in the map (but only one).
+        self.assertEqual(new_count, 1)
+
+
 @unittest.skipIf(asyncio is None, "asyncio module not present")
 class AnyThreadEventLoopPolicyTest(unittest.TestCase):
     def setUp(self):