[issue33674] asyncio: race condition in SSLProtocol

Yury Selivanov added the comment: Closing this one. Please open new issues to track regressions. -- resolution: -> fixed stage: patch review -> resolved status: open -> closed ___ Python tracker

[issue33674] asyncio: race condition in SSLProtocol

Yury Selivanov added the comment: yes, the method is idempotent. -- ___ Python tracker ___ ___ Python-bugs-list mailing list

[issue33674] asyncio: race condition in SSLProtocol

STINNER Victor added the comment: > bpo-33674: Pause the transport as early as possible (#7192) > https://github.com/python/cpython/commit/f295587c45f96b62d24f9a12cef6931b0805f596 Is it ok to always resume reading? Previously reading was only resumed if the transport was reading. --

[issue33674] asyncio: race condition in SSLProtocol

STINNER Victor added the comment: I fixed the main issue, so I remove the "release blocker" priority. -- priority: release blocker -> ___ Python tracker ___

[issue33674] asyncio: race condition in SSLProtocol

miss-islington added the comment: New changeset eca085993cb8620ba9232560f46d91326a13cdd2 by Miss Islington (bot) in branch '3.7': bpo-33674: Pause the transport as early as possible (GH-7192) https://github.com/python/cpython/commit/eca085993cb8620ba9232560f46d91326a13cdd2 -- nosy:

[issue33674] asyncio: race condition in SSLProtocol

Change by miss-islington : -- pull_requests: +6827 ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue33674] asyncio: race condition in SSLProtocol

Yury Selivanov added the comment: New changeset f295587c45f96b62d24f9a12cef6931b0805f596 by Yury Selivanov in branch 'master': bpo-33674: Pause the transport as early as possible (#7192) https://github.com/python/cpython/commit/f295587c45f96b62d24f9a12cef6931b0805f596 --

[issue33674] asyncio: race condition in SSLProtocol

Yury Selivanov added the comment: New changeset 7593b8a5075ff45d71be9f62980be6a9c005afa9 by Yury Selivanov (Victor Stinner) in branch '3.6': bpo-33674: asyncio: Fix SSLProtocol race (GH-7175) (GH-7188) https://github.com/python/cpython/commit/7593b8a5075ff45d71be9f62980be6a9c005afa9

[issue33674] asyncio: race condition in SSLProtocol

Change by Yury Selivanov : -- pull_requests: +6826 ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue33674] asyncio: race condition in SSLProtocol

Ned Deily added the comment: New changeset 0dd8fd03584b61cd769be88f5a2fb59b0d8f6d18 by Ned Deily (Miss Islington (bot)) in branch '3.7': bpo-33674: asyncio: Fix SSLProtocol race (GH-7175) (#7187) https://github.com/python/cpython/commit/0dd8fd03584b61cd769be88f5a2fb59b0d8f6d18 --

[issue33674] asyncio: race condition in SSLProtocol

Change by STINNER Victor : -- pull_requests: +6823 ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue33674] asyncio: race condition in SSLProtocol

Change by miss-islington : -- pull_requests: +6822 ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue33674] asyncio: race condition in SSLProtocol

STINNER Victor added the comment: New changeset be00a5583a2cb696335c527b921d1868266a42c6 by Victor Stinner in branch 'master': bpo-33674: asyncio: Fix SSLProtocol race (GH-7175) https://github.com/python/cpython/commit/be00a5583a2cb696335c527b921d1868266a42c6 --

[issue33674] asyncio: race condition in SSLProtocol

Yury Selivanov added the comment: And I agree, this should make it to 3.7.0 -- ___ Python tracker ___ ___ Python-bugs-list mailing

[issue33674] asyncio: race condition in SSLProtocol

Yury Selivanov added the comment: The fix is correct and the bug is now obvious: data_received() occur pretty much any time after connection_made() call; if call_soon() is used in connection_made(), data_received() may find the protocol in an incorrect state. Kudos Victor for debugging

[issue33674] asyncio: race condition in SSLProtocol

STINNER Victor added the comment: Yury Selivanov told me that usually it's safer to add call_soon(), than to remove call_soon(). But adding many call_soon() can make asyncio SSL slower. SSLProtocol doesn't seem to like call_soon(), it's only used at: * connection_lost():

[issue33674] asyncio: race condition in SSLProtocol

Change by STINNER Victor : -- keywords: +patch pull_requests: +6810 stage: -> patch review ___ Python tracker ___ ___

[issue33674] asyncio: race condition in SSLProtocol

STINNER Victor added the comment: loop.start_tls() method is new in Python 3.7. If possible, I would prefer to not see it with a builtin race condition, since such race condition is really hard to debug :-( So I raise the priority to release blocker. Sorry again Ned! -- nosy:

[issue33674] asyncio: race condition in SSLProtocol

STINNER Victor added the comment: Workaround: diff --git a/Lib/asyncio/sslproto.py b/Lib/asyncio/sslproto.py index 2bfa45dd15..4a5dbb38a1 100644 --- a/Lib/asyncio/sslproto.py +++ b/Lib/asyncio/sslproto.py @@ -592,7 +592,7 @@ class SSLProtocol(protocols.Protocol): # (b'', 1) is a

[issue33674] asyncio: race condition in SSLProtocol

New submission from STINNER Victor : While debugging bpo-32458 (functional test on START TLS), I found a race condition in SSLProtocol of asyncio/sslproto.py. Sometimes, _sslpipe.feed_ssldata() is called before _sslpipe.shutdown(). * SSLProtocol.connection_made() ->