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
(cherry picked from commit 55bf4ed9468ad467a0b681d2d041edbc2a5a4d21)
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>

2 files changed, 70 insertions, 1 deletions

diff --git a/src/network/ssl/qsslcertificate.cpp b/src/network/ssl/qsslcertificate.cpp
index 2edb2021da..dca37ddad6 100644
--- a/src/network/ssl/qsslcertificate.cpp
+++ b/src/network/ssl/qsslcertificate.cpp
@@ -121,6 +121,8 @@
 #include <QtCore/qfile.h>
 #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>
 
@@ -252,6 +254,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);
@@ -267,6 +270,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;
         // if we cannot convert to a long, just output the hexadecimal number
@@ -321,6 +325,7 @@ static QString _q_SubjectInfoToString(QSslCertificate::SubjectInfo info)
 */
 QString QSslCertificate::issuerInfo(SubjectInfo info) const
 {
+    QMutexLocker lock(QMutexPool::globalInstanceGet(d.data()));
     // lazy init
     if (d->issuerInfo.isEmpty() && d->x509)
         d->issuerInfo =
@@ -338,6 +343,7 @@ QString QSslCertificate::issuerInfo(SubjectInfo info) const
 */
 QString QSslCertificate::issuerInfo(const QByteArray &tag) const
 {
+    QMutexLocker lock(QMutexPool::globalInstanceGet(d.data()));
     // lazy init
     if (d->issuerInfo.isEmpty() && d->x509)
         d->issuerInfo =
@@ -357,6 +363,7 @@ QString QSslCertificate::issuerInfo(const QByteArray &tag) const
 */
 QString QSslCertificate::subjectInfo(SubjectInfo info) const
 {
+    QMutexLocker lock(QMutexPool::globalInstanceGet(d.data()));
     // lazy init
     if (d->subjectInfo.isEmpty() && d->x509)
         d->subjectInfo =
@@ -373,6 +380,7 @@ QString QSslCertificate::subjectInfo(SubjectInfo info) const
 */
 QString QSslCertificate::subjectInfo(const QByteArray &tag) const
 {
+    QMutexLocker lock(QMutexPool::globalInstanceGet(d.data()));
     // lazy init
     if (d->subjectInfo.isEmpty() && d->x509)
         d->subjectInfo =
diff --git a/tests/auto/qsslcertificate/tst_qsslcertificate.cpp b/tests/auto/qsslcertificate/tst_qsslcertificate.cpp
index 4f4a5a0d01..c73416d038 100644
--- a/tests/auto/qsslcertificate/tst_qsslcertificate.cpp
+++ b/tests/auto/qsslcertificate/tst_qsslcertificate.cpp
@@ -113,6 +113,7 @@ private slots:
     void largeSerialNumber();
     void largeExpirationDate();
     void blacklistedCertificates();
+    void threadSafeConstMethods();
 
 // ### add tests for certificate bundles (multiple certificates concatenated into a single
 //     structure); both PEM and DER formatted
@@ -850,7 +851,67 @@ void tst_QSslCertificate::blacklistedCertificates()
     }
 }
 
-#endif // QT_NO_OPENSSL
+class TestThread : public QThread
+{
+public:
+    void run()
+    {
+        effectiveDate = cert.effectiveDate();
+        expiryDate = cert.expiryDate();
+        isValid = cert.isValid();
+        issuerInfo = cert.issuerInfo(QSslCertificate::CommonName);
+        publicKey = cert.publicKey();
+        serialNumber = cert.serialNumber();
+        subjectInfo = cert.subjectInfo(QSslCertificate::CommonName);
+        toDer = cert.toDer();
+        toPem = cert.toPem();
+        version = cert.version();
+    }
+    QSslCertificate cert;
+    QDateTime effectiveDate;
+    QDateTime expiryDate;
+    bool isValid;
+    QString issuerInfo;
+    QSslKey publicKey;
+    QByteArray serialNumber;
+    QString subjectInfo;
+    QByteArray toDer;
+    QByteArray toPem;
+    QByteArray version;
+};
+
+void tst_QSslCertificate::threadSafeConstMethods()
+{
+    if (!QSslSocket::supportsSsl())
+        return;
+
+    QByteArray encoded = readFile(QLatin1String(SRCDIR "/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.isValid == t2.isValid);
+    QVERIFY(t1.issuerInfo == t2.issuerInfo);
+    QVERIFY(t1.publicKey == t2.publicKey);
+    QVERIFY(t1.serialNumber == t2.serialNumber);
+    QVERIFY(t1.subjectInfo == t2.subjectInfo);
+    QVERIFY(t1.toDer == t2.toDer);
+    QVERIFY(t1.toPem == t2.toPem);
+    QVERIFY(t1.version == t2.version);
+
+}
+
+#endif // QT_NO_SSL
 
 QTEST_MAIN(tst_QSslCertificate)
 #include "tst_qsslcertificate.moc"