[IP6NS Grant] Improve hostname resolution #1166
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Kaiepi
commented
Aug 27, 2019
Kaiepi
changed the title
|
OK, this should be ready for review now |
Kaiepi
force-pushed
the
ipv6-unsupported
branch
2 times, most recently
from
4ab431e
to
3b1cf34
Compare
AlexDaniel
suggested changes
Sep 6, 2019
There was a problem hiding this comment.
Waiting for a fix/confirmation of a leak, please don't merge before that is resolved.
https://colabti.org/irclogger/irclogger_log/perl6-dev?date=2019-09-06#l1
Kaiepi
force-pushed
the
ipv6-unsupported
branch
4 times, most recently
from
83b2633
to
c8c4198
Compare
|
I can't find any memory leaks specific to async sockets with OpenBSD's The last commit also includes some fixes to |
timo
reviewed
Sep 11, 2019
timo
reviewed
Sep 11, 2019
Kaiepi
force-pushed
the
ipv6-unsupported
branch
from
c8c4198
to
b0b4b02
Compare
|
Fixed the GC issues I found with |
Kaiepi
force-pushed
the
ipv6-unsupported
branch
from
b0b4b02
to
3092521
Compare
This makes it so AI_ADDRCONFIG and AI_NUMERICSERV get used, which fixes an issue where IPv6 addresses would still get resolved when IPv6 is disabled, and makes it so getaddrinfo doesn't treat the port like a service at first.
This fixes an issue where trying to connect to a host with AF_UNSPEC using an IPv6 address first would never reattempt using an IPv4 address when it inevitably fails if IPv6 support is enabled, but misconfigured.
Sometimes, async TCP sockets were being closed after they checked to see if they were closed, but before actions on the socket were done. Async sockets were not passing all the object pointers they have to MVMROOT. which could possibly lead to them getting moved in the middle of an op. Roughly 500KB less memory gets leaked when simply binding an async TCP socket, connecting to it, and echoing a message now. Doing the same with async UDP sockets leaks around 2.5MB less memory.
This makes debugging sockets easier.
Kaiepi
force-pushed
the
ipv6-unsupported
branch
from
3092521
to
379ec77
Compare
Kaiepi
added a commit
to Kaiepi/MoarVM
that referenced
this pull request
Oct 24, 2019
While only the change to make all addresses get used when connecting or binding sockets needs to be reverted from this, not every commit had only one change included with it, making it impossible to tell if it had truly been reverted or not. They will be remade later. This reverts commit 39f5774, reversing changes made to 01f85c8.
AlexDaniel
added a commit
that referenced
this pull request
Oct 26, 2019
Revert "Merge pull request #1166 from Kaiepi/ipv6-unsupported"
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This makes a few different changes to hostname resolution. First, it makes it so when resolving hostnames on systems where no IPv6 addresses actually exist on the interface used, an IPv6 address will never be returned by
MVM_io_resolve_host_name, and likewise for IPv4. Second, rather thanMVM_io_resolve_host_nameonly returning the first address returned bygetaddrinfo, it returns all of them instead, and functions using its return value now attempt to use all of them before they actually throw. Finally,MVM_io_resolve_host_namenow takes parameters for socket types, protocols, and passivity, which makes it so it doesn't return addresses that can't actually be used when those are specified.This is currently broken, as the
uv_udp_thandle bound whenMVM_io_async_socket_udpgets called is zeroed out inwrite_setup. Interestingly, if I don't sethandle->datatoNULL, theSocketSetupInfostruct there ends up being the only struct member not to be zeroed out. Maybe the GC's involved getting involved here somehow?Fixes rakudo/rakudo#3007
Fixes rakudo/rakudo#2162