From 475bb3bb8167068bce4114c08d5985d50986bb52 Mon Sep 17 00:00:00 2001 From: Jack O'Connor Date: Wed, 19 Jul 2017 12:40:36 -0400 Subject: [PATCH 1/2] govendor update go-framed-msgpack-rpc --- .../go-framed-msgpack-rpc/rpc/connection.go | 10 ++++++++++ .../rpc/resinit/resinit.go | 20 +++++++++++++++++++ .../rpc/resinit/resinit_android.go | 15 ++++++++++++++ .../rpc/resinit/resinit_nix.go | 14 +++++++++++++ .../rpc/resinit/resinit_windows.go | 10 ++++++++++ go/vendor/vendor.json | 12 ++++++++--- 6 files changed, 78 insertions(+), 3 deletions(-) create mode 100644 go/vendor/github.com/keybase/go-framed-msgpack-rpc/rpc/resinit/resinit.go create mode 100644 go/vendor/github.com/keybase/go-framed-msgpack-rpc/rpc/resinit/resinit_android.go create mode 100644 go/vendor/github.com/keybase/go-framed-msgpack-rpc/rpc/resinit/resinit_nix.go create mode 100644 go/vendor/github.com/keybase/go-framed-msgpack-rpc/rpc/resinit/resinit_windows.go diff --git a/go/vendor/github.com/keybase/go-framed-msgpack-rpc/rpc/connection.go b/go/vendor/github.com/keybase/go-framed-msgpack-rpc/rpc/connection.go index 8d713a1dd173..cd446293c0ee 100644 --- a/go/vendor/github.com/keybase/go-framed-msgpack-rpc/rpc/connection.go +++ b/go/vendor/github.com/keybase/go-framed-msgpack-rpc/rpc/connection.go @@ -12,6 +12,7 @@ import ( "time" "github.com/keybase/backoff" + "github.com/keybase/go-framed-msgpack-rpc/rpc/resinit" "golang.org/x/net/context" ) @@ -73,6 +74,15 @@ func (t *connTransport) Dial(context.Context) (Transporter, error) { var err error t.conn, err = t.uri.Dial() if err != nil { + // If we get a DNS error, it could be because glibc has cached an old + // version of /etc/resolv.conf. The res_init() libc function busts that + // cache and keeps us from getting stuck in a state where DNS requests + // keep failing even though the network is up. This is similar to what + // the Rust standard library does: + // https://github.com/rust-lang/rust/blob/028569ab1b/src/libstd/sys_common/net.rs#L186-L190 + // Note that we still propagate the error here, and we expect callers + // to retry. + resinit.ResInitIfDNSError(err) return nil, err } diff --git a/go/vendor/github.com/keybase/go-framed-msgpack-rpc/rpc/resinit/resinit.go b/go/vendor/github.com/keybase/go-framed-msgpack-rpc/rpc/resinit/resinit.go new file mode 100644 index 000000000000..b1b9de32e502 --- /dev/null +++ b/go/vendor/github.com/keybase/go-framed-msgpack-rpc/rpc/resinit/resinit.go @@ -0,0 +1,20 @@ +package resinit + +import "net" + +// If we get a DNS error, it could be because glibc has cached an old version +// of /etc/resolv.conf. The res_init() libc function busts that cache and keeps +// us from getting stuck in a state where DNS requests keep failing even though +// the network is up. This is similar to what the Rust standard library does: +// https://github.com/rust-lang/rust/blob/028569ab1b/src/libstd/sys_common/net.rs#L186-L190 +func ResInitIfDNSError(err error) { + // There are two error cases we need to handle, a raw *net.DNSError, and + // one wrapped in a *net.OpError. Detect that second case, and unwrap it. + if opErr, isOpErr := err.(*net.OpError); isOpErr { + err = opErr.Err + } + if _, isDNSError := err.(*net.DNSError); isDNSError { + // defined per platform in resinit_*.go + resInit() + } +} diff --git a/go/vendor/github.com/keybase/go-framed-msgpack-rpc/rpc/resinit/resinit_android.go b/go/vendor/github.com/keybase/go-framed-msgpack-rpc/rpc/resinit/resinit_android.go new file mode 100644 index 000000000000..df892b09d98c --- /dev/null +++ b/go/vendor/github.com/keybase/go-framed-msgpack-rpc/rpc/resinit/resinit_android.go @@ -0,0 +1,15 @@ +// Copyright 2015 Keybase, Inc. All rights reserved. Use of +// this source code is governed by the included BSD license. + +// +build android + +package resinit + +// /* Bionic has res_init() but it's not in any header. Patterned off of: */ +// /* https://mail.gnome.org/archives/commits-list/2013-May/msg01329.html */ +// int res_init (void); +import "C" + +func resInit() { + C.res_init() +} diff --git a/go/vendor/github.com/keybase/go-framed-msgpack-rpc/rpc/resinit/resinit_nix.go b/go/vendor/github.com/keybase/go-framed-msgpack-rpc/rpc/resinit/resinit_nix.go new file mode 100644 index 000000000000..212fb7b2296e --- /dev/null +++ b/go/vendor/github.com/keybase/go-framed-msgpack-rpc/rpc/resinit/resinit_nix.go @@ -0,0 +1,14 @@ +// Copyright 2015 Keybase, Inc. All rights reserved. Use of +// this source code is governed by the included BSD license. + +// +build !windows,!android + +package resinit + +// #cgo LDFLAGS: -lresolv +// #include +import "C" + +func resInit() { + C.res_init() +} diff --git a/go/vendor/github.com/keybase/go-framed-msgpack-rpc/rpc/resinit/resinit_windows.go b/go/vendor/github.com/keybase/go-framed-msgpack-rpc/rpc/resinit/resinit_windows.go new file mode 100644 index 000000000000..e7d7f0b61122 --- /dev/null +++ b/go/vendor/github.com/keybase/go-framed-msgpack-rpc/rpc/resinit/resinit_windows.go @@ -0,0 +1,10 @@ +// Copyright 2015 Keybase, Inc. All rights reserved. Use of +// this source code is governed by the included BSD license. + +// +build windows + +package resinit + +func resInit() { + // no-op +} diff --git a/go/vendor/vendor.json b/go/vendor/vendor.json index afd13a68e39a..1fd69501607a 100644 --- a/go/vendor/vendor.json +++ b/go/vendor/vendor.json @@ -259,10 +259,16 @@ "revisionTime": "2017-05-08T13:19:45Z" }, { - "checksumSHA1": "EByEHRqLqn43oRFBg+pr542YooU=", + "checksumSHA1": "gjj20DcVyDDJz+DaYpCuu4usgA8=", "path": "github.com/keybase/go-framed-msgpack-rpc/rpc", - "revision": "6c50f8c9981723baf4eea80f5de8f5b33493a126", - "revisionTime": "2017-06-08T21:54:08Z" + "revision": "9a3cbac42f5013d0a5d9ebb67ee5a2122b02e925", + "revisionTime": "2017-07-18T14:09:20Z" + }, + { + "checksumSHA1": "pi51SBDmToLthgwMA+5+wjyQans=", + "path": "github.com/keybase/go-framed-msgpack-rpc/rpc/resinit", + "revision": "9a3cbac42f5013d0a5d9ebb67ee5a2122b02e925", + "revisionTime": "2017-07-18T14:09:20Z" }, { "checksumSHA1": "rvlJlCMLMSpfITBgFv6OdWxlhco=", From 2deb33ee35d351bfce685f21065a6a80cd04e9bf Mon Sep 17 00:00:00 2001 From: Jack O'Connor Date: Tue, 18 Jul 2017 10:51:38 -0400 Subject: [PATCH 2/2] call libc::res_init() in response to DNS failures Go's DNS resolution often defers to the libc implementation, and glibc's resolver has a serious bug: https://sourceware.org/bugzilla/show_bug.cgi?id=984 It will cache the contents of /etc/resolv.conf, which can put the client in a state where all DNS requests fail forever after a network change. The conditions where Go calls into libc are complicated and platform-specific, and the resolver cache involves thread-local state, so repros tend to be inconsistent. But when you hit this on your laptop on the subway or whatever, the effect is that everything is broken until you restart the process. One way to fix this would be to force using the pure-Go resolver (net.DefaultResolver.PreferGo = true), which refreshes /etc/resolv.conf every 5 seconds. I'm wary of doing that, because the Go devs went through an enormous amount of trouble to enable cgo fallback, for various platform- and environment-specific reasons. See all the comments in net/conf.go::initConfVal() and net/conf.go::hostLookupOrder() in the standard library. Instead, we're trying the same workaround that the Rust standard library chose, where we call libc::res_init() after DNS failures. See https://github.com/rust-lang/rust/issues/41570. The downside here is that we have to remember to do this after we make network calls, and that we have to use cgo in the build, but the upside is that it should never break a DNS environment that was working before. --- go/libkb/client.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/go/libkb/client.go b/go/libkb/client.go index 142f5a2194d5..83d300bbbc95 100644 --- a/go/libkb/client.go +++ b/go/libkb/client.go @@ -17,6 +17,7 @@ import ( "time" "github.com/keybase/go-framed-msgpack-rpc/rpc" + "github.com/keybase/go-framed-msgpack-rpc/rpc/resinit" "h12.me/socks" ) @@ -146,6 +147,13 @@ func NewClient(e *Env, config *ClientConfig, needCookie bool) *Client { xprt.Dial = func(network, addr string) (c net.Conn, err error) { c, err = net.Dial(network, addr) if err != nil { + // If we get a DNS error, it could be because glibc has cached an + // old version of /etc/resolv.conf. The res_init() libc function + // busts that cache and keeps us from getting stuck in a state + // where DNS requests keep failing even though the network is up. + // This is similar to what the Rust standard library does: + // https://github.com/rust-lang/rust/blob/028569ab1b/src/libstd/sys_common/net.rs#L186-L190 + resinit.ResInitIfDNSError(err) return c, err } if err = rpc.DisableSigPipe(c); err != nil { @@ -159,6 +167,7 @@ func NewClient(e *Env, config *ClientConfig, needCookie bool) *Client { xprt.TLSClientConfig = &tls.Config{RootCAs: config.RootCAs} } if e.GetTorMode().Enabled() { + // TODO: should we call res_init on DNS errors here as well? dialSocksProxy := socks.DialSocksProxy(socks.SOCKS5, e.GetTorProxy()) xprt.Dial = dialSocksProxy } else {