From 4d44564e51055b75ffa3204c95bcdce3d5ba5a12 Mon Sep 17 00:00:00 2001 From: Mauricio Paez Date: Mon, 2 Nov 2026 15:51:29 -0500 Subject: [PATCH] gh-159875: Keep the SSL protocol of a closed transport after connection loss (GH-160069) * gh-159875: Add tests for an SSL transport closed after connection loss An application that closes its transport from connection_lost(), as aiohttp does, must be left with a transport whose write(), get_write_buffer_size() and set_protocol() behave as after closing a live connection, and loop.sendfile()'s fallback must then raise ConnectionError rather than AttributeError from set_protocol(). The sendfile test reads the file synchronously, so that the fallback is waiting in drain() when the connection is lost; a read in the executor that completes in the same loop iteration as the reset lets the fallback finish first and hides the bug. test_create_connection_memory_leak now keeps the transport past its check, as its comment says it covers, so that it would see an SSLContext kept alive by a transport the application still holds. Co-Authored-By: Claude Opus 5.5 * gh-159875: Keep the SSL protocol of a closed transport after connection loss _SSLProtocolTransport.close() dropped its _ssl_protocol when called a second time, and the transport is already marked closed once the connection is lost, so an application closing it from connection_lost() left a transport whose write(), set_protocol(), get_write_buffer_size(), ... raise AttributeError. That branch was also what released the SSLContext of a transport the application keeps. Keep the protocol, and release the TLS state from SSLProtocol.connection_lost() instead: one loop iteration later when the application's connection_lost() was scheduled, so that it can still read get_extra_info('ssl_object') and ('sslcontext'). Co-Authored-By: Claude Opus 5.5 --------- (cherry picked from commit 0626b14d31eb13b405c8b47b8b5259651051deec) Co-authored-by: Mauricio Paez Co-authored-by: Claude Opus 5.5 --- Lib/asyncio/sslproto.py | 19 +++++++++-- Lib/test/test_asyncio/test_sendfile.py | 32 +++++++++++++++++++ Lib/test/test_asyncio/test_ssl.py | 7 +++- Lib/test/test_asyncio/test_sslproto.py | 20 ++++++++++++ ...-10-21-15-42-29.gh-issue-159875.p6j3yW.rst | 7 ++++ 5 files changed, 82 insertions(+), 3 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2026-10-21-15-42-29.gh-issue-159875.p6j3yW.rst diff --git a/Lib/asyncio/sslproto.py b/Lib/asyncio/sslproto.py index 3e3854a46f87048..51934bca52d9654 100644 --- a/Lib/asyncio/sslproto.py +++ b/Lib/asyncio/sslproto.py @@ -114,8 +114,6 @@ def close(self): if not self._closed: self._closed = True self._ssl_protocol._start_shutdown() - else: - self._ssl_protocol = None def __del__(self, _warnings=warnings): if not self._closed: @@ -412,12 +410,14 @@ def connection_lost(self, exc): # The app protocol gets connection_lost() only if it got # connection_made(). After start_tls() that is already the case # while the handshake is in progress. + app_notified = False if ( self._app_state == AppProtocolState.STATE_CON_MADE or self._app_state == AppProtocolState.STATE_EOF ): self._app_state = AppProtocolState.STATE_CON_LOST self._loop.call_soon(self._app_protocol.connection_lost, exc) + app_notified = True self._set_state(SSLProtocolState.UNWRAPPED) self._transport = None self._app_transport = None @@ -430,6 +430,21 @@ def connection_lost(self, exc): if self._handshake_timeout_handle: self._handshake_timeout_handle.cancel() self._handshake_timeout_handle = None + if app_notified: + # After the app protocol's connection_lost() (scheduled above), + # so that it can still read the TLS extra info. + self._loop.call_soon(self._release_tls_state) + else: + self._release_tls_state() + + def _release_tls_state(self): + # The connection is gone: drop everything that refers to the + # SSLContext, so that an application holding the transport does + # not keep the context alive. + self._sslobj = None + self._sslcontext = None + self._extra.pop('sslcontext', None) + self._extra.pop('ssl_object', None) def get_buffer(self, n): want = n diff --git a/Lib/test/test_asyncio/test_sendfile.py b/Lib/test/test_asyncio/test_sendfile.py index bdc3aa604a158f3..e9099b8da0f5a54 100644 --- a/Lib/test/test_asyncio/test_sendfile.py +++ b/Lib/test/test_asyncio/test_sendfile.py @@ -512,6 +512,38 @@ def test_sendfile_ssl_close_peer_after_receiving(self): self.assertEqual(srv_proto.data, self.DATA) self.assertEqual(self.file.tell(), len(self.DATA)) + def test_sendfile_ssl_close_peer_in_the_middle_closes_transport(self): + # The protocol closes its transport when the connection is lost, as + # aiohttp's does. That close() must leave a transport on which the + # fallback can still restore the protocol: sendfile() raises + # ConnectionError, not AttributeError. + srv_proto, cli_proto = self.prepare_sendfile( + is_ssl=True, close_after=1024) + connection_lost = cli_proto.connection_lost + + def close_on_lost(exc): + cli_proto.transport.close() + connection_lost(exc) + + cli_proto.connection_lost = close_on_lost + # Read the file synchronously, so that the fallback only waits in + # drain(). A read in the executor that completes in the same loop + # iteration as the reset lets the fallback finish before close() is + # called, which hides the bug. + def run_in_executor(executor, func, *args): + fut = self.loop.create_future() + fut.set_result(func(*args)) + return fut + + with mock.patch.object(self.loop, 'run_in_executor', run_in_executor): + with self.assertRaises(ConnectionError): + self.run_loop( + self.loop.sendfile(cli_proto.transport, self.file)) + self.run_loop(srv_proto.done) + self.run_loop(cli_proto.done) + self.assertTrue(1024 <= srv_proto.nbytes < len(self.DATA), + srv_proto.nbytes) + # On Solaris, lowering SO_RCVBUF on a TCP connection after it has been # established has no effect. Due to its age, this bug affects both Oracle # Solaris as well as all other OpenSolaris forks (unless they fixed it diff --git a/Lib/test/test_asyncio/test_ssl.py b/Lib/test/test_asyncio/test_ssl.py index e97d7581f9e7860..d03b117d802c304 100644 --- a/Lib/test/test_asyncio/test_ssl.py +++ b/Lib/test/test_asyncio/test_ssl.py @@ -791,16 +791,21 @@ async def client(addr): await on_eof tr.close() + return tr with self.tcp_server(serve, timeout=self.TIMEOUT) as srv: - self.loop.run_until_complete( + tr = self.loop.run_until_complete( asyncio.wait_for(client(srv.addr), timeout=support.SHORT_TIMEOUT)) + # Let the protocol's connection_lost() run. + self.loop.run_until_complete(asyncio.sleep(0)) # No garbage is left for SSL client from loop.create_connection, even # if user stores the SSLTransport in corresponding protocol instance + # (and keeps the transport after the connection is lost). client_context = weakref.ref(client_context) self.assertIsNone(client_context()) + del tr def test_start_tls_client_buf_proto_1(self): HELLO_MSG = b'1' * self.PAYLOAD_SIZE diff --git a/Lib/test/test_asyncio/test_sslproto.py b/Lib/test/test_asyncio/test_sslproto.py index 30dccbe73fb3df6..349990a50f09519 100644 --- a/Lib/test/test_asyncio/test_sslproto.py +++ b/Lib/test/test_asyncio/test_sslproto.py @@ -289,6 +289,26 @@ def test_write_after_closing(self): # should not raise self.assertIsNone(transp.write(b'data')) + def test_transport_usable_after_connection_lost_and_close(self): + # connection_lost() marks the app transport closed, so the + # application's own close() afterwards (often made from its + # protocol's connection_lost()) must not be taken as a second + # close() that leaves the transport without its SSL protocol. + ssl_proto = self.ssl_protocol() + self.connection_made(ssl_proto) + transp = ssl_proto._app_transport + ssl_proto.connection_lost(None) + transp.close() + + # should not raise, as after closing a live connection + self.assertIsNone(transp.write(b'data')) + self.assertEqual(transp.get_write_buffer_size(), 0) + new_proto = asyncio.Protocol() + transp.set_protocol(new_proto) # as loop.sendfile()'s fallback does + self.assertIs(transp.get_protocol(), new_proto) + transp.abort() + self.assertTrue(transp.is_closing()) + def test_pause_reading_if_ssl_replies_are_not_read(self): # Output which OpenSSL produces on its own while reading (alerts, # KeyUpdate, ...) must not pile up if the peer does not read it. diff --git a/Misc/NEWS.d/next/Library/2026-10-21-15-42-29.gh-issue-159875.p6j3yW.rst b/Misc/NEWS.d/next/Library/2026-10-21-15-42-29.gh-issue-159875.p6j3yW.rst new file mode 100644 index 000000000000000..824fa9567d79111 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2026-10-21-15-42-29.gh-issue-159875.p6j3yW.rst @@ -0,0 +1,7 @@ +Fix :exc:`AttributeError` from the methods of an :mod:`asyncio` SSL transport, +such as :meth:`~asyncio.WriteTransport.write` and +:meth:`~asyncio.BaseTransport.set_protocol`, when the transport is closed after +its connection was lost. This also affected the fallback of +:meth:`loop.sendfile() ` over SSL when the peer went +away mid-transfer. The SSL context is still released once the connection is +lost.