Skip to content

Commit 2f9ea1a

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 2f9ea1a

File tree

2 files changed

+72
-2
lines changed

2 files changed

+72
-2
lines changed

tests/test_tcp.py

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2651,6 +2651,70 @@ 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, ssl=ctx),
2668+
0.1, loop=self.loop)
2669+
except (ConnectionRefusedError, asyncio.TimeoutError):
2670+
pass
2671+
else:
2672+
self.fail('TimeoutError is not raised')
2673+
2674+
with s:
2675+
ctx = ssl.create_default_context()
2676+
self.loop.run_until_complete(test(ctx))
2677+
ctx = weakref.ref(ctx)
2678+
2679+
# SSLProtocol should be DECREF to 0
2680+
self.assertIsNone(ctx())
2681+
2682+
def test_shutdown_timeout_handler_leak(self):
2683+
loop = self.loop
2684+
2685+
def server(sock):
2686+
sslctx = self._create_server_ssl_context(self.ONLYCERT, self.ONLYKEY)
2687+
sock = sslctx.wrap_socket(sock, server_side=True)
2688+
sock.recv(32)
2689+
sock.close()
2690+
2691+
class Protocol(asyncio.Protocol):
2692+
def __init__(self):
2693+
self.fut = asyncio.Future(loop=loop)
2694+
2695+
def connection_lost(self, exc):
2696+
self.fut.set_result(None)
2697+
2698+
async def client(addr, ctx):
2699+
tr, pr = await loop.create_connection(Protocol, *addr, ssl=ctx)
2700+
tr.close()
2701+
await pr.fut
2702+
2703+
with self.tcp_server(server) as srv:
2704+
ctx = self._create_client_ssl_context()
2705+
loop.run_until_complete(client(srv.addr, ctx))
2706+
ctx = weakref.ref(ctx)
2707+
2708+
if self.implementation == 'asyncio':
2709+
# asyncio has no shutdown timeout, but it ends up with a circular reference
2710+
# loop - not ideal (introduces gc glitches), but at least not leaking
2711+
gc.collect()
2712+
gc.collect()
2713+
gc.collect()
2714+
2715+
# SSLProtocol should be DECREF to 0
2716+
self.assertIsNone(ctx())
2717+
26542718

26552719
class Test_UV_TCPSSL(_TestSSL, tb.UVTestCase):
26562720
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)