Fix deadlock in geometry validation

Fix a deadlock in geometry validation. This happens when the mainthread "waits for finished" of a checker thread.

What happened in this case was, that the main thread canceled the feedback and waited for any jobs to finish.
If a job was waiting for a feature source (or something else to be executed on the main thread) this resulted in a mighty deadlock.

What we do here is that we regularly check if we ought to cancel while waiting for our slot on the main thread and can bail out if we should cancel before the main thread gets around to take care of our function.
The difference is, we still execute the code on the background thread and make sure that the main thread is not doing anything during this time to avoid working on the same data structures in parallel.

Author: m-kuhn

python/analysis/auto_generated/vector/geometry_checker/qgsfeaturepool.sip.in

@@ -30,11 +30,12 @@ A feature pool is based on a vector layer and caches features.
     QgsFeaturePool( QgsVectorLayer *layer );
     virtual ~QgsFeaturePool();
 
-    bool getFeature( QgsFeatureId id, QgsFeature &feature );
+    bool getFeature( QgsFeatureId id, QgsFeature &feature, QgsFeedback *feedback = 0 );
 %Docstring
 Retrieve the feature with the specified ``id`` into ``feature``.
 It will be retrieved from the cache or from the underlying layer if unavailable.
 If the feature is neither available from the cache nor from the layer it will return false.
+If ``feedback`` is specified, the call may return if the feedback is canceled.
 %End
 
 

src/analysis/vector/geometry_checker/qgsfeaturepool.cpp

@@ -36,7 +36,7 @@ QgsFeaturePool::QgsFeaturePool( QgsVectorLayer *layer )
 
 }
 
-bool QgsFeaturePool::getFeature( QgsFeatureId id, QgsFeature &feature )
+bool QgsFeaturePool::getFeature( QgsFeatureId id, QgsFeature &feature, QgsFeedback *feedback )
 {
   QgsReadWriteLocker locker( mCacheLock, QgsReadWriteLocker::Read );
   QgsFeature *cachedFeature = mFeatureCache.object( id );
@@ -47,11 +47,11 @@ bool QgsFeaturePool::getFeature( QgsFeatureId id, QgsFeature &feature )
   }
   else
   {
-    std::unique_ptr<QgsVectorLayerFeatureSource> source = QgsVectorLayerUtils::getFeatureSource( mLayer );
+    std::unique_ptr<QgsVectorLayerFeatureSource> source = QgsVectorLayerUtils::getFeatureSource( mLayer, feedback );
 
     // Feature not in cache, retrieve from layer
     // TODO: avoid always querying all attributes (attribute values are needed when merging by attribute)
-    if ( !source->getFeatures( QgsFeatureRequest( id ) ).nextFeature( feature ) )
+    if ( !source || !source->getFeatures( QgsFeatureRequest( id ) ).nextFeature( feature ) )
     {
       return false;
     }
@@ -62,11 +62,11 @@ bool QgsFeaturePool::getFeature( QgsFeatureId id, QgsFeature &feature )
   return true;
 }
 
