Skip to content
Permalink
Browse files
use signals/slots between QgsWmsCapabilitiesDownload and QgsNetworkAc…
…cessManager, fixes crash on start up in #13271
  • Loading branch information
blazek committed Sep 1, 2015
1 parent 4e60022 commit e95bf6d3e016fe8484b47fd1c54606601875054c
@@ -341,3 +341,20 @@ void QgsNetworkAccessManager::setupDefaultProxyAndCache()
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();
}
@@ -84,10 +84,29 @@ class CORE_EXPORT QgsNetworkAccessManager : public QNetworkAccessManager

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 * );
/** Emited 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();
@@ -1859,6 +1859,7 @@ QgsWmsCapabilitiesDownload::QgsWmsCapabilitiesDownload( QObject *parent )
, mCapabilitiesReply( 0 )
, mIsAborted( false )
{
connectManager();
}

QgsWmsCapabilitiesDownload::QgsWmsCapabilitiesDownload( const QString& baseUrl, const QgsWmsAuthorization& auth, QObject *parent )
@@ -1868,6 +1869,22 @@ QgsWmsCapabilitiesDownload::QgsWmsCapabilitiesDownload( const QString& baseUrl,
, mCapabilitiesReply( 0 )
, mIsAborted( false )
{
connectManager();
}

void QgsWmsCapabilitiesDownload::connectManager()
{
// The instance of this class may live on a thread different from QgsNetworkAccessManager instance's thread,
// so we cannot call QgsNetworkAccessManager::get() directly and we must send a signal instead.
connect( this, SIGNAL( sendRequest( const QNetworkRequest & ) ),
QgsNetworkAccessManager::instance(), SLOT( sendGet( const QNetworkRequest & ) ) );
connect( this, SIGNAL( deleteReply( QNetworkReply * ) ),
QgsNetworkAccessManager::instance(), SLOT( deleteReply( QNetworkReply * ) ) );
}

QgsWmsCapabilitiesDownload::~QgsWmsCapabilitiesDownload()
{
abort();
}

bool QgsWmsCapabilitiesDownload::downloadCapabilities( const QString& baseUrl, const QgsWmsAuthorization& auth )
@@ -1880,7 +1897,9 @@ bool QgsWmsCapabilitiesDownload::downloadCapabilities( const QString& baseUrl, c
bool QgsWmsCapabilitiesDownload::downloadCapabilities()
{
QgsDebugMsg( "entering." );
abort(); // cancel previous
mIsAborted = false;

QString url = mBaseUrl;
QgsDebugMsg( "url = " + url );
if ( !url.contains( "SERVICE=WMTS" ) &&
@@ -1896,17 +1915,9 @@ bool QgsWmsCapabilitiesDownload::downloadCapabilities()
request.setAttribute( QNetworkRequest::CacheLoadControlAttribute, QNetworkRequest::PreferNetwork );
request.setAttribute( QNetworkRequest::CacheSaveControlAttribute, true );

QgsDebugMsg( QString( "getcapabilities: %1" ).arg( url ) );
// This is causing Qt warning: "Cannot create children for a parent that is in a different thread."
// but it only means that the reply will have no parent
if ( mIsAborted )
{
return false;
}
mCapabilitiesReply = QgsNetworkAccessManager::instance()->get( request );

connect( mCapabilitiesReply, SIGNAL( finished() ), this, SLOT( capabilitiesReplyFinished() ), Qt::DirectConnection );
connect( mCapabilitiesReply, SIGNAL( downloadProgress( qint64, qint64 ) ), this, SLOT( capabilitiesReplyProgress( qint64, qint64 ) ), Qt::DirectConnection );
connect( QgsNetworkAccessManager::instance(), SIGNAL( requestSent( QNetworkReply *, QObject * ) ),
SLOT( requestSent( QNetworkReply *, QObject * ) ) );
emit sendRequest( request );

QEventLoop loop;
connect( this, SIGNAL( downloadFinished() ), &loop, SLOT( quit() ) );
@@ -1915,13 +1926,41 @@ bool QgsWmsCapabilitiesDownload::downloadCapabilities()
return mError.isEmpty();
}

void QgsWmsCapabilitiesDownload::requestSent( QNetworkReply * reply, QObject *sender )
{
QgsDebugMsg( "Entered" );
if ( sender != this ) // it is not our reply
{
return;
}
disconnect( QgsNetworkAccessManager::instance(), SIGNAL( requestSent( QNetworkReply *, QObject * ) ),
this, SLOT( requestSent( QNetworkReply *, QObject * ) ) );

if ( !reply )
{
emit downloadFinished();
return;
}
if ( mIsAborted )
{
emit deleteReply( reply );
emit downloadFinished();
return;
}
// Note: the reply was created on QgsNetworkAccessManager's thread
mCapabilitiesReply = reply;
connect( mCapabilitiesReply, SIGNAL( finished() ), this, SLOT( capabilitiesReplyFinished() ), Qt::DirectConnection );
connect( mCapabilitiesReply, SIGNAL( downloadProgress( qint64, qint64 ) ), this, SLOT( capabilitiesReplyProgress( qint64, qint64 ) ), Qt::DirectConnection );
}

void QgsWmsCapabilitiesDownload::abort()
{
QgsDebugMsg( "Entered" );
mIsAborted = true;
if ( mCapabilitiesReply )
{
mCapabilitiesReply->abort();
emit deleteReply( mCapabilitiesReply );
mCapabilitiesReply = 0;
}
}

@@ -1935,7 +1974,7 @@ void QgsWmsCapabilitiesDownload::capabilitiesReplyProgress( qint64 bytesReceived
void QgsWmsCapabilitiesDownload::capabilitiesReplyFinished()
{
QgsDebugMsg( "entering." );
if ( !mIsAborted )
if ( !mIsAborted && mCapabilitiesReply )
{
if ( mCapabilitiesReply->error() == QNetworkReply::NoError )
{
@@ -1962,10 +2001,8 @@ void QgsWmsCapabilitiesDownload::capabilitiesReplyFinished()

mCapabilitiesReply->deleteLater();
QgsDebugMsg( QString( "redirected getcapabilities: %1" ).arg( redirect.toString() ) );
mCapabilitiesReply = QgsNetworkAccessManager::instance()->get( request );

connect( mCapabilitiesReply, SIGNAL( finished() ), this, SLOT( capabilitiesReplyFinished() ) );
connect( mCapabilitiesReply, SIGNAL( downloadProgress( qint64, qint64 ) ), this, SLOT( capabilitiesReplyProgress( qint64, qint64 ) ) );
//mCapabilitiesReply = QgsNetworkAccessManager::instance()->get( request );
emit sendRequest( request );
return;
}
}
@@ -1987,8 +2024,11 @@ void QgsWmsCapabilitiesDownload::capabilitiesReplyFinished()
}
}

mCapabilitiesReply->deleteLater();
mCapabilitiesReply = 0;
if ( mCapabilitiesReply )
{
mCapabilitiesReply->deleteLater();
mCapabilitiesReply = 0;
}

emit downloadFinished();
}
@@ -677,7 +677,13 @@ class QgsWmsCapabilities



/** Class that handles download of capabilities */
/** Class that handles download of capabilities.
* Methods of this class may only be called directly from the thread to which instance of the class has affinity.
* It is possible to connect to abort() slot from another thread however.
*/
/* The requirement to call methods only from the thread to which this class instance has affinity guarantees that
* abort() cannot be called in the middle of another method and makes it simple to check if the request was aborted.
*/
class QgsWmsCapabilitiesDownload : public QObject
{
Q_OBJECT
@@ -687,6 +693,8 @@ class QgsWmsCapabilitiesDownload : public QObject

QgsWmsCapabilitiesDownload( const QString& baseUrl, const QgsWmsAuthorization& auth, QObject* parent = 0 );

virtual ~QgsWmsCapabilitiesDownload();

bool downloadCapabilities();

bool downloadCapabilities( const QString& baseUrl, const QgsWmsAuthorization& auth );
@@ -695,16 +703,25 @@ class QgsWmsCapabilitiesDownload : public QObject

QByteArray response() const { return mHttpCapabilitiesResponse; }

public slots:
/** Abort network request immediately */
void abort();

signals:
/** \brief emit a signal to be caught by qgisapp and display a msg on status bar */
void statusChanged( QString const & theStatusQString );

/** \brief emit a signal once the download is finished */
void downloadFinished();

/** Send request via signal/slot to main another thread */
void sendRequest( const QNetworkRequest & request );

/** Abort request through QgsNetworkAccessManager */
void deleteReply( QNetworkReply * reply );

protected slots:
void requestSent( QNetworkReply * reply, QObject *sender );
void capabilitiesReplyFinished();
void capabilitiesReplyProgress( qint64, qint64 );

@@ -727,6 +744,9 @@ class QgsWmsCapabilitiesDownload : public QObject
QByteArray mHttpCapabilitiesResponse;

bool mIsAborted;

private:
void connectManager();
};


2 comments on commit e95bf6d

@blazek

This comment has been minimized.

Copy link
Member Author

@blazek blazek replied Sep 1, 2015

Maybe overcomplicated? Could it be done in a simpler way?

@nirvn

This comment has been minimized.

Copy link
Contributor

@nirvn nirvn replied Sep 2, 2015

@blazek @wonder-sk I've compiled and tested this fix, which I confirm fixed both the thread warnings as well as the segfault crasher (on my machine).

Kudos for getting rid of an obscure crasher.

Please sign in to comment.