Disallow Singleton and LazyInstance on non-joinable threads.

Fix all known instances or explicitly allow them.  Usually the fix involves switching from Default traits to Lazy traits.

BUG=61753
TEST=none

Review URL: http://codereview.chromium.org/4635012

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@65996 0039d316-1c4b-4281-b951-d872f2087c98

15 files changed, 213 insertions, 75 deletions

diff --git a/base/lazy_instance.h b/base/lazy_instance.h
index ebf1e73..27cb784 100644
--- a/base/lazy_instance.h
+++ b/base/lazy_instance.h
@@ -36,14 +36,18 @@
 #define BASE_LAZY_INSTANCE_H_
 #pragma once
 
+#include <new>  // For placement new.
 #include "base/atomicops.h"
 #include "base/basictypes.h"
 #include "base/third_party/dynamic_annotations/dynamic_annotations.h"
+#include "base/thread_restrictions.h"
 
 namespace base {
 
 template <typename Type>
 struct DefaultLazyInstanceTraits {
+  static const bool kAllowedToAccessOnNonjoinableThread = false;
+
   static Type* New(void* instance) {
     // Use placement new to initialize our instance in our preallocated space.
     // The parenthesis is very important here to force POD type initialization.
@@ -57,6 +61,8 @@ struct DefaultLazyInstanceTraits {
 
 template <typename Type>
 struct LeakyLazyInstanceTraits {
+  static const bool kAllowedToAccessOnNonjoinableThread = true;
+
   static Type* New(void* instance) {
     return DefaultLazyInstanceTraits<Type>::New(instance);
   }
@@ -112,6 +118,9 @@ class LazyInstance : public LazyInstanceHelper {
   }
 
   Type* Pointer() {
+    if (!Traits::kAllowedToAccessOnNonjoinableThread)
+      base::ThreadRestrictions::AssertSingletonAllowed();
+
     // We will hopefully have fast access when the instance is already created.
     if ((base::subtle::NoBarrier_Load(&state_) != STATE_CREATED) &&
         NeedsInstance()) {
diff --git a/base/platform_thread_posix.cc b/base/platform_thread_posix.cc
index 66f3928..e442e9c 100644
--- a/base/platform_thread_posix.cc
+++ b/base/platform_thread_posix.cc
@@ -7,6 +7,11 @@
 #include <errno.h>
 #include <sched.h>
 
+#include "base/logging.h"
+#include "base/safe_strerror_posix.h"
+#include "base/scoped_ptr.h"
+#include "base/thread_restrictions.h"
+
 #if defined(OS_MACOSX)
 #include <mach/mach.h>
 #include <sys/resource.h>
@@ -24,18 +29,27 @@
 #include <sys/nacl_syscalls.h>
 #endif
 
-#include "base/logging.h"
-#include "base/safe_strerror_posix.h"
-
 #if defined(OS_MACOSX)
 namespace base {
 void InitThreading();
 }  // namespace base
 #endif
 
-static void* ThreadFunc(void* closure) {
-  PlatformThread::Delegate* delegate =
-      static_cast<PlatformThread::Delegate*>(closure);
+namespace {
+
+struct ThreadParams {
+  PlatformThread::Delegate* delegate;
+  bool joinable;
+};
+
+}  // namespace
+
+static void* ThreadFunc(void* params) {
+  ThreadParams* thread_params = static_cast<ThreadParams*>(params);
+  PlatformThread::Delegate* delegate = thread_params->delegate;
+  if (!thread_params->joinable)
+    base::ThreadRestrictions::SetSingletonAllowed(false);
+  delete thread_params;
   delegate->ThreadMain();
   return NULL;
 }
@@ -174,9 +188,14 @@ bool CreateThread(size_t stack_size, bool joinable,
   if (stack_size > 0)
     pthread_attr_setstacksize(&attributes, stack_size);
 
-  success = !pthread_create(thread_handle, &attributes, ThreadFunc, delegate);
+  ThreadParams* params = new ThreadParams;
+  params->delegate = delegate;
+  params->joinable = joinable;
+  success = !pthread_create(thread_handle, &attributes, ThreadFunc, params);
 
   pthread_attr_destroy(&attributes);
+  if (!success)
+    delete params;
   return success;
 }
 
diff --git a/base/platform_thread_win.cc b/base/platform_thread_win.cc
index a4a1b52..ce865da 100644
--- a/base/platform_thread_win.cc
+++ b/base/platform_thread_win.cc
@@ -5,6 +5,7 @@
 #include "base/platform_thread.h"
 
 #include "base/logging.h"
+#include "base/thread_restrictions.h"
 #include "base/win/windows_version.h"
 
 namespace {
@@ -20,9 +21,17 @@ typedef struct tagTHREADNAME_INFO {
   DWORD dwFlags;  // Reserved for future use, must be zero.
 } THREADNAME_INFO;
 
-DWORD __stdcall ThreadFunc(void* closure) {
-  PlatformThread::Delegate* delegate =
-      static_cast<PlatformThread::Delegate*>(closure);
+struct ThreadParams {
+  PlatformThread::Delegate* delegate;
+  bool joinable;
+};
+
+DWORD __stdcall ThreadFunc(void* params) {
+  ThreadParams* thread_params = static_cast<ThreadParams*>(params);
+  PlatformThread::Delegate* delegate = thread_params->delegate;
+  if (!thread_params->joinable)
+    base::ThreadRestrictions::SetSingletonAllowed(false);
+  delete thread_params;
   delegate->ThreadMain();
   return NULL;
 }
@@ -66,7 +75,8 @@ void PlatformThread::SetName(const char* name) {
 
 // static
 bool PlatformThread::Create(size_t stack_size, Delegate* delegate,
-                            PlatformThreadHandle* thread_handle) {
+                            PlatformThreadHandle* out_thread_handle) {
+  PlatformThreadHandle thread_handle;
   unsigned int flags = 0;
   if (stack_size > 0 && base::win::GetVersion() >= base::win::VERSION_XP) {
     flags = STACK_SIZE_PARAM_IS_A_RESERVATION;
@@ -74,22 +84,32 @@ bool PlatformThread::Create(size_t stack_size, Delegate* delegate,
     stack_size = 0;
   }
 
+  ThreadParams* params = new ThreadParams;
+  params->delegate = delegate;
+  params->joinable = out_thread_handle != NULL;
+
   // Using CreateThread here vs _beginthreadex makes thread creation a bit
   // faster and doesn't require the loader lock to be available.  Our code will
   // have to work running on CreateThread() threads anyway, since we run code
   // on the Windows thread pool, etc.  For some background on the difference:
   //   http://www.microsoft.com/msj/1099/win32/win321099.aspx
-  *thread_handle = CreateThread(
-      NULL, stack_size, ThreadFunc, delegate, flags, NULL);
-  return *thread_handle != NULL;
+  thread_handle = CreateThread(
+      NULL, stack_size, ThreadFunc, params, flags, NULL);
+  if (!thread_handle) {
+    delete params;
+    return false;
+  }
+
+  if (out_thread_handle)
+    *out_thread_handle = thread_handle;
+  else
+    CloseHandle(thread_handle);
+  return true;
 }
 
 // static
 bool PlatformThread::CreateNonJoinable(size_t stack_size, Delegate* delegate) {
-  PlatformThreadHandle thread_handle;
-  bool result = Create(stack_size, delegate, &thread_handle);
-  CloseHandle(thread_handle);
-  return result;
+  return Create(stack_size, delegate, NULL);
 }
 
 // static
diff --git a/base/singleton.h b/base/singleton.h
index 564b686..7caa8ee 100644
--- a/base/singleton.h
+++ b/base/singleton.h
@@ -9,6 +9,7 @@
 #include "base/at_exit.h"
 #include "base/atomicops.h"
 #include "base/third_party/dynamic_annotations/dynamic_annotations.h"
+#include "base/thread_restrictions.h"
 
 // Default traits for Singleton<Type>. Calls operator new and operator delete on
 // the object. Registers automatic deletion at process exit.
@@ -30,6 +31,11 @@ struct DefaultSingletonTraits {
   // Set to true to automatically register deletion of the object on process
   // exit. See below for the required call that makes this happen.
   static const bool kRegisterAtExit = true;
+
+  // Set to false to disallow access on a non-joinable thread.  This is
+  // different from kRegisterAtExit because StaticMemorySingletonTraits allows
+  // access on non-joinable threads, and gracefully handles this.
+  static const bool kAllowedToAccessOnNonjoinableThread = false;
 };
 
 
@@ -39,6 +45,7 @@ struct DefaultSingletonTraits {
 template<typename Type>
 struct LeakySingletonTraits : public DefaultSingletonTraits<Type> {
   static const bool kRegisterAtExit = false;
+  static const bool kAllowedToAccessOnNonjoinableThread = true;
 };
 
 
@@ -85,6 +92,7 @@ struct StaticMemorySingletonTraits {
   }
 
   static const bool kRegisterAtExit = true;
+  static const bool kAllowedToAccessOnNonjoinableThread = true;
 
   // Exposed for unittesting.
   static void Resurrect() {
@@ -176,6 +184,9 @@ class Singleton {
 
   // Return a pointer to the one true instance of the class.
   static Type* get() {
+    if (!Traits::kAllowedToAccessOnNonjoinableThread)
+      base::ThreadRestrictions::AssertSingletonAllowed();
+
     // Our AtomicWord doubles as a spinlock, where a value of
     // kBeingCreatedMarker means the spinlock is being held for creation.
     static const base::subtle::AtomicWord kBeingCreatedMarker = 1;
diff --git a/base/thread_restrictions.cc b/base/thread_restrictions.cc
index 270d663..1f5794a 100644
--- a/base/thread_restrictions.cc
+++ b/base/thread_restrictions.cc
@@ -18,6 +18,9 @@ namespace {
 LazyInstance<ThreadLocalBoolean, LeakyLazyInstanceTraits<ThreadLocalBoolean> >
     g_io_disallowed(LINKER_INITIALIZED);
 
+LazyInstance<ThreadLocalBoolean, LeakyLazyInstanceTraits<ThreadLocalBoolean> >
+    g_singleton_disallowed(LINKER_INITIALIZED);
+
 }  // anonymous namespace
 
 // static
@@ -39,6 +42,21 @@ void ThreadRestrictions::AssertIOAllowed() {
   }
 }
 
+bool ThreadRestrictions::SetSingletonAllowed(bool allowed) {
+  bool previous_allowed = g_singleton_disallowed.Get().Get();
+  g_singleton_disallowed.Get().Set(!allowed);
+  return !previous_allowed;
+}
+
+// static
+void ThreadRestrictions::AssertSingletonAllowed() {
+  if (g_singleton_disallowed.Get().Get())
+    LOG(FATAL) << "LazyInstance/Singleton is not allowed to be used on this "
+               << "thread.  Most likely it's because this thread is not "
+               << "joinable, so AtExitManager may have deleted the object "
+               << "on shutdown, leading to a potential shutdown crash.";
+}
+
 }  // namespace base
 
 #endif  // NDEBUG
diff --git a/base/thread_restrictions.h b/base/thread_restrictions.h
index a10aaec..2d73927 100644
--- a/base/thread_restrictions.h
+++ b/base/thread_restrictions.h
@@ -9,8 +9,13 @@
 
 namespace base {
 
-// ThreadRestrictions helps protect threads that should not block from
-// making blocking calls.  It works like this:
+// Certain behavior is disallowed on certain threads.  ThreadRestrictions helps
+// enforce these rules.  Examples of such rules:
+//
+// * Do not do blocking IO (makes the thread janky)
+// * Do not access Singleton/LazyInstance (may lead to shutdown crashes)
+//
+// Here's more about how the IO protection works:
 //
 // 1) If a thread should not be allowed to make IO calls, mark it:
 //      base::ThreadRestrictions::SetIOAllowed(false);
@@ -45,6 +50,20 @@ class ThreadRestrictions {
     DISALLOW_COPY_AND_ASSIGN(ScopedAllowIO);
   };
 
+  // Constructing a ScopedAllowSingleton temporarily allows accessing for the
+  // current thread.  Doing this is almost always incorrect.
+  class ScopedAllowSingleton {
+   public:
+    ScopedAllowSingleton() { previous_value_ = SetSingletonAllowed(true); }
+    ~ScopedAllowSingleton() { SetSingletonAllowed(previous_value_); }
+   private:
+    // Whether singleton use is allowed when the ScopedAllowSingleton was
+    // constructed.
+    bool previous_value_;
+
+    DISALLOW_COPY_AND_ASSIGN(ScopedAllowSingleton);
+  };
+
 #ifndef NDEBUG
   // Set whether the current thread to make IO calls.
   // Threads start out in the *allowed* state.
@@ -55,15 +74,25 @@ class ThreadRestrictions {
   // and DCHECK if not.  See the block comment above the class for
   // a discussion of where to add these checks.
   static void AssertIOAllowed();
+
+  // Set whether the current thread can use singletons.  Returns the previous
+  // value.
+  static bool SetSingletonAllowed(bool allowed);
+
+  // Check whether the current thread is allowed to use singletons (Singleton /
+  // LazyInstance).  DCHECKs if not.
+  static void AssertSingletonAllowed();
 #else
   // In Release builds, inline the empty definitions of these functions so
   // that they can be compiled out.
   static bool SetIOAllowed(bool allowed) { return true; }
   static void AssertIOAllowed() {}
+  static bool SetSingletonAllowed(bool allowed) { return true; }
+  static void AssertSingletonAllowed() {}
 #endif
 
  private:
-  ThreadRestrictions();  // class for namespacing only
+  DISALLOW_IMPLICIT_CONSTRUCTORS(ThreadRestrictions);
 };
 
 }  // namespace base
diff --git a/base/tracked_objects.cc b/base/tracked_objects.cc
index 4631f34..9db25ff 100644
--- a/base/tracked_objects.cc
+++ b/base/tracked_objects.cc
@@ -10,6 +10,7 @@
 #include "base/message_loop.h"
 #include "base/string_util.h"
 #include "base/stringprintf.h"
+#include "base/thread_restrictions.h"
 
 using base::TimeDelta;
 
@@ -89,7 +90,13 @@ Lock ThreadData::list_lock_;
 // static
 ThreadData::Status ThreadData::status_ = ThreadData::UNINITIALIZED;
 
-ThreadData::ThreadData() : next_(NULL), message_loop_(MessageLoop::current()) {}
+ThreadData::ThreadData() : next_(NULL) {
+  // This shouldn't use the MessageLoop::current() LazyInstance since this might
+  // be used on a non-joinable thread.
+  // http://crbug.com/62728
+  base::ThreadRestrictions::ScopedAllowSingleton scoped_allow_singleton;
+  message_loop_ = MessageLoop::current();
+}
 
 ThreadData::~ThreadData() {}
 
@@ -260,8 +267,14 @@ void ThreadData::WriteHTMLTotalAndSubtotals(
 }
 
 Births* ThreadData::TallyABirth(const Location& location) {
-  if (!message_loop_)  // In case message loop wasn't yet around...
-    message_loop_ = MessageLoop::current();  // Find it now.
+  {
+    // This shouldn't use the MessageLoop::current() LazyInstance since this
+    // might be used on a non-joinable thread.
+    // http://crbug.com/62728
+    base::ThreadRestrictions::ScopedAllowSingleton scoped_allow_singleton;
+    if (!message_loop_)  // In case message loop wasn't yet around...
+      message_loop_ = MessageLoop::current();  // Find it now.
+  }
 
   BirthMap::iterator it = birth_map_.find(location);
   if (it != birth_map_.end()) {
@@ -279,8 +292,12 @@ Births* ThreadData::TallyABirth(const Location& location) {
 
 void ThreadData::TallyADeath(const Births& lifetimes,
                              const TimeDelta& duration) {
-  if (!message_loop_)  // In case message loop wasn't yet around...
-    message_loop_ = MessageLoop::current();  // Find it now.
+  {
+    // http://crbug.com/62728
+    base::ThreadRestrictions::ScopedAllowSingleton scoped_allow_singleton;
+    if (!message_loop_)  // In case message loop wasn't yet around...
+      message_loop_ = MessageLoop::current();  // Find it now.
+  }
 
   DeathMap::iterator it = death_map_.find(&lifetimes);
   if (it != death_map_.end()) {
diff --git a/ceee/ie/broker/executors_manager.h b/ceee/ie/broker/executors_manager.h
index 3b9ac3b..cba7b3f 100644
--- a/ceee/ie/broker/executors_manager.h
+++ b/ceee/ie/broker/executors_manager.h
@@ -90,14 +90,10 @@ class ExecutorsManager {
 
   // Traits for Singleton<ExecutorsManager> so that we can pass an argument
   // to the constructor.
-  struct SingletonTraits {
+  struct SingletonTraits : public DefaultSingletonTraits<ExecutorsManager> {
     static ExecutorsManager* New() {
       return new ExecutorsManager(false);  // By default, we want a thread.
     }
-    static void Delete(ExecutorsManager* x) {
-      delete x;
-    }
-    static const bool kRegisterAtExit = true;
   };
 
  protected:
diff --git a/net/base/dns_reload_timer.cc b/net/base/dns_reload_timer.cc
index 5931c5b..1bfe535 100644
--- a/net/base/dns_reload_timer.cc
+++ b/net/base/dns_reload_timer.cc
@@ -5,11 +5,11 @@
 #include "net/base/dns_reload_timer.h"
 
 #if defined(OS_POSIX) && !defined(OS_MACOSX) && !defined(OS_OPENBSD)
-#include "base/singleton.h"
+#include "base/lazy_instance.h"
 #include "base/thread_local_storage.h"
 #include "base/time.h"
 
-namespace net {
+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
@@ -58,7 +58,7 @@ class DnsReloadTimer {
   }
 
  private:
-  friend struct DefaultSingletonTraits<DnsReloadTimer>;
+  friend struct base::DefaultLazyInstanceTraits<DnsReloadTimer>;
 
   DnsReloadTimer() {
     // During testing the DnsReloadTimer Singleton may be created and destroyed
@@ -81,8 +81,16 @@ class DnsReloadTimer {
 // static
 ThreadLocalStorage::Slot DnsReloadTimer::tls_index_(base::LINKER_INITIALIZED);
 
+base::LazyInstance<DnsReloadTimer,
+                   base::LeakyLazyInstanceTraits<DnsReloadTimer> >
+    g_dns_reload_timer(base::LINKER_INITIALIZED);
+
+}  // namespace
+
+namespace net {
+
 bool DnsReloadTimerHasExpired() {
-  DnsReloadTimer* dns_timer = Singleton<DnsReloadTimer>::get();
+  DnsReloadTimer* dns_timer = g_dns_reload_timer.Pointer();
   return dns_timer->Expired();
 }
 
diff --git a/net/base/ev_root_ca_metadata.cc b/net/base/ev_root_ca_metadata.cc
index 661b652..a721357 100644
--- a/net/base/ev_root_ca_metadata.cc
+++ b/net/base/ev_root_ca_metadata.cc
@@ -13,8 +13,8 @@
 #include <stdlib.h>
 #endif
 
+#include "base/lazy_instance.h"
 #include "base/logging.h"
-#include "base/singleton.h"
 
 namespace net {
 
@@ -283,9 +283,13 @@ const EVRootCAMetadata::PolicyOID EVRootCAMetadata::policy_oids_[] = {
 };
 #endif
 
+static base::LazyInstance<EVRootCAMetadata,
+                          base::LeakyLazyInstanceTraits<EVRootCAMetadata> >
+    g_ev_root_ca_metadata(base::LINKER_INITIALIZED);
+
 // static
 EVRootCAMetadata* EVRootCAMetadata::GetInstance() {
-  return Singleton<EVRootCAMetadata>::get();
+  return g_ev_root_ca_metadata.Pointer();
 }
 
 bool EVRootCAMetadata::GetPolicyOID(
diff --git a/net/base/ev_root_ca_metadata.h b/net/base/ev_root_ca_metadata.h
index e0961f3..832ebe2 100644
--- a/net/base/ev_root_ca_metadata.h
+++ b/net/base/ev_root_ca_metadata.h
@@ -17,8 +17,10 @@
 
 #include "net/base/x509_certificate.h"
 
+namespace base {
 template <typename T>
-struct DefaultSingletonTraits;
+struct DefaultLazyInstanceTraits;
+}  // namespace base
 
 namespace net {
 
@@ -55,7 +57,7 @@ class EVRootCAMetadata {
                       PolicyOID policy_oid) const;
 
  private:
-  friend struct DefaultSingletonTraits<EVRootCAMetadata>;
+  friend struct base::DefaultLazyInstanceTraits<EVRootCAMetadata>;
 
   typedef std::map<SHA1Fingerprint, PolicyOID,
                    SHA1FingerprintLessThan> PolicyOidMap;
diff --git a/net/base/keygen_handler_unittest.cc b/net/base/keygen_handler_unittest.cc
index 62c5191..d3bf4f5 100644
--- a/net/base/keygen_handler_unittest.cc
+++ b/net/base/keygen_handler_unittest.cc
@@ -16,6 +16,7 @@
 #include "base/logging.h"
 #include "base/nss_util.h"
 #include "base/task.h"
+#include "base/thread_restrictions.h"
 #include "base/waitable_event.h"
 #include "base/worker_pool.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -90,6 +91,9 @@ class ConcurrencyTestTask : public Task {
   }
 
   virtual void Run() {
+    // We allow Singleton use on the worker thread here since we use a
+    // WaitableEvent to synchronize, so it's safe.
+    base::ThreadRestrictions::ScopedAllowSingleton scoped_allow_singleton;
     KeygenHandler handler(768, "some challenge",
                           GURL("http://www.example.com"));
     handler.set_stores_key(false); // Don't leave the key-pair behind.
diff --git a/net/base/x509_certificate.cc b/net/base/x509_certificate.cc
index d93d270..a1dc2c3 100644
--- a/net/base/x509_certificate.cc
+++ b/net/base/x509_certificate.cc
@@ -6,9 +6,9 @@
 
 #include <map>
 
+#include "base/lazy_instance.h"
 #include "base/logging.h"
 #include "base/metrics/histogram.h"
-#include "base/singleton.h"
 #include "base/string_piece.h"
 #include "base/time.h"
 #include "net/base/pem_tokenizer.h"
@@ -39,17 +39,6 @@ const char kCertificateHeader[] = "CERTIFICATE";
 // The PEM block header used for PKCS#7 data
 const char kPKCS7Header[] = "PKCS7";
 
-}  // namespace
-
-bool X509Certificate::LessThan::operator()(X509Certificate* lhs,
-                                           X509Certificate* rhs) const {
-  if (lhs == rhs)
-    return false;
-
-  SHA1FingerprintLessThan fingerprint_functor;
-  return fingerprint_functor(lhs->fingerprint_, rhs->fingerprint_);
-}
-
 // A thread-safe cache for X509Certificate objects.
 //
 // The cache does not hold a reference to the certificate objects.  The objects
@@ -57,9 +46,8 @@ bool X509Certificate::LessThan::operator()(X509Certificate* lhs,
 // will be holding dead pointers to the objects).
 // TODO(rsleevi): There exists a chance of a use-after-free, due to a race
 // between Find() and Remove(). See http://crbug.com/49377
-class X509Certificate::Cache {
+class X509CertificateCache {
  public:
-  static Cache* GetInstance();
   void Insert(X509Certificate* cert);
   void Remove(X509Certificate* cert);
   X509Certificate* Find(const SHA1Fingerprint& fingerprint);
@@ -69,8 +57,9 @@ class X509Certificate::Cache {
       CertMap;
 
   // Obtain an instance of X509Certificate::Cache via GetInstance().
-  Cache() {}
-  friend struct DefaultSingletonTraits<Cache>;
+  X509CertificateCache() {}
+  ~X509CertificateCache() {}
+  friend struct base::DefaultLazyInstanceTraits<X509CertificateCache>;
 
   // You must acquire this lock before using any private data of this object.
   // You must not block while holding this lock.
@@ -79,18 +68,16 @@ class X509Certificate::Cache {
   // The certificate cache.  You must acquire |lock_| before using |cache_|.
   CertMap cache_;
 
-  DISALLOW_COPY_AND_ASSIGN(Cache);
+  DISALLOW_COPY_AND_ASSIGN(X509CertificateCache);
 };
 
-// Get the singleton object for the cache.
-// static
-X509Certificate::Cache* X509Certificate::Cache::GetInstance() {
-  return Singleton<X509Certificate::Cache>::get();
-}
+base::LazyInstance<X509CertificateCache,
+                   base::LeakyLazyInstanceTraits<X509CertificateCache> >
+    g_x509_certificate_cache(base::LINKER_INITIALIZED);
 
 // Insert |cert| into the cache.  The cache does NOT AddRef |cert|.
 // Any existing certificate with the same fingerprint will be replaced.
-void X509Certificate::Cache::Insert(X509Certificate* cert) {
+void X509CertificateCache::Insert(X509Certificate* cert) {
   AutoLock lock(lock_);
 
   DCHECK(!IsNullFingerprint(cert->fingerprint())) <<
@@ -100,7 +87,7 @@ void X509Certificate::Cache::Insert(X509Certificate* cert) {
 
 // Remove |cert| from the cache.  The cache does not assume that |cert| is
 // already in the cache.
-void X509Certificate::Cache::Remove(X509Certificate* cert) {
+void X509CertificateCache::Remove(X509Certificate* cert) {
   AutoLock lock(lock_);
 
   CertMap::iterator pos(cache_.find(cert->fingerprint()));
@@ -111,7 +98,7 @@ void X509Certificate::Cache::Remove(X509Certificate* cert) {
 
 // Find a certificate in the cache with the given fingerprint.  If one does
 // not exist, this method returns NULL.
-X509Certificate* X509Certificate::Cache::Find(
+X509Certificate* X509CertificateCache::Find(
     const SHA1Fingerprint& fingerprint) {
   AutoLock lock(lock_);
 
@@ -122,6 +109,17 @@ X509Certificate* X509Certificate::Cache::Find(
   return pos->second;
 };
 
+}  // namespace
+
+bool X509Certificate::LessThan::operator()(X509Certificate* lhs,
+                                           X509Certificate* rhs) const {
+  if (lhs == rhs)
+    return false;
+
+  SHA1FingerprintLessThan fingerprint_functor;
+  return fingerprint_functor(lhs->fingerprint_, rhs->fingerprint_);
+}
+
 // static
 X509Certificate* X509Certificate::CreateFromHandle(
     OSCertHandle cert_handle,
@@ -131,7 +129,7 @@ X509Certificate* X509Certificate::CreateFromHandle(
   DCHECK(source != SOURCE_UNUSED);
 
   // Check if we already have this certificate in memory.
-  X509Certificate::Cache* cache = X509Certificate::Cache::GetInstance();
+  X509CertificateCache* cache = g_x509_certificate_cache.Pointer();
   X509Certificate* cached_cert =
       cache->Find(CalculateFingerprint(cert_handle));
   if (cached_cert) {
@@ -311,7 +309,7 @@ X509Certificate::X509Certificate(const std::string& subject,
 
 X509Certificate::~X509Certificate() {
   // We might not be in the cache, but it is safe to remove ourselves anyway.
-  X509Certificate::Cache::GetInstance()->Remove(this);
+  g_x509_certificate_cache.Get().Remove(this);
   if (cert_handle_)
     FreeOSCertHandle(cert_handle_);
   for (size_t i = 0; i < intermediate_ca_certs_.size(); ++i)
diff --git a/net/base/x509_certificate.h b/net/base/x509_certificate.h
index 2a7e6d7..bb958e6 100644
--- a/net/base/x509_certificate.h
+++ b/net/base/x509_certificate.h
@@ -277,8 +277,6 @@ class X509Certificate : public base::RefCountedThreadSafe<X509Certificate> {
   FRIEND_TEST_ALL_PREFIXES(X509CertificateTest, Cache);
   FRIEND_TEST_ALL_PREFIXES(X509CertificateTest, IntermediateCertificates);
 
-  class Cache;
-
   // Construct an X509Certificate from a handle to the certificate object
   // in the underlying crypto library.
   X509Certificate(OSCertHandle cert_handle, Source source,
diff --git a/net/base/x509_certificate_mac.cc b/net/base/x509_certificate_mac.cc
index a2a0eea..2a604ee 100644
--- a/net/base/x509_certificate_mac.cc
+++ b/net/base/x509_certificate_mac.cc
@@ -8,6 +8,7 @@
 #include <Security/Security.h>
 #include <time.h>
 
+#include "base/lazy_instance.h"
 #include "base/logging.h"
 #include "base/pickle.h"
 #include "base/mac/scoped_cftyperef.h"
@@ -21,6 +22,8 @@ using base::Time;
 
 namespace net {
 
+namespace {
+
 class MacTrustedCertificates {
  public:
   // Sets the trusted root certificate used by tests. Call with |cert| set
@@ -57,7 +60,7 @@ class MacTrustedCertificates {
     return merged_array;
   }
  private:
-  friend struct DefaultSingletonTraits<MacTrustedCertificates>;
+  friend struct base::DefaultLazyInstanceTraits<MacTrustedCertificates>;
 
   // Obtain an instance of MacTrustedCertificates via the singleton
   // interface.
@@ -73,11 +76,9 @@ class MacTrustedCertificates {
   DISALLOW_COPY_AND_ASSIGN(MacTrustedCertificates);
 };
 
-void SetMacTestCertificate(X509Certificate* cert) {
-  Singleton<MacTrustedCertificates>::get()->SetTestCertificate(cert);
-}
-
-namespace {
+base::LazyInstance<MacTrustedCertificates,
+    base::LeakyLazyInstanceTraits<MacTrustedCertificates> >
+    g_mac_trusted_certificates(base::LINKER_INITIALIZED);
 
 typedef OSStatus (*SecTrustCopyExtendedResultFuncPtr)(SecTrustRef,
                                                       CFDictionaryRef*);
@@ -443,6 +444,10 @@ void AddCertificatesFromBytes(const char* data, size_t length,
 
 }  // namespace
 
+void SetMacTestCertificate(X509Certificate* cert) {
+  g_mac_trusted_certificates.Get().SetTestCertificate(cert);
+}
+
 void X509Certificate::Initialize() {
   const CSSM_X509_NAME* name;
   OSStatus status = SecCertificateGetSubject(cert_handle_, &name);
@@ -545,7 +550,7 @@ int X509Certificate::Verify(const std::string& hostname, int flags,
   // Set the trusted anchor certificates for the SecTrustRef by merging the
   // system trust anchors and the test root certificate.
   CFArrayRef anchor_array =
-      Singleton<MacTrustedCertificates>::get()->CopyTrustedCertificateArray();
+      g_mac_trusted_certificates.Get().CopyTrustedCertificateArray();
   ScopedCFTypeRef<CFArrayRef> scoped_anchor_array(anchor_array);
   if (anchor_array) {
     status = SecTrustSetAnchorCertificates(trust_ref, anchor_array);