Issue 30698: asyncio sslproto do not shutdown ssl layer cleanly (original) (raw)
Created on 2017-06-18 22:08 by grzgrzgrz3, last changed 2022-04-11 14:58 by admin. This issue is now closed.
Messages (9)
Author: Grzegorz Grzywacz (grzgrzgrz3) *
Date: 2017-06-18 22:08
Asyncio on shutdown do not send shutdown confirmation to the other side. _SSLPipe after doing unwrap is calling shutdown callback where transport is closed and quit ssldata wont be sent.
Author: Grzegorz Grzywacz (grzgrzgrz3) *
Date: 2017-06-21 09:17
No one yet responded, maybe this is unclear. I will clarify what is going on, why i made this change, what we gain from this and why this is not ideal solution.
I will focus on ssl layer shutdown as this issue regards.
We have connection asyncio <-> nginx
Lets see first situation asyncio initiates shutdown:
ideal:
shutdown
asyncio ----------> nginx <---------- shutdown Ideal situation asyncio sending shutdown and nginx replies back. This is how it works before attached PR.
we can't relay on nginx
shutdown
asyncio ----------> nginx ||||||||||| shutdown
At this point everything looks great, but what will happen when nginx do not sent shutdown - we will wait forever. We have this situation here #29406.
Attached PR "fix" this problem (note is not ideal fix, more like workaround):
with fix:
shutdown
asyncio ----------> nginx ||?-------- shutdown asyncio is sending shutdown ssl data to nginx but not waiting for nginx response, transport is closed anyway. I think ideal will be to wait for certain amount of time for response like Nikolay in #29406 propose. This will allow to implement SSL downgrade to plain text.
Second situation, nginx sent ssl eof.
- before fix: shutdown
nginx ----------> asyncio ||||||||||| shutdown In this case we are receiving nginx shutdown and correctly process it but after that, shutdown callback will close the transport before shutdown is sent back. Asyncio will try to send this data but fail due to closed transport. There is another issue should be not possible to write to closed transport. We are getting false-positive result to write. I do not analyze this deeper, maybe there is a reason to it.
- after fix: shutdown
nginx ----------> asyncio <---------- shutdown
This is clean, shutdown callback in _SSLPipe is removed. We close transport in ssl protocol.
I think connections between _SSLPipe and ssl protocol has design problems, _SSLPipe should be, as name suggest, only a pipe. Callback for handshake and shutdown feels wrong. Ssl protocol based on _SSLPipie mode and state can figure out when to call connection_made or close transport.
Author: Alexander Mohr (thehesiod) *
Date: 2018-01-11 18:53
@grzgrzgrz3, does this resolve the issue in https://bugs.python.org/issue29406 ? I'm guessing you based this PR on that issue. If so I'd like it merged ASAP as otherwise our prod services will be incompatible with all future python releases given the original "fix" was reverted. Thanks!
Author: Grzegorz Grzywacz (grzgrzgrz3) *
Date: 2018-01-11 19:44
No, this PR originally fix similar but different issue.
However it also fix as a side effect.
Author: Yury Selivanov (yselivanov) * 
Date: 2018-05-25 19:39
Is this issue resolved now, or do we need to work on this?
Author: Wei-Cheng Pan (legnaleurc) *
Date: 2018-10-15 11:49
The SSL connection still cannot close cleanly in 3.7.0. It will keep throwing read error during shutdown, so shutdown callback will never be called.
I've been test this PR for a while, and at least it fixes my problem.
Hope this PR can be included in 3.7.1, or at least 3.7.2.
Author: Manjusaka (Manjusaka) *
Date: 2018-12-22 19:13
Ping!
Agree with Wei-Cheng.
The leak still bothers me for times, and still bother for third-party like https://aiohttp.readthedocs.io/en/stable/client_reference.html#tcpconnector
Yury, I think it's worth working on it. Using uvloop should not be a good idea for some people.
Author: Kumar Aditya (kumaraditya) * 
Date: 2022-02-28 07:48
Since https://bugs.python.org/issue44011 is fixed now, this can be closed. @asvetlov
Author: Andrew Svetlov (asvetlov) *
Date: 2022-02-28 11:36
Agree
History
Date
User
Action
Args
2022-04-11 14:58:47
admin
set
github: 74883
2022-02-28 11:36:37
asvetlov
set
status: open -> closed
resolution: duplicate
messages: +
stage: resolved
2022-02-28 07:48:58
kumaraditya
set
nosy: + kumaraditya
messages: +
versions: + Python 3.11, - Python 3.5, Python 3.6, Python 3.7
2018-12-22 19:13:50
Manjusaka
set
nosy: + Manjusaka
messages: +
2018-10-15 11:49:15
legnaleurc
set
nosy: + legnaleurc
messages: +
2018-05-25 19:39:53
yselivanov
set
messages: +
2018-02-05 10:59:38
asvetlov
set
nosy: + asvetlov
2018-01-11 19:44:22
grzgrzgrz3
set
messages: +
2018-01-11 18:53:28
thehesiod
set
messages: +
2017-06-21 09:17:37
grzgrzgrz3
set
messages: +
2017-06-19 19:49:35
thehesiod
set
nosy: + thehesiod
2017-06-18 22:13:00
grzgrzgrz3
set
pull_requests: + <pull%5Frequest2320>
2017-06-18 22:08:35
grzgrzgrz3
create