Skip to content

Commit e2bba47

Browse files
committed
Refs #220, fix sslproto memory leak
There were two issues: 1) at `connection_lost()` the timeout handle was never cancelled because getting a cdef property on a cdef class with `getattr` always return `None`, and 2) for now cancelling a uvloop timeout handle only clears references to call arguments but not the callback object itself, therefore the closure is now a part of the arguments to avoid SSLProtocol object stays in memory until the cancelled timeout handle actually get deallocated.
1 parent 83ee1e3 commit e2bba47

File tree

2 files changed

+35
-2
lines changed

2 files changed

+35
-2
lines changed

tests/test_tcp.py

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2651,6 +2651,33 @@ async def test():
26512651
else:
26522652
self.fail('Unexpected ResourceWarning: {}'.format(cm.warning))
26532653

2654+
def test_handshake_timeout_handler_leak(self):
2655+
s = socket.socket(socket.AF_INET)
2656+
s.bind(('127.0.0.1', 0))
2657+
s.listen(1)
2658+
addr = s.getsockname()
2659+
2660+
async def test(ctx):
2661+
try:
2662+
await asyncio.wait_for(
2663+
self.loop.create_connection(asyncio.Protocol, *addr, ssl=ctx),
2664+
0.1, loop=self.loop)
2665+
except (ConnectionRefusedError, asyncio.TimeoutError):
2666+
pass
2667+
else:
2668+
self.fail('TimeoutError is not raised')
2669+
2670+
with s:
2671+
ctx = ssl.create_default_context()
2672+
self.loop.run_until_complete(test(ctx))
2673+
ctx = weakref.ref(ctx)
2674+
2675+
# SSLProtocol should be DECREF to 0
2676+
gc.collect()
2677+
gc.collect()
2678+
gc.collect()
2679+
self.assertIsNone(ctx())
2680+
26542681

26552682
class Test_UV_TCPSSL(_TestSSL, tb.UVTestCase):
26562683
pass

uvloop/sslproto.pyx

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -341,9 +341,9 @@ cdef class SSLProtocol:
341341
self._app_transport = None
342342
self._wakeup_waiter(exc)
343343

344-
if getattr(self, '_shutdown_timeout_handle', None):
344+
if self._shutdown_timeout_handle:
345345
self._shutdown_timeout_handle.cancel()
346-
if getattr(self, '_handshake_timeout_handle', None):
346+
if self._handshake_timeout_handle:
347347
self._handshake_timeout_handle.cancel()
348348

349349
def get_buffer(self, n):
@@ -453,8 +453,11 @@ cdef class SSLProtocol:
453453
self._set_state(DO_HANDSHAKE)
454454

455455
# start handshake timeout count down
456+
# XXX: This is a workaround to call cdef method and clear reference to self when
457+
# the handle is cancelled (because uvloop currently clears only args)
456458
self._handshake_timeout_handle = \
457459
self._loop.call_later(self._ssl_handshake_timeout,
460+
lambda x: x(),
458461
lambda: self._check_handshake_timeout())
459462

460463
try:
@@ -533,8 +536,11 @@ cdef class SSLProtocol:
533536
self._abort(None)
534537
else:
535538
self._set_state(FLUSHING)
539+
# XXX: This is a workaround to call cdef method and clear reference to self
540+
# when the handle is cancelled (because uvloop currently clears only args)
536541
self._shutdown_timeout_handle = \
537542
self._loop.call_later(self._ssl_shutdown_timeout,
543+
lambda x: x(),
538544
lambda: self._check_shutdown_timeout())
539545
self._do_flush()
540546

0 commit comments

Comments
 (0)