Make RSA key-pair class a Runnable that's owned by a threadpool

libraries/networking/src/AccountManager.cpp

@@ -22,6 +22,7 @@
 #include <QtCore/QStringList>
 #include <QtCore/QStandardPaths>
 #include <QtCore/QUrlQuery>
+#include <QtCore/QThreadPool>
 #include <QtNetwork/QHttpMultiPart>
 #include <QtNetwork/QNetworkRequest>
 #include <qthread.h>
@@ -94,10 +95,6 @@ const QString DOUBLE_SLASH_SUBSTITUTE = "slashslash";
 const QString ACCOUNT_MANAGER_REQUESTED_SCOPE = "owner";
 
 AccountManager::~AccountManager() {
-    QMutexLocker lock(&_rsaKeygenLock);
-    while (_rsaKeygenThread) {
-        _rsaKeygenWait.wait(&_rsaKeygenLock);
-    }
 }
 
 void AccountManager::logout() {
@@ -747,96 +744,75 @@ void AccountManager::generateNewKeypair(bool isUserKeypair, const QUuid& domainI
         qCDebug(networking) << "Clearing current private key in DataServerAccountInfo";
         _accountInfo.setPrivateKey(QByteArray());
 
-        // setup a new QThread to generate the keypair on, in case it takes a while
-        QThread* generateThread = new QThread(this);
-        generateThread->setObjectName("Account Manager Generator Thread");
-
-        // setup a keypair generator
+        // Create a runnable keypair generated to create an RSA pair and exit.
         RSAKeypairGenerator* keypairGenerator = new RSAKeypairGenerator;
 
         if (!isUserKeypair) {
-            keypairGenerator->setDomainID(domainID);
             _accountInfo.setDomainID(domainID);
         }
 
-        // start keypair generation when the thread starts
-        connect(generateThread, &QThread::started, keypairGenerator, &RSAKeypairGenerator::generateKeypair);
-
         // handle success or failure of keypair generation
-        connect(keypairGenerator, &RSAKeypairGenerator::generatedKeypair, this, &AccountManager::processGeneratedKeypair);
-        connect(keypairGenerator, &RSAKeypairGenerator::errorGeneratingKeypair,
-                this, &AccountManager::handleKeypairGenerationError);
-
-        connect(keypairGenerator, &QObject::destroyed, generateThread, &QThread::quit);
-        connect(generateThread, &QThread::finished, this, &AccountManager::rsaKeygenThreadFinished);
-        connect(generateThread, &QThread::finished, generateThread, &QThread::deleteLater);
-        _rsaKeygenThread = generateThread;
-
-        keypairGenerator->moveToThread(generateThread);
+        connect(keypairGenerator, &RSAKeypairGenerator::generatedKeypair, this,
+            &AccountManager::processGeneratedKeypair);
+        connect(keypairGenerator, &RSAKeypairGenerator::errorGeneratingKeypair, this,
+            &AccountManager::handleKeypairGenerationError);
 
         qCDebug(networking) << "Starting worker thread to generate 2048-bit RSA keypair.";
-        generateThread->start();
+        // Start on Qt's global thread pool.
+        QThreadPool::globalInstance()->start(keypairGenerator);
     }
 }
 
