QSslSocketPrivate::systemCaCertificates() hangs sometimes on Symbianv4.7.0

The patch fixes the hanging issues on some Symbian devices that occurs while
retrieving certificates from the Symbian certificate store. The hanging was
caused by the certificate info array not being closed before exiting the
thread. This alone wouldn't make the existing implementation work, so the patch
replaces it with a pure Symbian style implementation which doesn't seem to be
affected (probably some OpenC threads issue).

Merge-request: 808
Reviewed-by: Shane Kearns
Reviewed-by: Simon Hausmann <simon.hausmann@nokia.com>
Task: QTBUG-13033
(cherry picked from commit 5342be5ceffc84b56476fff57dd7d5e1bbfeb471)

1 files changed, 103 insertions, 90 deletions

diff --git a/src/network/ssl/qsslsocket_openssl.cpp b/src/network/ssl/qsslsocket_openssl.cpp
index 3174501c57..5182f118ed 100644
--- a/src/network/ssl/qsslsocket_openssl.cpp
+++ b/src/network/ssl/qsslsocket_openssl.cpp
@@ -71,6 +71,9 @@ QT_BEGIN_NAMESPACE
     PtrCertFindCertificateInStore QSslSocketPrivate::ptrCertFindCertificateInStore = 0;
     PtrCertCloseStore QSslSocketPrivate::ptrCertCloseStore = 0;
 #elif defined(Q_OS_SYMBIAN)
+#include <e32base.h>
+#include <e32std.h>
+#include <e32debug.h>
 #include <QtCore/private/qcore_symbian_p.h>
 #endif
 
@@ -569,120 +572,138 @@ void QSslSocketPrivate::resetDefaultCiphers()
 
 #if defined(Q_OS_SYMBIAN)
 
-QCertificateRetriever::QCertificateRetriever(QCertificateConsumer* parent)
-    : CActive(EPriorityStandard)
-    , certStore(0)
-    , certFilter(0)
-    , consumer(parent)
-    , currentCertificateIndex(0)
-    , certDescriptor(0, 0)
+CSymbianCertificateRetriever::CSymbianCertificateRetriever() : CActive(CActive::EPriorityStandard),
+        iSequenceError(KErrNone)
 {
-    CActiveScheduler::Add(this);
-    QT_TRAP_THROWING(certStore = CUnifiedCertStore::NewL(qt_s60GetRFs(), EFalse));
-    QT_TRAP_THROWING(certFilter = CCertAttributeFilter::NewL());
-    certFilter->SetFormat(EX509Certificate);
 }
 
-QCertificateRetriever::~QCertificateRetriever()
+CSymbianCertificateRetriever::~CSymbianCertificateRetriever()
 {
-    delete certFilter;
-    delete certStore;
-    Cancel();
+    iThread.Close();
 }
 
-void QCertificateRetriever::fetch()
+CSymbianCertificateRetriever* CSymbianCertificateRetriever::NewL()
 {
-    certStore->Initialize(iStatus);
-    state = Initializing;
-    SetActive();
+    CSymbianCertificateRetriever* self = new (ELeave) CSymbianCertificateRetriever();
+    CleanupStack::PushL(self);
+    self->ConstructL();
+    CleanupStack::Pop();
+    return self;
 }
 
-void QCertificateRetriever::list()
+int CSymbianCertificateRetriever::GetCertificates(QList<QByteArray> &certificates)
 {
-    certStore->List(certs, *certFilter, iStatus);
-    state = Listing;
-    SetActive();
+    iCertificates = &certificates;
+
+    TRequestStatus status;
+    iThread.Logon(status);
+    iThread.Resume();
+    User::WaitForRequest(status);
+    if (iThread.ExitType() == EExitKill)
+        return KErrDied;
+    else
+        return status.Int();    // Logon() completes with the thread's exit value
 }
 
-void QCertificateRetriever::retrieveNextCertificate()
+void CSymbianCertificateRetriever::doThreadEntryL()
 {
-    CCTCertInfo* cert = certs[currentCertificateIndex];
-    currentCertificate.resize(cert->Size());
-    certDescriptor.Set((TUint8*)currentCertificate.data(), 0, currentCertificate.size());
-    certStore->Retrieve(*cert, certDescriptor, iStatus);
-    state = RetrievingCertificates;
+    CActiveScheduler* activeScheduler = new (ELeave) CActiveScheduler;
+    CleanupStack::PushL(activeScheduler);
+    CActiveScheduler::Install(activeScheduler);
+
+    CActiveScheduler::Add(this);
+
+    // These aren't deleted in the destructor so leaving the to CS is ok
+    iCertStore = CUnifiedCertStore::NewLC(qt_s60GetRFs(), EFalse);
+    iCertFilter = CCertAttributeFilter::NewLC();
+
+    // Kick off the sequence by initializing the cert store
+    iState = Initializing;
+    iCertStore->Initialize(iStatus);
     SetActive();
-}
 
-void QCertificateRetriever::RunL()
-{
-    QT_TRYCATCH_LEAVING(run());
+    CActiveScheduler::Start();
+
+    // Sequence complete, clean up
+
+    // These MUST be cleaned up before the installed CActiveScheduler is destroyed and can't be left to the
+    // destructor of CSymbianCertificateRetriever. Otherwise the destructor of CActiveScheduler will get
+    // stuck.
+    iCertInfos.Close();
+    CleanupStack::PopAndDestroy(3);     // activeScheduler, iCertStore, iCertFilter
 }
 
