bug 1344478 - isAlreadyShutDown should return true for nsNSSShutDownObjects created after NSS shut down r=Cykesiopka,ttaubert

MozReview-Commit-ID: 5bUTLz6mGKC

In general, it is possible to create a new nsNSSShutDownObject after
nsNSSShutDownList::shutdown() had been called. Before this patch, at that point,
isAlreadyShutDown() would incorrectly return false, which could lead to code
calling NSS functions, which would probably lead to a crash (because NSS could
be uninitialized at that point). This change merges
nsNSSShutDownList::shutdown() with evaporateAllNSSResources() into
evaporateAllNSSResourcesAndShutDown() for simplicity and makes it so
isAlreadyShutDown() returns true if called after that point.

--HG--
extra : rebase_source : badab89a9e197f18fcd943f16cc77c6aa6664f0d

security/manager/ssl/nsNSSComponent.cpp

@@ -197,7 +197,6 @@ nsNSSComponent::~nsNSSComponent()
   SharedSSLState::GlobalCleanup();
   RememberCertErrorsTable::Cleanup();
   --mInstanceCount;
-  nsNSSShutDownList::shutdown();
 
   MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("nsNSSComponent::dtor finished\n"));
 }
@@ -1809,7 +1808,7 @@ nsNSSComponent::ShutdownNSS()
     Unused << SSL_ShutdownServerSessionIDCache();
 
     MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("evaporating psm resources"));
-    if (NS_FAILED(nsNSSShutDownList::evaporateAllNSSResources())) {
+    if (NS_FAILED(nsNSSShutDownList::evaporateAllNSSResourcesAndShutDown())) {
       MOZ_LOG(gPIPNSSLog, LogLevel::Error, ("failed to evaporate resources"));
       return;
     }

security/manager/ssl/nsNSSShutDown.cpp

@@ -50,6 +50,8 @@ nsNSSShutDownList::nsNSSShutDownList()
 nsNSSShutDownList::~nsNSSShutDownList()
 {
   MOZ_ASSERT(this == singleton);
+  MOZ_ASSERT(sInShutdown,
+             "evaporateAllNSSResourcesAndShutDown() should have been called");
   singleton = nullptr;
 }
 
@@ -126,7 +128,7 @@ nsresult nsNSSShutDownList::doPK11Logout()
   return NS_OK;
 }
 
-nsresult nsNSSShutDownList::evaporateAllNSSResources()
+nsresult nsNSSShutDownList::evaporateAllNSSResourcesAndShutDown()
 {
   MOZ_RELEASE_ASSERT(NS_IsMainThread());
   if (!NS_IsMainThread()) {
@@ -150,6 +152,8 @@ nsresult nsNSSShutDownList::evaporateAllNSSResources()
     return NS_OK;
   }
 
+  sInShutdown = true;
+
   {
     StaticMutexAutoUnlock unlock(sListLock);
     PRStatus rv = singleton->mActivityState.restrictActivityToCurrentThread();
@@ -183,6 +187,8 @@ nsresult nsNSSShutDownList::evaporateAllNSSResources()
   }
 
   singleton->mActivityState.releaseCurrentThreadActivityRestriction();
+  delete singleton;
+
   return NS_OK;
 }
 
@@ -211,17 +217,6 @@ bool nsNSSShutDownList::construct(const StaticMutexAutoLock& /*proofOfLock*/)
   return !!singleton;
 }
 
-void nsNSSShutDownList::shutdown()
-{
-  MOZ_RELEASE_ASSERT(NS_IsMainThread());
-  StaticMutexAutoLock lock(sListLock);
-  sInShutdown = true;
-
-  if (singleton) {
-    delete singleton;
-  }
-}
-
 nsNSSActivityState::nsNSSActivityState()
 :mNSSActivityStateLock("nsNSSActivityState.mNSSActivityStateLock"), 
  mNSSActivityChanged(mNSSActivityStateLock,
@@ -286,3 +281,9 @@ nsNSSShutDownPreventionLock::~nsNSSShutDownPreventionLock()
 {
   nsNSSShutDownList::leaveActivityState();
 }
+
+bool
+nsNSSShutDownObject::isAlreadyShutDown() const
+{
+  return mAlreadyShutDown || sInShutdown;
+}

security/manager/ssl/nsNSSShutDown.h

@@ -64,8 +64,6 @@ public:
 class nsNSSShutDownList
 {
 public:
-  static void shutdown();
-
   // track instances that support early cleanup
   static void remember(nsNSSShutDownObject *o);
   static void forget(nsNSSShutDownObject *o);
@@ -75,8 +73,9 @@ public:
   static void remember(nsOnPK11LogoutCancelObject *o);
   static void forget(nsOnPK11LogoutCancelObject *o);
 
-  // Do the "early cleanup", if possible.
-  static nsresult evaporateAllNSSResources();
+  // Release all tracked NSS resources and prevent nsNSSShutDownObjects from
+  // using NSS functions.
+  static nsresult evaporateAllNSSResourcesAndShutDown();
 
   // PSM has been asked to log out of a token.
   // Notify all registered instances that want to react to that event.
@@ -228,7 +227,7 @@ public:
     }
   }
 
-  bool isAlreadyShutDown() const { return mAlreadyShutDown; }
+  bool isAlreadyShutDown() const;
 
 protected:
   virtual void virtualDestroyNSSReference() = 0;