Skip to content

Commit cdafca5

Browse files
committedJun 25, 2018
[auth] Add mutex; remove qApp parents; do not set parent in wrong thread
1 parent 5ee767a commit cdafca5

File tree

5 files changed

+21
-10
lines changed

5 files changed

+21
-10
lines changed
 

‎src/auth/oauth2/qgsauthoauth2config.cpp

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -676,7 +676,7 @@ QgsStringMap QgsAuthOAuth2Config::mapOAuth2Configs(
676676
return configs;
677677
}
678678

679-
QgsStringMap QgsAuthOAuth2Config::mappedOAuth2ConfigsCache( const QString &extradir )
679+
QgsStringMap QgsAuthOAuth2Config::mappedOAuth2ConfigsCache(QObject *parent, const QString &extradir )
680680
{
681681
QgsStringMap configs;
682682
bool ok = false;
@@ -701,7 +701,7 @@ QgsStringMap QgsAuthOAuth2Config::mappedOAuth2ConfigsCache( const QString &extra
701701
continue;
702702
}
703703
QgsStringMap newconfigs = QgsAuthOAuth2Config::mapOAuth2Configs(
704-
configdirinfo.canonicalFilePath(), qApp, QgsAuthOAuth2Config::JSON, &ok );
704+
configdirinfo.canonicalFilePath(), parent, QgsAuthOAuth2Config::JSON, &ok );
705705
if ( ok )
706706
{
707707
QgsStringMap::const_iterator i = newconfigs.constBegin();
@@ -735,6 +735,7 @@ QString QgsAuthOAuth2Config::configTypeString( QgsAuthOAuth2Config::ConfigType c
735735
case QgsAuthOAuth2Config::Custom:
736736
return tr( "Custom" );
737737
case QgsAuthOAuth2Config::Predefined:
738+
default:
738739
return tr( "Predefined" );
739740
}
740741
}
@@ -749,6 +750,7 @@ QString QgsAuthOAuth2Config::grantFlowString( QgsAuthOAuth2Config::GrantFlow flo
749750
case QgsAuthOAuth2Config::Implicit:
750751
return tr( "Implicit" );
751752
case QgsAuthOAuth2Config::ResourceOwner:
753+
default:
752754
return tr( "Resource Owner" );
753755
}
754756
}
@@ -763,6 +765,7 @@ QString QgsAuthOAuth2Config::accessMethodString( QgsAuthOAuth2Config::AccessMeth
763765
case QgsAuthOAuth2Config::Form:
764766
return tr( "Form (POST only)" );
765767
case QgsAuthOAuth2Config::Query:
768+
default:
766769
return tr( "URL Query" );
767770
}
768771
}

‎src/auth/oauth2/qgsauthoauth2config.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,7 @@ class QgsAuthOAuth2Config : public QObject
203203
bool *ok = nullptr );
204204

205205
//! Load and parse standard directories of configs (e.g. JSON) to a mapped cache
206-
static QgsStringMap mappedOAuth2ConfigsCache( const QString &extradir = QString::null );
206+
static QgsStringMap mappedOAuth2ConfigsCache( QObject *parent, const QString &extradir = QString::null );
207207

208208
//!
209209
static QString oauth2ConfigsPkgDataDir();

‎src/auth/oauth2/qgsauthoauth2edit.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -623,7 +623,7 @@ void QgsAuthOAuth2Edit::updateDefinedConfigsCache()
623623
{
624624
QString extradir = leDefinedDirPath->text();
625625
mDefinedConfigsCache.clear();
626-
mDefinedConfigsCache = QgsAuthOAuth2Config::mappedOAuth2ConfigsCache( extradir );
626+
mDefinedConfigsCache = QgsAuthOAuth2Config::mappedOAuth2ConfigsCache( this, extradir );
627627
}
628628

629629
// slot

‎src/auth/oauth2/qgsauthoauth2method.cpp

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
#include <QDir>
3434
#include <QEventLoop>
3535
#include <QString>
36+
#include <QMutexLocker>
3637

3738

