Skip to content

Commit

Permalink
Cleanup refs to callback in Handle/TimerHandle on cancel
Browse files Browse the repository at this point in the history
Currently there's a lot of potential to create reference cycles
between callback handles and protocols/transports that use them.
Basically any bound-method has a potential to create such a cycle.
This happens especially often for TimerHandles used to ensure
timeouts.

Fix that by ensuring that the handle clears the ref to its callback
when the 'cancel()' method is called.
  • Loading branch information
1st1 committed Feb 11, 2019
1 parent 8462981 commit f0a945d
Showing 1 changed file with 20 additions and 13 deletions.
33 changes: 20 additions & 13 deletions uvloop/cbhandles.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,10 @@ cdef class Handle:

if cb_type == 1:
callback = self.arg1
if callback is None:
raise RuntimeError(
'cannot run Handle; callback is not set')

args = self.arg2

if args is None:
Expand Down Expand Up @@ -106,11 +110,11 @@ cdef class Handle:
cdef _cancel(self):
self._cancelled = 1
self.callback = NULL
self.arg2 = self.arg3 = self.arg4 = None
self.arg1 = self.arg2 = self.arg3 = self.arg4 = None

cdef _format_handle(self):
# Mirrors `asyncio.base_events._format_handle`.
if self.cb_type == 1:
if self.cb_type == 1 and self.arg1 is not None:
cb = self.arg1
if isinstance(getattr(cb, '__self__', None), aio_Task):
try:
Expand All @@ -135,7 +139,7 @@ cdef class Handle:
if self._cancelled:
info.append('cancelled')

if self.cb_type == 1:
if self.cb_type == 1 and self.arg1 is not None:
func = self.arg1
# Cython can unset func.__qualname__/__name__, hence the checks.
if hasattr(func, '__qualname__') and func.__qualname__:
Expand All @@ -146,7 +150,7 @@ cdef class Handle:
cb_name = repr(func)

info.append(cb_name)
else:
elif self.meth_name is not None:
info.append(self.meth_name)

if self._source_traceback is not None:
Expand Down Expand Up @@ -214,6 +218,7 @@ cdef class TimerHandle:
if self.timer is None:
return

self.callback = None
self.args = None

try:
Expand All @@ -225,10 +230,11 @@ cdef class TimerHandle:
cdef _run(self):
if self._cancelled == 1:
return
if self.callback is None:
raise RuntimeError('cannot run TimerHandle; callback is not set')

callback = self.callback
args = self.args
self._clear()

Py_INCREF(self) # Since _run is a cdef and there's no BoundMethod,
# we guard 'self' manually.
Expand Down Expand Up @@ -275,15 +281,16 @@ cdef class TimerHandle:
if self._cancelled:
info.append('cancelled')

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.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)

info.append(cb_name)
info.append(cb_name)

if self._source_traceback is not None:
frame = self._source_traceback[-1]
Expand Down

0 comments on commit f0a945d

Please sign in to comment.