Bug 1235634 - Construct nsNSSShutdownList::singleton lazily on first use r=keeler
☠☠ backed out by 7c465dd93903 ☠ ☠
authorTim Taubert <ttaubert@mozilla.com>
Tue, 22 Mar 2016 15:13:05 +0100
changeset 289785 917819510b3f7390a26805156fbcb9c5c04e1da6
parent 289784 c0f5c3d87ac83880eec8cd9d4ac2139a57fab54c
child 289786 38aa13f30b92d21f60ae149f1bde82a9a0599b60
push id73990
push userttaubert@mozilla.com
push dateTue, 22 Mar 2016 14:13:55 +0000
treeherdermozilla-inbound@917819510b3f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskeeler
bugs1235634
milestone48.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1235634 - Construct nsNSSShutdownList::singleton lazily on first use r=keeler
security/manager/ssl/nsNSSComponent.cpp
security/manager/ssl/nsNSSComponent.h
security/manager/ssl/nsNSSShutDown.cpp
security/manager/ssl/nsNSSShutDown.h
--- a/security/manager/ssl/nsNSSComponent.cpp
+++ b/security/manager/ssl/nsNSSComponent.cpp
@@ -220,17 +220,16 @@ nsNSSComponent::nsNSSComponent()
    mThreadList(nullptr),
 #endif
    mCertVerificationThread(nullptr)
 {
   MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("nsNSSComponent::ctor\n"));
 
   NS_ASSERTION( (0 == mInstanceCount), "nsNSSComponent is a singleton, but instantiated multiple times!");
   ++mInstanceCount;
-  mShutdownObjectList = nsNSSShutDownList::construct();
 }
 
 void
 nsNSSComponent::deleteBackgroundThreads()
 {
   if (mCertVerificationThread)
   {
     mCertVerificationThread->requestExit();
@@ -264,17 +263,17 @@ nsNSSComponent::~nsNSSComponent()
   deleteBackgroundThreads();
 
   // All cleanup code requiring services needs to happen in xpcom_shutdown
 
   ShutdownNSS();
   SharedSSLState::GlobalCleanup();
   RememberCertErrorsTable::Cleanup();
   --mInstanceCount;
-  delete mShutdownObjectList;
+  nsNSSShutDownList::shutdown();
 
   // We are being freed, drop the haveLoaded flag to re-enable
   // potential nss initialization later.
   EnsureNSSInitialized(nssShutdown);
 
   MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("nsNSSComponent::dtor finished\n"));
 }
 
@@ -1154,17 +1153,17 @@ nsNSSComponent::ShutdownNSS()
     ShutdownSmartCardThreads();
 #endif
     SSL_ClearSessionCache();
     UnloadLoadableRoots();
 #ifndef MOZ_NO_EV_CERTS
     CleanupIdentityInfo();
 #endif
     MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("evaporating psm resources\n"));
-    mShutdownObjectList->evaporateAllNSSResources();
+    nsNSSShutDownList::evaporateAllNSSResources();
     EnsureNSSInitialized(nssShutdown);
     if (SECSuccess != ::NSS_Shutdown()) {
       MOZ_LOG(gPIPNSSLog, LogLevel::Error, ("NSS SHUTDOWN FAILURE\n"));
     }
     else {
       MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("NSS shutdown =====>> OK <<=====\n"));
     }
   }
@@ -1175,22 +1174,16 @@ nsNSSComponent::Init()
 {
   // No mutex protection.
   // Assume Init happens before any concurrency on "this" can start.
 
   nsresult rv = NS_OK;
 
   MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("Beginning NSS initialization\n"));
 
-  if (!mShutdownObjectList)
-  {
-    MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("NSS init, out of memory in constructor\n"));
-    return NS_ERROR_OUT_OF_MEMORY;
-  }
-
   rv = InitializePIPNSSBundle();
   if (NS_FAILED(rv)) {
     MOZ_LOG(gPIPNSSLog, LogLevel::Error, ("Unable to create pipnss bundle.\n"));
     return rv;
   }
 
   // Access our string bundles now, this prevents assertions from I/O
   // - nsStandardURL not thread-safe
