Skip to content

[3.13] gh-118950: Fix SSLProtocol.connection_lost not being called when OSError is thrown (GH-118960) #125931

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion Lib/asyncio/sslproto.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ def get_protocol(self):
return self._ssl_protocol._app_protocol

def is_closing(self):
return self._closed
return self._closed or self._ssl_protocol._is_transport_closing()

def close(self):
"""Close the transport.
Expand Down Expand Up @@ -379,6 +379,9 @@ def _get_app_transport(self):
self._app_transport_created = True
return self._app_transport

def _is_transport_closing(self):
return self._transport is not None and self._transport.is_closing()

def connection_made(self, transport):
"""Called when the low-level connection is made.

Expand Down
48 changes: 48 additions & 0 deletions Lib/test/test_asyncio/test_sslproto.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,54 @@ def test_connection_lost(self):
test_utils.run_briefly(self.loop)
self.assertIsInstance(waiter.exception(), ConnectionAbortedError)

def test_connection_lost_when_busy(self):
# gh-118950: SSLProtocol.connection_lost not being called when OSError
# is thrown on asyncio.write.
sock = mock.Mock()
sock.fileno = mock.Mock(return_value=12345)
sock.send = mock.Mock(side_effect=BrokenPipeError)

# construct StreamWriter chain that contains loop dependant logic this emulates
# what _make_ssl_transport() does in BaseSelectorEventLoop
reader = asyncio.StreamReader(limit=2 ** 16, loop=self.loop)
protocol = asyncio.StreamReaderProtocol(reader, loop=self.loop)
ssl_proto = self.ssl_protocol(proto=protocol)

# emulate reading decompressed data
sslobj = mock.Mock()
sslobj.read.side_effect = ssl.SSLWantReadError
sslobj.write.side_effect = ssl.SSLWantReadError
ssl_proto._sslobj = sslobj

# emulate outgoing data
data = b'An interesting message'

outgoing = mock.Mock()
outgoing.read = mock.Mock(return_value=data)
outgoing.pending = len(data)
ssl_proto._outgoing = outgoing

# use correct socket transport to initialize the SSLProtocol
self.loop._make_socket_transport(sock, ssl_proto)

transport = ssl_proto._app_transport
writer = asyncio.StreamWriter(transport, protocol, reader, self.loop)

async def main():
# writes data to transport
async def write():
writer.write(data)
await writer.drain()

# try to write for the first time
await write()
# try to write for the second time, this raises as the connection_lost
# callback should be done with error
with self.assertRaises(ConnectionResetError):
await write()

self.loop.run_until_complete(main())

def test_close_during_handshake(self):
# bpo-29743 Closing transport during handshake process leaks socket
waiter = self.loop.create_future()
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix bug where SSLProtocol.connection_lost wasn't getting called when OSError was thrown on writing to socket.
Loading