-void AccountManager::processGeneratedKeypair() {
+void AccountManager::processGeneratedKeypair(QByteArray publicKey, QByteArray privateKey) {
 
     qCDebug(networking) << "Generated 2048-bit RSA keypair. Uploading public key now.";
 
-    RSAKeypairGenerator* keypairGenerator = qobject_cast<RSAKeypairGenerator*>(sender());
+    // hold the private key to later set our metaverse API account info if upload succeeds
+    _pendingPrivateKey = privateKey;
 
-    if (keypairGenerator) {
-        // hold the private key to later set our metaverse API account info if upload succeeds
-        _pendingPrivateKey = keypairGenerator->getPrivateKey();
+    // upload the public key so data-web has an up-to-date key
+    const QString USER_PUBLIC_KEY_UPDATE_PATH = "api/v1/user/public_key";
+    const QString DOMAIN_PUBLIC_KEY_UPDATE_PATH = "api/v1/domains/%1/public_key";
 
-        // upload the public key so data-web has an up-to-date key
-        const QString USER_PUBLIC_KEY_UPDATE_PATH = "api/v1/user/public_key";
-        const QString DOMAIN_PUBLIC_KEY_UPDATE_PATH = "api/v1/domains/%1/public_key";
-
-        QString uploadPath;
-        const auto& domainID = keypairGenerator->getDomainID();
-        if (domainID.isNull()) {
-            uploadPath = USER_PUBLIC_KEY_UPDATE_PATH;
-        } else {
-            uploadPath = DOMAIN_PUBLIC_KEY_UPDATE_PATH.arg(uuidStringWithoutCurlyBraces(domainID));
-        }
-
-        // setup a multipart upload to send up the public key
-        QHttpMultiPart* requestMultiPart = new QHttpMultiPart(QHttpMultiPart::FormDataType);
-
-        QHttpPart publicKeyPart;
-        publicKeyPart.setHeader(QNetworkRequest::ContentTypeHeader, QVariant("application/octet-stream"));
-
-        publicKeyPart.setHeader(QNetworkRequest::ContentDispositionHeader,
-                          QVariant("form-data; name=\"public_key\"; filename=\"public_key\""));
-        publicKeyPart.setBody(keypairGenerator->getPublicKey());
-        requestMultiPart->append(publicKeyPart);
-
-        if (!domainID.isNull()) {
-            const auto& key = getTemporaryDomainKey(domainID);
-            QHttpPart apiKeyPart;
-            publicKeyPart.setHeader(QNetworkRequest::ContentTypeHeader, QVariant("application/octet-stream"));
-            apiKeyPart.setHeader(QNetworkRequest::ContentDispositionHeader,
-                              QVariant("form-data; name=\"api_key\""));
-            apiKeyPart.setBody(key.toUtf8());
-            requestMultiPart->append(apiKeyPart);
-        }
-
-        // setup callback parameters so we know once the keypair upload has succeeded or failed
-        JSONCallbackParameters callbackParameters;
-        callbackParameters.jsonCallbackReceiver = this;
-        callbackParameters.jsonCallbackMethod = "publicKeyUploadSucceeded";
-        callbackParameters.errorCallbackReceiver = this;
-        callbackParameters.errorCallbackMethod = "publicKeyUploadFailed";
-
-        sendRequest(uploadPath, AccountManagerAuth::Optional, QNetworkAccessManager::PutOperation,
-                    callbackParameters, QByteArray(), requestMultiPart);
-
-        keypairGenerator->deleteLater();
+    QString uploadPath;
+    const auto& domainID = _accountInfo.getDomainID();
+    if (domainID.isNull()) {
+        uploadPath = USER_PUBLIC_KEY_UPDATE_PATH;
     } else {
-        qCWarning(networking) << "Expected processGeneratedKeypair to be called by a live RSAKeypairGenerator"
-            << "but the casted sender is NULL. Will not process generated keypair.";
+        uploadPath = DOMAIN_PUBLIC_KEY_UPDATE_PATH.arg(uuidStringWithoutCurlyBraces(domainID));
     }
+
+    // setup a multipart upload to send up the public key
+    QHttpMultiPart* requestMultiPart = new QHttpMultiPart(QHttpMultiPart::FormDataType);
+
+    QHttpPart publicKeyPart;
+    publicKeyPart.setHeader(QNetworkRequest::ContentTypeHeader, QVariant("application/octet-stream"));
+
+    publicKeyPart.setHeader(QNetworkRequest::ContentDispositionHeader,
+                        QVariant("form-data; name=\"public_key\"; filename=\"public_key\""));
+    publicKeyPart.setBody(publicKey);
+    requestMultiPart->append(publicKeyPart);
+
+    // Currently broken? We don't have the temporary domain key.
+    if (!domainID.isNull()) {
+        const auto& key = getTemporaryDomainKey(domainID);
+        QHttpPart apiKeyPart;
+        publicKeyPart.setHeader(QNetworkRequest::ContentTypeHeader, QVariant("application/octet-stream"));
+        apiKeyPart.setHeader(QNetworkRequest::ContentDispositionHeader,
+                            QVariant("form-data; name=\"api_key\""));
+        apiKeyPart.setBody(key.toUtf8());
+        requestMultiPart->append(apiKeyPart);
+    }
+
+    // setup callback parameters so we know once the keypair upload has succeeded or failed
+    JSONCallbackParameters callbackParameters;
+    callbackParameters.jsonCallbackReceiver = this;
+    callbackParameters.jsonCallbackMethod = "publicKeyUploadSucceeded";
+    callbackParameters.errorCallbackReceiver = this;
+    callbackParameters.errorCallbackMethod = "publicKeyUploadFailed";
+
+    sendRequest(uploadPath, AccountManagerAuth::Optional, QNetworkAccessManager::PutOperation,
+                callbackParameters, QByteArray(), requestMultiPart);
 }
 
 void AccountManager::publicKeyUploadSucceeded(QNetworkReply& reply) {
@@ -875,12 +851,4 @@ void AccountManager::handleKeypairGenerationError() {
 
     // reset our waiting state for keypair response
     _isWaitingForKeypairResponse = false;
-
-    sender()->deleteLater();
-}
-
-void AccountManager::rsaKeygenThreadFinished() {
-    QMutexLocker lock(&_rsaKeygenLock);
-    _rsaKeygenThread = nullptr;
-    _rsaKeygenWait.wakeAll();
 }

libraries/networking/src/AccountManager.h

@@ -15,8 +15,6 @@
 #include <QtCore/QByteArray>
 #include <QtCore/QObject>
 #include <QtCore/QUrl>
-#include <QtCore/QMutex>
-#include <QtCore/QWaitCondition>
 #include <QtNetwork/QNetworkReply>
 #include <QUrlQuery>
 
@@ -130,11 +128,10 @@ signals:
 private slots:
     void processReply();
     void handleKeypairGenerationError();
-    void processGeneratedKeypair();
+    void processGeneratedKeypair(QByteArray publicKey, QByteArray privateKey);
     void publicKeyUploadSucceeded(QNetworkReply& reply);
     void publicKeyUploadFailed(QNetworkReply& reply);
     void generateNewKeypair(bool isUserKeypair = true, const QUuid& domainID = QUuid());
-    void rsaKeygenThreadFinished();
 
 private:
     AccountManager(AccountManager const& other) = delete;
@@ -160,9 +157,6 @@ private:
     QByteArray _pendingPrivateKey;
 
     QUuid _sessionID { QUuid::createUuid() };
-    QMutex _rsaKeygenLock;
-    QWaitCondition _rsaKeygenWait;
-    QThread* _rsaKeygenThread { nullptr };
 };
 
 #endif // hifi_AccountManager_h

libraries/networking/src/RSAKeypairGenerator.cpp

@@ -25,7 +25,10 @@
 RSAKeypairGenerator::RSAKeypairGenerator(QObject* parent) :
     QObject(parent)
 {
+}
 
+void RSAKeypairGenerator::run() {
+    generateKeypair();
 }
 
 void RSAKeypairGenerator::generateKeypair() {
@@ -92,5 +95,5 @@ void RSAKeypairGenerator::generateKeypair() {
     OPENSSL_free(publicKeyDER);
     OPENSSL_free(privateKeyDER);
     
-    emit generatedKeypair();
+    emit generatedKeypair(_publicKey, _privateKey);
 }

libraries/networking/src/RSAKeypairGenerator.h

@@ -13,25 +13,20 @@
 #define hifi_RSAKeypairGenerator_h
 
 #include <QtCore/QObject>
+#include <QtCore/QRunnable>
 #include <QtCore/QUuid>
 
-class RSAKeypairGenerator : public QObject {
+class RSAKeypairGenerator : public QObject, public QRunnable {
     Q_OBJECT
 public:
-    RSAKeypairGenerator(QObject* parent = 0);
+    RSAKeypairGenerator(QObject* parent = nullptr);
 
-    void setDomainID(const QUuid& domainID) { _domainID = domainID; }
-    const QUuid& getDomainID() const { return _domainID; }
-
-    const QByteArray& getPublicKey() const { return _publicKey; }
-    const QByteArray& getPrivateKey() const { return _privateKey; }
-    
-public slots:
+    virtual void run() override;
     void generateKeypair();
 
 signals:
     void errorGeneratingKeypair();
-    void generatedKeypair();
+    void generatedKeypair(QByteArray publicKey, QByteArray privateKey);
 
 private:
     QUuid _domainID;