-
-
Notifications
You must be signed in to change notification settings - Fork 30.6k
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
test_asyncio: test_create_server_ssl_over_ssl() failed on ARM64 macOS 3.x #95573
Comments
#95613 adds a debug callback for BIO (both mem and socket type). It prints to stderr. Could somebody use it to capture a failing test run? |
Findings so far:
Now the open question is: did the new test surface a pre-existing flakiness or was the flakiness introduced by the SSL rewrite in GH-31275? |
Crash log from 13c10bf:
|
Progress! Using a variation of the "easiest way" from @ambv, I am now able to reliably produce the test failure on (some) Intel Macs, Intel Macs on a VM as well as multiple Apple Silicon (M1) Macs using either OpenSSL 1.1 or 3, with or without pydebug, with or without Mac framework build, top of 3.11 source build or with python.org 3.11.0b5 macOS installer, and on multiple macOS versions (in particular, 11.6.8, 12.5, and 13.0 developer beta).
The key is setting the I'm going to step away from this now but ping me if you need help reproducing. I'll also ping @ronaldoussoren here in case this behavior might ring a bell. |
I haven't had the time to read though all the conversations about this on Discord, but this reminds me of #26893. I'll see if I can put aside some time tonight for debugging this. |
We essentially need to decide today if we're reverting #31275. |
We have decided to revert to unblock the release of 3.11.0rc1 |
In May 2021, Andrew Svetlov copied the asyncio SSL implementation from uvloop to Python (asyncio). Sadly, Pablo had to revert it (GH-88177) in Python 3.10, due to test_asyncio failures on buildbots. In Feburary 2022, Kumar Aditya basically copied it again to Python upstream. Sadly, the first release candidate of Python 3.11 is scheduled for tomorrow and there is no asyncio expert available to debug this tricky issue. Moreover, the issue only occurs on macOS which makes is harder to debug (for example, I don't have access to any macOS machine). By the way, the commit message lacks author information from source repository, and the motivation behind the change is missing. Most of the code was written by Yury and Fantix.
The discussion occurred on Discord #python-dev. For example, Pablo wrote there:
The failing asyncio test_create_server_ssl_over_ssl() test is testing "TLS over TLS". |
I found the root reason by temporarily disabling this suppression: cpython/Lib/test/test_asyncio/test_ssl.py Lines 1159 to 1160 in 8a0d9a6
I think a reasonable fix is to reduce the test data size: cpython/Lib/test/test_asyncio/test_ssl.py Lines 1027 to 1028 in 8a0d9a6
Reducing from 1MB to 64KB seems to work fine on an M1 macOS with |
I am confused, the original error is not
but
Could you provide a bit more context on how that error is making the other one appear? |
@fantix, thanks for your successful investigation. Exciting find! However, I agree with Pablo that it's not quite clear how silencing a warning might have lead to silencing of the OSError that in turn lead to the SSLEOFError. It's a bit of a head scratcher. Additionally, speaking of the proposed workaround (I can't make myself call it a fix), don't you think a 1MB blob is realistic enough that it should be supported even in cases of raised concurrency? 64 processes still isn't that much, honestly, but we were able to easily overwhelm the test with just Am I right thinking that this suggests that tunneling SSL in SSL with this sslproto implementation isn't feasible for production use? I mean, I've seen JavaScript files over 1MB 😆 Let me know what you think. |
Thanks for checking! Those are good questions. Let me try to answer one of them here first: cpython/Lib/test/test_asyncio/test_ssl.py Lines 148 to 157 in 6a5104f
I think this In our case here, For the second question of why the |
@fantix's discovery led @ronaldoussoren to create a C-only repoducer for the problem, proving it's not an issue with asyncio's new SSL implementation. He filed FB11063974 about this issue in Apple's tracker. I could reproduce the issue using Ronald's code. Instead of reverting the new asyncio.sslproto, we should instead special-case macOS to decrease the buffer size as @fantix is suggesting. Let's only decrease the buffer for the Mac. |
…gs (pythonGH-95687) (cherry picked from commit e1d68b3) Co-authored-by: Fantix King <[email protected]>
…-95687) (GH-95699) (cherry picked from commit e1d68b3) Co-authored-by: Fantix King <[email protected]>
Co-authored-by: Łukasz Langa <[email protected]>
…thonGH-95668) Co-authored-by: Łukasz Langa <[email protected]> (cherry picked from commit 3a9e1fd) Co-authored-by: Fantix King <[email protected]>
GH-95705) Co-authored-by: Łukasz Langa <[email protected]> (cherry picked from commit 3a9e1fd) Co-authored-by: Fantix King <[email protected]>
This can now be closed and the 3.11 release is unblocked. Thanks for investigating @tiran and @ned-deily, thanks for identifying the root cause and issuing PRs @fantix, and thanks @ronaldoussoren for ensuring the problem is not Python-specific and following up with Apple. |
…thonGH-95668) Co-authored-by: Łukasz Langa <[email protected]>
test_create_server_ssl_over_ssl() of test_asyncio failed on ARM64 macOS 3.x: https://buildbot.python.org/all/#/builders/725/builds/2392
The same test failed when test_asyncio was re-run.
The text was updated successfully, but these errors were encountered: