From a608d2d5a7f1aabe9bcbfc220135c5e126189390 Mon Sep 17 00:00:00 2001 From: Nikolay Kim Date: Fri, 9 Jun 2017 21:04:39 -0700 Subject: bpo-29406: asyncio SSL contexts leak sockets after calling close with certain servers (#409) * asyncio SSL contexts leak sockets after calling close with certain servers * cleanup _shutdown_timeout_handle on _fatal_error --- Lib/asyncio/sslproto.py | 25 ++++++++++++++++++++++++- Lib/test/test_asyncio/test_sslproto.py | 34 ++++++++++++++++++++++++++++++++++ Misc/NEWS | 4 ++++ 3 files changed, 62 insertions(+), 1 deletion(-) diff --git a/Lib/asyncio/sslproto.py b/Lib/asyncio/sslproto.py index 68499e5..a36725e 100644 --- a/Lib/asyncio/sslproto.py +++ b/Lib/asyncio/sslproto.py @@ -6,6 +6,8 @@ except ImportError: # pragma: no cover ssl = None from . import base_events +from . import compat +from . import futures from . import protocols from . import transports from .log import logger @@ -407,7 +409,7 @@ class SSLProtocol(protocols.Protocol): def __init__(self, loop, app_protocol, sslcontext, waiter, server_side=False, server_hostname=None, - call_connection_made=True): + call_connection_made=True, shutdown_timeout=5.0): if ssl is None: raise RuntimeError('stdlib ssl module not available') @@ -438,6 +440,8 @@ class SSLProtocol(protocols.Protocol): self._session_established = False self._in_handshake = False self._in_shutdown = False + self._shutdown_timeout = shutdown_timeout + self._shutdown_timeout_handle = None # transport, ex: SelectorSocketTransport self._transport = None self._call_connection_made = call_connection_made @@ -552,6 +556,15 @@ class SSLProtocol(protocols.Protocol): self._in_shutdown = True self._write_appdata(b'') + if self._shutdown_timeout is not None: + self._shutdown_timeout_handle = self._loop.call_later( + self._shutdown_timeout, self._on_shutdown_timeout) + + def _on_shutdown_timeout(self): + if self._transport is not None: + self._fatal_error( + futures.TimeoutError(), 'Can not complete shitdown operation') + def _write_appdata(self, data): self._write_backlog.append((data, 0)) self._write_buffer_size += len(data) @@ -679,12 +692,22 @@ class SSLProtocol(protocols.Protocol): }) if self._transport: self._transport._force_close(exc) + self._transport = None + + if self._shutdown_timeout_handle is not None: + self._shutdown_timeout_handle.cancel() + self._shutdown_timeout_handle = None def _finalize(self): self._sslpipe = None if self._transport is not None: self._transport.close() + self._transport = None + + if self._shutdown_timeout_handle is not None: + self._shutdown_timeout_handle.cancel() + self._shutdown_timeout_handle = None def _abort(self): try: diff --git a/Lib/test/test_asyncio/test_sslproto.py b/Lib/test/test_asyncio/test_sslproto.py index bcd236e..6bcaa9e 100644 --- a/Lib/test/test_asyncio/test_sslproto.py +++ b/Lib/test/test_asyncio/test_sslproto.py @@ -96,6 +96,40 @@ class SslProtoHandshakeTests(test_utils.TestCase): test_utils.run_briefly(self.loop) self.assertIsInstance(waiter.exception(), ConnectionAbortedError) + def test_close_abort(self): + # From issue #bpo-29406 + # abort connection if server does not complete shutdown procedure + ssl_proto = self.ssl_protocol() + transport = self.connection_made(ssl_proto) + ssl_proto._on_handshake_complete(None) + ssl_proto._start_shutdown() + self.assertIsNotNone(ssl_proto._shutdown_timeout_handle) + + exc_handler = mock.Mock() + self.loop.set_exception_handler(exc_handler) + ssl_proto._shutdown_timeout_handle._run() + + exc_handler.assert_called_with( + self.loop, {'message': 'Can not complete shitdown operation', + 'exception': mock.ANY, + 'transport': transport, + 'protocol': ssl_proto} + ) + self.assertIsNone(ssl_proto._shutdown_timeout_handle) + + def test_close(self): + # From issue #bpo-29406 + # abort connection if server does not complete shutdown procedure + ssl_proto = self.ssl_protocol() + transport = self.connection_made(ssl_proto) + ssl_proto._on_handshake_complete(None) + ssl_proto._start_shutdown() + self.assertIsNotNone(ssl_proto._shutdown_timeout_handle) + + ssl_proto._finalize() + self.assertIsNone(ssl_proto._transport) + self.assertIsNone(ssl_proto._shutdown_timeout_handle) + def test_close_during_handshake(self): # bpo-29743 Closing transport during handshake process leaks socket waiter = asyncio.Future(loop=self.loop) diff --git a/Misc/NEWS b/Misc/NEWS index 9db4e34..36ce9ce 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -356,6 +356,10 @@ Library - bpo-29743: Closing transport during handshake process leaks open socket. Patch by Nikolay Kim +- bpo-29406: asyncio SSL contexts leak sockets after calling close with + certain servers. + Patch by Nikolay Kim + - bpo-27585: Fix waiter cancellation in asyncio.Lock. Patch by Mathieu Sornay. -- cgit v0.12