fix issue 'QFutureWatcher::connect: connecting after calling setFutur…

…e() is likely to produce race'
qgis · Nov 4, 2021 · d21e015 · d21e015
1 parent 9c2613c
commit d21e015
Show file tree

Hide file tree

Showing 7 changed files with 50 additions and 46 deletions.
diff --git a/src/3d/qgspointcloudlayerchunkloader_p.cpp b/src/3d/qgspointcloudlayerchunkloader_p.cpp
@@ -79,6 +79,9 @@ QgsPointCloudLayerChunkLoader::QgsPointCloudLayerChunkLoader( const QgsPointClou
   //
   // this will be run in a background thread
   //
+  mFutureWatcher = new QFutureWatcher<void>( this );
+  connect( mFutureWatcher, &QFutureWatcher<void>::finished, this, &QgsChunkQueueJob::finished );
+
   const QFuture<void> future = QtConcurrent::run( [pc, pcNode, this]
   {
     const QgsEventTracing::ScopedEvent e( QStringLiteral( "3D" ), QStringLiteral( "PC chunk load" ) );
@@ -92,10 +95,7 @@ QgsPointCloudLayerChunkLoader::QgsPointCloudLayerChunkLoader( const QgsPointClou
   } );
 
   // emit finished() as soon as the handler is populated with features
-  mFutureWatcher = new QFutureWatcher<void>( this );
   mFutureWatcher->setFuture( future );
-  connect( mFutureWatcher, &QFutureWatcher<void>::finished, this, &QgsChunkQueueJob::finished );
-
 }
 
 QgsPointCloudLayerChunkLoader::~QgsPointCloudLayerChunkLoader()

diff --git a/src/3d/qgsrulebasedchunkloader_p.cpp b/src/3d/qgsrulebasedchunkloader_p.cpp
@@ -73,6 +73,8 @@ QgsRuleBasedChunkLoader::QgsRuleBasedChunkLoader( const QgsRuleBasedChunkLoaderF
   //
   // this will be run in a background thread
   //
+  mFutureWatcher = new QFutureWatcher<void>( this );
+  connect( mFutureWatcher, &QFutureWatcher<void>::finished, this, &QgsChunkQueueJob::finished );
 
   const QFuture<void> future = QtConcurrent::run( [req, this]
   {
@@ -90,9 +92,7 @@ QgsRuleBasedChunkLoader::QgsRuleBasedChunkLoader( const QgsRuleBasedChunkLoaderF
   } );
 
   // emit finished() as soon as the handler is populated with features
-  mFutureWatcher = new QFutureWatcher<void>( this );
   mFutureWatcher->setFuture( future );
-  connect( mFutureWatcher, &QFutureWatcher<void>::finished, this, &QgsChunkQueueJob::finished );
 }
 
 QgsRuleBasedChunkLoader::~QgsRuleBasedChunkLoader()

diff --git a/src/3d/qgsvectorlayerchunkloader_p.cpp b/src/3d/qgsvectorlayerchunkloader_p.cpp
@@ -83,6 +83,8 @@ QgsVectorLayerChunkLoader::QgsVectorLayerChunkLoader( const QgsVectorLayerChunkL
   //
   // this will be run in a background thread
   //
+  mFutureWatcher = new QFutureWatcher<void>( this );
+  connect( mFutureWatcher, &QFutureWatcher<void>::finished, this, &QgsChunkQueueJob::finished );
 
   const QFuture<void> future = QtConcurrent::run( [req, this]
   {
@@ -100,9 +102,7 @@ QgsVectorLayerChunkLoader::QgsVectorLayerChunkLoader( const QgsVectorLayerChunkL
   } );
 
   // emit finished() as soon as the handler is populated with features
-  mFutureWatcher = new QFutureWatcher<void>( this );
   mFutureWatcher->setFuture( future );
-  connect( mFutureWatcher, &QFutureWatcher<void>::finished, this, &QgsChunkQueueJob::finished );
 }
 
 QgsVectorLayerChunkLoader::~QgsVectorLayerChunkLoader()

diff --git a/src/3d/terrain/qgsdemterraintileloader_p.cpp b/src/3d/terrain/qgsdemterraintileloader_p.cpp
@@ -229,16 +229,18 @@ int QgsDemHeightMapGenerator::render( const QgsChunkNodeId &nodeId )
   jd.tileId = nodeId;
   jd.extent = extent;
   jd.timer.start();
+
+  QFutureWatcher<QByteArray> *fw = new QFutureWatcher<QByteArray>( nullptr );
+  connect( fw, &QFutureWatcher<QByteArray>::finished, this, &QgsDemHeightMapGenerator::onFutureFinished );
+  connect( fw, &QFutureWatcher<QByteArray>::finished, fw, &QObject::deleteLater );
+
   // make a clone of the data provider so it is safe to use in worker thread
   if ( mDtm )
     jd.future = QtConcurrent::run( _readDtmData, mClonedProvider, extent, mResolution, mTilingScheme.crs() );
   else
     jd.future = QtConcurrent::run( _readOnlineDtm, mDownloader.get(), extent, mResolution, mTilingScheme.crs(), mTransformContext );
 
-  QFutureWatcher<QByteArray> *fw = new QFutureWatcher<QByteArray>( nullptr );
   fw->setFuture( jd.future );
-  connect( fw, &QFutureWatcher<QByteArray>::finished, this, &QgsDemHeightMapGenerator::onFutureFinished );
-  connect( fw, &QFutureWatcher<QByteArray>::finished, fw, &QObject::deleteLater );
 
   mJobs.insert( fw, jd );
 

diff --git a/src/analysis/vector/geometry_checker/qgsgeometrychecker.cpp b/src/analysis/vector/geometry_checker/qgsgeometrychecker.cpp
@@ -82,15 +82,16 @@ QFuture<void> QgsGeometryChecker::execute( int *totalSteps )
       }
     }
   }
