[auth] Add mutex; remove qApp parents; do not set parent in wrong thread

qgis · Jun 25, 2018 · cdafca5 · cdafca5
1 parent 5ee767a
commit cdafca5
Show file tree

Hide file tree

Showing 5 changed files with 21 additions and 10 deletions.
diff --git a/src/auth/oauth2/qgsauthoauth2config.cpp b/src/auth/oauth2/qgsauthoauth2config.cpp
@@ -676,7 +676,7 @@ QgsStringMap QgsAuthOAuth2Config::mapOAuth2Configs(
   return configs;
 }
 
-QgsStringMap QgsAuthOAuth2Config::mappedOAuth2ConfigsCache( const QString &extradir )
+QgsStringMap QgsAuthOAuth2Config::mappedOAuth2ConfigsCache(QObject *parent, const QString &extradir )
 {
   QgsStringMap configs;
   bool ok = false;
@@ -701,7 +701,7 @@ QgsStringMap QgsAuthOAuth2Config::mappedOAuth2ConfigsCache( const QString &extra
       continue;
     }
     QgsStringMap newconfigs = QgsAuthOAuth2Config::mapOAuth2Configs(
-                                configdirinfo.canonicalFilePath(), qApp, QgsAuthOAuth2Config::JSON, &ok );
+                                configdirinfo.canonicalFilePath(), parent, QgsAuthOAuth2Config::JSON, &ok );
     if ( ok )
     {
       QgsStringMap::const_iterator i = newconfigs.constBegin();
@@ -735,6 +735,7 @@ QString QgsAuthOAuth2Config::configTypeString( QgsAuthOAuth2Config::ConfigType c
     case QgsAuthOAuth2Config::Custom:
       return tr( "Custom" );
     case QgsAuthOAuth2Config::Predefined:
+    default:
       return tr( "Predefined" );
   }
 }
@@ -749,6 +750,7 @@ QString QgsAuthOAuth2Config::grantFlowString( QgsAuthOAuth2Config::GrantFlow flo
     case QgsAuthOAuth2Config::Implicit:
       return tr( "Implicit" );
     case QgsAuthOAuth2Config::ResourceOwner:
+    default:
       return tr( "Resource Owner" );
   }
 }
@@ -763,6 +765,7 @@ QString QgsAuthOAuth2Config::accessMethodString( QgsAuthOAuth2Config::AccessMeth
     case QgsAuthOAuth2Config::Form:
       return tr( "Form (POST only)" );
     case QgsAuthOAuth2Config::Query:
+    default:
       return tr( "URL Query" );
   }
 }

diff --git a/src/auth/oauth2/qgsauthoauth2config.h b/src/auth/oauth2/qgsauthoauth2config.h
@@ -203,7 +203,7 @@ class QgsAuthOAuth2Config : public QObject
       bool *ok = nullptr );
 
     //! Load and parse standard directories of configs (e.g. JSON) to a mapped cache
-    static QgsStringMap mappedOAuth2ConfigsCache( const QString &extradir = QString::null );
+    static QgsStringMap mappedOAuth2ConfigsCache( QObject *parent, const QString &extradir = QString::null );
 
     //!
     static QString oauth2ConfigsPkgDataDir();

diff --git a/src/auth/oauth2/qgsauthoauth2edit.cpp b/src/auth/oauth2/qgsauthoauth2edit.cpp
@@ -623,7 +623,7 @@ void QgsAuthOAuth2Edit::updateDefinedConfigsCache()
 {
   QString extradir = leDefinedDirPath->text();
   mDefinedConfigsCache.clear();
-  mDefinedConfigsCache = QgsAuthOAuth2Config::mappedOAuth2ConfigsCache( extradir );
+  mDefinedConfigsCache = QgsAuthOAuth2Config::mappedOAuth2ConfigsCache( this, extradir );
 }
 
 // slot

diff --git a/src/auth/oauth2/qgsauthoauth2method.cpp b/src/auth/oauth2/qgsauthoauth2method.cpp
@@ -33,6 +33,7 @@
 #include <QDir>
 #include <QEventLoop>
 #include <QString>
+#include <QMutexLocker>
 
 
 static const QString AUTH_METHOD_KEY = QStringLiteral( "OAuth2" );
