From 3070ec85c3533632298eb13c235f83820cadac87 Mon Sep 17 00:00:00 2001 From: Fantix King Date: Fri, 8 Mar 2019 18:41:36 -0600 Subject: [PATCH 1/7] Fix SSL memory leak Refs bpo-34745, protocol and transport form circular reference, causing SSLContext stack up ugly. --- tests/test_tcp.py | 66 +++++++++++++++++++++++++++++++++++++++++++++ uvloop/sslproto.pyx | 1 + 2 files changed, 67 insertions(+) diff --git a/tests/test_tcp.py b/tests/test_tcp.py index dcf497f6..3fbad009 100644 --- a/tests/test_tcp.py +++ b/tests/test_tcp.py @@ -1655,6 +1655,72 @@ async def client(addr): self.loop.run_until_complete( asyncio.wait_for(client(srv.addr), loop=self.loop, timeout=10)) + def test_create_connection_memory_leak(self): + if self.implementation == 'asyncio': + raise unittest.SkipTest() + + HELLO_MSG = b'1' * self.PAYLOAD_SIZE + + server_context = self._create_server_ssl_context( + self.ONLYCERT, self.ONLYKEY) + client_context = self._create_client_ssl_context() + + def serve(sock): + sock.settimeout(self.TIMEOUT) + + sock.starttls(server_context, server_side=True) + + sock.sendall(b'O') + data = sock.recv_all(len(HELLO_MSG)) + self.assertEqual(len(data), len(HELLO_MSG)) + + sock.unwrap() + sock.close() + + class ClientProto(asyncio.Protocol): + def __init__(self, on_data, on_eof): + self.on_data = on_data + self.on_eof = on_eof + self.con_made_cnt = 0 + + def connection_made(proto, tr): + # XXX: We assume user stores the transport in protocol + proto.tr = tr + proto.con_made_cnt += 1 + # Ensure connection_made gets called only once. + self.assertEqual(proto.con_made_cnt, 1) + + def data_received(self, data): + self.on_data.set_result(data) + + def eof_received(self): + self.on_eof.set_result(True) + + async def client(addr): + await asyncio.sleep(0.5, loop=self.loop) + + on_data = self.loop.create_future() + on_eof = self.loop.create_future() + + tr, proto = await self.loop.create_connection( + lambda: ClientProto(on_data, on_eof), *addr, + ssl=client_context) + + self.assertEqual(await on_data, b'O') + tr.write(HELLO_MSG) + await on_eof + + tr.close() + + with self.tcp_server(serve, timeout=self.TIMEOUT) as srv: + self.loop.run_until_complete( + asyncio.wait_for(client(srv.addr), loop=self.loop, timeout=10)) + + # No garbage is left for SSL client from loop.create_connection, even + # if user stores the SSLTransport in corresponding protocol instance + client_context = weakref.ref(client_context) + self.assertIsNone(client_context()) + def test_start_tls_client_buf_proto_1(self): if self.implementation == 'asyncio': raise unittest.SkipTest() diff --git a/uvloop/sslproto.pyx b/uvloop/sslproto.pyx index 54004349..f914c7df 100644 --- a/uvloop/sslproto.pyx +++ b/uvloop/sslproto.pyx @@ -339,6 +339,7 @@ cdef class SSLProtocol: self._set_state(UNWRAPPED) self._transport = None self._app_transport = None + self._app_protocol = None self._wakeup_waiter(exc) if self._shutdown_timeout_handle: From 1a5dbc28342215ecd97fecf043ac4b2afccb6b36 Mon Sep 17 00:00:00 2001 From: Yury Selivanov Date: Wed, 20 Mar 2019 00:28:08 -0400 Subject: [PATCH 2/7] 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. --- tests/test_base.py | 36 ++++++++++++++++++++++++++++++- uvloop/cbhandles.pxd | 6 +++--- uvloop/cbhandles.pyx | 51 +++++++++++++++++++++++++++++++------------- 3 files changed, 74 insertions(+), 19 deletions(-) diff --git a/tests/test_base.py b/tests/test_base.py index bc7f7299..566422f3 100644 --- a/tests/test_base.py +++ b/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 = [] diff --git a/uvloop/cbhandles.pxd b/uvloop/cbhandles.pxd index 80978b98..ba65d9d0 100644 --- a/uvloop/cbhandles.pxd +++ b/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) diff --git a/uvloop/cbhandles.pyx b/uvloop/cbhandles.pyx index 169efaeb..a7d62523 100644 --- a/uvloop/cbhandles.pyx +++ b/uvloop/cbhandles.pyx @@ -192,7 +192,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, self._run, self, delay) @@ -202,6 +207,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 @@ -225,7 +235,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: @@ -256,8 +266,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: @@ -272,6 +282,7 @@ cdef class TimerHandle: Py_DECREF(self) if PY37: Context_Exit(context) + self._clear() # Public API @@ -281,19 +292,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) + '>' @@ -305,6 +317,15 @@ 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 From d9a111be06c810b0097bbb949d19c6d852f37a4c Mon Sep 17 00:00:00 2001 From: Jelle Zijlstra Date: Mon, 25 Feb 2019 14:49:51 -0800 Subject: [PATCH 3/7] fix compilation warnings on older Python versions --- uvloop/includes/compat.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/uvloop/includes/compat.h b/uvloop/includes/compat.h index 860fa674..4bad369b 100644 --- a/uvloop/includes/compat.h +++ b/uvloop/includes/compat.h @@ -34,19 +34,19 @@ int epoll_ctl(int epfd, int op, int fd, struct epoll_event *event) {}; PyObject * Context_CopyCurrent(void) { PyErr_SetString(PyExc_NotImplementedError, - '"contextvars" support requires Python 3.7+'); + "\"contextvars\" support requires Python 3.7+"); return NULL; }; int Context_Enter(PyObject *ctx) { PyErr_SetString(PyExc_NotImplementedError, - '"contextvars" support requires Python 3.7+'); + "\"contextvars\" support requires Python 3.7+"); return -1; } int Context_Exit(PyObject *ctx) { PyErr_SetString(PyExc_NotImplementedError, - '"contextvars" support requires Python 3.7+'); + "\"contextvars\" support requires Python 3.7+"); return -1; } From 8f037a68f766d64f6fae77598900b640e7d574fa Mon Sep 17 00:00:00 2001 From: Fantix King Date: Sun, 17 Mar 2019 09:44:13 -0500 Subject: [PATCH 4/7] Round delay in call_later() * Fixes #233. --- tests/test_base.py | 13 +++++++++++++ uvloop/loop.pyx | 2 +- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/tests/test_base.py b/tests/test_base.py index 566422f3..6fbb2e4f 100644 --- a/tests/test_base.py +++ b/tests/test_base.py @@ -205,6 +205,19 @@ def cb(arg): self.loop.run_forever() self.assertEqual(calls, ['a']) + def test_call_later_rounding(self): + # Refs #233, call_later() and call_at() shouldn't call cb early + + def cb(): + self.loop.stop() + + for i in range(8): + self.loop.call_later(0.06 + 0.01, cb) # 0.06999999999999999 + started = int(round(self.loop.time() * 1000)) + self.loop.run_forever() + finished = int(round(self.loop.time() * 1000)) + self.assertGreaterEqual(finished - started, 70) + def test_call_at(self): if os.environ.get('TRAVIS_OS_NAME'): # Time seems to be really unpredictable on Travis. diff --git a/uvloop/loop.pyx b/uvloop/loop.pyx index 740d55ce..9f60b0e4 100644 --- a/uvloop/loop.pyx +++ b/uvloop/loop.pyx @@ -1291,7 +1291,7 @@ cdef class Loop: # infinity for a Python application. delay = 3600 * 24 * 365 * 100 - when = (delay * 1000) + when = round(delay * 1000) if not args: args = None if when == 0: From 06335f3e37503a4299791d86e6edcacd03be517e Mon Sep 17 00:00:00 2001 From: Yury Selivanov Date: Wed, 20 Mar 2019 13:24:47 -0400 Subject: [PATCH 5/7] Make test_call_later_rounding more stable (but less strict) --- tests/test_base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_base.py b/tests/test_base.py index 6fbb2e4f..2d76eda8 100644 --- a/tests/test_base.py +++ b/tests/test_base.py @@ -216,7 +216,7 @@ def cb(): started = int(round(self.loop.time() * 1000)) self.loop.run_forever() finished = int(round(self.loop.time() * 1000)) - self.assertGreaterEqual(finished - started, 70) + self.assertGreaterEqual(finished - started, 69) def test_call_at(self): if os.environ.get('TRAVIS_OS_NAME'): From 88608b8e6ab686e85a9fea2acd309c0a7c2f3e01 Mon Sep 17 00:00:00 2001 From: Yury Selivanov Date: Wed, 20 Mar 2019 13:25:48 -0400 Subject: [PATCH 6/7] Bump the version to 0.12.2 --- uvloop/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/uvloop/__init__.py b/uvloop/__init__.py index e6ef95ff..214a9903 100644 --- a/uvloop/__init__.py +++ b/uvloop/__init__.py @@ -7,7 +7,7 @@ from .loop import Loop as __BaseLoop # NOQA -__version__ = '0.12.2.dev0' +__version__ = '0.12.2' __all__ = ('new_event_loop', 'install', 'EventLoopPolicy') From 4ad2f46a0ba0a90417b144c3b520836384e4ecc1 Mon Sep 17 00:00:00 2001 From: Yury Selivanov Date: Wed, 20 Mar 2019 16:19:58 -0400 Subject: [PATCH 7/7] Bump the version to 0.12.3.dev0 --- uvloop/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/uvloop/__init__.py b/uvloop/__init__.py index 214a9903..60a86368 100644 --- a/uvloop/__init__.py +++ b/uvloop/__init__.py @@ -7,7 +7,7 @@ from .loop import Loop as __BaseLoop # NOQA -__version__ = '0.12.2' +__version__ = '0.12.3.dev0' __all__ = ('new_event_loop', 'install', 'EventLoopPolicy')