Skip to content
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.

Sign up for GitHub

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

Reload nameserver information on lookup failure #41582

Merged
merged 1 commit into from
May 6, 2017
Merged

Reload nameserver information on lookup failure #41582

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/liblibc
Open in desktop
Submodule liblibc updated 25 files
+2 −2 Cargo.lock
+1 −1 Cargo.toml
+19 −0 README.md
+1 −1 appveyor.yml
+0 −2 ci/run-docker.sh
+16 −0 libc-test/build.rs
+2 −0 src/unix/bsd/mod.rs
+17 −0 src/unix/mod.rs
+1 −1 src/unix/notbsd/android/b64/mod.rs
+12 −0 src/unix/notbsd/android/mod.rs
+1 −1 src/unix/notbsd/linux/mips/mips32.rs
+1 −1 src/unix/notbsd/linux/mips/mips64.rs
+1 −0 src/unix/notbsd/linux/mips/mod.rs
+37 −0 src/unix/notbsd/linux/mod.rs
+2 −1 src/unix/notbsd/linux/musl/mod.rs
+1 −0 src/unix/notbsd/linux/other/b32/arm.rs
+1 −0 src/unix/notbsd/linux/other/b32/powerpc.rs
+1 −0 src/unix/notbsd/linux/other/b32/x86.rs
+1 −0 src/unix/notbsd/linux/other/b64/aarch64.rs
+1 −0 src/unix/notbsd/linux/other/b64/powerpc64.rs
+1 −0 src/unix/notbsd/linux/other/b64/sparc64.rs
+1 −0 src/unix/notbsd/linux/other/b64/x86_64.rs
+1 −1 src/unix/notbsd/linux/other/mod.rs
+2 −1 src/unix/notbsd/linux/s390x.rs
+113 −45 src/unix/solaris/mod.rs
5 changes: 5 additions & 0 deletions src/libstd/build.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,16 @@ fn main() {
println!("cargo:rustc-link-lib=pthread");
} else if target.contains("apple-darwin") {
println!("cargo:rustc-link-lib=System");

// res_init and friends require -lresolv on macOS/iOS.
// See #41582 and http://blog.achernya.com/2013/03/os-x-has-silly-libsystem.html
println!("cargo:rustc-link-lib=resolv");
} else if target.contains("apple-ios") {
println!("cargo:rustc-link-lib=System");
println!("cargo:rustc-link-lib=objc");
println!("cargo:rustc-link-lib=framework=Security");
println!("cargo:rustc-link-lib=framework=Foundation");
println!("cargo:rustc-link-lib=resolv");
} else if target.contains("windows") {
println!("cargo:rustc-link-lib=advapi32");
println!("cargo:rustc-link-lib=ws2_32");
Expand Down
19 changes: 16 additions & 3 deletions src/libstd/sys_common/net.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,9 +177,22 @@ pub fn lookup_host(host: &str) -> io::Result<LookupHost> {
};
let mut res = ptr::null_mut();
unsafe {
cvt_gai(c::getaddrinfo(c_host.as_ptr(), ptr::null(), &hints,
&mut res))?;
Ok(LookupHost { original: res, cur: res })
match cvt_gai(c::getaddrinfo(c_host.as_ptr(), ptr::null(), &hints, &mut res)) {
Ok(_) => {
Ok(LookupHost { original: res, cur: res })
},
#[cfg(unix)]
Err(e) => {
// The lookup failure could be caused by using a stale /etc/resolv.conf.
// See https://github.com/rust-lang/rust/issues/41570.
// We therefore force a reload of the nameserver information.
c::res_init();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't this still result in surprising behaviour if e.g. the contents of /etc/resolv.conf change without the old resolver becoming unusable?

For instance, if I change my DNS resolver without making the old resolver unreachable, I'll never hit this error and any running rust applications will continue to use the old resolver...indefinitely.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Though if the resolution happens successfully, what is the problem? It's also quite hard to get around that particular case. We could always call res_init, but that seems a little wasteful. The real solution to this is to fix libc (most libcs do not have this problem — glibc is the major exception). Applications that want to be robust against this could always call libc::res_init directly though of course.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Though if the resolution happens successfully, what is the problem?

Playing devil's advocate, "successful" doesn't imply "correct".

We could always call res_init, but that seems a little wasteful.

How wasteful? Perhaps this is worth measuring.

The real solution to this is to fix libc (most libcs do not have this problem — glibc is the major exception).

What do you mean? What would "fixing" libc look like? What do other libcs do in contrast to glibc?

Copy link
Contributor Author

@jonhoo jonhoo May 25, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Though if the resolution happens successfully, what is the problem?

Playing devil's advocate, "successful" doesn't imply "correct".

True, though that sounds like a very weird setup indeed. One in which you can connect using the resolution information from the old server, but you need to instead connect to the server provided by a new resolver?

We could always call res_init, but that seems a little wasteful.

How wasteful? Perhaps this is worth measuring.

I did some benchmarks above (#41582 (comment)), and it's not terrible (especially because it doesn't require a syscall), but if we can avoid doing something...

The real solution to this is to fix libc (most libcs do not have this problem — glibc is the major exception).

What do you mean? What would "fixing" libc look like? What do other libcs do in contrast to glibc?

No other libcs have this issue. Some of them don't cache /etc/resolv.conf, some integrate with NSS or similar services, which know when the cache should be flushed. I haven't looked into it too carefully. It is unclear what the "right" solution is given that glibc wants to be both fast (i.e., don't do a file read on every connect), and not rely on other services (like NSS).

Err(e)
},
// the cfg is needed here to avoid an "unreachable pattern" warning
#[cfg(not(unix))]
Err(e) => Err(e),
}
}
}

Expand Down
1 change: 1 addition & 0 deletions src/test/run-make/tools.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ else
endif
else
ifeq ($(UNAME),Darwin)
EXTRACFLAGS := -lresolv
else
ifeq ($(UNAME),FreeBSD)
EXTRACFLAGS := -lm -lpthread -lgcc_s
Expand Down