-
Notifications
You must be signed in to change notification settings - Fork 141
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
avoid refcycles in tracebacks from happy eyeballs exceptions #809
base: master
Are you sure you want to change the base?
Conversation
41264d3
to
c217d95
Compare
bd67006
to
a2baf53
Compare
This is failing on Python3.9 because anyio is using asyncio.get_running_loop().create_connection() which in 3.9 didn't delete its reference to |
tests/test_sockets.py
Outdated
"asyncio.BaseEventLoop.create_connection creates refcycles on py 3.9" | ||
) | ||
ip = "127.0.0.1" | ||
port = ephemeral_port_reserve.reserve(ip=ip) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have plenty of tests that need ephemeral ports. Why do we have to add a new test dependency for this specific test? (I'd be happy to add such a fixture to anyio's pytest plugin though!)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh is there already a tool for this? I'll have a look
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't say there was a tool yet (that's what my suggestion in parentheses was about). Rather, we currently do this in a rather manual fashion.
@@ -133,6 +135,31 @@ def check_asyncio_bug(anyio_backend_name: str, family: AnyIPAddressFamily) -> No | |||
pytest.skip("Does not work due to a known bug (39148)") | |||
|
|||
|
|||
if sys.version_info >= (3, 14): | |||
|
|||
async def no_other_refs() -> list[object]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't really need to duplicate this from test_taskgroups.py
, do we? Why not just move it to a utility module in the test suite, or something?
ip = "127.0.0.1" | ||
with socket.socket(AddressFamily.AF_INET6) as dummy_socket: | ||
dummy_socket.bind(("::", 0)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wait, if you're trying to connect with IPv4, why are you trying to get an ephemeral port with IPv6?
Changes
Checklist
If this is a user-facing code change, like a bugfix or a new feature, please ensure that
you've fulfilled the following conditions (where applicable):
tests/
) added which would fail without your patchdocs/
, in case of behavior changes or newfeatures)
docs/versionhistory.rst
).If this is a trivial change, like a typo fix or a code reformatting, then you can ignore
these instructions.
Updating the changelog
If there are no entries after the last release, use
**UNRELEASED**
as the version.If, say, your patch fixes issue #123, the entry should look like this:
* Fix big bad boo-boo in task groups (#123 <https://github.com/agronholm/anyio/issues/123>_; PR by @yourgithubaccount)
If there's no issue linked, just link to your pull request instead by updating the
changelog after you've created the PR.