@@ -1393,17 +1386,17 @@ nsresult nsNSSComponent::LogoutAuthentic
   if (icos) {
     icos->ClearValidityOverride(
             NS_LITERAL_CSTRING("all:temporary-certificates"),
             0);
   }
 
   nsClientAuthRememberService::ClearAllRememberedDecisions();
 
-  return mShutdownObjectList->doPK11Logout();
+  return nsNSSShutDownList::doPK11Logout();
 }
 
 nsresult
 nsNSSComponent::RegisterObservers()
 {
   // Happens once during init only, no mutex protection.
 
   nsCOMPtr<nsIObserverService> observerService(
--- a/security/manager/ssl/nsNSSComponent.h
+++ b/security/manager/ssl/nsNSSComponent.h
@@ -171,17 +171,16 @@ private:
   void DoProfileBeforeChange();
 
   mozilla::Mutex mutex;
 
   nsCOMPtr<nsIStringBundle> mPIPNSSBundle;
   nsCOMPtr<nsIStringBundle> mNSSErrorsBundle;
   bool mNSSInitialized;
   static int mInstanceCount;
-  nsNSSShutDownList* mShutdownObjectList;
 #ifndef MOZ_NO_SMART_CARDS
   SmartCardThreadList* mThreadList;
 #endif
 
 #ifdef DEBUG
   nsAutoString mTestBuiltInRootHash;
 #endif
 
--- a/security/manager/ssl/nsNSSShutDown.cpp
+++ b/security/manager/ssl/nsNSSShutDown.cpp
@@ -30,131 +30,166 @@ ObjectSetInitEntry(PLDHashEntryHdr *hdr,
 static const PLDHashTableOps gSetOps = {
   PLDHashTable::HashVoidPtrKeyStub,
   ObjectSetMatchEntry,
   PLDHashTable::MoveEntryStub,
   PLDHashTable::ClearEntryStub,
   ObjectSetInitEntry
 };
 
+StaticMutex nsNSSShutDownList::sListLock;
 nsNSSShutDownList *nsNSSShutDownList::singleton = nullptr;
 
 nsNSSShutDownList::nsNSSShutDownList()
-  : mListLock("nsNSSShutDownList.mListLock")
-  , mObjects(&gSetOps, sizeof(ObjectHashEntry))
+  : mObjects(&gSetOps, sizeof(ObjectHashEntry))
   , mPK11LogoutCancelObjects(&gSetOps, sizeof(ObjectHashEntry))
 {
 }
 
 nsNSSShutDownList::~nsNSSShutDownList()
 {
   PR_ASSERT(this == singleton);
   singleton = nullptr;
 }
 
 void nsNSSShutDownList::remember(nsNSSShutDownObject *o)
 {
-  if (!singleton)
-    return;
+  StaticMutexAutoLock lock(sListLock);
+  nsNSSShutDownList::construct(lock);
   
   PR_ASSERT(o);
-  MutexAutoLock lock(singleton->mListLock);
   singleton->mObjects.Add(o, fallible);
 }
 
 void nsNSSShutDownList::forget(nsNSSShutDownObject *o)
 {
+  StaticMutexAutoLock lock(sListLock);
   if (!singleton)
     return;
   
   PR_ASSERT(o);
-  MutexAutoLock lock(singleton->mListLock);
   singleton->mObjects.Remove(o);
 }
 
 void nsNSSShutDownList::remember(nsOnPK11LogoutCancelObject *o)
 {
-  if (!singleton)
-    return;
+  StaticMutexAutoLock lock(sListLock);
+  nsNSSShutDownList::construct(lock);
   
   PR_ASSERT(o);
-  MutexAutoLock lock(singleton->mListLock);
   singleton->mPK11LogoutCancelObjects.Add(o, fallible);
 }
 
 void nsNSSShutDownList::forget(nsOnPK11LogoutCancelObject *o)
 {
+  StaticMutexAutoLock lock(sListLock);
   if (!singleton)
     return;
   
   PR_ASSERT(o);
-  MutexAutoLock lock(singleton->mListLock);
   singleton->mPK11LogoutCancelObjects.Remove(o);
 }
 
 nsresult nsNSSShutDownList::doPK11Logout()
 {
-    MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("canceling all open SSL sockets to disallow future IO\n"));
+  StaticMutexAutoLock lock(sListLock);
+  if (!singleton) {
+    return NS_OK;
+  }
+
+  MOZ_LOG(gPIPNSSLog, LogLevel::Debug,
+          ("canceling all open SSL sockets to disallow future IO\n"));
+
   // During our iteration we will set a bunch of PRBools to true.
   // Nobody else ever modifies that bool, only we do.
   // We only must ensure that our objects do not go away.
   // This is guaranteed by holding the list lock.
 
-  MutexAutoLock lock(singleton->mListLock);
-  for (auto iter = mPK11LogoutCancelObjects.Iter(); !iter.Done(); iter.Next()) {
+  for (auto iter = singleton->mPK11LogoutCancelObjects.Iter();
+       !iter.Done();
+       iter.Next()) {
     auto entry = static_cast<ObjectHashEntry*>(iter.Get());
     nsOnPK11LogoutCancelObject *pklco =
       reinterpret_cast<nsOnPK11LogoutCancelObject*>(entry->obj);
     if (pklco) {
       pklco->logout();
     }
   }
 
   return NS_OK;
 }
 
 nsresult nsNSSShutDownList::evaporateAllNSSResources()
 {
-  if (PR_SUCCESS != mActivityState.restrictActivityToCurrentThread()) {
+  StaticMutexAutoLock lock(sListLock);
+  if (!singleton) {
+    return NS_OK;
+  }
+
+  PRStatus rv = singleton->mActivityState.restrictActivityToCurrentThread();
+  if (rv != PR_SUCCESS) {
     MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("failed to restrict activity to current thread\n"));
     return NS_ERROR_FAILURE;
   }
 
   MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("now evaporating NSS resources\n"));
 
   // Never free more than one entry, because other threads might be calling
   // us and remove themselves while we are iterating over the list,
   // and the behaviour of changing the list while iterating is undefined.
-  while (true) {
-    MutexAutoLock lock(mListLock);
-    auto iter = mObjects.Iter();
+  while (singleton) {
+    auto iter = singleton->mObjects.Iter();
     if (iter.Done()) {
       break;
     }
     auto entry = static_cast<ObjectHashEntry*>(iter.Get());
     {
-      MutexAutoUnlock unlock(singleton->mListLock);
+      StaticMutexAutoUnlock unlock(sListLock);
       entry->obj->shutdown(nsNSSShutDownObject::calledFromList);
     }
     iter.Remove();
   }
 
-  mActivityState.releaseCurrentThreadActivityRestriction();
+  if (!singleton) {
+    return NS_ERROR_FAILURE;
+  }
+
+  singleton->mActivityState.releaseCurrentThreadActivityRestriction();
   return NS_OK;
 }
 
-nsNSSShutDownList *nsNSSShutDownList::construct()
+void nsNSSShutDownList::enterActivityState()
 {
+  StaticMutexAutoLock lock(sListLock);
+  nsNSSShutDownList::construct(lock);
+  singleton->mActivityState.enter();
+}
+
+void nsNSSShutDownList::leaveActivityState()
+{
+  StaticMutexAutoLock lock(sListLock);
   if (singleton) {
-    // we should never ever be called twice
-    return nullptr;
+    singleton->mActivityState.leave();
+  }
+}
+
+void nsNSSShutDownList::construct(const StaticMutexAutoLock& /*proofOfLock*/)
+{
+  if (!singleton) {
+    singleton = new nsNSSShutDownList();
   }
+}
 
-  singleton = new nsNSSShutDownList();
-  return singleton;
+void nsNSSShutDownList::shutdown()
+{
+  StaticMutexAutoLock lock(sListLock);
+
+  if (singleton) {
+    delete singleton;
+  }
 }
 
 nsNSSActivityState::nsNSSActivityState()
 :mNSSActivityStateLock("nsNSSActivityState.mNSSActivityStateLock"), 
  mNSSActivityChanged(mNSSActivityStateLock,
                      "nsNSSActivityState.mNSSActivityStateLock"),
  mNSSActivityCounter(0),
  mNSSRestrictedThread(nullptr)
@@ -204,23 +239,15 @@ void nsNSSActivityState::releaseCurrentT
 
   mNSSRestrictedThread = nullptr;
 
   mNSSActivityChanged.NotifyAll();
 }
 
 nsNSSShutDownPreventionLock::nsNSSShutDownPreventionLock()
 {
-  nsNSSActivityState *state = nsNSSShutDownList::getActivityState();
-  if (!state)
-    return;
-
-  state->enter();
+  nsNSSShutDownList::enterActivityState();
 }
 
 nsNSSShutDownPreventionLock::~nsNSSShutDownPreventionLock()
 {
-  nsNSSActivityState *state = nsNSSShutDownList::getActivityState();
-  if (!state)
-    return;
-  
-  state->leave();
+  nsNSSShutDownList::leaveActivityState();
 }
--- a/security/manager/ssl/nsNSSShutDown.h
+++ b/security/manager/ssl/nsNSSShutDown.h
@@ -4,22 +4,22 @@
 
 #ifndef _INC_NSSShutDown_H
 #define _INC_NSSShutDown_H
 
 #include "nscore.h"
 #include "nspr.h"
 #include "PLDHashTable.h"
 #include "mozilla/CondVar.h"
-#include "mozilla/Mutex.h"
+#include "mozilla/StaticMutex.h"
 
 class nsNSSShutDownObject;
 class nsOnPK11LogoutCancelObject;
 
-// Singleton, owner by nsNSSShutDownList
+// Singleton, owned by nsNSSShutDownList
 class nsNSSActivityState
 {
 public:
   nsNSSActivityState();
   ~nsNSSActivityState();
 
   // Call enter/leave when PSM enters a scope during which
   // shutting down NSS is prohibited.
@@ -57,46 +57,46 @@ public:
   ~nsNSSShutDownPreventionLock();
 };
 
 // Singleton, used by nsNSSComponent to track the list of PSM objects,
 // which hold NSS resources and support the "early cleanup mechanism".
 class nsNSSShutDownList
 {
 public:
-  ~nsNSSShutDownList();
-
-  static nsNSSShutDownList *construct();
+  static void shutdown();
   
   // track instances that support early cleanup
   static void remember(nsNSSShutDownObject *o);
   static void forget(nsNSSShutDownObject *o);
 
   // track instances that would like notification when
   // a PK11 logout operation is performed.
   static void remember(nsOnPK11LogoutCancelObject *o);
   static void forget(nsOnPK11LogoutCancelObject *o);
 
   // Do the "early cleanup", if possible.
-  nsresult evaporateAllNSSResources();
+  static nsresult evaporateAllNSSResources();
 
   // PSM has been asked to log out of a token.
   // Notify all registered instances that want to react to that event.
-  nsresult doPK11Logout();
-  
-  static nsNSSActivityState *getActivityState()
-  {
-    return singleton ? &singleton->mActivityState : nullptr;
-  }
+  static nsresult doPK11Logout();
+
+  // Signal entering/leaving a scope where shutting down NSS is prohibited.
+  static void enterActivityState();
+  static void leaveActivityState();
   
 private:
   nsNSSShutDownList();
+  ~nsNSSShutDownList();
+
+  static void construct(const mozilla::StaticMutexAutoLock& /*proofOfLock*/);
 
 protected:
-  mozilla::Mutex mListLock;
+  static mozilla::StaticMutex sListLock;
   static nsNSSShutDownList *singleton;
   PLDHashTable mObjects;
   PLDHashTable mPK11LogoutCancelObjects;
   nsNSSActivityState mActivityState;
 };
 
 /*
   A class deriving from nsNSSShutDownObject will have its instances