-QgsFeatureIds QgsFeaturePool::getFeatures( const QgsFeatureRequest &request )
+QgsFeatureIds QgsFeaturePool::getFeatures( const QgsFeatureRequest &request, QgsFeedback *feedback )
 {
   QgsFeatureIds fids;
 
-  std::unique_ptr<QgsVectorLayerFeatureSource> source = QgsVectorLayerUtils::getFeatureSource( mLayer );
+  std::unique_ptr<QgsVectorLayerFeatureSource> source = QgsVectorLayerUtils::getFeatureSource( mLayer, feedback );
 
   QgsFeatureIterator it = source->getFeatures( request );
   QgsFeature feature;

src/analysis/vector/geometry_checker/qgsfeaturepool.h

@@ -46,17 +46,19 @@ class ANALYSIS_EXPORT QgsFeaturePool : public QgsFeatureSink SIP_ABSTRACT
      * Retrieve the feature with the specified \a id into \a feature.
      * It will be retrieved from the cache or from the underlying layer if unavailable.
      * If the feature is neither available from the cache nor from the layer it will return false.
+     * If \a feedback is specified, the call may return if the feedback is canceled.
      */
-    bool getFeature( QgsFeatureId id, QgsFeature &feature );
+    bool getFeature( QgsFeatureId id, QgsFeature &feature, QgsFeedback *feedback = nullptr );
 
     /**
      * Get features for the provided \a request. No features will be fetched
      * from the cache and the request is sent directly to the underlying feature source.
      * Results of the request are cached in the pool and the ids of all the features
      * are returned. This can be used to warm the cache for a particular area of interest
      * (bounding box) or other set of features.
+     * If \a feedback is specified, the call may return if the feedback is canceled.
      */
-    QgsFeatureIds getFeatures( const QgsFeatureRequest &request ) SIP_SKIP;
+    QgsFeatureIds getFeatures( const QgsFeatureRequest &request, QgsFeedback *feedback = nullptr ) SIP_SKIP;
 
     /**
      * Updates a feature in this pool.

src/analysis/vector/geometry_checker/qgsgeometrymissingvertexcheck.cpp

@@ -150,7 +150,7 @@ void QgsGeometryMissingVertexCheck::processPolygon( const QgsCurvePolygon *polyg
     if ( fid == currentFeature.id() )
       continue;
 
-    if ( featurePool->getFeature( fid, compareFeature ) )
+    if ( featurePool->getFeature( fid, compareFeature, feedback ) )
     {
       if ( feedback->isCanceled() )
         break;

src/core/qgsthreadingutils.h

@@ -20,7 +20,11 @@
 
 #include "qgis_core.h"
 
+#include "qgsfeedback.h"
+
 #include <QThread>
+#include <QSemaphore>
+#include <memory>
 
 /**
  * \ingroup core
@@ -39,24 +43,83 @@ class CORE_EXPORT QgsThreadingUtils
      * This is useful to quickly access information from objects that live on the
      * main thread and copying this information into worker threads. Avoid running
      * expensive code inside \a func.
+     * If a \a feedback is provided, it will observe if the feedback is canceled.
+     * In case the feedback is canceled before the main thread started to run the
+     * function, it will return without executing the function.
      *
      * \note Only works with Qt >= 5.10, earlier versions will execute the code
      *       in the worker thread.
      *
      * \since QGIS 3.4
      */
     template <typename Func>
-    static void runOnMainThread( const Func &func )
+    static bool runOnMainThread( const Func &func, QgsFeedback *feedback = nullptr )
     {
 #if QT_VERSION >= QT_VERSION_CHECK( 5, 10, 0 )
       // Make sure we only deal with the vector layer on the main thread where it lives.
       // Anything else risks a crash.
       if ( QThread::currentThread() == qApp->thread() )
+      {
         func();
+        return true;
+      }
       else
+      {
+        if ( feedback )
+        {
+          // This semaphore will block the worker thread until the main thread is ready.
+          // Ready means the event to execute the waitFunc has arrived in the event loop
+          // and is being executed.
+          QSemaphore semaphoreMainThreadReady( 1 );
+
+          // This semaphore will block the main thread until the worker thread is ready.
+          // Once the main thread is executing the waitFunc, it will wait for this semaphore
+          // to be released. This way we can make sure that
+          QSemaphore semaphoreWorkerThreadReady( 1 );
+
+          // Acquire both semaphores. We want the main thread and the current thread to be blocked
+          // until it's save to continue.
+          semaphoreMainThreadReady.acquire();
+          semaphoreWorkerThreadReady.acquire();
+
+          std::function<void()> waitFunc = [&semaphoreMainThreadReady, &semaphoreWorkerThreadReady]()
+          {
+            // This function is executed on the main thread. As soon as it's executed
+            // it will tell the worker thread that the main thread is blocked by releasing
+            // the semaphore.
+            semaphoreMainThreadReady.release();
+
+            // ... and wait for the worker thread to release its semaphore
+            semaphoreWorkerThreadReady.acquire();
+          };
+
+          QMetaObject::invokeMethod( qApp, waitFunc, Qt::QueuedConnection );
+
+          // while we are in the event queue for the main thread and not yet
+          // being executed, check all 100 ms if the feedback is canceled.
+          while ( !semaphoreMainThreadReady.tryAcquire( 1, 100 ) )
+          {
+            if ( feedback->isCanceled() )
+            {
+              semaphoreWorkerThreadReady.release();
+              return false;
+            }
+          }
+
+          // finally, the main thread is blocked and we are (most likely) not canceled.
+          // let's do the real work!!
+          func();
+
+          // work done -> tell the main thread he may continue
+          semaphoreWorkerThreadReady.release();
+          return true;
+        }
         QMetaObject::invokeMethod( qApp, func, Qt::BlockingQueuedConnection );
+        return true;
+      }
 #else
       func();
+      return true;
 #endif
     }
 

src/core/qgsvectorlayerutils.cpp

@@ -510,14 +510,14 @@ QgsFeature QgsVectorLayerUtils::duplicateFeature( QgsVectorLayer *layer, const Q
   return newFeature;
 }
 
-std::unique_ptr<QgsVectorLayerFeatureSource> QgsVectorLayerUtils::getFeatureSource( QPointer<QgsVectorLayer> layer )
+std::unique_ptr<QgsVectorLayerFeatureSource> QgsVectorLayerUtils::getFeatureSource( QPointer<QgsVectorLayer> layer, QgsFeedback *feedback )
 {
   std::unique_ptr<QgsVectorLayerFeatureSource> featureSource;
 
-  auto getFeatureSource = [ layer, &featureSource ]
+  auto getFeatureSource = [ layer, &featureSource, feedback ]
   {
 #if QT_VERSION >= QT_VERSION_CHECK( 5, 10, 0 )
-    Q_ASSERT( QThread::currentThread() == qApp->thread() );
+    Q_ASSERT( QThread::currentThread() == qApp->thread() || feedback );
 #endif
     QgsVectorLayer *lyr = layer.data();
 
@@ -527,7 +527,7 @@ std::unique_ptr<QgsVectorLayerFeatureSource> QgsVectorLayerUtils::getFeatureSour
     }
   };
 
-  QgsThreadingUtils::runOnMainThread( getFeatureSource );
+  QgsThreadingUtils::runOnMainThread( getFeatureSource, feedback );
 
   return featureSource;
 }

src/core/qgsvectorlayerutils.h

@@ -166,7 +166,7 @@ class CORE_EXPORT QgsVectorLayerUtils
      * \note Requires Qt >= 5.10 to make use of the thread-safe implementation
      * \since QGIS 3.4
      */
-    static std::unique_ptr<QgsVectorLayerFeatureSource> getFeatureSource( QPointer<QgsVectorLayer> layer ) SIP_SKIP;
+    static std::unique_ptr<QgsVectorLayerFeatureSource> getFeatureSource( QPointer<QgsVectorLayer> layer, QgsFeedback *feedback ) SIP_SKIP;
 
     /**
      * Matches the attributes in \a feature to the specified \a fields.