Add res_init

[jonhoo]

The res_init function, while deprecated, is critical for making networked applications work correctly when switching between networks, or between being offline and online. By default, getaddrinfo and friends use a cached version of /etc/resolv.conf, which means that network changes are not picked up by applications after they first start up. This has bitten Firefox, Pidgin, MongoDB, and more in the past.

The logic behind exposing only res_init is that it is the only res_* function that is frequently called directly by user applications. The other res_* functions provide low-level access to domain lookups, whereas res_init is useful even if the application itself is not concerned with doing lookups.

Behind the scenes, getaddrinfo in glibc ultimately calls res_nsearch with &_res. res_init operates directly on this global reference, and is thus more useful to expose for most applications than the non-deprecated res_ninit function (which operators on an arbitrary res_state).

As far as I can tell, res_init is available in FreeBSD, NetBSD, OpenBSD, Solaris, Linux, and macOS.

[alexcrichton]

@bors: r+

Thanks!

[bors]

📌 Commit ae294f9 has been approved by alexcrichton

[jonhoo]

@alexcrichton looking at the Travis build thus far, it looks like a number of tests are failing with empty logs.. Is this a known issue?

[alexcrichton]

I think Travis is doing some DB maintenance right now, and tha may be causing issues with logs :(

[bors]

⌛ Testing commit ae294f9 with merge 703a5e7...

[bors]

💔 Test failed - status-travis

[alexcrichton]

Maybe a header file isn't #include'd?

[jonhoo]

Ah, good catch! Added.
Any chance you could also take a look at the related rust-lang/rust#41570 ?

[alexcrichton]

Oh hm, is this required to link to the resolv library? That changes the calculus a bit here as it runs a high risk of causing accidental breakage :(

[jonhoo]

@alexcrichton yeah, unfortunately it seems like it does, at least on some platforms. Which is kind of strange, given that getaddrinfo internally calls these methods (and thus must somehow be able to get at the relevant resolv symbols). musl doesn't require this, since res_init is implemented directly, nor do the x86_64 targets (why?), but some other glibc targets do...

[jonhoo]

Also, interestingly, cargo complains about "undefined reference to res_init" even with -lresolv, despite

#include <netinet/in.h>
#include <resolv.h>

int main() {
        res_init();
}

compiling just fine locally for me with both gcc and clang without additional linking options. Have also now tested with gcc on i686, and still compile fine without -lresolv.

[jonhoo]

Actually, we may be in luck @alexcrichton. See ae5cf30

[alexcrichton]

Nice! Perhaps we could leave in the leading double undescore?

Or is that just the symbol name and the C identifier is res_init?

[jonhoo]

The C identifier (and what shows up in all the manpages) is res_init. The rename to __res_init is completely masked from C applications by the resolv.h macro:

#define res_init __res_init

[aidanhs]

I can only see res_init in musl, not __res_init.

(it's also a no-op in musl, but that's an aside)

[alexcrichton]

@bors: r+

Ok thanks for the info @jonhoo!

@aidanhs ah we'll probably get a link error then, but let's see what travis says.

[bors]

📌 Commit ae5cf30 has been approved by alexcrichton

[bors]

⌛ Testing commit ae5cf30 with merge 5e16276...

[jonhoo]

See https://stackoverflow.com/questions/16741732/using-getaddrinfo-only-checks-nscd-cache-first-time-if-dns-times-out for another discussion of why res_init is necessary

[jonhoo]

@alexcrichton @aidanhs I think I tracked down all the major ones in fe94999

[alexcrichton]

@bors: r+

[bors]

📌 Commit fe94999 has been approved by alexcrichton

[bors]

⌛ Testing commit fe94999 with merge faa15ce...

[malbarbo]

I also tested the example in rust-lang/rust#41570 and it worked on a real android phone. The program prints connected when the phone is back online.

[jonhoo]

Interesting.. I suspect this is because Android probably uses nscd or something similar, which does automatically refresh the cache. If you're not running a network manager, that is not the case, and you must call res_init again (at least on Linux). I guess we could then expose res_init only for Linux, since that then seems like the only place it is needed.. Thoughts @alexcrichton?

[semarie]

Android doesn't use glibc as Linux does (their libc version has at least parts from OpenBSD, but I dunno for details).

@jonhoo your underline problem seems to be unexpected behaviour in glibc (maybe a bug should be opened over there ?). But it could be more simple to workaround it with a global solution (like calling res_init unconditionnally). From your bug report, it seems Mozilla already solves it this way, so it works, no need to diverge from a working solution.

[jonhoo]

@semarie I don't know that this is actually unexpected behavior as far as glibc goes. It seems quite reasonable for it to cache /etc/resolv.conf so that it doesn't need to do an additional two system calls for every new connection. My question above was more whether, since this only seems to be an issue on Linux when no network manager is in use, it's sufficient for us to just expose libc::res_init on Linux (and not on other targets)? And place the call to res_init in a cfg!(linux) in the modified lookup_host.

[jonhoo]

@alexcrichton would you prefer just exposing the call on Linux, or keep the current version which exposes it on all UNIX platforms?

[alexcrichton]

Ok getting back to this, glad it's green! I'm fine still exposing this on all platforms, I mainly just want to be as careful as possible adding a new library to link to. I figure we can just be super conservative and only add it to libstd builds and then if it gets past rust-lang/rust CI I'll be happy.

[alexcrichton]

Perhaps this could be converted to a whitelist? It's mostly just targeting Linux/android/FreebSD it looks like?

[alexcrichton]

I'm a little wary of doing this, especially outside the stdbuild blocks. Just in terms of possible breakage on new platforms. I do realize though that this is required to link correctly as libc on crates.io otherwise doesn't pull in new libs (it relies on libc in std to do that).

Perhaps we could elide this section (or move it down below after the clause above) and then update the copy in libstd? We can rely on rust-lang/rust CI to block changes if it'd otherwise cause linkage problems. In the meantime we could just disable verification of the res_init symbol in libc as we already know it works!

[jonhoo]

I'm not sure I follow this? What do you mean by "move it down below after the clause above"? And by "update the copy in libstd"?

[alexcrichton]

Ah sorry, basically what I mean is:

• Ideally, no libs are added
• If a lib must be added, it should be added without adding clauses to cfg_if!
• If a lib must be added, disable tests in libc-test and we'll test that it works in rust-lang/rust instead

[jonhoo]

So we should simply not link with resolv on macOS/iOS, both on stdbuild and not(stdbuild) , and disable the test for res_init if apple?

[alexcrichton]

Let's start off by disabling linking to resolv everywhere. Let's then only enable it in the not(stdbuild) blocks to ensure we don't accidentally break crates.io. By only enabling it in not(stdbuild) then we can't test it in this PR, but it's ok to leave it enabled on all platforms you have it defined on currently.

[jonhoo]

Okay, I've removed all the linking with libresolv in fcf5fc0. As noted in my discussion comment though, it seems like res_init is just never tested any more following the res_init if android => true addition made in afebd98.

[jonhoo]

Hmm, following afebd98, it doesn't look like res_init is tested for any of the platforms..? :/

[alexcrichton]

@jonhoo hm but the current failures on CI seem to contradict your findings that it's not being tested?

[jonhoo]

Huh, weird.. You're right. I guess I must have actually fixed some linking errors then :p
And now you'd like me to skip tests for if apple? Done in be7e45f

[jonhoo]

@alexcrichton all right, I think that brings the changes in line with what you wanted?

[alexcrichton]

@bors: r+

Looks great! Thanks and sorry for the runaround!

[bors]

📌 Commit be7e45f has been approved by alexcrichton

[bors]

⌛ Testing commit be7e45f with merge 03562b0...

[bors]

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing 03562b0 to master...

[jonhoo]

No worries at all! It's important to get the details of these things right, and gave me some further insight into libc's innards. Now onto rust-lang/rust#41570!