Skip to content

Commit

Permalink
fix crash when redownloading or canceling
Browse files Browse the repository at this point in the history
  • Loading branch information
3nids committed May 7, 2018
1 parent 9d45077 commit 5610ebe
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 37 deletions.
7 changes: 6 additions & 1 deletion python/core/qgsnetworkcontentfetcherregistry.sip.in
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,11 @@ Emitted when the download actually starts
void cancelTriggered();
%Docstring
Emitted when download is canceled.
%End

void taskCompleted();
%Docstring
Emitted when the download is finished (although file not accessible yet)
%End

};
Expand Down Expand Up @@ -118,7 +123,7 @@ Create the registry for temporary downloaded files

~QgsNetworkContentFetcherRegistry();

const QgsFetchedContent *fetch( const QUrl &url, const FetchingMode &fetchingMode = DownloadLater );
const QgsFetchedContent *fetch( const QUrl &url, const FetchingMode fetchingMode = DownloadLater );
%Docstring
Initialize a download for the given URL

Expand Down
72 changes: 43 additions & 29 deletions src/core/qgsnetworkcontentfetcherregistry.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
#include "qgsnetworkcontentfetcherregistry.h"

#include "qgsapplication.h"
#include "qgsnetworkcontentfetchertask.h"

QgsNetworkContentFetcherRegistry::QgsNetworkContentFetcherRegistry()
: QObject()
Expand All @@ -37,7 +36,7 @@ QgsNetworkContentFetcherRegistry::~QgsNetworkContentFetcherRegistry()
mFileRegistry.clear();
}

const QgsFetchedContent *QgsNetworkContentFetcherRegistry::fetch( const QUrl &url, const FetchingMode &fetchingMode )
const QgsFetchedContent *QgsNetworkContentFetcherRegistry::fetch( const QUrl &url, const FetchingMode fetchingMode )
{
QMutexLocker locker( &mMutex );
if ( mFileRegistry.contains( url ) )
Expand All @@ -46,72 +45,87 @@ const QgsFetchedContent *QgsNetworkContentFetcherRegistry::fetch( const QUrl &ur
}

QgsFetchedContent *content = new QgsFetchedContent( nullptr, QgsFetchedContent::NotStarted );
content->mFetchingTask = new QgsNetworkContentFetcherTask( url );

// start
QObject::connect( content, &QgsFetchedContent::downloadStarted, this, [ = ]( const bool redownload )
{
QMutexLocker locker( &mMutex );
if ( mFileRegistry.contains( url ) && redownload )
if ( redownload && content->status() == QgsFetchedContent::Downloading )
{
QgsFetchedContent *content = mFileRegistry[url];
if ( mFileRegistry.value( url )->status() == QgsFetchedContent::Downloading )
{
content->cancel();
QMutexLocker locker( &mMutex );
if ( content->mFetchingTask )
disconnect( content->mFetchingTask, &QgsNetworkContentFetcherTask::fetched, content, &QgsFetchedContent::taskCompleted );
}
// no locker when calling cancel!
content->cancel();
}
if ( ( mFileRegistry.contains( url ) && mFileRegistry.value( url )->status() == QgsFetchedContent::NotStarted ) || redownload )
QMutexLocker locker( &mMutex );
if ( redownload ||
content->status() == QgsFetchedContent::NotStarted ||
content->status() == QgsFetchedContent::Failed )
{
content->mFetchingTask = new QgsNetworkContentFetcherTask( url );
connect( content->mFetchingTask, &QgsNetworkContentFetcherTask::fetched, content, &QgsFetchedContent::taskCompleted );
QgsApplication::instance()->taskManager()->addTask( content->mFetchingTask );
content->mStatus = QgsFetchedContent::Downloading;
}
} );

// cancel
QObject::connect( content, &QgsFetchedContent::cancelTriggered, this, [ = ]()
{
QMutexLocker locker( &mMutex );
if ( content->mFetchingTask->canCancel() )
if ( content->mFetchingTask && content->mFetchingTask->canCancel() )
{
content->mFetchingTask->cancel();
}
if ( content->mFile )
{
content->mFile->deleteLater();
mFileRegistry[url]->setFilePath( QStringLiteral() );
content->mFilePath = QString();
}
} );

// finished
QObject::connect( content->mFetchingTask, &QgsNetworkContentFetcherTask::fetched, this, [ = ]()
connect( content, &QgsFetchedContent::taskCompleted, this, [ = ]()
{
QMutexLocker locker( &mMutex );
QNetworkReply *reply = content->mFetchingTask->reply();
QgsFetchedContent *content = mFileRegistry.value( url );
if ( reply->error() == QNetworkReply::NoError )
if ( !content->mFetchingTask || !content->mFetchingTask->reply() )
{
QTemporaryFile *tf = new QTemporaryFile( QStringLiteral( "XXXXXX" ) );
content->setFile( tf );
tf->open();
content->mFile->write( reply->readAll() );
// Qt docs notes that on some system if fileName is not called before close, file might get deleted
content->setFilePath( tf->fileName() );
tf->close();
content->setStatus( QgsFetchedContent::Finished );
// if no reply, it has been canceled
content->mStatus = QgsFetchedContent::Failed;
content->mError = QNetworkReply::OperationCanceledError;
content->mFilePath = QString();
}
else
{
content->setStatus( QgsFetchedContent::Failed );
content->setError( reply->error() );
content->setFilePath( QStringLiteral() );
QNetworkReply *reply = content->mFetchingTask->reply();
if ( reply->error() == QNetworkReply::NoError )
{
QTemporaryFile *tf = new QTemporaryFile( QStringLiteral( "XXXXXX" ) );
content->mFile = tf;
tf->open();
content->mFile->write( reply->readAll() );
// Qt docs notes that on some system if fileName is not called before close, file might get deleted
content->mFilePath = tf->fileName();
tf->close();
content->mStatus = QgsFetchedContent::Finished;
}
else
{
content->mStatus = QgsFetchedContent::Failed;
content->mError = reply->error();
content->mFilePath = QString();
}
}
content->emitFetched();
} );

mFileRegistry.insert( url, content );

if ( fetchingMode == DownloadImmediately )
content->download();

mFileRegistry.insert( url, content );

return content;
}

Expand Down Expand Up @@ -162,7 +176,7 @@ QString QgsNetworkContentFetcherRegistry::localPath( const QString &filePathOrUr
else
{
// if the file is not downloaded yet or has failed, return empty string
path = QStringLiteral();
path = QString();
}
}
else
Expand Down
14 changes: 7 additions & 7 deletions src/core/qgsnetworkcontentfetcherregistry.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@

class QTemporaryFile;

class QgsNetworkContentFetcherTask;
#include "qgstaskmanager.h"
#include "qgsnetworkcontentfetchertask.h"


/**
Expand Down Expand Up @@ -94,15 +95,14 @@ class CORE_EXPORT QgsFetchedContent : public QObject
//! Emitted when download is canceled.
void cancelTriggered();

//! Emitted when the download is finished (although file not accessible yet)
void taskCompleted();

private:
void setFile( QTemporaryFile *file ) {mFile = file;}
void setStatus( ContentStatus status ) {mStatus = status;}
void setError( QNetworkReply::NetworkError error ) {mError = error;}
void setFilePath( const QString &filePath ) {mFilePath = filePath;}
void emitFetched() {emit fetched();}
QTemporaryFile *mFile;
QString mFilePath = QStringLiteral();
QgsNetworkContentFetcherTask *mFetchingTask;
QgsNetworkContentFetcherTask *mFetchingTask = nullptr;
ContentStatus mStatus;
QNetworkReply::NetworkError mError = QNetworkReply::NoError;

Expand Down Expand Up @@ -145,7 +145,7 @@ class CORE_EXPORT QgsNetworkContentFetcherRegistry : public QObject
* \param fetchingMode defines if the download will start immediately or shall be manually triggered
* \note If the download starts immediately, it will not redownload any already fetched or currently fetching file.
*/
const QgsFetchedContent *fetch( const QUrl &url, const FetchingMode &fetchingMode = DownloadLater );
const QgsFetchedContent *fetch( const QUrl &url, const FetchingMode fetchingMode = DownloadLater );

#ifndef SIP_RUN

Expand Down

0 comments on commit 5610ebe

Please sign in to comment.