Backed out changeset 917819510b3f (bug 1235634) for memory leaks on a CLOSED TREE

security/manager/ssl/nsNSSComponent.cpp

@@ -225,6 +225,7 @@ nsNSSComponent::nsNSSComponent()
 
   NS_ASSERTION( (0 == mInstanceCount), "nsNSSComponent is a singleton, but instantiated multiple times!");
   ++mInstanceCount;
+  mShutdownObjectList = nsNSSShutDownList::construct();
 }
 
 void
@@ -268,7 +269,7 @@ nsNSSComponent::~nsNSSComponent()
   SharedSSLState::GlobalCleanup();
   RememberCertErrorsTable::Cleanup();
   --mInstanceCount;
-  nsNSSShutDownList::shutdown();
+  delete mShutdownObjectList;
 
   // We are being freed, drop the haveLoaded flag to re-enable
   // potential nss initialization later.
@@ -1158,7 +1159,7 @@ nsNSSComponent::ShutdownNSS()
     CleanupIdentityInfo();
 #endif
     MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("evaporating psm resources\n"));
-    nsNSSShutDownList::evaporateAllNSSResources();
+    mShutdownObjectList->evaporateAllNSSResources();
     EnsureNSSInitialized(nssShutdown);
     if (SECSuccess != ::NSS_Shutdown()) {
       MOZ_LOG(gPIPNSSLog, LogLevel::Error, ("NSS SHUTDOWN FAILURE\n"));
@@ -1179,6 +1180,12 @@ nsNSSComponent::Init()
 
   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"));
@@ -1391,7 +1398,7 @@ nsresult nsNSSComponent::LogoutAuthenticatedPK11()
 
   nsClientAuthRememberService::ClearAllRememberedDecisions();
 
-  return nsNSSShutDownList::doPK11Logout();
+  return mShutdownObjectList->doPK11Logout();
 }
 
 nsresult

security/manager/ssl/nsNSSComponent.h

@@ -176,6 +176,7 @@ private:
   nsCOMPtr<nsIStringBundle> mNSSErrorsBundle;
   bool mNSSInitialized;
   static int mInstanceCount;
+  nsNSSShutDownList* mShutdownObjectList;
 #ifndef MOZ_NO_SMART_CARDS
   SmartCardThreadList* mThreadList;
 #endif

security/manager/ssl/nsNSSShutDown.cpp

@@ -35,11 +35,11 @@ static const PLDHashTableOps gSetOps = {
   ObjectSetInitEntry
 };
 
-StaticMutex nsNSSShutDownList::sListLock;
 nsNSSShutDownList *nsNSSShutDownList::singleton = nullptr;
 
 nsNSSShutDownList::nsNSSShutDownList()
-  : mObjects(&gSetOps, sizeof(ObjectHashEntry))
+  : mListLock("nsNSSShutDownList.mListLock")
+  , mObjects(&gSetOps, sizeof(ObjectHashEntry))
   , mPK11LogoutCancelObjects(&gSetOps, sizeof(ObjectHashEntry))
 {
 }