-void QCertificateRetriever::run()
+
+TInt CSymbianCertificateRetriever::ThreadEntryPoint(TAny* aParams)
 {
-    switch (state) {
-    case Initializing:
-        list();
-        break;
-    case Listing:
-        currentCertificateIndex = 0;
-        retrieveNextCertificate();
-        break;
-    case RetrievingCertificates:
-        consumer->addEncodedCertificate(currentCertificate);
-        currentCertificate = QByteArray();
+    CTrapCleanup* cleanupStack = CTrapCleanup::New();
 
-        currentCertificateIndex++;
+    CSymbianCertificateRetriever* self = (CSymbianCertificateRetriever*) aParams;
+    TRAPD(err, self->doThreadEntryL());
+    delete cleanupStack;
 
-        if (currentCertificateIndex < certs.Count())
-            retrieveNextCertificate();
-        else
-            consumer->finish();
-        break;
-    }
+    // doThreadEntryL() can leave only before the retrieval sequence is started
+    if (err)
+        return err;
+    else
+        return self->iSequenceError;    // return any error that occured during the retrieval
 }
 
-void QCertificateRetriever::DoCancel()
+void CSymbianCertificateRetriever::ConstructL()
 {
-    switch (state) {
-    case Initializing:
-        certStore->CancelInitialize();
-        break;
-    case Listing:
-        certStore->CancelList();
-        break;
-    case RetrievingCertificates:
-        certStore->CancelRetrieve();
-        break;
-    }
+    User::LeaveIfError(iThread.Create(_L("CertWorkerThread"),
+        CSymbianCertificateRetriever::ThreadEntryPoint, 16384, NULL, this));
 }
 
-QCertificateConsumer::QCertificateConsumer(QObject* parent)
-    : QObject(parent)
-    , retriever(0)
+void CSymbianCertificateRetriever::DoCancel()
 {
+    // We never cancel the sequence
 }
 
-QCertificateConsumer::~QCertificateConsumer()
+TInt CSymbianCertificateRetriever::RunError(TInt aError)
 {
-    delete retriever;
+    // If something goes wrong in the sequence, abort the sequence
+    iSequenceError = aError;    // this gets reported to the client in the TRequestStatus
+    CActiveScheduler::Stop();
+    return KErrNone;
 }
 
-void QCertificateConsumer::finish()
+void CSymbianCertificateRetriever::GetCertificateL()
 {
-    delete retriever;
-    retriever = 0;
-    emit finished();
+    CCTCertInfo* certInfo = iCertInfos[iCurrentCertIndex];
+    iCertificateData.resize(certInfo->Size());
+    TPtr8 des((TUint8*)iCertificateData.data(), 0, iCertificateData.size());
+    iCertStore->Retrieve(*certInfo, des, iStatus);
+    iState = RetrievingCertificates;
+    SetActive();
 }
 
-void QCertificateConsumer::start()
+void CSymbianCertificateRetriever::RunL()
 {
-    retriever = new QCertificateRetriever(this);
-    Q_CHECK_PTR(retriever);
-    retriever->fetch();
-}
+    switch (iState) {
+    case Initializing:
+        iState = Listing;
+        iCertStore->List(iCertInfos, *iCertFilter, iStatus);
+        SetActive();
+        break;
 
+    case Listing:
+        iCurrentCertIndex = 0;
+        GetCertificateL();
+        break;
+
+    case RetrievingCertificates:
+        iCertificates->append(iCertificateData);
+        iCertificateData = QByteArray();
+        iCurrentCertIndex++;
+        if (iCurrentCertIndex < iCertInfos.Count())
+            GetCertificateL();
+        else
+            // Stop the scheduler to return to the thread entry function
+            CActiveScheduler::Stop();
+        break;
+    }
+}
 #endif // defined(Q_OS_SYMBIAN)
 
 QList<QSslCertificate> QSslSocketPrivate::systemCaCertificates()
@@ -759,23 +780,15 @@ QList<QSslCertificate> QSslSocketPrivate::systemCaCertificates()
     systemCerts.append(QSslCertificate::fromPath(QLatin1String("/usr/share/ssl/*.pem"), QSsl::Pem, QRegExp::Wildcard)); // Centos, Redhat, SuSE
     systemCerts.append(QSslCertificate::fromPath(QLatin1String("/usr/local/ssl/*.pem"), QSsl::Pem, QRegExp::Wildcard)); // Normal OpenSSL Tarball
 #elif defined(Q_OS_SYMBIAN)
-    QThread* certThread = new QThread;
+    QList<QByteArray> certs;
+    QScopedPointer<CSymbianCertificateRetriever> retriever(CSymbianCertificateRetriever::NewL());
 
-    QCertificateConsumer *consumer = new QCertificateConsumer();
-    consumer->moveToThread(certThread);
-    QObject::connect(certThread, SIGNAL(started()), consumer, SLOT(start()));
-    QObject::connect(consumer, SIGNAL(finished()), certThread, SLOT(quit()), Qt::DirectConnection);
-
-    certThread->start();
-    certThread->wait();
-    foreach (const QByteArray &encodedCert, consumer->encodedCertificates()) {
+    retriever->GetCertificates(certs);
+    foreach (const QByteArray &encodedCert, certs) {
         QSslCertificate cert(encodedCert, QSsl::Der);
         if (!cert.isNull())
             systemCerts.append(cert);
     }
-
-    delete consumer;
-    delete certThread;
 #endif
 
     return systemCerts;