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.
--- a/security/manager/ssl/nsNSSComponent.cpp
+++ b/security/manager/ssl/nsNSSComponent.cpp
@@ -192,17 +192,16 @@ nsNSSComponent::~nsNSSComponent()
MOZ_RELEASE_ASSERT(NS_IsMainThread());
// All cleanup code requiring services needs to happen in xpcom_shutdown
ShutdownNSS();
SharedSSLState::GlobalCleanup();
RememberCertErrorsTable::Cleanup();
--mInstanceCount;
- nsNSSShutDownList::shutdown();
MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("nsNSSComponent::dtor finished\n"));
}
NS_IMETHODIMP
nsNSSComponent::PIPBundleFormatStringFromName(const char* name,
const char16_t** params,
uint32_t numParams,
@@ -1804,17 +1803,17 @@ nsNSSComponent::ShutdownNSS()
ShutdownSmartCardThreads();
#endif
SSL_ClearSessionCache();
// TLSServerSocket may be run with the session cache enabled. This ensures
// those resources are cleaned up.
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;
}
UnloadLoadableRoots();
if (SECSuccess != ::NSS_Shutdown()) {
MOZ_LOG(gPIPNSSLog, LogLevel::Error, ("NSS SHUTDOWN FAILURE"));
} else {
MOZ_LOG(gPIPNSSLog, LogLevel::Debug, ("NSS shutdown =====>> OK <<====="));
--- a/security/manager/ssl/nsNSSShutDown.cpp
+++ b/security/manager/ssl/nsNSSShutDown.cpp
@@ -45,16 +45,18 @@ nsNSSShutDownList::nsNSSShutDownList()
: mObjects(&gSetOps, sizeof(ObjectHashEntry))
, mPK11LogoutCancelObjects(&gSetOps, sizeof(ObjectHashEntry))
{
}
nsNSSShutDownList::~nsNSSShutDownList()
{
MOZ_ASSERT(this == singleton);
+ MOZ_ASSERT(sInShutdown,
+ "evaporateAllNSSResourcesAndShutDown() should have been called");
singleton = nullptr;
}
void nsNSSShutDownList::remember(nsNSSShutDownObject *o)
{
StaticMutexAutoLock lock(sListLock);
if (!nsNSSShutDownList::construct(lock)) {
return;
@@ -121,17 +123,17 @@ nsresult nsNSSShutDownList::doPK11Logout
if (pklco) {
pklco->logout();
}
}
return NS_OK;
}
-nsresult nsNSSShutDownList::evaporateAllNSSResources()
+nsresult nsNSSShutDownList::evaporateAllNSSResourcesAndShutDown()
{
MOZ_RELEASE_ASSERT(NS_IsMainThread());
if (!NS_IsMainThread()) {
return NS_ERROR_NOT_SAME_THREAD;
}
StaticMutexAutoLock lock(sListLock);
// Other threads can acquire an nsNSSShutDownPreventionLock and cause this
@@ -145,16 +147,18 @@ nsresult nsNSSShutDownList::evaporateAll
// and destroyed on the main thread, and if we similarly enforce that this
// function is only called on the main thread, what we can do is check that
// the singleton hasn't already gone away and then we don't actually have to
// hold sListLock while calling restrictActivityToCurrentThread.
if (!singleton) {
return NS_OK;
}
+ sInShutdown = true;
+
{
StaticMutexAutoUnlock unlock(sListLock);
PRStatus rv = singleton->mActivityState.restrictActivityToCurrentThread();
if (rv != PR_SUCCESS) {
MOZ_LOG(gPIPNSSLog, LogLevel::Debug,
("failed to restrict activity to current thread"));
return NS_ERROR_FAILURE;
}
@@ -178,16 +182,18 @@ nsresult nsNSSShutDownList::evaporateAll
iter.Remove();
}
if (!singleton) {
return NS_ERROR_FAILURE;
}
singleton->mActivityState.releaseCurrentThreadActivityRestriction();
+ delete singleton;
+
return NS_OK;
}
void nsNSSShutDownList::enterActivityState()
{
StaticMutexAutoLock lock(sListLock);
if (nsNSSShutDownList::construct(lock)) {
singleton->mActivityState.enter();
@@ -206,27 +212,16 @@ bool nsNSSShutDownList::construct(const
{
if (!singleton && !sInShutdown && XRE_IsParentProcess()) {
singleton = new nsNSSShutDownList();
}
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,
"nsNSSActivityState.mNSSActivityStateLock"),
mNSSActivityCounter(0),
mNSSRestrictedThread(nullptr)
{
}
@@ -281,8 +276,14 @@ nsNSSShutDownPreventionLock::nsNSSShutDo
{
nsNSSShutDownList::enterActivityState();
}
nsNSSShutDownPreventionLock::~nsNSSShutDownPreventionLock()
{
nsNSSShutDownList::leaveActivityState();
}
+
+bool
+nsNSSShutDownObject::isAlreadyShutDown() const
+{
+ return mAlreadyShutDown || sInShutdown;
+}
--- a/security/manager/ssl/nsNSSShutDown.h
+++ b/security/manager/ssl/nsNSSShutDown.h
@@ -59,29 +59,28 @@ 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:
- 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.
- 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.
static nsresult doPK11Logout();
// Signal entering/leaving a scope where shutting down NSS is prohibited.
static void enterActivityState();
static void leaveActivityState();
@@ -223,17 +222,17 @@ public:
break;
default:
MOZ_CRASH("shutdown() called from an unknown source");
}
mAlreadyShutDown = true;
}
}
- bool isAlreadyShutDown() const { return mAlreadyShutDown; }
+ bool isAlreadyShutDown() const;
protected:
virtual void virtualDestroyNSSReference() = 0;
private:
volatile bool mAlreadyShutDown;
};
class nsOnPK11LogoutCancelObject