@@ -52,60 +52,54 @@ nsNSSShutDownList::~nsNSSShutDownList()
 
 void nsNSSShutDownList::remember(nsNSSShutDownObject *o)
 {
-  StaticMutexAutoLock lock(sListLock);
-  nsNSSShutDownList::construct(lock);
+  if (!singleton)
+    return;
   
   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)
 {
-  StaticMutexAutoLock lock(sListLock);
-  nsNSSShutDownList::construct(lock);
+  if (!singleton)
+    return;
   
   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()
 {
-  StaticMutexAutoLock lock(sListLock);
-  if (!singleton) {
-    return NS_OK;
-  }
-
-  MOZ_LOG(gPIPNSSLog, LogLevel::Debug,
-          ("canceling all open SSL sockets to disallow future IO\n"));
-
+    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.
 
-  for (auto iter = singleton->mPK11LogoutCancelObjects.Iter();
-       !iter.Done();
-       iter.Next()) {
+  MutexAutoLock lock(singleton->mListLock);
+  for (auto iter = mPK11LogoutCancelObjects.Iter(); !iter.Done(); iter.Next()) {
     auto entry = static_cast<ObjectHashEntry*>(iter.Get());
     nsOnPK11LogoutCancelObject *pklco =
       reinterpret_cast<nsOnPK11LogoutCancelObject*>(entry->obj);
@@ -119,13 +113,7 @@ nsresult nsNSSShutDownList::doPK11Logout()
 
 nsresult nsNSSShutDownList::evaporateAllNSSResources()
 {
-  StaticMutexAutoLock lock(sListLock);
-  if (!singleton) {
-    return NS_OK;
-  }
-
-  PRStatus rv = singleton->mActivityState.restrictActivityToCurrentThread();
-  if (rv != PR_SUCCESS) {
+  if (PR_SUCCESS != mActivityState.restrictActivityToCurrentThread()) {
     MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("failed to restrict activity to current thread\n"));
     return NS_ERROR_FAILURE;
   }
@@ -135,56 +123,33 @@ nsresult nsNSSShutDownList::evaporateAllNSSResources()
   // 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 (singleton) {
-    auto iter = singleton->mObjects.Iter();
+  while (true) {
+    MutexAutoLock lock(mListLock);
+    auto iter = mObjects.Iter();
     if (iter.Done()) {
       break;
     }
     auto entry = static_cast<ObjectHashEntry*>(iter.Get());
     {
-      StaticMutexAutoUnlock unlock(sListLock);
+      MutexAutoUnlock unlock(singleton->mListLock);
       entry->obj->shutdown(nsNSSShutDownObject::calledFromList);
     }
     iter.Remove();
   }
 
-  if (!singleton) {
-    return NS_ERROR_FAILURE;
-  }
-
-  singleton->mActivityState.releaseCurrentThreadActivityRestriction();
+  mActivityState.releaseCurrentThreadActivityRestriction();
   return NS_OK;
 }
 
-void nsNSSShutDownList::enterActivityState()
+nsNSSShutDownList *nsNSSShutDownList::construct()
 {
-  StaticMutexAutoLock lock(sListLock);
-  nsNSSShutDownList::construct(lock);
-  singleton->mActivityState.enter();
-}
-
-void nsNSSShutDownList::leaveActivityState()
-{
-  StaticMutexAutoLock lock(sListLock);
   if (singleton) {
-    singleton->mActivityState.leave();
+    // we should never ever be called twice
+    return nullptr;
   }
-}
 
-void nsNSSShutDownList::construct(const StaticMutexAutoLock& /*proofOfLock*/)
-{
-  if (!singleton) {
-    singleton = new nsNSSShutDownList();
-  }
-}
-
-void nsNSSShutDownList::shutdown()
-{
-  StaticMutexAutoLock lock(sListLock);
-
-  if (singleton) {
-    delete singleton;
-  }
+  singleton = new nsNSSShutDownList();
+  return singleton;
 }
 
 nsNSSActivityState::nsNSSActivityState()
@@ -244,10 +209,18 @@ void nsNSSActivityState::releaseCurrentThreadActivityRestriction()
 
 nsNSSShutDownPreventionLock::nsNSSShutDownPreventionLock()
 {
-  nsNSSShutDownList::enterActivityState();
+  nsNSSActivityState *state = nsNSSShutDownList::getActivityState();
+  if (!state)
+    return;
+
+  state->enter();
 }
 
 nsNSSShutDownPreventionLock::~nsNSSShutDownPreventionLock()
 {
-  nsNSSShutDownList::leaveActivityState();
+  nsNSSActivityState *state = nsNSSShutDownList::getActivityState();
+  if (!state)
+    return;
+  
+  state->leave();
 }

security/manager/ssl/nsNSSShutDown.h

@@ -9,12 +9,12 @@
 #include "nspr.h"
 #include "PLDHashTable.h"
 #include "mozilla/CondVar.h"
-#include "mozilla/StaticMutex.h"
+#include "mozilla/Mutex.h"
 
 class nsNSSShutDownObject;
 class nsOnPK11LogoutCancelObject;
 
-// Singleton, owned by nsNSSShutDownList
+// Singleton, owner by nsNSSShutDownList
 class nsNSSActivityState
 {
 public:
@@ -62,7 +62,9 @@ public:
 class nsNSSShutDownList
 {
 public:
-  static void shutdown();
+  ~nsNSSShutDownList();
+
+  static nsNSSShutDownList *construct();
   
   // track instances that support early cleanup
   static void remember(nsNSSShutDownObject *o);
@@ -74,24 +76,22 @@ public:
   static void forget(nsOnPK11LogoutCancelObject *o);
 
   // Do the "early cleanup", if possible.
-  static nsresult evaporateAllNSSResources();
+  nsresult evaporateAllNSSResources();
 
   // PSM has been asked to log out of a token.
   // Notify all registered instances that want to react to that event.
-  static nsresult doPK11Logout();
-
-  // Signal entering/leaving a scope where shutting down NSS is prohibited.
-  static void enterActivityState();
-  static void leaveActivityState();
+  nsresult doPK11Logout();
+  
+  static nsNSSActivityState *getActivityState()
+  {
+    return singleton ? &singleton->mActivityState : nullptr;
+  }
   
 private:
   nsNSSShutDownList();
-  ~nsNSSShutDownList();
-
-  static void construct(const mozilla::StaticMutexAutoLock& /*proofOfLock*/);
 
 protected:
-  static mozilla::StaticMutex sListLock;
+  mozilla::Mutex mListLock;
   static nsNSSShutDownList *singleton;
   PLDHashTable mObjects;
   PLDHashTable mPK11LogoutCancelObjects;