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

net/base/dns_reloader.cc

+// Copyright (c) 2010 The Chromium Authors. All rights reserved.

[agl, 2011/07/13 22:17:48]

wrong year.

[Craig, 2011/07/18 16:18:12]

Done.

+// 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/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
+// 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() {

[agl, 2011/07/13 22:17:48]

needs a comment to say which thread it runs on. (An assert is a perfectly good
comment.)

[Craig, 2011/07/18 16:18:12]

Done with a DCHECK but it's a touch ugly.

+    resolver_generation_++;
+  }
+
+  void MaybeReload() {
+    ReloadState* reload_state = static_cast<ReloadState*>(tls_index_.Get());
+
+    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:
+  int resolver_generation_;

[agl, 2011/07/13 22:17:48]

it appears that this is read/written across different threads so I think it
should be guarded by a lock. There's always some case there some CPU won't see
updates/will write crap without a lock. You could look at the atomic stuff in
base/, but I don't immediately see a good candidate wrapper around subtle::.

[Craig, 2011/07/14 17:23:30]

I assumed that it was thread-safe based on the assumption that access to the int
should be atomic and that it is only written to from one thread (and read from
several other threads) but I'll throw a lock at the problem to be sure.

[Craig, 2011/07/18 16:18:12]

Done.

+  friend struct base::DefaultLazyInstanceTraits<DnsReloader>;
+
+  DnsReloader() : resolver_generation_(0) {
+    // During testing the DnsReloader Singleton may be created and destroyed
+    // multiple times. Initialize the ThreadLocalStorage slot only once.

[willchan no longer on Chromium, 2011/07/14 15:42:32]

Really? Which test does this?

[Craig, 2011/07/14 17:23:30]

I copied the code blindly from dns_reload_timer :( The comment/code comes from
r33594 from Pawel - back then the net test suite was being tweaked with a call
to test_suite.EnforceTestIsolation() that seemed to fiddle with LazyInstance in
interesting ways. Test isolation seems to have been backed out in r56229 but
this snippet was left in place. I'll stare at this in more detail but it's
probably not necessary anymore and I shouldn't just have copied it without
making sure I knew what it did.

[Craig, 2011/07/18 16:18:13]

Done.

+    if (!tls_index_.initialized())
+      tls_index_.Initialize(SlotReturnFunction);
+    net::NetworkChangeNotifier::AddDNSObserver(this);
+  }
+
+  ~DnsReloader() {
+    net::NetworkChangeNotifier::RemoveDNSObserver(this);
+  }
+
+  // We use thread local storage to identify which ReloadState to interact with.
+  static base::ThreadLocalStorage::Slot tls_index_ ;

[willchan no longer on Chromium, 2011/07/14 15:42:32]

We generally use ThreadLocalPointer instead of ThreadLocalStorage, which is used
for implementing ThreadLocalPointer.

[Craig, 2011/07/18 16:18:13]

With ThreadLocalStorage, I  can hook into the SlotReturnFunction and use that to
do some cleanups but I can't see how I'd do that with a ThreadLocalPointer so I
haven't changed this for now.

+
+  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 DnsReloaderInit() {
+  DnsReloader* t ALLOW_UNUSED = g_dns_reloader.Pointer();
+}
+
+void DnsReloaderMaybeReload() {

[agl, 2011/07/13 22:17:48]

Needs a comment to say that it runs on any DNS worker thread.

[Craig, 2011/07/18 16:18:13]

Done.

+  DnsReloader* dns_reloader = g_dns_reloader.Pointer();
+  dns_reloader->MaybeReload();
+}
+
+}  // namespace net
+
+#endif  // defined(OS_POSIX) && !defined(OS_MACOSX) && !defined(OS_OPENBSD)