Skip to content

Commit

Permalink
use individual network managers for threads (fixes #13721, fixes #14401
Browse files Browse the repository at this point in the history
…, implements #14192)
  • Loading branch information
jef-n committed Mar 5, 2016
1 parent 26d6195 commit 2eb8243
Show file tree
Hide file tree
Showing 18 changed files with 341 additions and 271 deletions.
19 changes: 0 additions & 19 deletions python/core/qgsnetworkaccessmanager.sip
Original file line number Diff line number Diff line change
Expand Up @@ -62,29 +62,10 @@ class QgsNetworkAccessManager : QNetworkAccessManager

bool useSystemProxy();

public slots:

This comment has been minimized.

Copy link
@Gustry

Gustry Apr 15, 2016

Contributor

These lines has been removed from the public API. Some plugins were using these public slots (InaSAFE, QuickOSM ...).
The API shouldn't change between two minor releases (2.14.0 and 2.14.1) and I think that before to remove public slots, they should be tagged deprecated for a few months @jef-n .

/** Send GET request, calls get().
* Emits requestSent().
* @param request request to be sent
*/
void sendGet( const QNetworkRequest & request );
/** Abort and delete reply. This slot may be used to abort reply created by instance of this class
* (and which was not moved to another thread) from a different thread. Such reply cannot
* be aborted directly from a different thread. The reply must be also deleted
* in this slot, otherwise it could happen that abort signal comes after the reply was deleted.
* @param reply reply to be aborted.
*/
void deleteReply( QNetworkReply * reply );

signals:
void requestAboutToBeCreated( QNetworkAccessManager::Operation, const QNetworkRequest &, QIODevice * );
void requestCreated( QNetworkReply * );
void requestTimedOut( QNetworkReply * );
/** Emitted when request was sent by request()
* @param reply request reply
* @param sender the object which called request() slot.
*/
void requestSent( QNetworkReply * reply, QObject *sender );

protected:
virtual QNetworkReply *createRequest( QNetworkAccessManager::Operation op, const QNetworkRequest &req, QIODevice *outgoingData = 0 );
Expand Down
69 changes: 42 additions & 27 deletions src/app/qgisapp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,6 @@
#include <QNetworkReply>
#include <QNetworkProxy>
#include <QAuthenticator>
#include <QNetworkDiskCache>

//
// Mac OS X Includes
Expand Down Expand Up @@ -398,7 +397,7 @@ static void setTitleBarText_( QWidget & qgisApp )
*/
static QgsMessageOutput *messageOutputViewer_()
{
if ( QThread::currentThread() == QApplication::instance()->thread() )
if ( QThread::currentThread() == qApp->thread() )
return new QgsMessageViewer( QgisApp::instance() );
else
return new QgsMessageOutputConsole();
Expand Down Expand Up @@ -10864,6 +10863,8 @@ void QgisApp::namSetup()

void QgisApp::namAuthenticationRequired( QNetworkReply *reply, QAuthenticator *auth )
{
Q_ASSERT( qApp->thread() == QThread::currentThread() );

QString username = auth->user();
QString password = auth->password();

Expand All @@ -10882,31 +10883,38 @@ void QgisApp::namAuthenticationRequired( QNetworkReply *reply, QAuthenticator *a
}
}

for ( ;; )
{
QMutexLocker lock( QgsCredentials::instance()->mutex() );
bool ok;

for ( ;; )
{
bool ok = QgsCredentials::instance()->get(
QString( "%1 at %2" ).arg( auth->realm(), reply->url().host() ),
username, password,
tr( "Authentication required" ) );
if ( !ok )
return;
QMutexLocker lock( QgsCredentials::instance()->mutex() );
ok = QgsCredentials::instance()->get(
QString( "%1 at %2" ).arg( auth->realm(), reply->url().host() ),
username, password,
tr( "Authentication required" ) );
}
if ( !ok )
return;

if ( reply->isFinished() )
return;
if ( reply->isFinished() )
return;

if ( auth->user() != username || ( password != auth->password() && !password.isNull() ) )
break;
if ( auth->user() != username || ( password != auth->password() && !password.isNull() ) )
break;

// credentials didn't change - stored ones probably wrong? clear password and retry
// credentials didn't change - stored ones probably wrong? clear password and retry
{
QMutexLocker lock( QgsCredentials::instance()->mutex() );
QgsCredentials::instance()->put(
QString( "%1 at %2" ).arg( auth->realm(), reply->url().host() ),
username, QString::null );
}
}

// save credentials
// save credentials
{
QMutexLocker lock( QgsCredentials::instance()->mutex() );
QgsCredentials::instance()->put(
QString( "%1 at %2" ).arg( auth->realm(), reply->url().host() ),
username, password
Expand All @@ -10930,27 +10938,34 @@ void QgisApp::namProxyAuthenticationRequired( const QNetworkProxy &proxy, QAuthe
QString username = auth->user();
QString password = auth->password();

for ( ;; )
{
QMutexLocker lock( QgsCredentials::instance()->mutex() );
bool ok;

for ( ;; )
{
bool ok = QgsCredentials::instance()->get(
QString( "proxy %1:%2 [%3]" ).arg( proxy.hostName() ).arg( proxy.port() ).arg( auth->realm() ),
username, password,
tr( "Proxy authentication required" ) );
if ( !ok )
return;
QMutexLocker lock( QgsCredentials::instance()->mutex() );
ok = QgsCredentials::instance()->get(
QString( "proxy %1:%2 [%3]" ).arg( proxy.hostName() ).arg( proxy.port() ).arg( auth->realm() ),
username, password,
tr( "Proxy authentication required" ) );
}
if ( !ok )
return;

if ( auth->user() != username || ( password != auth->password() && !password.isNull() ) )
break;
if ( auth->user() != username || ( password != auth->password() && !password.isNull() ) )
break;

// credentials didn't change - stored ones probably wrong? clear password and retry
// credentials didn't change - stored ones probably wrong? clear password and retry
{
QMutexLocker lock( QgsCredentials::instance()->mutex() );
QgsCredentials::instance()->put(
QString( "proxy %1:%2 [%3]" ).arg( proxy.hostName() ).arg( proxy.port() ).arg( auth->realm() ),
username, QString::null );
}
}

{
QMutexLocker lock( QgsCredentials::instance()->mutex() );
QgsCredentials::instance()->put(
QString( "proxy %1:%2 [%3]" ).arg( proxy.hostName() ).arg( proxy.port() ).arg( auth->realm() ),
username, password
Expand Down
2 changes: 2 additions & 0 deletions src/core/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ SET(QGIS_CORE_SRCS
qgsmimedatautils.cpp
qgsmultirenderchecker.cpp
qgsnetworkaccessmanager.cpp
qgsnetworkdiskcache.cpp
qgsnetworkcontentfetcher.cpp
qgsnetworkreplyparser.cpp
qgsobjectcustomproperties.cpp
Expand Down Expand Up @@ -452,6 +453,7 @@ SET(QGIS_CORE_MOC_HDRS
qgsmessagelog.h
qgsmessageoutput.h
qgsnetworkaccessmanager.h
qgsnetworkdiskcache.h
qgsnetworkcontentfetcher.h
qgsnetworkreplyparser.h
qgsofflineediting.h
Expand Down
72 changes: 34 additions & 38 deletions src/core/qgsnetworkaccessmanager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,17 @@
#include <QSettings>
#include <QTimer>
#include <QNetworkReply>
#include <QNetworkDiskCache>
#include <QThreadStorage>

#ifndef QT_NO_OPENSSL
#include <QSslConfiguration>
#endif

#include "qgsnetworkdiskcache.h"
#include "qgsauthmanager.h"

QgsNetworkAccessManager *QgsNetworkAccessManager::smMainNAM = 0;

/// @cond PRIVATE
class QgsNetworkProxyFactory : public QNetworkProxyFactory
{
Expand Down Expand Up @@ -99,13 +102,22 @@ class QgsNetworkProxyFactory : public QNetworkProxyFactory
//
QgsNetworkAccessManager* QgsNetworkAccessManager::instance()
{
static QgsNetworkAccessManager* sInstance( new QgsNetworkAccessManager( QApplication::instance() ) );
return sInstance;
static QThreadStorage<QgsNetworkAccessManager> sInstances;
QgsNetworkAccessManager *nam = &sInstances.localData();

if ( nam->thread() == qApp->thread() )
smMainNAM = nam;

if ( !nam->mInitialized )
nam->setupDefaultProxyAndCache();

return nam;
}

QgsNetworkAccessManager::QgsNetworkAccessManager( QObject *parent )
: QNetworkAccessManager( parent )
, mUseSystemProxy( false )
, mInitialized( false )
{
setProxyFactory( new QgsNetworkProxyFactory() );
}
Expand Down Expand Up @@ -221,6 +233,8 @@ void QgsNetworkAccessManager::abortRequest()
QNetworkReply *reply = qobject_cast<QNetworkReply *>( timer->parent() );
Q_ASSERT( reply );

QgsDebugMsg( QString( "Abort [reply:%1]" ).arg(( qint64 ) reply, 0, 16 ) );

QgsMessageLog::logMessage( tr( "Network request %1 timed out" ).arg( reply->url().toString() ), tr( "Network" ) );

if ( reply->isRunning() )
Expand Down Expand Up @@ -270,36 +284,36 @@ QNetworkRequest::CacheLoadControl QgsNetworkAccessManager::cacheLoadControlFromN

void QgsNetworkAccessManager::setupDefaultProxyAndCache()
{
QNetworkProxy proxy;
QStringList excludes;

QSettings settings;

mInitialized = true;
mUseSystemProxy = false;

if ( this != instance() )
{
Qt::ConnectionType connectionType = thread() == instance()->thread() ? Qt::AutoConnection : Qt::BlockingQueuedConnection;
Q_ASSERT( smMainNAM );

if ( smMainNAM != this )
{
connect( this, SIGNAL( authenticationRequired( QNetworkReply *, QAuthenticator * ) ),
instance(), SIGNAL( authenticationRequired( QNetworkReply *, QAuthenticator * ) ),
connectionType );
smMainNAM, SIGNAL( authenticationRequired( QNetworkReply *, QAuthenticator * ) ),
Qt::BlockingQueuedConnection );

connect( this, SIGNAL( proxyAuthenticationRequired( const QNetworkProxy &, QAuthenticator * ) ),
instance(), SIGNAL( proxyAuthenticationRequired( const QNetworkProxy &, QAuthenticator * ) ),
connectionType );
smMainNAM, SIGNAL( proxyAuthenticationRequired( const QNetworkProxy &, QAuthenticator * ) ),
Qt::BlockingQueuedConnection );

connect( this, SIGNAL( requestTimedOut( QNetworkReply* ) ),
instance(), SIGNAL( requestTimedOut( QNetworkReply* ) ) );
smMainNAM, SIGNAL( requestTimedOut( QNetworkReply* ) ) );

#ifndef QT_NO_OPENSSL
connect( this, SIGNAL( sslErrors( QNetworkReply *, const QList<QSslError> & ) ),
instance(), SIGNAL( sslErrors( QNetworkReply *, const QList<QSslError> & ) ),
connectionType );
smMainNAM, SIGNAL( sslErrors( QNetworkReply *, const QList<QSslError> & ) ),
Qt::BlockingQueuedConnection );
#endif
}

// check if proxy is enabled
QSettings settings;
QNetworkProxy proxy;
QStringList excludes;

bool proxyEnabled = settings.value( "proxy/proxyEnabled", false ).toBool();
if ( proxyEnabled )
{
Expand Down Expand Up @@ -354,9 +368,9 @@ void QgsNetworkAccessManager::setupDefaultProxyAndCache()

setFallbackProxyAndExcludes( proxy, excludes );

QNetworkDiskCache *newcache = qobject_cast<QNetworkDiskCache*>( cache() );
QgsNetworkDiskCache *newcache = qobject_cast<QgsNetworkDiskCache*>( cache() );
if ( !newcache )
newcache = new QNetworkDiskCache( this );
newcache = new QgsNetworkDiskCache( this );

QString cacheDirectory = settings.value( "cache/directory", QgsApplication::qgisSettingsDirPath() + "cache" ).toString();
qint64 cacheSize = settings.value( "cache/size", 50 * 1024 * 1024 ).toULongLong();
Expand All @@ -370,21 +384,3 @@ void QgsNetworkAccessManager::setupDefaultProxyAndCache()
if ( cache() != newcache )
setCache( newcache );
}

void QgsNetworkAccessManager::sendGet( const QNetworkRequest & request )
{
QgsDebugMsg( "Entered" );
QNetworkReply * reply = get( request );
emit requestSent( reply, QObject::sender() );
}

void QgsNetworkAccessManager::deleteReply( QNetworkReply * reply )
{
QgsDebugMsg( "Entered" );
if ( !reply )
{
return;
}
reply->abort();
reply->deleteLater();
}
22 changes: 3 additions & 19 deletions src/core/qgsnetworkaccessmanager.h
Original file line number Diff line number Diff line change
Expand Up @@ -82,31 +82,13 @@ class CORE_EXPORT QgsNetworkAccessManager : public QNetworkAccessManager
//! Setup the NAM according to the user's settings
void setupDefaultProxyAndCache();

//! return whether the system proxy should be used
bool useSystemProxy() { return mUseSystemProxy; }

public slots:
/** Send GET request, calls get().
* Emits requestSent().
* @param request request to be sent
*/
void sendGet( const QNetworkRequest & request );
/** Abort and delete reply. This slot may be used to abort reply created by instance of this class
* (and which was not moved to another thread) from a different thread. Such reply cannot
* be aborted directly from a different thread. The reply must be also deleted
* in this slot, otherwise it could happen that abort signal comes after the reply was deleted.
* @param reply reply to be aborted.
*/
void deleteReply( QNetworkReply * reply );

signals:
void requestAboutToBeCreated( QNetworkAccessManager::Operation, const QNetworkRequest &, QIODevice * );
void requestCreated( QNetworkReply * );
void requestTimedOut( QNetworkReply * );
/** Emitted when request was sent by request()
* @param reply request reply
* @param sender the object which called request() slot.
*/
void requestSent( QNetworkReply * reply, QObject *sender );

private slots:
void abortRequest();
Expand All @@ -119,6 +101,8 @@ class CORE_EXPORT QgsNetworkAccessManager : public QNetworkAccessManager
QNetworkProxy mFallbackProxy;
QStringList mExcludedURLs;
bool mUseSystemProxy;
bool mInitialized;
static QgsNetworkAccessManager *smMainNAM;
};

#endif // QGSNETWORKACCESSMANAGER_H
Expand Down
Loading

2 comments on commit 2eb8243

@m-kuhn
Copy link
Member

@m-kuhn m-kuhn commented on 2eb8243 Mar 5, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot @jef-n !

@nirvn
Copy link
Contributor

@nirvn nirvn commented on 2eb8243 Mar 6, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jef-n big thanks.

Please sign in to comment.