Use a weak reference to clean up the link for local objects if they die before...

Use a weak reference to clean up the link for local objects if they die before the greenlet does (#983)

Fixes #981

CHANGES.rst

@@ -7,7 +7,11 @@
 1.2.3 (unreleased)
 ==================
 
-- Nothing changed yet.
+- If a single greenlet created and destroyed many
+  :class:`gevent.local.local` objects without ever exiting, there
+  would be a leak of the function objects intended to clean up the
+  locals after the greenlet exited. Introduce a weak reference to
+  avoid that. Reported in :issue:`981` by Heungsub Lee.
 
 
 1.2.2 (2017-06-05)

src/gevent/local.py

@@ -133,6 +133,10 @@ affects what we see:
    This results in locals being eligible for garbage collection as soon
    as their greenlet exits.
 
+.. versionchanged:: 1.2.3
+   Use a weak-reference to clear the greenlet link we establish in case
+   the local object dies before the greenlet does.
+
 """
 
 from copy import copy
@@ -174,39 +178,44 @@ class _localimpl(object):
         thread = getcurrent()
         idt = id(thread)
 
-        # If we are working with a gevent.greenlet.Greenlet, we can
-        # pro-actively clear out with a link. Use rawlink to avoid
-        # spawning any more greenlets
+        wrdicts = ref(self.dicts)
+
+        def thread_deleted(_, idt=idt, wrdicts=wrdicts):
+            # When the thread is deleted, remove the local dict.
+            # Note that this is suboptimal if the thread object gets
+            # caught in a reference loop. We would like to be called
+            # as soon as the OS-level thread ends instead.
+
+            # If we are working with a gevent.greenlet.Greenlet, we
+            # can pro-actively clear out with a link, avoiding the
+            # issue described above.Use rawlink to avoid spawning any
+            # more greenlets.
+            dicts = wrdicts()
+            if dicts:
+                dicts.pop(idt, None)
+
         try:
             rawlink = thread.rawlink
         except AttributeError:
-            # Otherwise we need to do it with weak refs
-            def local_deleted(_, key=key):
-                # When the localimpl is deleted, remove the thread attribute.
-                thread = wrthread()
-                if thread is not None:
-                    del thread.__dict__[key]
-
-            def thread_deleted(_, idt=idt):
-                # When the thread is deleted, remove the local dict.
-                # Note that this is suboptimal if the thread object gets
-                # caught in a reference loop. We would like to be called
-                # as soon as the OS-level thread ends instead.
-                _local = wrlocal()
-                if _local is not None:
-                    _local.dicts.pop(idt, None)
-            wrlocal = ref(self, local_deleted)
             wrthread = ref(thread, thread_deleted)
-            thread.__dict__[key] = wrlocal
         else:
-            wrdicts = ref(self.dicts)
-
-            def clear(_):
-                dicts = wrdicts()
-                if dicts:
-                    dicts.pop(idt, None)
-            rawlink(clear)
-            wrthread = None
+            rawlink(thread_deleted)
+            wrthread = ref(thread)
+
+        def local_deleted(_, key=key, wrthread=wrthread):
+            # When the localimpl is deleted, remove the thread attribute.
+            thread = wrthread()
+            if thread is not None:
+                try:
+                    unlink = thread.unlink
+                except AttributeError:
+                    pass
+                else:
+                    unlink(thread_deleted)
+                del thread.__dict__[key]
+
+        wrlocal = ref(self, local_deleted)
+        thread.__dict__[key] = wrlocal
 
         self.dicts[idt] = wrthread, localdict
         return localdict

src/greentest/test__local.py

@@ -25,7 +25,6 @@ class Obj(object):
 deleted_sentinels = []
 created_sentinels = []
 
-
 class Sentinel(object):
     def __del__(self):
         deleted_sentinels.append(id(self))
@@ -105,5 +104,43 @@ class GeventLocalTestCase(greentest.TestCase):
         # The sentinels should be gone too
         self.assertEqual(len(deleted_sentinels), len(greenlets))
 
+    def test_locals_collected_when_unreferenced_even_in_running_greenlet(self):
+        # https://github.com/gevent/gevent/issues/981
+        import gevent
+        import gc
+        gc.collect()
+
+        del created_sentinels[:]
+        del deleted_sentinels[:]
+
+        count = 1000
+
+        running_greenlet = None
+
+        def demonstrate_my_local():
+            for i in range(1000):
+                x = MyLocal()
+                self.assertIsNotNone(x.sentinel)
+                x = None
+
+            gc.collect()
+            gc.collect()
+
+            self.assertEqual(count, len(created_sentinels))
+            # They're all dead, even though this greenlet is
+            # still running
+            self.assertEqual(count, len(deleted_sentinels))
+
+            # The links were removed as well.
+            self.assertEqual(list(running_greenlet._links), [])
+
+
+        running_greenlet = gevent.spawn(demonstrate_my_local)
+        gevent.sleep()
+        running_greenlet.join()
+
+        self.assertEqual(count, len(deleted_sentinels))
+
+
 if __name__ == '__main__':
     greentest.main()