Skip to content

Commit c3e8ce6

Browse files
committed
Fixes #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 c3e8ce6

File tree

2 files changed

+79
-2
lines changed

2 files changed

+79
-2
lines changed

tests/test_tcp.py

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

2654+
def test_handshake_timeout_handler_leak(self):
2655+
if self.implementation == 'asyncio':
2656+
# Okay this turns out to be an issue for asyncio.sslproto too
2657+
raise unittest.SkipTest()
2658+
2659+
s = socket.socket(socket.AF_INET)
2660+
s.bind(('127.0.0.1', 0))
2661+
s.listen(1)
2662+
addr = s.getsockname()
2663+
2664+
async def test(ctx):
2665+
try:
2666+
await asyncio.wait_for(
2667+
self.loop.create_connection(asyncio.Protocol, *addr,
2668+
ssl=ctx),
2669+
0.1, loop=self.loop)
2670+
except (ConnectionRefusedError, asyncio.TimeoutError):
2671+
pass
2672+
else:
2673+
self.fail('TimeoutError is not raised')
2674+
2675+
with s:
2676+
ctx = ssl.create_default_context()
2677+
self.loop.run_until_complete(test(ctx))
2678+
ctx = weakref.ref(ctx)
2679+
2680+
# SSLProtocol should be DECREF to 0
2681+
self.assertIsNone(ctx())
2682+
2683+
def test_shutdown_timeout_handler_leak(self):
2684+
loop = self.loop
2685+
2686+
def server(sock):
2687+
sslctx = self._create_server_ssl_context(self.ONLYCERT,
2688+
self.ONLYKEY)
2689+
sock = sslctx.wrap_socket(sock, server_side=True)
2690+
sock.recv(32)
2691+
sock.close()
2692+
2693+
class Protocol(asyncio.Protocol):
2694+
def __init__(self):
2695+
self.fut = asyncio.Future(loop=loop)
2696+
2697+
def connection_lost(self, exc):
2698+
self.fut.set_result(None)
2699+
2700+
async def client(addr, ctx):
2701+
tr, pr = await loop.create_connection(Protocol, *addr, ssl=ctx)
2702+
tr.close()
2703+
await pr.fut
2704+
2705+
with self.tcp_server(server) as srv:
2706+
ctx = self._create_client_ssl_context()
2707+
loop.run_until_complete(client(srv.addr, ctx))
2708+
ctx = weakref.ref(ctx)
2709+
2710+
if self.implementation == 'asyncio':
2711+
# asyncio has no shutdown timeout, but it ends up with a circular
2712+
# reference loop - not ideal (introduces gc glitches), but at least
2713+
# not leaking
2714+
gc.collect()
2715+
gc.collect()
2716+
gc.collect()
2717+
2718+
# SSLProtocol should be DECREF to 0
2719+
self.assertIsNone(ctx())
2720+
26542721

26552722
class Test_UV_TCPSSL(_TestSSL, tb.UVTestCase):
26562723
pass

uvloop/sslproto.pyx

Lines changed: 12 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,13 @@ 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
457+
# self when the handle is cancelled (because uvloop currently clears
458+
# only args, it'll be okay to remove the lambda x: x() after the fix to
459+
# clear also the callback)
456460
self._handshake_timeout_handle = \
457461
self._loop.call_later(self._ssl_handshake_timeout,
462+
lambda x: x(),
458463
lambda: self._check_handshake_timeout())
459464

460465
try:
@@ -533,8 +538,13 @@ cdef class SSLProtocol:
533538
self._abort(None)
534539
else:
535540
self._set_state(FLUSHING)
541+
# XXX: This is a workaround to call cdef method and clear reference
542+
# to self when the handle is cancelled (because uvloop currently
543+
# clears only args, it'll be okay to remove the lambda x: x() after
544+
# the fix to clear also the callback)
536545
self._shutdown_timeout_handle = \
537546
self._loop.call_later(self._ssl_shutdown_timeout,
547+
lambda x: x(),
538548
lambda: self._check_shutdown_timeout())
539549
self._do_flush()
540550

0 commit comments

Comments
 (0)