diff options
-rw-r--r-- | base/lazy_instance.h | 11 | ||||
-rw-r--r-- | base/message_loop_proxy_impl.cc | 7 | ||||
-rw-r--r-- | base/platform_thread_posix.cc | 33 | ||||
-rw-r--r-- | base/platform_thread_win.cc | 74 | ||||
-rw-r--r-- | base/singleton.h | 11 | ||||
-rw-r--r-- | base/thread_restrictions.cc | 23 | ||||
-rw-r--r-- | base/thread_restrictions.h | 35 | ||||
-rw-r--r-- | base/tracked_objects.cc | 27 | ||||
-rw-r--r-- | ceee/ie/broker/executors_manager.h | 6 | ||||
-rw-r--r-- | chrome/browser/browser_thread.cc | 11 | ||||
-rw-r--r-- | net/base/dns_reload_timer.cc | 16 | ||||
-rw-r--r-- | net/base/ev_root_ca_metadata.cc | 8 | ||||
-rw-r--r-- | net/base/ev_root_ca_metadata.h | 6 | ||||
-rw-r--r-- | net/base/keygen_handler_unittest.cc | 4 | ||||
-rw-r--r-- | net/base/x509_certificate.cc | 54 | ||||
-rw-r--r-- | net/base/x509_certificate.h | 2 | ||||
-rw-r--r-- | net/base/x509_certificate_mac.cc | 19 | ||||
-rw-r--r-- | net/ocsp/nss_ocsp.cc | 11 |
18 files changed, 100 insertions, 258 deletions
diff --git a/base/lazy_instance.h b/base/lazy_instance.h index f8d5987..ebf1e73 100644 --- a/base/lazy_instance.h +++ b/base/lazy_instance.h @@ -36,19 +36,14 @@ #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. @@ -62,8 +57,6 @@ struct DefaultLazyInstanceTraits { template <typename Type> struct LeakyLazyInstanceTraits { - static const bool kAllowedToAccessOnNonjoinableThread = true; - static Type* New(void* instance) { return DefaultLazyInstanceTraits<Type>::New(instance); } @@ -119,9 +112,6 @@ 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()) { @@ -136,6 +126,7 @@ class LazyInstance : public LazyInstanceHelper { // and CompleteInstance(...) happens before "return instance_" below. // See the corresponding HAPPENS_BEFORE in CompleteInstance(...). ANNOTATE_HAPPENS_AFTER(&state_); + return instance_; } diff --git a/base/message_loop_proxy_impl.cc b/base/message_loop_proxy_impl.cc index 7998a94..2e0f809 100644 --- a/base/message_loop_proxy_impl.cc +++ b/base/message_loop_proxy_impl.cc @@ -3,7 +3,6 @@ // found in the LICENSE file. #include "base/message_loop_proxy_impl.h" -#include "base/thread_restrictions.h" namespace base { @@ -46,11 +45,6 @@ bool MessageLoopProxyImpl::PostNonNestableDelayedTask( } bool MessageLoopProxyImpl::BelongsToCurrentThread() { - // We shouldn't use MessageLoop::current() since it uses LazyInstance which - // may be deleted by ~AtExitManager when a WorkerPool thread calls this - // function. - // http://crbug.com/63678 - base::ThreadRestrictions::ScopedAllowSingleton allow_singleton; AutoLock lock(message_loop_lock_); return (target_message_loop_ && (MessageLoop::current() == target_message_loop_)); @@ -104,3 +98,4 @@ MessageLoopProxy::CreateForCurrentThread() { } } // namespace base + diff --git a/base/platform_thread_posix.cc b/base/platform_thread_posix.cc index e442e9c..66f3928 100644 --- a/base/platform_thread_posix.cc +++ b/base/platform_thread_posix.cc @@ -7,11 +7,6 @@ #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> @@ -29,27 +24,18 @@ #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 -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; +static void* ThreadFunc(void* closure) { + PlatformThread::Delegate* delegate = + static_cast<PlatformThread::Delegate*>(closure); delegate->ThreadMain(); return NULL; } @@ -188,14 +174,9 @@ bool CreateThread(size_t stack_size, bool joinable, if (stack_size > 0) pthread_attr_setstacksize(&attributes, stack_size); - ThreadParams* params = new ThreadParams; - params->delegate = delegate; - params->joinable = joinable; - success = !pthread_create(thread_handle, &attributes, ThreadFunc, params); + success = !pthread_create(thread_handle, &attributes, ThreadFunc, delegate); 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 e5afc52..a4a1b52 100644 --- a/base/platform_thread_win.cc +++ b/base/platform_thread_win.cc @@ -5,7 +5,6 @@ #include "base/platform_thread.h" #include "base/logging.h" -#include "base/thread_restrictions.h" #include "base/win/windows_version.h" namespace { @@ -21,58 +20,13 @@ typedef struct tagTHREADNAME_INFO { DWORD dwFlags; // Reserved for future use, must be zero. } THREADNAME_INFO; -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; +DWORD __stdcall ThreadFunc(void* closure) { + PlatformThread::Delegate* delegate = + static_cast<PlatformThread::Delegate*>(closure); delegate->ThreadMain(); return NULL; } -// CreateThreadInternal() matches PlatformThread::Create(), except that -// |out_thread_handle| may be NULL, in which case a non-joinable thread is -// created. -bool CreateThreadInternal(size_t stack_size, - PlatformThread::Delegate* delegate, - 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; - } else { - 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, 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; -} - } // namespace // static @@ -113,13 +67,29 @@ void PlatformThread::SetName(const char* name) { // static bool PlatformThread::Create(size_t stack_size, Delegate* delegate, PlatformThreadHandle* thread_handle) { - DCHECK(thread_handle); - return CreateThreadInternal(stack_size, delegate, thread_handle); + unsigned int flags = 0; + if (stack_size > 0 && base::win::GetVersion() >= base::win::VERSION_XP) { + flags = STACK_SIZE_PARAM_IS_A_RESERVATION; + } else { + stack_size = 0; + } + + // 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; } // static bool PlatformThread::CreateNonJoinable(size_t stack_size, Delegate* delegate) { - return CreateThreadInternal(stack_size, delegate, NULL); + PlatformThreadHandle thread_handle; + bool result = Create(stack_size, delegate, &thread_handle); + CloseHandle(thread_handle); + return result; } // static diff --git a/base/singleton.h b/base/singleton.h index 7caa8ee..564b686 100644 --- a/base/singleton.h +++ b/base/singleton.h @@ -9,7 +9,6 @@ #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. @@ -31,11 +30,6 @@ 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; }; @@ -45,7 +39,6 @@ struct DefaultSingletonTraits { template<typename Type> struct LeakySingletonTraits : public DefaultSingletonTraits<Type> { static const bool kRegisterAtExit = false; - static const bool kAllowedToAccessOnNonjoinableThread = true; }; @@ -92,7 +85,6 @@ struct StaticMemorySingletonTraits { } static const bool kRegisterAtExit = true; - static const bool kAllowedToAccessOnNonjoinableThread = true; // Exposed for unittesting. static void Resurrect() { @@ -184,9 +176,6 @@ 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 6767d80..270d663 100644 --- a/base/thread_restrictions.cc +++ b/base/thread_restrictions.cc @@ -18,16 +18,13 @@ namespace { LazyInstance<ThreadLocalBoolean, LeakyLazyInstanceTraits<ThreadLocalBoolean> > g_io_disallowed(LINKER_INITIALIZED); -LazyInstance<ThreadLocalBoolean, LeakyLazyInstanceTraits<ThreadLocalBoolean> > - g_singleton_disallowed(LINKER_INITIALIZED); - } // anonymous namespace // static bool ThreadRestrictions::SetIOAllowed(bool allowed) { - bool previous_disallowed = g_io_disallowed.Get().Get(); + bool previous_allowed = g_io_disallowed.Get().Get(); g_io_disallowed.Get().Set(!allowed); - return !previous_disallowed; + return !previous_allowed; } // static @@ -42,22 +39,6 @@ void ThreadRestrictions::AssertIOAllowed() { } } -bool ThreadRestrictions::SetSingletonAllowed(bool allowed) { - bool previous_disallowed = g_singleton_disallowed.Get().Get(); - g_singleton_disallowed.Get().Set(!allowed); - return !previous_disallowed; -} - -// 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 51e5a15..a10aaec 100644 --- a/base/thread_restrictions.h +++ b/base/thread_restrictions.h @@ -9,13 +9,8 @@ namespace base { -// 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 protection works: +// ThreadRestrictions helps protect threads that should not block from +// making blocking calls. It works like this: // // 1) If a thread should not be allowed to make IO calls, mark it: // base::ThreadRestrictions::SetIOAllowed(false); @@ -50,20 +45,6 @@ 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. @@ -74,25 +55,15 @@ 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: - DISALLOW_IMPLICIT_CONSTRUCTORS(ThreadRestrictions); + ThreadRestrictions(); // class for namespacing only }; } // namespace base diff --git a/base/tracked_objects.cc b/base/tracked_objects.cc index 9db25ff..4631f34 100644 --- a/base/tracked_objects.cc +++ b/base/tracked_objects.cc @@ -10,7 +10,6 @@ #include "base/message_loop.h" #include "base/string_util.h" #include "base/stringprintf.h" -#include "base/thread_restrictions.h" using base::TimeDelta; @@ -90,13 +89,7 @@ Lock ThreadData::list_lock_; // static ThreadData::Status ThreadData::status_ = ThreadData::UNINITIALIZED; -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() : next_(NULL), message_loop_(MessageLoop::current()) {} ThreadData::~ThreadData() {} @@ -267,14 +260,8 @@ void ThreadData::WriteHTMLTotalAndSubtotals( } Births* ThreadData::TallyABirth(const Location& location) { - { - // 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. - } + 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()) { @@ -292,12 +279,8 @@ Births* ThreadData::TallyABirth(const Location& location) { void ThreadData::TallyADeath(const Births& lifetimes, const TimeDelta& duration) { - { - // 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. - } + 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 cba7b3f..3b9ac3b 100644 --- a/ceee/ie/broker/executors_manager.h +++ b/ceee/ie/broker/executors_manager.h @@ -90,10 +90,14 @@ class ExecutorsManager { // Traits for Singleton<ExecutorsManager> so that we can pass an argument // to the constructor. - struct SingletonTraits : public DefaultSingletonTraits<ExecutorsManager> { + struct SingletonTraits { 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/chrome/browser/browser_thread.cc b/chrome/browser/browser_thread.cc index e6edfd0..7ba53e0 100644 --- a/chrome/browser/browser_thread.cc +++ b/chrome/browser/browser_thread.cc @@ -6,7 +6,6 @@ #include "base/message_loop.h" #include "base/message_loop_proxy.h" -#include "base/thread_restrictions.h" // Friendly names for the well-known threads. static const char* browser_thread_names[BrowserThread::ID_COUNT] = { @@ -113,11 +112,6 @@ bool BrowserThread::IsWellKnownThread(ID identifier) { // static bool BrowserThread::CurrentlyOn(ID identifier) { - // We shouldn't use MessageLoop::current() since it uses LazyInstance which - // may be deleted by ~AtExitManager when a WorkerPool thread calls this - // function. - // http://crbug.com/63678 - base::ThreadRestrictions::ScopedAllowSingleton allow_singleton; AutoLock lock(lock_); DCHECK(identifier >= 0 && identifier < ID_COUNT); return browser_threads_[identifier] && @@ -166,11 +160,6 @@ bool BrowserThread::PostNonNestableDelayedTask( // static bool BrowserThread::GetCurrentThreadIdentifier(ID* identifier) { - // We shouldn't use MessageLoop::current() since it uses LazyInstance which - // may be deleted by ~AtExitManager when a WorkerPool thread calls this - // function. - // http://crbug.com/63678 - base::ThreadRestrictions::ScopedAllowSingleton allow_singleton; MessageLoop* cur_message_loop = MessageLoop::current(); for (int i = 0; i < ID_COUNT; ++i) { if (browser_threads_[i] && diff --git a/net/base/dns_reload_timer.cc b/net/base/dns_reload_timer.cc index 1bfe535..5931c5b 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/lazy_instance.h" +#include "base/singleton.h" #include "base/thread_local_storage.h" #include "base/time.h" -namespace { +namespace net { // 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 base::DefaultLazyInstanceTraits<DnsReloadTimer>; + friend struct DefaultSingletonTraits<DnsReloadTimer>; DnsReloadTimer() { // During testing the DnsReloadTimer Singleton may be created and destroyed @@ -81,16 +81,8 @@ 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 = g_dns_reload_timer.Pointer(); + DnsReloadTimer* dns_timer = Singleton<DnsReloadTimer>::get(); return dns_timer->Expired(); } diff --git a/net/base/ev_root_ca_metadata.cc b/net/base/ev_root_ca_metadata.cc index a721357..661b652 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,13 +283,9 @@ 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 g_ev_root_ca_metadata.Pointer(); + return Singleton<EVRootCAMetadata>::get(); } bool EVRootCAMetadata::GetPolicyOID( diff --git a/net/base/ev_root_ca_metadata.h b/net/base/ev_root_ca_metadata.h index 832ebe2..e0961f3 100644 --- a/net/base/ev_root_ca_metadata.h +++ b/net/base/ev_root_ca_metadata.h @@ -17,10 +17,8 @@ #include "net/base/x509_certificate.h" -namespace base { template <typename T> -struct DefaultLazyInstanceTraits; -} // namespace base +struct DefaultSingletonTraits; namespace net { @@ -57,7 +55,7 @@ class EVRootCAMetadata { PolicyOID policy_oid) const; private: - friend struct base::DefaultLazyInstanceTraits<EVRootCAMetadata>; + friend struct DefaultSingletonTraits<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 d3bf4f5..62c5191 100644 --- a/net/base/keygen_handler_unittest.cc +++ b/net/base/keygen_handler_unittest.cc @@ -16,7 +16,6 @@ #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" @@ -91,9 +90,6 @@ 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 310defb..d93d270 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,6 +39,17 @@ 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 @@ -46,8 +57,9 @@ const char kPKCS7Header[] = "PKCS7"; // 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 X509CertificateCache { +class X509Certificate::Cache { public: + static Cache* GetInstance(); void Insert(X509Certificate* cert); void Remove(X509Certificate* cert); X509Certificate* Find(const SHA1Fingerprint& fingerprint); @@ -56,10 +68,9 @@ class X509CertificateCache { typedef std::map<SHA1Fingerprint, X509Certificate*, SHA1FingerprintLessThan> CertMap; - // Obtain an instance of X509CertificateCache via a LazyInstance. - X509CertificateCache() {} - ~X509CertificateCache() {} - friend struct base::DefaultLazyInstanceTraits<X509CertificateCache>; + // Obtain an instance of X509Certificate::Cache via GetInstance(). + Cache() {} + friend struct DefaultSingletonTraits<Cache>; // You must acquire this lock before using any private data of this object. // You must not block while holding this lock. @@ -68,16 +79,18 @@ class X509CertificateCache { // The certificate cache. You must acquire |lock_| before using |cache_|. CertMap cache_; - DISALLOW_COPY_AND_ASSIGN(X509CertificateCache); + DISALLOW_COPY_AND_ASSIGN(Cache); }; -base::LazyInstance<X509CertificateCache, - base::LeakyLazyInstanceTraits<X509CertificateCache> > - g_x509_certificate_cache(base::LINKER_INITIALIZED); +// Get the singleton object for the cache. +// static +X509Certificate::Cache* X509Certificate::Cache::GetInstance() { + return Singleton<X509Certificate::Cache>::get(); +} // Insert |cert| into the cache. The cache does NOT AddRef |cert|. // Any existing certificate with the same fingerprint will be replaced. -void X509CertificateCache::Insert(X509Certificate* cert) { +void X509Certificate::Cache::Insert(X509Certificate* cert) { AutoLock lock(lock_); DCHECK(!IsNullFingerprint(cert->fingerprint())) << @@ -87,7 +100,7 @@ void X509CertificateCache::Insert(X509Certificate* cert) { // Remove |cert| from the cache. The cache does not assume that |cert| is // already in the cache. -void X509CertificateCache::Remove(X509Certificate* cert) { +void X509Certificate::Cache::Remove(X509Certificate* cert) { AutoLock lock(lock_); CertMap::iterator pos(cache_.find(cert->fingerprint())); @@ -98,7 +111,7 @@ void X509CertificateCache::Remove(X509Certificate* cert) { // Find a certificate in the cache with the given fingerprint. If one does // not exist, this method returns NULL. -X509Certificate* X509CertificateCache::Find( +X509Certificate* X509Certificate::Cache::Find( const SHA1Fingerprint& fingerprint) { AutoLock lock(lock_); @@ -109,17 +122,6 @@ X509Certificate* X509CertificateCache::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, @@ -129,7 +131,7 @@ X509Certificate* X509Certificate::CreateFromHandle( DCHECK(source != SOURCE_UNUSED); // Check if we already have this certificate in memory. - X509CertificateCache* cache = g_x509_certificate_cache.Pointer(); + X509Certificate::Cache* cache = X509Certificate::Cache::GetInstance(); X509Certificate* cached_cert = cache->Find(CalculateFingerprint(cert_handle)); if (cached_cert) { @@ -309,7 +311,7 @@ X509Certificate::X509Certificate(const std::string& subject, X509Certificate::~X509Certificate() { // We might not be in the cache, but it is safe to remove ourselves anyway. - g_x509_certificate_cache.Get().Remove(this); + X509Certificate::Cache::GetInstance()->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 9f44952..1866a17 100644 --- a/net/base/x509_certificate.h +++ b/net/base/x509_certificate.h @@ -284,6 +284,8 @@ 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 2a604ee..a2a0eea 100644 --- a/net/base/x509_certificate_mac.cc +++ b/net/base/x509_certificate_mac.cc @@ -8,7 +8,6 @@ #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" @@ -22,8 +21,6 @@ using base::Time; namespace net { -namespace { - class MacTrustedCertificates { public: // Sets the trusted root certificate used by tests. Call with |cert| set @@ -60,7 +57,7 @@ class MacTrustedCertificates { return merged_array; } private: - friend struct base::DefaultLazyInstanceTraits<MacTrustedCertificates>; + friend struct DefaultSingletonTraits<MacTrustedCertificates>; // Obtain an instance of MacTrustedCertificates via the singleton // interface. @@ -76,9 +73,11 @@ class MacTrustedCertificates { DISALLOW_COPY_AND_ASSIGN(MacTrustedCertificates); }; -base::LazyInstance<MacTrustedCertificates, - base::LeakyLazyInstanceTraits<MacTrustedCertificates> > - g_mac_trusted_certificates(base::LINKER_INITIALIZED); +void SetMacTestCertificate(X509Certificate* cert) { + Singleton<MacTrustedCertificates>::get()->SetTestCertificate(cert); +} + +namespace { typedef OSStatus (*SecTrustCopyExtendedResultFuncPtr)(SecTrustRef, CFDictionaryRef*); @@ -444,10 +443,6 @@ 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); @@ -550,7 +545,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 = - g_mac_trusted_certificates.Get().CopyTrustedCertificateArray(); + Singleton<MacTrustedCertificates>::get()->CopyTrustedCertificateArray(); ScopedCFTypeRef<CFArrayRef> scoped_anchor_array(anchor_array); if (anchor_array) { status = SecTrustSetAnchorCertificates(trust_ref, anchor_array); diff --git a/net/ocsp/nss_ocsp.cc b/net/ocsp/nss_ocsp.cc index 7618f9e..fafaa68 100644 --- a/net/ocsp/nss_ocsp.cc +++ b/net/ocsp/nss_ocsp.cc @@ -85,8 +85,7 @@ class OCSPIOLoop { DISALLOW_COPY_AND_ASSIGN(OCSPIOLoop); }; -base::LazyInstance<OCSPIOLoop, base::LeakyLazyInstanceTraits<OCSPIOLoop> > - g_ocsp_io_loop(base::LINKER_INITIALIZED); +base::LazyInstance<OCSPIOLoop> g_ocsp_io_loop(base::LINKER_INITIALIZED); const int kRecvBufferSize = 4096; @@ -561,6 +560,7 @@ OCSPNSSInitialization::~OCSPNSSInitialization() {} SECStatus OCSPCreateSession(const char* host, PRUint16 portnum, SEC_HTTP_SERVER_SESSION* pSession) { VLOG(1) << "OCSP create session: host=" << host << " port=" << portnum; + DCHECK(!MessageLoop::current()); pthread_mutex_lock(&g_request_context_lock); URLRequestContext* request_context = g_request_context; pthread_mutex_unlock(&g_request_context_lock); @@ -579,6 +579,7 @@ SECStatus OCSPCreateSession(const char* host, PRUint16 portnum, SECStatus OCSPKeepAliveSession(SEC_HTTP_SERVER_SESSION session, PRPollDesc **pPollDesc) { VLOG(1) << "OCSP keep alive"; + DCHECK(!MessageLoop::current()); if (pPollDesc) *pPollDesc = NULL; return SECSuccess; @@ -586,6 +587,7 @@ SECStatus OCSPKeepAliveSession(SEC_HTTP_SERVER_SESSION session, SECStatus OCSPFreeSession(SEC_HTTP_SERVER_SESSION session) { VLOG(1) << "OCSP free session"; + DCHECK(!MessageLoop::current()); delete reinterpret_cast<OCSPServerSession*>(session); return SECSuccess; } @@ -600,6 +602,7 @@ SECStatus OCSPCreate(SEC_HTTP_SERVER_SESSION session, << " path_and_query=" << path_and_query_string << " http_request_method=" << http_request_method << " timeout=" << timeout; + DCHECK(!MessageLoop::current()); OCSPServerSession* ocsp_session = reinterpret_cast<OCSPServerSession*>(session); @@ -621,6 +624,7 @@ SECStatus OCSPSetPostData(SEC_HTTP_REQUEST_SESSION request, const PRUint32 http_data_len, const char* http_content_type) { VLOG(1) << "OCSP set post data len=" << http_data_len; + DCHECK(!MessageLoop::current()); OCSPRequestSession* req = reinterpret_cast<OCSPRequestSession*>(request); req->SetPostData(http_data, http_data_len, http_content_type); @@ -632,6 +636,7 @@ SECStatus OCSPAddHeader(SEC_HTTP_REQUEST_SESSION request, const char* http_header_value) { VLOG(1) << "OCSP add header name=" << http_header_name << " value=" << http_header_value; + DCHECK(!MessageLoop::current()); OCSPRequestSession* req = reinterpret_cast<OCSPRequestSession*>(request); req->AddHeader(http_header_name, http_header_value); @@ -691,6 +696,7 @@ SECStatus OCSPTrySendAndReceive(SEC_HTTP_REQUEST_SESSION request, } VLOG(1) << "OCSP try send and receive"; + DCHECK(!MessageLoop::current()); OCSPRequestSession* req = reinterpret_cast<OCSPRequestSession*>(request); // We support blocking mode only. if (pPollDesc) @@ -768,6 +774,7 @@ SECStatus OCSPTrySendAndReceive(SEC_HTTP_REQUEST_SESSION request, SECStatus OCSPFree(SEC_HTTP_REQUEST_SESSION request) { VLOG(1) << "OCSP free"; + DCHECK(!MessageLoop::current()); OCSPRequestSession* req = reinterpret_cast<OCSPRequestSession*>(request); req->Cancel(); req->Release(); |