Skip to content

Commit b0e91bd

Browse files
gh-155027: Flush close_notify before closing SSL transport
When SSL shutdown failed, SSLProtocol copied close_notify to the raw transport and immediately force-closed it. Selector transports discarded buffered output and proactor transports cancelled the active write. Close the raw transport normally after SSL write flow resumes, keep the shutdown timeout active while it drains, and preserve the SSL exception for the application protocol unless the raw transport reports its own error.
1 parent c443e33 commit b0e91bd

2 files changed

Lines changed: 94 additions & 7 deletions

File tree

Lib/asyncio/sslproto.py

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -323,6 +323,8 @@ def __init__(self, loop, app_protocol, sslcontext, waiter,
323323
self._outgoing = ssl.MemoryBIO()
324324
self._state = SSLProtocolState.UNWRAPPED
325325
self._conn_lost = 0 # Set when connection_lost called
326+
self._shutdown_exc = None
327+
self._shutdown_close_pending = False
326328
if call_connection_made:
327329
self._app_state = AppProtocolState.STATE_INIT
328330
else:
@@ -397,6 +399,11 @@ def connection_lost(self, exc):
397399
meaning a regular EOF is received or the connection was
398400
aborted or closed).
399401
"""
402+
if exc is None and self._shutdown_exc is not None:
403+
exc = self._shutdown_exc
404+
self._shutdown_exc = None
405+
self._shutdown_close_pending = False
406+
400407
self._write_backlog.clear()
401408
self._outgoing.read()
402409
self._conn_lost += 1
@@ -669,14 +676,21 @@ def _do_shutdown(self):
669676
self._on_shutdown_complete(None)
670677

671678
def _on_shutdown_complete(self, shutdown_exc):
672-
if self._shutdown_timeout_handle is not None:
673-
self._shutdown_timeout_handle.cancel()
674-
self._shutdown_timeout_handle = None
679+
# close() lets the raw transport flush data queued by
680+
# _process_outgoing(). _fatal_error() would force-close it and
681+
# discard the close_notify that shutdown just produced. Keep the
682+
# shutdown timeout active until connection_lost() bounds the drain.
683+
if shutdown_exc is not None:
684+
self._shutdown_exc = shutdown_exc
685+
self._shutdown_close_pending = True
686+
if not self._ssl_writing_paused:
687+
self._loop.call_soon(self._close_transport)
675688

676-
if shutdown_exc:
677-
self._fatal_error(shutdown_exc)
678-
else:
679-
self._loop.call_soon(self._transport.close)
689+
def _close_transport(self):
690+
if self._shutdown_close_pending:
691+
self._shutdown_close_pending = False
692+
if self._transport is not None:
693+
self._transport.close()
680694

681695
def _abort(self, exc):
682696
self._set_state(SSLProtocolState.UNWRAPPED)
@@ -927,6 +941,8 @@ def resume_writing(self):
927941
assert self._ssl_writing_paused
928942
self._ssl_writing_paused = False
929943
self._process_outgoing()
944+
if self._shutdown_close_pending:
945+
self._loop.call_soon(self._close_transport)
930946

931947
def _fatal_error(self, exc, message='Fatal error on transport'):
932948
if self._transport:

Lib/test/test_asyncio/test_sslproto.py

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,77 @@ def test_fatal_error_no_name_error(self):
9696
# Restore error logging.
9797
log.logger.setLevel(log_level)
9898

99+
def test_shutdown_error_closes_after_flushing(self):
100+
app_proto = mock.Mock(spec=asyncio.Protocol)
101+
app_proto.eof_received.return_value = False
102+
ssl_proto = self.ssl_protocol(proto=app_proto)
103+
ssl_proto._state = sslproto.SSLProtocolState.SHUTDOWN
104+
ssl_proto._app_state = sslproto.AppProtocolState.STATE_CON_MADE
105+
transport = mock.Mock()
106+
ssl_proto._transport = transport
107+
ssl_proto._sslobj = mock.Mock()
108+
shutdown_exc = ssl.SSLError(ssl.SSL_ERROR_SSL, 'shutdown failed')
109+
ssl_proto._sslobj.unwrap.side_effect = shutdown_exc
110+
ssl_proto._outgoing = mock.Mock()
111+
ssl_proto._outgoing.read.side_effect = [b'close notify', b'', b'']
112+
ssl_proto._outgoing.pending = 0
113+
timeout_handle = mock.Mock()
114+
ssl_proto._shutdown_timeout_handle = timeout_handle
115+
116+
ssl_proto._do_shutdown()
117+
ssl_proto.eof_received()
118+
119+
transport.write.assert_called_once_with(b'close notify')
120+
transport._force_close.assert_not_called()
121+
test_utils.run_briefly(self.loop)
122+
transport.close.assert_called_once_with()
123+
timeout_handle.cancel.assert_not_called()
124+
125+
ssl_proto.connection_lost(None)
126+
test_utils.run_briefly(self.loop)
127+
app_proto.connection_lost.assert_called_once_with(shutdown_exc)
128+
timeout_handle.cancel.assert_called_once_with()
129+
130+
def test_shutdown_waits_for_resume_writing_before_close(self):
131+
app_proto = mock.Mock(spec=asyncio.Protocol)
132+
ssl_proto = self.ssl_protocol(proto=app_proto)
133+
ssl_proto._state = sslproto.SSLProtocolState.SHUTDOWN
134+
ssl_proto._app_state = sslproto.AppProtocolState.STATE_CON_MADE
135+
transport = mock.Mock()
136+
ssl_proto._transport = transport
137+
ssl_proto._sslobj = mock.Mock()
138+
shutdown_exc = ssl.SSLError(ssl.SSL_ERROR_SSL, 'shutdown failed')
139+
ssl_proto._sslobj.unwrap.side_effect = shutdown_exc
140+
ssl_proto._outgoing = mock.Mock()
141+
ssl_proto._outgoing.read.return_value = b'close notify'
142+
ssl_proto._outgoing.pending = 0
143+
ssl_proto._ssl_writing_paused = True
144+
145+
ssl_proto._do_shutdown()
146+
test_utils.run_briefly(self.loop)
147+
148+
transport.write.assert_not_called()
149+
transport.close.assert_not_called()
150+
151+
ssl_proto.resume_writing()
152+
transport.write.assert_called_once_with(b'close notify')
153+
test_utils.run_briefly(self.loop)
154+
transport.close.assert_called_once_with()
155+
156+
def test_shutdown_raw_error_takes_precedence(self):
157+
app_proto = mock.Mock(spec=asyncio.Protocol)
158+
ssl_proto = self.ssl_protocol(proto=app_proto)
159+
ssl_proto._state = sslproto.SSLProtocolState.SHUTDOWN
160+
ssl_proto._app_state = sslproto.AppProtocolState.STATE_CON_MADE
161+
ssl_proto._shutdown_exc = ssl.SSLError(
162+
ssl.SSL_ERROR_SSL, 'shutdown failed')
163+
raw_exc = ConnectionResetError('raw write failed')
164+
165+
ssl_proto.connection_lost(raw_exc)
166+
test_utils.run_briefly(self.loop)
167+
168+
app_proto.connection_lost.assert_called_once_with(raw_exc)
169+
99170
def test_connection_lost(self):
100171
# From issue #472.
101172
# yield from waiter hang if lost_connection was called.

0 commit comments

Comments
 (0)