3839
static const QString AUTH_METHOD_KEY = QStringLiteral( "OAuth2" );
@@ -105,6 +106,8 @@ bool QgsAuthOAuth2Method::updateNetworkRequest( QNetworkRequest &request, const
105106
{
106107
Q_UNUSED( dataprovider )
107108

109+
QMutexLocker locker( &mNetworkRequestMutex );
110+
108111
QString msg;
109112

110113
QgsO2 *o2 = getOAuth2Bundle( authcfg );
@@ -144,7 +147,7 @@ bool QgsAuthOAuth2Method::updateNetworkRequest( QNetworkRequest &request, const
144147

145148
// Try to get a refresh token first
146149
// go into local event loop and wait for a fired refresh-related slot
147-
QEventLoop rloop( qApp );
150+
QEventLoop rloop( nullptr );
148151
connect( o2, &QgsO2::refreshFinished, &rloop, &QEventLoop::quit );
149152

150153
// Asynchronously attempt the refresh
@@ -182,13 +185,13 @@ bool QgsAuthOAuth2Method::updateNetworkRequest( QNetworkRequest &request, const
182185
settings.setValue( timeoutkey, reqtimeout );
183186

184187
// go into local event loop and wait for a fired linking-related slot
185-
QEventLoop loop( qApp );
188+
QEventLoop loop( nullptr );
186189
connect( o2, &QgsO2::linkingFailed, &loop, &QEventLoop::quit );
187190
connect( o2, &QgsO2::linkingSucceeded, &loop, &QEventLoop::quit );
188191

189192
// add singlshot timer to quit linking after an alloted timeout
190193
// this should keep the local event loop from blocking forever
191-
QTimer timer( this );
194+
QTimer timer( nullptr );
192195
timer.setInterval( reqtimeout * 5 );
193196
timer.setSingleShot( true );
194197
connect( &timer, &QTimer::timeout, o2, &QgsO2::linkingFailed );
@@ -270,6 +273,7 @@ bool QgsAuthOAuth2Method::updateNetworkRequest( QNetworkRequest &request, const
270273
bool QgsAuthOAuth2Method::updateNetworkReply( QNetworkReply *reply, const QString &authcfg, const QString &dataprovider )
271274
{
272275
Q_UNUSED( dataprovider )
276+
QMutexLocker locker( &mNetworkRequestMutex );
273277

274278
// TODO: handle token refresh error on the reply, see O2Requestor::onRequestError()
275279
// Is this doable if the errors are also handled in qgsapp (and/or elsewhere)?
@@ -389,6 +393,7 @@ void QgsAuthOAuth2Method::onReplyFinished()
389393

390394
void QgsAuthOAuth2Method::onNetworkError( QNetworkReply::NetworkError err )
391395
{
396+
QMutexLocker locker( &mNetworkRequestMutex );
392397
QString msg;
393398
QNetworkReply *reply = qobject_cast<QNetworkReply *>( sender() );
394399
if ( !reply )
@@ -489,7 +494,7 @@ QgsO2 *QgsAuthOAuth2Method::getOAuth2Bundle( const QString &authcfg, bool fullco
489494
return sOAuth2ConfigCache.value( authcfg );
490495
}
491496

492-
QgsAuthOAuth2Config *config = new QgsAuthOAuth2Config( qApp );
497+
QgsAuthOAuth2Config *config = new QgsAuthOAuth2Config( );
493498
QgsO2 *nullbundle = nullptr;
494499

495500
// else build oauth2 config
@@ -542,7 +547,7 @@ QgsO2 *QgsAuthOAuth2Method::getOAuth2Bundle( const QString &authcfg, bool fullco
542547
QgsDebugMsg( QStringLiteral( "No custom defined dir path to load OAuth2 config" ) );
543548
}
544549

545-
QgsStringMap definedcache = QgsAuthOAuth2Config::mappedOAuth2ConfigsCache( extradir );
550+
QgsStringMap definedcache = QgsAuthOAuth2Config::mappedOAuth2ConfigsCache( this, extradir );
546551

547552
if ( !definedcache.contains( definedid ) )
548553
{
@@ -595,7 +600,7 @@ QgsO2 *QgsAuthOAuth2Method::getOAuth2Bundle( const QString &authcfg, bool fullco
595600
QgsDebugMsg( QStringLiteral( "Loading authenticator object with %1 flow properties of OAuth2 config: %2" )
596601
.arg( QgsAuthOAuth2Config::grantFlowString( config->grantFlow() ), authcfg ) );
597602

598-
QgsO2 *o2 = new QgsO2( authcfg, config, qApp, QgsNetworkAccessManager::instance() );
603+
QgsO2 *o2 = new QgsO2( authcfg, config, nullptr, QgsNetworkAccessManager::instance() );
599604

600605
// cache bundle
601606
putOAuth2Bundle( authcfg, o2 );

‎src/auth/oauth2/qgsauthoauth2method.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
#include <QDialog>
2121
#include <QEventLoop>
2222
#include <QTimer>
23+
#include <QMutex>
2324

2425
#include "qgsauthmethod.h"
2526

@@ -77,6 +78,8 @@ class QgsAuthOAuth2Method : public QgsAuthMethod
7778
static QMap<QString, QgsO2 *> sOAuth2ConfigCache;
7879

7980
QgsO2 *authO2( const QString &authcfg );
81+
82+
QMutex mNetworkRequestMutex;
8083
};
8184

8285
#endif // QGSAUTHOAUTH2METHOD_H

0 commit comments

Comments
 (0)
Please sign in to comment.