pop3: move blocking operations to a secondary thread

Similar to what kimap and ksmtp do.

resources/pop3/jobs.cpp

@@ -15,15 +15,23 @@
 
 #include "pop3protocol.h"
 
+#include <QThread>
+
 POPSession::POPSession(Settings &settings, const QString &password)
-    : mPassword(password)
-    , mSettings(settings)
+    : mProtocol(std::make_unique<POP3Protocol>(settings, password))
+    , mThread(new QThread)
 {
+    qRegisterMetaType<Result>();
+    connect(mProtocol.get(), &POP3Protocol::sslError, this, &POPSession::handleSslError, Qt::BlockingQueuedConnection);
+    mProtocol->moveToThread(mThread.get());
+    mThread->start();
 }
 
 POPSession::~POPSession()
 {
     closeSession();
+    mThread->quit();
+    mThread->wait();
 }
 
 void POPSession::setCurrentJob(BaseJob *job)
@@ -37,13 +45,6 @@ void POPSession::handleSslError(const KSslErrorUiData &errorData)
     mProtocol->setContinueAfterSslError(cont);
 }
 
-Result POPSession::createProtocol()
-{
-    mProtocol.reset(new POP3Protocol(mSettings, mPassword));
-    connect(mProtocol.get(), &POP3Protocol::sslError, this, &POPSession::handleSslError);
-    return mProtocol->openConnection();
-}
-
 POP3Protocol *POPSession::getProtocol() const
 {
     return mProtocol.get();
@@ -60,7 +61,10 @@ void POPSession::abortCurrentJob()
 
 void POPSession::closeSession()
 {
-    mProtocol->closeConnection();
+    QMetaObject::invokeMethod(mProtocol.get(), [=]() {
+        Q_ASSERT(QThread::currentThread() != qApp->thread());
+        mProtocol->closeConnection();
+    });
 }
 
 static QByteArray cleanupListResponse(const QByteArray &response)
@@ -89,22 +93,33 @@ BaseJob::BaseJob(POPSession *POPSession)
     : mPOPSession(POPSession)
 {
     mPOPSession->setCurrentJob(this);
+    connect(this, &BaseJob::jobDone, this, &BaseJob::handleJobDone);
 }
 
 BaseJob::~BaseJob()
 {
     // Don't do that here, the job might be destroyed after another one was started
     // and therefore overwrite the current job
-    // mPOPSession->setCurrentJob( 0 );
+    // mPOPSession->setCurrentJob(nullptr);
 }
 
 void BaseJob::startJob(const QString &path)
 {
-    connect(mPOPSession->getProtocol(), &POP3Protocol::data, this, &BaseJob::slotData);
-    const Result result = mPOPSession->getProtocol()->get(path);
+    Q_ASSERT(QThread::currentThread() == qApp->thread());
+    POP3Protocol *protocol = mPOPSession->getProtocol();
+    connect(protocol, &POP3Protocol::data, this, &BaseJob::slotData);
+    // Important: copy the arguments into the lambda, it'll crash if you capture by reference
+    QMetaObject::invokeMethod(protocol, [=]() {
+        Q_ASSERT(QThread::currentThread() != qApp->thread());
+        const Result result = protocol->get(path);
+        disconnect(protocol, &POP3Protocol::data, this, &BaseJob::slotData);
+        Q_EMIT jobDone(result);
+    });
+}
 
-    // get() is sync, so we're done
-    disconnect(mPOPSession->getProtocol(), &POP3Protocol::data, this, &BaseJob::slotData);
+void BaseJob::handleJobDone(const Result &result)
+{
+    Q_ASSERT(QThread::currentThread() == qApp->thread());
     mPOPSession->setCurrentJob(nullptr);
     if (!result.success) {
         setError(result.error);
@@ -131,12 +146,13 @@ void LoginJob::start()
         emitResult();
     }
 
-    const Result result = mPOPSession->createProtocol();
-    if (!result.success) {
-        setError(result.error);
-        setErrorText(result.errorString);
-    }
-    emitResult();
+    Q_ASSERT(QThread::currentThread() == qApp->thread());
+    POP3Protocol *protocol = mPOPSession->getProtocol();
+    QMetaObject::invokeMethod(protocol, [=]() {
+        Q_ASSERT(QThread::currentThread() != qApp->thread());
+        const Result result = protocol->openConnection();
+        Q_EMIT jobDone(result);
+    });
 }
 
 ListJob::ListJob(POPSession *popSession)
@@ -288,7 +304,6 @@ void FetchJob::start()
     setTotalAmount(KJob::Bytes, mTotalBytesToDownload);
     connect(mPOPSession->getProtocol(), &POP3Protocol::messageComplete, this, &FetchJob::slotMessageComplete);
     startJob(QLatin1String("/download/") + intListToString(mIdsPendingDownload));
-    disconnect(mPOPSession->getProtocol(), &POP3Protocol::messageComplete, this, &FetchJob::slotMessageComplete);
 }
 
 void FetchJob::slotData(const QByteArray &data)
