Fix memory leak related to call_later() and call_at()

The crux of the problem is that TimerHandle did not clean up a strong
reference from the event loop to `self`.  This typically isn't a problem
unless there's another strong reference to the loop from the callback or
from its arguments (such as a Future).

A few new unit tests should ensure this kind of bugs won't happen again.

Fixes: #239.

Author: 1st1

tests/test_base.py

@@ -36,7 +36,7 @@ def test_handle_weakref(self):
         h = self.loop.call_soon(lambda: None)
         wd['h'] = h  # Would fail without __weakref__ slot.
 
-    def test_call_soon(self):
+    def test_call_soon_1(self):
         calls = []
 
         def cb(inc):
@@ -56,6 +56,22 @@ def cb(inc):
 
         self.assertEqual(calls, [10, 1])
 
+    def test_call_soon_2(self):
+        waiter = self.loop.create_future()
+        waiter_r = weakref.ref(waiter)
+        self.loop.call_soon(lambda f: f.set_result(None), waiter)
+        self.loop.run_until_complete(waiter)
+        del waiter
+        self.assertIsNone(waiter_r())
+
+    def test_call_soon_3(self):
+        waiter = self.loop.create_future()
+        waiter_r = weakref.ref(waiter)
+        self.loop.call_soon(lambda f=waiter: f.set_result(None))
+        self.loop.run_until_complete(waiter)
+        del waiter
+        self.assertIsNone(waiter_r())
+
     def test_call_soon_base_exc(self):
         def cb():
             raise KeyboardInterrupt()
@@ -160,6 +176,24 @@ async def main():
         delta = time.monotonic() - started
         self.assertGreater(delta, 0.019)
 
+    def test_call_later_3(self):
+        # a memory leak regression test
+        waiter = self.loop.create_future()
+        waiter_r = weakref.ref(waiter)
+        self.loop.call_later(0.01, lambda f: f.set_result(None), waiter)
+        self.loop.run_until_complete(waiter)
+        del waiter
+        self.assertIsNone(waiter_r())
+
+    def test_call_later_4(self):
+        # a memory leak regression test
+        waiter = self.loop.create_future()
+        waiter_r = weakref.ref(waiter)
+        self.loop.call_later(0.01, lambda f=waiter: f.set_result(None))
+        self.loop.run_until_complete(waiter)
+        del waiter
+        self.assertIsNone(waiter_r())
+
     def test_call_later_negative(self):
         calls = []
 

uvloop/cbhandles.pxd

@@ -24,15 +24,15 @@ cdef class Handle:
 
 cdef class TimerHandle:
     cdef:
-        object callback, args
+        object callback
+        tuple args
         bint _cancelled
         UVTimer timer
         Loop loop
         object context
+        tuple _debug_info
         object __weakref__
 
-        readonly _source_traceback
-
     cdef _run(self)
     cdef _cancel(self)
     cdef inline _clear(self)

uvloop/cbhandles.pyx

@@ -194,7 +194,12 @@ cdef class TimerHandle:
             self.context = None
 
         if loop._debug:
-            self._source_traceback = extract_stack()
+            self._debug_info = (
+                format_callback_name(callback),
+                extract_stack()
+            )
+        else:
+            self._debug_info = None
 
         self.timer = UVTimer.new(
             loop, <method_t>self._run, self, delay)
@@ -204,6 +209,11 @@ cdef class TimerHandle:
         # Only add to loop._timers when `self.timer` is successfully created
         loop._timers.add(self)
 
+    property _source_traceback:
+        def __get__(self):
+            if self._debug_info is not None:
+                return self._debug_info[1]
+
     def __dealloc__(self):
         if UVLOOP_DEBUG:
             self.loop._debug_cb_timer_handles_count -= 1
@@ -227,7 +237,7 @@ cdef class TimerHandle:
             self.loop._timers.remove(self)
         finally:
             self.timer._close()
-            self.timer = None  # let it die asap
+            self.timer = None  # let the UVTimer handle GC
 
     cdef _run(self):
         if self._cancelled == 1:
@@ -260,8 +270,8 @@ cdef class TimerHandle:
                 'handle': self,
             }
 
-            if self._source_traceback is not None:
-                context['source_traceback'] = self._source_traceback
+            if self._debug_info is not None:
+                context['source_traceback'] = self._debug_info[1]
 
             self.loop.call_exception_handler(context)
         else:
@@ -276,6 +286,7 @@ cdef class TimerHandle:
             Py_DECREF(self)
             if PY37:
                 Context_Exit(context)
+            self._clear()
 
     # Public API
 
@@ -285,19 +296,20 @@ cdef class TimerHandle:
         if self._cancelled:
             info.append('cancelled')
 
-        if self.callback is not None:
-            func = self.callback
-            if hasattr(func, '__qualname__'):
-                cb_name = getattr(func, '__qualname__')
-            elif hasattr(func, '__name__'):
-                cb_name = getattr(func, '__name__')
-            else:
-                cb_name = repr(func)
+        if self._debug_info is not None:
+            callback_name = self._debug_info[0]
+            source_traceback = self._debug_info[1]
+        else:
+            callback_name = None
+            source_traceback = None
 
-            info.append(cb_name)
+        if callback_name is not None:
+            info.append(callback_name)
+        elif self.callback is not None:
+            info.append(format_callback_name(self.callback))
 
-        if self._source_traceback is not None:
-            frame = self._source_traceback[-1]
+        if source_traceback is not None:
+            frame = source_traceback[-1]
             info.append('created at {}:{}'.format(frame[0], frame[1]))
 
         return '<' + ' '.join(info) + '>'
@@ -309,6 +321,16 @@ cdef class TimerHandle:
         self._cancel()
 
 
+cdef format_callback_name(func):
+    if hasattr(func, '__qualname__'):
+        cb_name = getattr(func, '__qualname__')
+    elif hasattr(func, '__name__'):
+        cb_name = getattr(func, '__name__')
+    else:
+        cb_name = repr(func)
+    return cb_name
+
+
 cdef new_Handle(Loop loop, object callback, object args, object context):
     cdef Handle handle
     handle = Handle.__new__(Handle)