add asynch getaddrinfo support #6746
Conversation
![sonatype-lift[bot]](https://avatars.githubusercontent.com/in/9843?s=60&v=4){kind=small}
| } gai; | ||
| struct { | ||
| char *hostname; | ||
| char service[12]; |
There was a problem hiding this comment.
Why 12? I think this is used for a port number string, right? If so, 65535 is the largest value so 6 bytes should be enough?
There was a problem hiding this comment.
Copied from asyn-thread.c. I guess because max chars of stringified int are 12.
There was a problem hiding this comment.
glibc says it is NI_MAXSERV .
m6w6
force-pushed
the
asynchdns/getaddrinfo_a
branch
from
daa773d
to
18aaaf1
Compare
m6w6
force-pushed
the
asynchdns/getaddrinfo_a
branch
from
18aaaf1
to
6043a10
Compare
m6w6
force-pushed
the
asynchdns/getaddrinfo_a
branch
from
6043a10
to
88821a3
Compare
bagder
added
name lookup
|
How do you invoke configure to build this? I tried both with just ... which leads to: we should also add a CI build using the async resolver to verify that it works. |
|
Yes: |
|
That doesn't work for me. I'll investigate. |
|
Okay, it works if I use I would like a way for |
| #endif | ||
|
|
||
| Curl_expire(data, resolver->data.expire, EXPIRE_ASYNC_NAME); | ||
| return 0; |
There was a problem hiding this comment.
What about using a socketpair here to signal when the resolve is complete? Otherwise we need to fall back to polling, which is ineffective and we've tried to work it off for the other resolvers.
|
|
||
| do { | ||
| rc = Curl_resolver_is_resolved(data, &dns); | ||
| } while(CURLE_OK == rc && !dns); |
There was a problem hiding this comment.
This is a (nasty) busy-loop, which I see as another good argument to use a socketpair to wait for,
| } | ||
|
|
||
| /* time out if no more time is left for us */ | ||
| left = Curl_timeleft(data, NULL, 1); |
There was a problem hiding this comment.
Please use TRUE instead of '1' for the boolean argument
bagder
added
needs-info
and removed
feature-window
|
@m6w6 what's the status here? I would like to see this move forward and I could probably help out a bit from here if you'd like to. |
|
Sorry, it's been a bit stiff for me. Help is definitely appreciated especially since I got mad with the autoconf stuff 🤷 |
There is something wrong.. You have to AC_USE_SYSTEM_EXTENSIONS in early configure.in (before AC_PROG_CC is called) for this to be correct, this is the only "right way" in the autoconf build, in the CMake build you are to pass as a CPPFLAG |
|
You must also note that what this do is moving the blocking somewhere else.. to a differrent thread pool and that's pretty much it. (all of the libc resolver bugs apply) Maybe just maybe work on a libunbound backend is better.. oh well.. |
Resolving a host name will mean waiting for the response to come back. It's not unimportant who does the waiting and how it is done.
What would libunbound do better than c-ares already does if going with an external dependency is deemed okay? Does libunbound work as a "perfect" getaddrinfo() replacement? |
No, it willof course not be perfect. but It does DNSSEC by itself , DNS over TLS (or add any modern accepted way here) and does not rely on the libc resolver, which is lacking of love and bug fixing since quite a while unfortunately. |
In my experience, people want reliable name resolution and they want it to work the same as it does in other applications which is why any replacement library has to have very close to feature-parity with This is hard and large teams have failed this task before. This is also why the threaded stock resolver is still the default one in curl, even if c-ares gives more flexibility, more features and allows us to avoid threads. It still doesn't work exactly as getaddrinfo. |
|
I'm going to close this and add this task to the |
Somewhat niche, because it needs:
-D_GNU_SOURCEgetaddrinfo_a(glibc only? in-lanl)Advantages:
asyn-thread.c