Skip to content

bpo-35998: Avoid TimeoutError in test_asyncio: test_start_tls_server_1() #14080

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Jun 14, 2019
Merged

bpo-35998: Avoid TimeoutError in test_asyncio: test_start_tls_server_1() #14080

merged 2 commits into from
Jun 14, 2019

Conversation

asvetlov
Copy link
Contributor

@asvetlov asvetlov commented Jun 14, 2019 •
edited by bedevere-bot
Loading

The test was not robust because it used to rely on sock.shutdown(socket.SHUT_RDWR) before sock.close().

Actually, sometimes these socket state changes are merged into the just transition into the socket-closed state on the peer side. I don't want to add sleep(N) between calls, it is the easiest way to add new test flakiness.

The start_tls test doesn't need to check proto.eof_received() callback receiving, there are other explicit tests for it.

The test is changed to receive TSL encrypted data and send an acknowledgment back before client socket closing.

Now everything works smoothly, I don't see any timeout exceptions even under very heavy testing.

https://bugs.python.org/issue35998

@asvetlov asvetlov requested a review from tiran as a code owner June 14, 2019 14:14
@bedevere-bot bedevere-bot added tests Tests in the Lib/test dir awaiting core review labels Jun 14, 2019
vstinner
vstinner approved these changes Jun 14, 2019
View reviewed changes
Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, I just have a minor comment.

On Fedora 30, reproducing the take takes me 5 seconds using (the test hangs for a few minutes and then fail):

./python -m test test_asyncio -m test_start_tls_server_1 -F

With this change, the test no longer hangs.

I can ./python -m test test_asyncio -m test_start_tls_server_1 -F 6 times in parallel for 5 minutes, my system load is around 7.0 (the system is very busy), and the test still doesn't hang. So I can confirm that this change fix the issue for me.

vstinner
vstinner approved these changes Jun 14, 2019
View reviewed changes
Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@vstinner vstinner merged commit f0749da into python:master Jun 14, 2019
@miss-islington
Copy link
Contributor

Thanks @asvetlov for the PR, and @vstinner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.7, 3.8.
🐍🍒⛏🤖

@bedevere-bot
Copy link

GH-14084 is a backport of this pull request to the 3.8 branch.

@miss-islington
Copy link
Contributor

Sorry, @asvetlov and @vstinner, I could not cleanly backport this to 3.7 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker f0749da9a535375f05a2015e8960e8ae54877349 3.7

@asvetlov
Copy link
Contributor Author

I'll make backport myself

@bedevere-bot
Copy link

GH-14086 is a backport of this pull request to the 3.7 branch.

@vstinner
Copy link
Member

@asvetlov: I just made it :-) I really want this bug to go away, it annoy me on buildbots for 6 months at least. It was a stupid conflict on additional "loop=self.loop" in 3.7.

@asvetlov
Copy link
Contributor Author

Yes, I see.
Thank you!

@asvetlov asvetlov deleted the fix-ssl branch June 14, 2019 15:44
miss-islington added a commit that referenced this pull request Jun 14, 2019
@miss-islington @asvetlov
bpo-35998: Avoid TimeoutError in test_asyncio: test_start_tls_server_…
0c2eb6d 
…1() (GH-14080)

(cherry picked from commit f0749da)

Co-authored-by: Andrew Svetlov <andrew.svetlov@gmail.com>
vstinner added a commit that referenced this pull request Jun 14, 2019
@vstinner
bpo-35998: Avoid TimeoutError in test_asyncio: test_start_tls_server_…
33feb2e 
…1() (GH-14080) (GH-14086)

(cherry picked from commit f0749da)
lisroach pushed a commit to lisroach/cpython that referenced this pull request Sep 10, 2019
@asvetlov @lisroach
bpo-35998: Avoid TimeoutError in test_asyncio: test_start_tls_server_…
73e76e9 
…1() (pythonGH-14080)
DinoV pushed a commit to DinoV/cpython that referenced this pull request Jan 14, 2020
@asvetlov @DinoV
bpo-35998: Avoid TimeoutError in test_asyncio: test_start_tls_server_…
12fac6a 
…1() (pythonGH-14080)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vstinner vstinner vstinner approved these changes

@tiran tiran Awaiting requested review from tiran

@1st1 1st1 Awaiting requested review from 1st1

@pablogsal pablogsal Awaiting requested review from pablogsal

@ned-deily ned-deily Awaiting requested review from ned-deily

Assignees

@vstinner vstinner

Labels
tests Tests in the Lib/test dir topic-asyncio
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@asvetlov @miss-islington @bedevere-bot @vstinner @the-knights-who-say-ni