Linux: Monitor resolv.conf for changes and use that to reload resolver.

net/base/dns_reloader.cc

+// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "net/base/dns_reloader.h"
+
+#if defined(OS_POSIX) && !defined(OS_MACOSX) && !defined(OS_OPENBSD)
+
+#include <resolv.h>
+
+#include "base/basictypes.h"
+#include "base/lazy_instance.h"
+#include "base/logging.h"
+#include "base/message_loop.h"
+#include "base/synchronization/lock.h"
+#include "base/threading/thread_local_storage.h"
+#include "net/base/network_change_notifier.h"
+
+namespace {
+
+// On Linux/BSD, changes to /etc/resolv.conf can go unnoticed thus resulting
+// in DNS queries failing either because nameservers are unknown on startup
+// or because nameserver info has changed as a result of e.g. connecting to
+// a new network. Some distributions patch glibc to stat /etc/resolv.conf
+// to try to automatically detect such changes but these patches are not
+// universal and even patched systems such as Jaunty appear to need calls
+// to res_ninit to reload the nameserver information in different threads.
+//
+// To fix this, on systems with FilePathWatcher support, we use
+// NetworkChangeNotifier::DNSObserver to monitor /etc/resolv.conf to
+// enable us to respond to DNS changes and reload the resolver state.
+//
+// OpenBSD does not have thread-safe res_ninit/res_nclose so we can't do

[eroman, 2011/07/25 20:13:03]

Actually, it looks like since these calls are protected by locks,
res_ninit/res_nclose will never be run in parallel. Although maybe the
threadsafety issues has to do with running getaddrinfo() at the same time?

[Craig, 2011/07/26 15:15:32]

Ah, interesting!

I'm not that clued up on *BSD so treat these comments with caution - the
original comment there comes from pvalchev in
http://codereview.chromium.org/556068 btw.

We didn't have locks back then but it seems as if openbsd doesn't have
res_ninit/res_nclose at all, only res_init/close. The openbsd resolver does seem
to stat resolv.conf internally after 30 seconds and I think it calls init
internally as required so I'd speculate that things may work reasonably on
openbsd even without any DnsReloader support - see
http://openbsd.monkey.org/tech/200503/msg00114.html

It's not exactly clear how badly their resolver is affected by thread safety
issues though -  the comment in rev 1.28 of 
http://www.openbsd.org/cgi-bin/cvsweb/src/lib/libc/net/res_init.c says "thread
safer libc (note: safer, not safe)". There's an open bug for this stuff
crbug.com/33481 so I'm tempted to leave this as is rather than enabling this
code on openbsd (and, for example, using res_init/close instead of ninit/nclose
for them). If I did enable it, we'd still need filepathwatcher support too of
course since I've killed the timer stuff.

[eroman, 2011/07/26 16:53:13]

Agreed. I definitely am not suggesting you enable BSD support as part of this
CL. I just figured I would ask these questions while I had an expert on the line
:)

+// the same trick there and most *BSD's don't yet have support for
+// FilePathWatcher (but perhaps the new kqueue mac code just needs to be
+// ported to *BSD to support that).
+
+class DnsReloader : public net::NetworkChangeNotifier::DNSObserver {
+ public:
+  struct ReloadState {
+    int resolver_generation;
+  };
+
+  // NetworkChangeNotifier::OnDNSChanged methods:
+  virtual void OnDNSChanged() {
+    DCHECK(MessageLoop::current()->thread_name() == "Chrome_IOThread");

[eroman, 2011/07/25 20:13:03]

Please avoid referencing chrome thread names (since this code might run in other
projects alltogether, or even things like the chrome "service process" which may
name threads differently).

If you still want an assertion, you can test the messageloop's type is an IO
thread (which at least catches incorrect runs on worker threads and/or ui
threads).

[Craig, 2011/08/03 19:30:58]

Done.

+    base::AutoLock l(lock_);
+    resolver_generation_++;
+  }
+
+  void MaybeReload() {
+    ReloadState* reload_state = static_cast<ReloadState*>(tls_index_.Get());
+    base::AutoLock l(lock_);
+
+    if (!reload_state) {
+      reload_state = new ReloadState();
+      reload_state->resolver_generation = resolver_generation_;
+      res_ninit(&_res);
+      tls_index_.Set(reload_state);
+    } else if (reload_state->resolver_generation != resolver_generation_) {
+      reload_state->resolver_generation = resolver_generation_;
+      // It is safe to call res_nclose here since we know res_ninit will have
+      // been called above.
+      res_nclose(&_res);
+      res_ninit(&_res);
+    }
+  }
+
+  // Free the allocated state.
+  static void SlotReturnFunction(void* data) {
+    ReloadState* reload_state = static_cast<ReloadState*>(data);
+    if (reload_state)
+      res_nclose(&_res);
+    delete reload_state;
+  }
+
+ private:
+  base::Lock lock_;  // Protects resolver_generation_.
+  int resolver_generation_;
+  friend struct base::DefaultLazyInstanceTraits<DnsReloader>;
+
+  DnsReloader() : resolver_generation_(0) {

[eroman, 2011/07/25 20:13:03]

style-nit: please declare methods above data members. In this case, the ctor and
dtor should be at the top of this privae section.

[Craig, 2011/08/03 19:30:58]

Done.

+    tls_index_.Initialize(SlotReturnFunction);
+    net::NetworkChangeNotifier::AddDNSObserver(this);
+  }
+
+  ~DnsReloader() {
+    NOTREACHED();  // LeakyLazyInstance is not destructed.
+    net::NetworkChangeNotifier::RemoveDNSObserver(this);
+  }
+
+  // We use thread local storage to identify which ReloadState to interact with.
+  static base::ThreadLocalStorage::Slot tls_index_ ;
+
+  DISALLOW_COPY_AND_ASSIGN(DnsReloader);
+};
+
+// A TLS slot to the ReloadState for the current thread.
+// static
+base::ThreadLocalStorage::Slot DnsReloader::tls_index_(
+    base::LINKER_INITIALIZED);
+
+base::LazyInstance<DnsReloader,
+                   base::LeakyLazyInstanceTraits<DnsReloader> >
+    g_dns_reloader(base::LINKER_INITIALIZED);
+
+}  // namespace
+
+namespace net {
+
+void EnsureDnsReloaderInit() {
+  DnsReloader* t ALLOW_UNUSED = g_dns_reloader.Pointer();
+}
+
+void DnsReloaderMaybeReload() {
+  // This routine can be called by any of the DNS worker threads.
+  DnsReloader* dns_reloader = g_dns_reloader.Pointer();
+  dns_reloader->MaybeReload();
+}
+
+}  // namespace net
+
+#endif  // defined(OS_POSIX) && !defined(OS_MACOSX) && !defined(OS_OPENBSD)