From e87b8bbba3be2889489f5b4a94b708e67d0b8c67 Mon Sep 17 00:00:00 2001 From: "willchan@chromium.org" Date: Fri, 19 Nov 2010 01:53:38 +0000 Subject: Reland r65996. Disallows Singletons on non-joinable thread. Test breakages caused by this change have been fixed here or in other changelists. BUG=61753 TEST=none Review URL: http://codereview.chromium.org/5024003 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@66719 0039d316-1c4b-4281-b951-d872f2087c98 --- net/base/dns_reload_timer.cc | 16 ++++++++--- net/base/ev_root_ca_metadata.cc | 8 ++++-- net/base/ev_root_ca_metadata.h | 6 +++-- net/base/keygen_handler_unittest.cc | 4 +++ net/base/x509_certificate.cc | 54 ++++++++++++++++++------------------- net/base/x509_certificate.h | 2 -- net/base/x509_certificate_mac.cc | 19 ++++++++----- 7 files changed, 64 insertions(+), 45 deletions(-) (limited to 'net/base') 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; + friend struct base::DefaultLazyInstanceTraits; 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 > + g_dns_reload_timer(base::LINKER_INITIALIZED); + +} // namespace + +namespace net { + bool DnsReloadTimerHasExpired() { - DnsReloadTimer* dns_timer = Singleton::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 #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 > + g_ev_root_ca_metadata(base::LINKER_INITIALIZED); + // static EVRootCAMetadata* EVRootCAMetadata::GetInstance() { - return Singleton::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 -struct DefaultSingletonTraits; +struct DefaultLazyInstanceTraits; +} // namespace base namespace net { @@ -55,7 +57,7 @@ class EVRootCAMetadata { PolicyOID policy_oid) const; private: - friend struct DefaultSingletonTraits; + friend struct base::DefaultLazyInstanceTraits; typedef std::map 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..310defb 100644 --- a/net/base/x509_certificate.cc +++ b/net/base/x509_certificate.cc @@ -6,9 +6,9 @@ #include +#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); @@ -68,9 +56,10 @@ class X509Certificate::Cache { typedef std::map CertMap; - // Obtain an instance of X509Certificate::Cache via GetInstance(). - Cache() {} - friend struct DefaultSingletonTraits; + // Obtain an instance of X509CertificateCache via a LazyInstance. + X509CertificateCache() {} + ~X509CertificateCache() {} + friend struct base::DefaultLazyInstanceTraits; // 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::get(); -} +base::LazyInstance > + 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 1866a17..9f44952 100644 --- a/net/base/x509_certificate.h +++ b/net/base/x509_certificate.h @@ -284,8 +284,6 @@ class X509Certificate : public base::RefCountedThreadSafe { 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 #include +#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; + friend struct base::DefaultLazyInstanceTraits; // 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::get()->SetTestCertificate(cert); -} - -namespace { +base::LazyInstance > + 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::get()->CopyTrustedCertificateArray(); + g_mac_trusted_certificates.Get().CopyTrustedCertificateArray(); ScopedCFTypeRef scoped_anchor_array(anchor_array); if (anchor_array) { status = SecTrustSetAnchorCertificates(trust_ref, anchor_array); -- cgit v1.1