QSslCertificate - make lazy initialisation thread safe

QSslCertificate can be copied around into multiple threads,
without detaching. For example, the https worker threads inside
QNetworkAccessManager.
There are const methods, which lazily initialise members of
the private class without detaching (i.e. caching results of
expensive function calls)
These functions now lock the d pointer using QMutexPool to
avoid concurrency related crashes.

autotest crashes 20% of the time in release builds without
the fix, passes 100 times in a row with the fix.

Task-number: QTBUG-20452
Change-Id: I64a01af8159216f2dd6215a08669890f6c029ca8
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
Reviewed-by: Richard J. Moore <rich@kde.org>
This commit is contained in:
Shane Kearns 2012-02-29 15:53:24 +00:00 committed by Qt by Nokia
parent c78957766a
commit 00821ec710
2 changed files with 81 additions and 0 deletions

View File

@ -123,6 +123,7 @@
#include <QtCore/qfileinfo.h>
#include <QtCore/qmap.h>
#include <QtCore/qmutex.h>
#include <QtCore/private/qmutexpool_p.h>
#include <QtCore/qstring.h>
#include <QtCore/qstringlist.h>
#include <QtCore/qvarlengtharray.h>
@ -263,6 +264,7 @@ void QSslCertificate::clear()
*/
QByteArray QSslCertificate::version() const
{
QMutexLocker lock(QMutexPool::globalInstanceGet(d.data()));
if (d->versionString.isEmpty() && d->x509)
d->versionString =
QByteArray::number(qlonglong(q_ASN1_INTEGER_get(d->x509->cert_info->version)) + 1);
@ -275,6 +277,7 @@ QByteArray QSslCertificate::version() const
*/
QByteArray QSslCertificate::serialNumber() const
{
QMutexLocker lock(QMutexPool::globalInstanceGet(d.data()));
if (d->serialNumberString.isEmpty() && d->x509) {
ASN1_INTEGER *serialNumber = d->x509->cert_info->serialNumber;
QByteArray hexString;
@ -327,6 +330,7 @@ static QByteArray _q_SubjectInfoToString(QSslCertificate::SubjectInfo info)
*/
QStringList QSslCertificate::issuerInfo(SubjectInfo info) const
{
QMutexLocker lock(QMutexPool::globalInstanceGet(d.data()));
// lazy init
if (d->issuerInfo.isEmpty() && d->x509)
d->issuerInfo =
@ -344,6 +348,7 @@ QStringList QSslCertificate::issuerInfo(SubjectInfo info) const
*/
QStringList QSslCertificate::issuerInfo(const QByteArray &attribute) const
{
QMutexLocker lock(QMutexPool::globalInstanceGet(d.data()));
// lazy init
if (d->issuerInfo.isEmpty() && d->x509)
d->issuerInfo =
@ -363,6 +368,7 @@ QStringList QSslCertificate::issuerInfo(const QByteArray &attribute) const
*/
QStringList QSslCertificate::subjectInfo(SubjectInfo info) const
{
QMutexLocker lock(QMutexPool::globalInstanceGet(d.data()));
// lazy init
if (d->subjectInfo.isEmpty() && d->x509)
d->subjectInfo =
@ -379,6 +385,7 @@ QStringList QSslCertificate::subjectInfo(SubjectInfo info) const
*/
QStringList QSslCertificate::subjectInfo(const QByteArray &attribute) const
{
QMutexLocker lock(QMutexPool::globalInstanceGet(d.data()));
// lazy init
if (d->subjectInfo.isEmpty() && d->x509)
d->subjectInfo =
@ -398,6 +405,7 @@ QStringList QSslCertificate::subjectInfo(const QByteArray &attribute) const
*/
QList<QByteArray> QSslCertificate::subjectInfoAttributes() const
{
QMutexLocker lock(QMutexPool::globalInstanceGet(d.data()));
// lazy init
if (d->subjectInfo.isEmpty() && d->x509)
d->subjectInfo =
@ -417,6 +425,7 @@ QList<QByteArray> QSslCertificate::subjectInfoAttributes() const
*/
QList<QByteArray> QSslCertificate::issuerInfoAttributes() const
{
QMutexLocker lock(QMutexPool::globalInstanceGet(d.data()));
// lazy init
if (d->issuerInfo.isEmpty() && d->x509)
d->issuerInfo =

View File

@ -109,6 +109,7 @@ private slots:
void subjectAndIssuerAttributes();
void verify();
void extensions();
void threadSafeConstMethods();
// helper for verbose test failure messages
QString toString(const QList<QSslError>&);
@ -1059,6 +1060,77 @@ void tst_QSslCertificate::extensions()
}
class TestThread : public QThread
{
public:
void run()
{
effectiveDate = cert.effectiveDate();
expiryDate = cert.expiryDate();
extensions = cert.extensions();
isBlacklisted = cert.isBlacklisted();
issuerInfo = cert.issuerInfo(QSslCertificate::CommonName);
issuerInfoAttributes = cert.issuerInfoAttributes();
publicKey = cert.publicKey();
serialNumber = cert.serialNumber();
subjectInfo = cert.subjectInfo(QSslCertificate::CommonName);
subjectInfoAttributes = cert.subjectInfoAttributes();
toDer = cert.toDer();
toPem = cert.toPem();
toText = cert.toText();
version = cert.version();
}
QSslCertificate cert;
QDateTime effectiveDate;
QDateTime expiryDate;
QList<QSslCertificateExtension> extensions;
bool isBlacklisted;
QStringList issuerInfo;
QList<QByteArray> issuerInfoAttributes;
QSslKey publicKey;
QByteArray serialNumber;
QStringList subjectInfo;
QList<QByteArray> subjectInfoAttributes;
QByteArray toDer;
QByteArray toPem;
QByteArray toText;
QByteArray version;
};
void tst_QSslCertificate::threadSafeConstMethods()
{
if (!QSslSocket::supportsSsl())
return;
QByteArray encoded = readFile(testDataDir + "/certificates/cert.pem");
QSslCertificate certificate(encoded);
QVERIFY(!certificate.isNull());
TestThread t1;
t1.cert = certificate; //shallow copy
TestThread t2;
t2.cert = certificate; //shallow copy
t1.start();
t2.start();
QVERIFY(t1.wait(5000));
QVERIFY(t2.wait(5000));
QVERIFY(t1.cert == t2.cert);
QVERIFY(t1.effectiveDate == t2.effectiveDate);
QVERIFY(t1.expiryDate == t2.expiryDate);
//QVERIFY(t1.extensions == t2.extensions); // no equality operator, so not tested
QVERIFY(t1.isBlacklisted == t2.isBlacklisted);
QVERIFY(t1.issuerInfo == t2.issuerInfo);
QVERIFY(t1.issuerInfoAttributes == t2.issuerInfoAttributes);
QVERIFY(t1.publicKey == t2.publicKey);
QVERIFY(t1.serialNumber == t2.serialNumber);
QVERIFY(t1.subjectInfo == t2.subjectInfo);
QVERIFY(t1.subjectInfoAttributes == t2.subjectInfoAttributes);
QVERIFY(t1.toDer == t2.toDer);
QVERIFY(t1.toPem == t2.toPem);
QVERIFY(t1.toText == t2.toText);
QVERIFY(t1.version == t2.version);
}
#endif // QT_NO_SSL