bpo-30698: asyncio shutdown the ssl layer cleanly. #2270
Conversation
|
@grzgrzgrz3, thanks for your PR! By analyzing the history of the files in this pull request, we identified @1st1, @fafhrd91 and @serhiy-storchaka to be potential reviewers. |
| ssldata = self._sslpipe.shutdown(self._finalize) | ||
| ssldata = self._sslpipe.shutdown() | ||
| self._feed_ssl_data(ssldata) | ||
| self._finalize() | ||
| offset = 1 |
There was a problem hiding this comment.
There is no need. When we return - this code is Omitted :
del self._write_backlog[0]
If _process_write_backlog is called after shutdown, we ll do shutdown again. If we want to return here we need to add del either. I think this may complicate this method even more.
Maybe something like that gist, but code with write blocked on a read is not tested, i will suggest to not make more changes.
I am planing to add more unittests in future and refactor sslproto.
Lib/asyncio/sslproto.py
Outdated
| @@ -636,11 +633,12 @@ def _process_write_backlog(self): | |||
| self._on_handshake_complete) | |||
| offset = 1 | |||
| else: | |||
| ssldata = self._sslpipe.shutdown(self._finalize) | |||
| ssldata = self._sslpipe.shutdown() | |||
There was a problem hiding this comment.
Do we want to wrap this in try..finally to guarantee that self._finalize will be called?
There was a problem hiding this comment.
transport will be closed anyway in self._fatal_error, but why not.
Asyncio on shutdown do not send shutdown confirmation to the other side. _SSLPipe after doing unwrap is calling shutdown calback where transport is closed and quit ssldata wont be sent. Having this callback in _SSLPipe feels wrong and its removed. Removing callback from _SSLPipe.shutdown should not break any api.
grzgrzgrz3
force-pushed
the
bpo-30698-asyncio-ssl-shutdown
branch
from
24df62a
to
e983306
Compare
|
Could you please rebase the PR against the current master? NEWS should be generated with blurb |
|
It appears that the author of the pull request is no longer active. I'm going to close this PR so someone else can create a new one if they are interested. |
|
Agree, I'm working on the better fix here: #17975 |
Asyncio on shutdown do not send shutdown confirmation to the other side.
_SSLPipe after doing unwrap is calling shutdown calback where transport
is closed and quit ssldata wont be sent. Having this callback in
_SSLPipe feels wrong and its removed.
Removing callback from _SSLPipe.shutdown should not break any api.
https://bugs.python.org/issue30698