@@ -301,6 +316,12 @@ void FetchJob::slotData(const QByteArray &data)
     }
 }
 
+void FetchJob::handleJobDone(const Result &result)
+{
+    disconnect(mPOPSession->getProtocol(), &POP3Protocol::messageComplete, this, &FetchJob::slotMessageComplete);
+    BaseJob::handleJobDone(result);
+}
+
 void FetchJob::slotMessageComplete()
 {
     KMime::Message::Ptr msg(new KMime::Message);

resources/pop3/jobs.h

@@ -27,7 +27,8 @@ public:
     explicit POPSession(Settings &settings, const QString &password);
     ~POPSession() override;
 
-    Result createProtocol();
+    // Warning: this object lives in a different thread
+    // Do not make direct method calls to it
     POP3Protocol *getProtocol() const;
 
     void abortCurrentJob();
@@ -41,8 +42,7 @@ private:
 
     std::unique_ptr<POP3Protocol> mProtocol;
     BaseJob *mCurrentJob = nullptr;
-    const QString mPassword;
-    Settings &mSettings;
+    std::unique_ptr<QThread> mThread;
 };
 
 class BaseJob : public KJob
@@ -52,9 +52,14 @@ public:
     explicit BaseJob(POPSession *POPSession);
     ~BaseJob() override;
 
+Q_SIGNALS:
+    // internal signal
+    void jobDone(const Result &result);
+
 protected:
-    void startJob(const QString &path);
     virtual void slotData(const QByteArray &data);
+    virtual void handleJobDone(const Result &result);
+    void startJob(const QString &path);
 
     POPSession *const mPOPSession;
 };
@@ -133,6 +138,7 @@ Q_SIGNALS:
 
 private:
     void slotData(const QByteArray &data) override;
+    void handleJobDone(const Result &result) override;
     void slotMessageComplete();
 
     QList<int> mIdsPendingDownload;

resources/pop3/pop3protocol.cpp

@@ -26,6 +26,7 @@ extern "C" {
 
 #include <QSslCipher>
 #include <QSslSocket>
+#include <QThread>
 #include <string.h>
 
 #include <KIOCore/KSslErrorUiData>
@@ -537,6 +538,8 @@ Result POP3Protocol::loginPASS()
 
 Result POP3Protocol::openConnection()
 {
+    Q_ASSERT(QThread::currentThread() != qApp->thread());
+
     m_try_apop = mSettings.authenticationMethod() == MailTransport::Transport::EnumAuthenticationType::APOP;
     m_try_sasl = useSASL(mSettings);
 
@@ -685,6 +688,8 @@ size_t POP3Protocol::realGetSize(unsigned int msg_num)
 
 Result POP3Protocol::get(const QString &_commandString)
 {
+    Q_ASSERT(QThread::currentThread() != qApp->thread());
+
     qCDebug(POP3_LOG) << _commandString;
     // List of supported commands
     //
@@ -746,7 +751,7 @@ Result POP3Protocol::get(const QString &_commandString)
                 // sanders, changed -2 to -1 below
                 int bufStrLen = strlen(buf);
                 buf[bufStrLen - 2] = '\0';
-                Q_EMIT data(QByteArray::fromRawData(buf, bufStrLen));
+                Q_EMIT data(QByteArray(buf, bufStrLen));
             }
         }
         qCDebug(POP3_LOG) << "Finishing up list";
@@ -846,7 +851,7 @@ Result POP3Protocol::get(const QString &_commandString)
                     }
 
                     if (buf2 > destbuf) {
-                        Q_EMIT data(QByteArray::fromRawData(destbuf, buf2 - destbuf));
+                        Q_EMIT data(QByteArray(destbuf, buf2 - destbuf));
                     }
 
                     if (endOfMail) {
@@ -882,7 +887,7 @@ Result POP3Protocol::get(const QString &_commandString)
         if (command(commandStr.toLatin1(), buf, sizeof(buf) - 1) == Ok) {
             const int len = strlen(buf);
             // totalSize(len);
-            Q_EMIT data(QByteArray::fromRawData(buf, len));
+            Q_EMIT data(QByteArray(buf, len));
             // processedSize(len);
             qCDebug(POP3_LOG) << buf;
             qCDebug(POP3_LOG) << "Finishing up uid";

resources/pop3/result.h

@@ -6,6 +6,7 @@
 
 #pragma once
 
+#include <QMetaType>
 #include <QString>
 
 /**
@@ -32,3 +33,5 @@ struct Result {
         return Result{true, 0, QString()};
     }
 };
+
+Q_DECLARE_METATYPE(Result)