From 00821ec710ec8d9549b9f7e68837a7393a954754 Mon Sep 17 00:00:00 2001 From: Shane Kearns Date: Wed, 29 Feb 2012 15:53:24 +0000 Subject: 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 Reviewed-by: Richard J. Moore --- src/network/ssl/qsslcertificate.cpp | 9 +++ .../ssl/qsslcertificate/tst_qsslcertificate.cpp | 72 ++++++++++++++++++++++ 2 files changed, 81 insertions(+) diff --git a/src/network/ssl/qsslcertificate.cpp b/src/network/ssl/qsslcertificate.cpp index 966497be2e..a9573bf1d8 100644 --- a/src/network/ssl/qsslcertificate.cpp +++ b/src/network/ssl/qsslcertificate.cpp @@ -123,6 +123,7 @@ #include #include #include +#include #include #include #include @@ -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 QSslCertificate::subjectInfoAttributes() const { + QMutexLocker lock(QMutexPool::globalInstanceGet(d.data())); // lazy init if (d->subjectInfo.isEmpty() && d->x509) d->subjectInfo = @@ -417,6 +425,7 @@ QList QSslCertificate::subjectInfoAttributes() const */ QList QSslCertificate::issuerInfoAttributes() const { + QMutexLocker lock(QMutexPool::globalInstanceGet(d.data())); // lazy init if (d->issuerInfo.isEmpty() && d->x509) d->issuerInfo = diff --git a/tests/auto/network/ssl/qsslcertificate/tst_qsslcertificate.cpp b/tests/auto/network/ssl/qsslcertificate/tst_qsslcertificate.cpp index 5da9ff0476..93299c26c6 100644 --- a/tests/auto/network/ssl/qsslcertificate/tst_qsslcertificate.cpp +++ b/tests/auto/network/ssl/qsslcertificate/tst_qsslcertificate.cpp @@ -109,6 +109,7 @@ private slots: void subjectAndIssuerAttributes(); void verify(); void extensions(); + void threadSafeConstMethods(); // helper for verbose test failure messages QString toString(const QList&); @@ -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 extensions; + bool isBlacklisted; + QStringList issuerInfo; + QList issuerInfoAttributes; + QSslKey publicKey; + QByteArray serialNumber; + QStringList subjectInfo; + QList 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 -- cgit v1.2.3