bug 1361893 - remove two unnecessary mutexes and a cast from SSLServerCertVerification.cpp r=kmckinley

gSSLVerificationPK11Mutex isn't used at all - it can be removed
gSSLVerificationTelemetryMutex is unnecessary because telemetry has its own lock:

https://dxr.mozilla.org/mozilla-central/rev/a748acbebbde373a88868dc02910fb2bc5e6a023/toolkit/components/telemetry/TelemetryHistogram.cpp#1135
https://dxr.mozilla.org/mozilla-central/rev/a748acbebbde373a88868dc02910fb2bc5e6a023/toolkit/components/telemetry/TelemetryHistogram.cpp#1984

The nsNSSSocketInfo* cast in SSLServerCertVerificationResult::Run() is
unnecessary because mInfoObject is a RefPtr<nsNSSSocketInfo>.

MozReview-Commit-ID: DG7qWGg2amQ

--HG--
extra : rebase_source : 0a475d7aba75ba4debecc7cec2ed14319082d9ab
This commit is contained in:
David Keeler 2017-05-03 16:44:17 -07:00
parent d0a8596e13
commit f821131b3b

View File

@ -110,7 +110,6 @@
#include "cert.h"
#include "mozilla/Assertions.h"
#include "mozilla/Casting.h"
#include "mozilla/Mutex.h"
#include "mozilla/RefPtr.h"
#include "mozilla/Telemetry.h"
#include "mozilla/UniquePtr.h"
@ -150,14 +149,6 @@ namespace {
// do not use a nsCOMPtr to avoid static initializer/destructor
nsIThreadPool* gCertVerificationThreadPool = nullptr;
// We avoid using a mutex for the success case to avoid lock-related
// performance issues. However, we do use a lock in the error case to simplify
// the code, since performance in the error case is not important.
Mutex* gSSLVerificationTelemetryMutex = nullptr;
// We add a mutex to serialize PKCS11 database operations
Mutex* gSSLVerificationPK11Mutex = nullptr;
} // unnamed namespace
// Called when the socket transport thread starts, to initialize the SSL cert
@ -173,8 +164,6 @@ Mutex* gSSLVerificationPK11Mutex = nullptr;
void
InitializeSSLServerCertVerificationThreads()
{
gSSLVerificationTelemetryMutex = new Mutex("SSLVerificationTelemetryMutex");
gSSLVerificationPK11Mutex = new Mutex("SSLVerificationPK11Mutex");
// TODO: tuning, make parameters preferences
// XXX: instantiate nsThreadPool directly, to make this more bulletproof.
// Currently, the nsThreadPool.h header isn't exported for us to do so.
@ -207,14 +196,6 @@ void StopSSLServerCertVerificationThreads()
gCertVerificationThreadPool->Shutdown();
NS_RELEASE(gCertVerificationThreadPool);
}
if (gSSLVerificationTelemetryMutex) {
delete gSSLVerificationTelemetryMutex;
gSSLVerificationTelemetryMutex = nullptr;
}
if (gSSLVerificationPK11Mutex) {
delete gSSLVerificationPK11Mutex;
gSSLVerificationPK11Mutex = nullptr;
}
}
namespace {
@ -1609,11 +1590,10 @@ SSLServerCertVerificationJob::Run()
// Note: the interval is not calculated once as PR_GetError MUST be called
// before any other function call
error = PR_GetError();
{
TimeStamp now = TimeStamp::Now();
MutexAutoLock telemetryMutex(*gSSLVerificationTelemetryMutex);
Telemetry::AccumulateTimeDelta(failureTelemetry, mJobStartTime, now);
}
TimeStamp now = TimeStamp::Now();
Telemetry::AccumulateTimeDelta(failureTelemetry, mJobStartTime, now);
if (error != 0) {
RefPtr<CertErrorRunnable> runnable(
CreateCertErrorRunnable(*mCertVerifier, error, mInfoObject, mCert,
@ -1871,9 +1851,7 @@ SSLServerCertVerificationResult::Run()
if (mTelemetryID != Telemetry::HistogramCount) {
Telemetry::Accumulate(mTelemetryID, mTelemetryValue);
}
// XXX: This cast will be removed by the next patch
((nsNSSSocketInfo*) mInfoObject.get())
->SetCertVerificationResult(mErrorCode, mErrorMessageType);
mInfoObject->SetCertVerificationResult(mErrorCode, mErrorMessageType);
return NS_OK;
}