Skip to content

Conversation

@aeros
Copy link
Contributor

@aeros aeros commented Dec 12, 2019

Co-authored-by: @tirkarthi

The security patch #17311 (which was backported to 3.8, 3.7, and 3.6) added a few subtle warnings, including a ResourceWarning and two DeprecationWarnings in test_asyncio.test_base_events. @tirkarthi discovered the warnings, and proposed a fix for the ResourceWarning. See the bpo issue for more details.

https://bugs.python.org/issue37228

@ambv
Copy link
Contributor

ambv commented Dec 12, 2019

Wasn't the point of the security fix to not use reuse_addr=?

@aeros
Copy link
Contributor Author

aeros commented Dec 12, 2019

@ambv

Wasn't the point of the security fix to not use reuse_addr=?

The primary point of the security fix was to specifically remove usage of reuse_addr=True for create_datagram_endpoint() and no longer use it as a default. But as a part of it we also deprecated the reuse_address parameter itself since it was turned into a no-op. This PR is simply addressing some minor test changes that I missed, which @tirkarthi picked up on by running the tests with -Wall.

I accidentally introduced a couple deprecation warnings that were raised from previous tests that still had reuse_address=False, which should have been removed. Also, in the actual test for the deprecation warning, the transport and protocol were not closed, leading to a resource warning.

It wouldn't be the end of the world if this didn't make it into the next releases, but it would definitely be preferable to not have additional warnings lingering in the regression tests. Especially in this case where the fix is quite simple.

@ambv ambv merged commit 1988344 into python:master Dec 12, 2019
@bedevere-bot
Copy link

@ambv: Please replace # with GH- in the commit message next time. Thanks!

@miss-islington
Copy link
Contributor

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

@ambv
Copy link
Contributor

ambv commented Dec 12, 2019

Thanks! ✨ 🍰 ✨

@bedevere-bot
Copy link

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

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Dec 12, 2019
Co-authored-by: tirkarthi
(cherry picked from commit 1988344)

Co-authored-by: Kyle Stanley <aeros167@gmail.com>
@bedevere-bot
Copy link

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

@bedevere-bot
Copy link

GH-17581 is a backport of this pull request to the 3.6 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Dec 12, 2019
Co-authored-by: tirkarthi
(cherry picked from commit 1988344)

Co-authored-by: Kyle Stanley <aeros167@gmail.com>
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Dec 12, 2019
Co-authored-by: tirkarthi
(cherry picked from commit 1988344)

Co-authored-by: Kyle Stanley <aeros167@gmail.com>
@tirkarthi
Copy link
Member

Thanks @aeros :)

ambv pushed a commit that referenced this pull request Dec 12, 2019
Co-authored-by: tirkarthi
(cherry picked from commit 1988344)

Co-authored-by: Kyle Stanley <aeros167@gmail.com>
ned-deily pushed a commit that referenced this pull request Dec 17, 2019
Co-authored-by: tirkarthi
(cherry picked from commit 1988344)

Co-authored-by: Kyle Stanley <aeros167@gmail.com>
ned-deily pushed a commit that referenced this pull request Dec 17, 2019
Co-authored-by: tirkarthi
(cherry picked from commit 1988344)

Co-authored-by: Kyle Stanley <aeros167@gmail.com>
ned-deily pushed a commit to ned-deily/cpython that referenced this pull request Dec 18, 2019
…nGH-17580)

Co-authored-by: tirkarthi
(cherry picked from commit 1988344)

Co-authored-by: Kyle Stanley <aeros167@gmail.com>
shihai1991 pushed a commit to shihai1991/cpython that referenced this pull request Jan 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip news tests Tests in the Lib/test dir topic-asyncio type-bug An unexpected behavior, bug, or error

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants