avoid race condition

qgis · May 7, 2018 · 8308ce1 · 8308ce1
1 parent 0d6dcb2
commit 8308ce1
Show file tree

Hide file tree

Showing 3 changed files with 48 additions and 16 deletions.
diff --git a/python/core/qgsnetworkcontentfetcherregistry.sip.in b/python/core/qgsnetworkcontentfetcherregistry.sip.in
@@ -65,15 +65,25 @@ Return the potential error of the download
 :param redownload: if set to true, it will restart any achieved or pending download.
 %End
 
+    void cancel();
+%Docstring
+ Cancel the download operation
+%End
+
   signals:
     void fetched();
 %Docstring
-Sent when the file is fetched and accessible
+Emitted when the file is fetched and accessible
 %End
 
     void downloadStarted( const bool redownload );
 %Docstring
-Sent went the download actually starts
+Emitted when the download actually starts
+%End
+
+    void cancelTriggered();
+%Docstring
+Emitted when download is canceled.
 %End
 
 };

diff --git a/src/core/qgsnetworkcontentfetcherregistry.cpp b/src/core/qgsnetworkcontentfetcherregistry.cpp
@@ -39,40 +39,53 @@ QgsNetworkContentFetcherRegistry::~QgsNetworkContentFetcherRegistry()
 
 const QgsFetchedContent *QgsNetworkContentFetcherRegistry::fetch( const QUrl &url, const FetchingMode &fetchingMode )
 {
+  QMutexLocker locker( &mMutex );
   if ( mFileRegistry.contains( url ) )
   {
     return mFileRegistry.value( url );
   }
 
   QgsFetchedContent *content = new QgsFetchedContent( nullptr, QgsFetchedContent::NotStarted );
-  QgsNetworkContentFetcherTask *fetcher = new QgsNetworkContentFetcherTask( url );
+  content->mFetchingTask = new QgsNetworkContentFetcherTask( url );
 
+  // start
   QObject::connect( content, &QgsFetchedContent::downloadStarted, this, [ = ]( const bool redownload )
   {
     QMutexLocker locker( &mMutex );
     if ( mFileRegistry.contains( url ) && redownload )
     {
-      const QgsFetchedContent *content = mFileRegistry[url];
-      if ( mFileRegistry.value( url )->status() == QgsFetchedContent::Downloading && content->mFetchingTask )
+      QgsFetchedContent *content = mFileRegistry[url];
+      if ( mFileRegistry.value( url )->status() == QgsFetchedContent::Downloading )
       {
-        content->mFetchingTask->cancel();
-      }
-      if ( content->mFile )
-      {
-        content->mFile->deleteLater();
-        mFileRegistry[url]->setFilePath( QStringLiteral() );
+        content->cancel();
       }
     }
     if ( ( mFileRegistry.contains( url ) && mFileRegistry.value( url )->status() == QgsFetchedContent::NotStarted ) || redownload )
     {
-      QgsApplication::instance()->taskManager()->addTask( fetcher );
+      QgsApplication::instance()->taskManager()->addTask( content->mFetchingTask );
     }
   } );
 
-  QObject::connect( fetcher, &QgsNetworkContentFetcherTask::fetched, this, [ = ]()
+  // cancel
+  QObject::connect( content, &QgsFetchedContent::cancelTriggered, this, [ = ]()
   {
     QMutexLocker locker( &mMutex );
-    QNetworkReply *reply = fetcher->reply();
+    if ( content->mFetchingTask )
+    {
+      content->mFetchingTask->cancel();
+    }
+    if ( content->mFile )
+    {
+      content->mFile->deleteLater();
+      mFileRegistry[url]->setFilePath( QStringLiteral() );
+    }
+  } );
+
+  // finished
+  QObject::connect( content->mFetchingTask, &QgsNetworkContentFetcherTask::fetched, this, [ = ]()
+  {
+    QMutexLocker locker( &mMutex );
+    QNetworkReply *reply = content->mFetchingTask->reply();
     QgsFetchedContent *content = mFileRegistry.value( url );
     if ( reply->error() == QNetworkReply::NoError )
     {
@@ -96,6 +109,7 @@ const QgsFetchedContent *QgsNetworkContentFetcherRegistry::fetch( const QUrl &ur
 
   if ( fetchingMode == DownloadImmediately )
     content->download();
+
   mFileRegistry.insert( url, content );
 
   return content;

diff --git a/src/core/qgsnetworkcontentfetcherregistry.h b/src/core/qgsnetworkcontentfetcherregistry.h
@@ -79,13 +79,21 @@ class CORE_EXPORT QgsFetchedContent : public QObject
      */
     void download( bool redownload = false ) {emit downloadStarted( redownload );}
 
+    /**
+     * @brief Cancel the download operation
+     */
+    void cancel() {emit cancelTriggered();}
+
   signals:
-    //! Sent when the file is fetched and accessible
+    //! Emitted when the file is fetched and accessible
     void fetched();
 
-    //! Sent went the download actually starts
+    //! Emitted when the download actually starts
     void downloadStarted( const bool redownload );
 
+    //! Emitted when download is canceled.
+    void cancelTriggered();
+
   private:
     void setFile( QTemporaryFile *file ) {mFile = file;}
     void setStatus( ContentStatus status ) {mStatus = status;}