@@ -105,6 +106,8 @@ bool QgsAuthOAuth2Method::updateNetworkRequest( QNetworkRequest &request, const
 {
   Q_UNUSED( dataprovider )
 
+  QMutexLocker locker( &mNetworkRequestMutex );
+
   QString msg;
 
   QgsO2 *o2 = getOAuth2Bundle( authcfg );
@@ -144,7 +147,7 @@ bool QgsAuthOAuth2Method::updateNetworkRequest( QNetworkRequest &request, const
 
       // Try to get a refresh token first
       // go into local event loop and wait for a fired refresh-related slot
-      QEventLoop rloop( qApp );
+      QEventLoop rloop( nullptr );
       connect( o2, &QgsO2::refreshFinished, &rloop, &QEventLoop::quit );
 
       // Asynchronously attempt the refresh
@@ -182,13 +185,13 @@ bool QgsAuthOAuth2Method::updateNetworkRequest( QNetworkRequest &request, const
     settings.setValue( timeoutkey, reqtimeout );
 
     // go into local event loop and wait for a fired linking-related slot
-    QEventLoop loop( qApp );
+    QEventLoop loop( nullptr );
     connect( o2, &QgsO2::linkingFailed, &loop, &QEventLoop::quit );
     connect( o2, &QgsO2::linkingSucceeded, &loop, &QEventLoop::quit );
 
     // add singlshot timer to quit linking after an alloted timeout
     // this should keep the local event loop from blocking forever
-    QTimer timer( this );
+    QTimer timer( nullptr );
     timer.setInterval( reqtimeout * 5 );
     timer.setSingleShot( true );
     connect( &timer, &QTimer::timeout, o2, &QgsO2::linkingFailed );
@@ -270,6 +273,7 @@ bool QgsAuthOAuth2Method::updateNetworkRequest( QNetworkRequest &request, const
 bool QgsAuthOAuth2Method::updateNetworkReply( QNetworkReply *reply, const QString &authcfg, const QString &dataprovider )
 {
   Q_UNUSED( dataprovider )
+  QMutexLocker locker( &mNetworkRequestMutex );
 
   // TODO: handle token refresh error on the reply, see O2Requestor::onRequestError()
   // Is this doable if the errors are also handled in qgsapp (and/or elsewhere)?
@@ -389,6 +393,7 @@ void QgsAuthOAuth2Method::onReplyFinished()
 
 void QgsAuthOAuth2Method::onNetworkError( QNetworkReply::NetworkError err )
 {
+  QMutexLocker locker( &mNetworkRequestMutex );
   QString msg;
   QNetworkReply *reply = qobject_cast<QNetworkReply *>( sender() );
   if ( !reply )
@@ -489,7 +494,7 @@ QgsO2 *QgsAuthOAuth2Method::getOAuth2Bundle( const QString &authcfg, bool fullco
     return sOAuth2ConfigCache.value( authcfg );
   }
 
-  QgsAuthOAuth2Config *config = new QgsAuthOAuth2Config( qApp );
+  QgsAuthOAuth2Config *config = new QgsAuthOAuth2Config( );
   QgsO2 *nullbundle =  nullptr;
 
   // else build oauth2 config
@@ -542,7 +547,7 @@ QgsO2 *QgsAuthOAuth2Method::getOAuth2Bundle( const QString &authcfg, bool fullco
       QgsDebugMsg( QStringLiteral( "No custom defined dir path to load OAuth2 config" ) );
     }
 
-    QgsStringMap definedcache = QgsAuthOAuth2Config::mappedOAuth2ConfigsCache( extradir );
+    QgsStringMap definedcache = QgsAuthOAuth2Config::mappedOAuth2ConfigsCache( this, extradir );
 
     if ( !definedcache.contains( definedid ) )
     {
@@ -595,7 +600,7 @@ QgsO2 *QgsAuthOAuth2Method::getOAuth2Bundle( const QString &authcfg, bool fullco
   QgsDebugMsg( QStringLiteral( "Loading authenticator object with %1 flow properties of OAuth2 config: %2" )
                .arg( QgsAuthOAuth2Config::grantFlowString( config->grantFlow() ), authcfg ) );
 
-  QgsO2 *o2 = new QgsO2( authcfg, config, qApp, QgsNetworkAccessManager::instance() );
+  QgsO2 *o2 = new QgsO2( authcfg, config, nullptr, QgsNetworkAccessManager::instance() );
 
   // cache bundle
   putOAuth2Bundle( authcfg, o2 );

diff --git a/src/auth/oauth2/qgsauthoauth2method.h b/src/auth/oauth2/qgsauthoauth2method.h
@@ -20,6 +20,7 @@
 #include <QDialog>
 #include <QEventLoop>
 #include <QTimer>
+#include <QMutex>
 
 #include "qgsauthmethod.h"
 
@@ -77,6 +78,8 @@ class QgsAuthOAuth2Method : public QgsAuthMethod
     static QMap<QString, QgsO2 *> sOAuth2ConfigCache;
 
     QgsO2 *authO2( const QString &authcfg );
+
+    QMutex mNetworkRequestMutex;
 };
 
 #endif // QGSAUTHOAUTH2METHOD_H