-
-  QFuture<void> future = QtConcurrent::map( mChecks, RunCheckWrapper( this ) );
-
-  QFutureWatcher<void> *watcher = new QFutureWatcher<void>();
-  watcher->setFuture( future );
   QTimer *timer = new QTimer();
   connect( timer, &QTimer::timeout, this, &QgsGeometryChecker::emitProgressValue );
+
+  QFutureWatcher<void> *watcher = new QFutureWatcher<void>();
   connect( watcher, &QFutureWatcherBase::finished, timer, &QObject::deleteLater );
   connect( watcher, &QFutureWatcherBase::finished, watcher, &QObject::deleteLater );
+
+  QFuture<void> future = QtConcurrent::map( mChecks, RunCheckWrapper( this ) );
+  watcher->setFuture( future );
+
   timer->start( 500 );
 
   return future;

diff --git a/src/app/qgsgeometryvalidationservice.cpp b/src/app/qgsgeometryvalidationservice.cpp
@@ -456,6 +456,37 @@ void QgsGeometryValidationService::triggerTopologyChecks( QgsVectorLayer *layer,
 
   mLayerChecks[layer].topologyCheckFeedbacks = feedbacks.values();
 
+  QFutureWatcher<void> *futureWatcher = new QFutureWatcher<void>();
+
+  connect( futureWatcher, &QFutureWatcherBase::finished, this, [&allErrors, layer, feedbacks, futureWatcher, stopEditing, this]()
+  {
+    QgsReadWriteLocker errorLocker( mTopologyCheckLock, QgsReadWriteLocker::Read );
+    layer->setAllowCommit( allErrors.empty() && mLayerChecks[layer].singleFeatureCheckErrors.empty() );
+    errorLocker.unlock();
+    qDeleteAll( feedbacks );
+    futureWatcher->deleteLater();
+    if ( mLayerChecks[layer].topologyCheckFutureWatcher == futureWatcher )
+      mLayerChecks[layer].topologyCheckFutureWatcher = nullptr;
+
+    if ( !allErrors.empty() || !mLayerChecks[layer].singleFeatureCheckErrors.empty() )
+    {
+      if ( mLayerChecks[layer].commitPending )
+        showMessage( tr( "Geometry errors have been found. Please fix the errors before saving the layer." ) );
+      else
+        showMessage( tr( "Geometry errors have been found." ) );
+    }
+    if ( allErrors.empty() && mLayerChecks[layer].singleFeatureCheckErrors.empty() && mLayerChecks[layer].commitPending )
+    {
+      mBypassChecks = true;
+      layer->commitChanges( stopEditing );
+      mBypassChecks = false;
+      mMessageBar->popWidget( mMessageBarItem );
+      mMessageBarItem = nullptr;
+    }
+
+    mLayerChecks[layer].commitPending = false;
+  } );
+
   QFuture<void> future = QtConcurrent::map( checks, [&allErrors, layerFeatureIds, layer, layerId, feedbacks, affectedFeatureIds, this]( const QgsGeometryCheck * check )
   {
     // Watch out with the layer pointer in here. We are running in a thread, so we do not want to actually use it
@@ -501,37 +532,7 @@ void QgsGeometryValidationService::triggerTopologyChecks( QgsVectorLayer *layer,
     errorLocker.unlock();
   } );
 
-  QFutureWatcher<void> *futureWatcher = new QFutureWatcher<void>();
   futureWatcher->setFuture( future );
 
-  connect( futureWatcher, &QFutureWatcherBase::finished, this, [&allErrors, layer, feedbacks, futureWatcher, stopEditing, this]()
-  {
-    QgsReadWriteLocker errorLocker( mTopologyCheckLock, QgsReadWriteLocker::Read );
-    layer->setAllowCommit( allErrors.empty() && mLayerChecks[layer].singleFeatureCheckErrors.empty() );
-    errorLocker.unlock();
-    qDeleteAll( feedbacks );
-    futureWatcher->deleteLater();
-    if ( mLayerChecks[layer].topologyCheckFutureWatcher == futureWatcher )
-      mLayerChecks[layer].topologyCheckFutureWatcher = nullptr;
-
-    if ( !allErrors.empty() || !mLayerChecks[layer].singleFeatureCheckErrors.empty() )
-    {
-      if ( mLayerChecks[layer].commitPending )
-        showMessage( tr( "Geometry errors have been found. Please fix the errors before saving the layer." ) );
-      else
-        showMessage( tr( "Geometry errors have been found." ) );
-    }
-    if ( allErrors.empty() && mLayerChecks[layer].singleFeatureCheckErrors.empty() && mLayerChecks[layer].commitPending )
-    {
-      mBypassChecks = true;
-      layer->commitChanges( stopEditing );
-      mBypassChecks = false;
-      mMessageBar->popWidget( mMessageBarItem );
-      mMessageBarItem = nullptr;
-    }
-
-    mLayerChecks[layer].commitPending = false;
-  } );
-
   mLayerChecks[layer].topologyCheckFutureWatcher = futureWatcher;
 }
diff --git a/src/core/maprenderer/qgsmaprendererparalleljob.cpp b/src/core/maprenderer/qgsmaprendererparalleljob.cpp
@@ -288,9 +288,9 @@ void QgsMapRendererParallelJob::renderingFinished()
   {
     mStatus = RenderingSecondPass;
     // We have a second pass to do.
+    connect( &mSecondPassFutureWatcher, &QFutureWatcher<void>::finished, this, &QgsMapRendererParallelJob::renderLayersSecondPassFinished );
     mSecondPassFuture = QtConcurrent::map( mSecondPassLayerJobs, renderLayerStatic );
     mSecondPassFutureWatcher.setFuture( mSecondPassFuture );
-    connect( &mSecondPassFutureWatcher, &QFutureWatcher<void>::finished, this, &QgsMapRendererParallelJob::renderLayersSecondPassFinished );
   }
   else
   {