Add an optional QgsFeedback object for QgsFeatureRequest, and use

this to abort the acquisition of a connection in the OGR connection pool Provide a mechanism to avoid a deadlock when multiple OGR iterators are trying to obtain a connection to a resource at once. Refs #43572
qgis · Jun 18, 2021 · e416d77 · e416d77
1 parent d83a769
commit e416d77
Show file tree

Hide file tree

Showing 6 changed files with 90 additions and 3 deletions.
diff --git a/python/core/auto_generated/qgsfeaturerequest.sip.in b/python/core/auto_generated/qgsfeaturerequest.sip.in
@@ -744,6 +744,29 @@ For example, this should be set on requests that are issued from an
 expression function.
 
 .. versionadded:: 3.4
+%End
+
+    void setFeedback( QgsFeedback *feedback );
+%Docstring
+Attach a ``feedback`` object that can be queried regularly by the iterator to check
+if it should be canceled.
+
+Ownership of ``feedback`` is NOT transferred, and the caller must take care that it exists
+for the lifetime of the feature request and feature iterators.
+
+.. seealso:: :py:func:`feedback`
+
+.. versionadded:: 3.20
+%End
+
+    QgsFeedback *feedback() const;
+%Docstring
+Returns the feedback object that can be queried regularly by the iterator to check
+if it should be canceled, if set.
+
+.. seealso:: :py:func:`setFeedback`
+
+.. versionadded:: 3.20
 %End
 
   protected:

diff --git a/src/core/providers/ogr/qgsogrfeatureiterator.cpp b/src/core/providers/ogr/qgsogrfeatureiterator.cpp
@@ -83,9 +83,13 @@ QgsOgrFeatureIterator::QgsOgrFeatureIterator( QgsOgrFeatureSource *source, bool
   else
   {
     //QgsDebugMsg( "Feature iterator of " + mSource->mLayerName + ": acquiring connection");
-    mConn = QgsOgrConnPool::instance()->acquireConnection( QgsOgrProviderUtils::connectionPoolId( mSource->mDataSource, mSource->mShareSameDatasetAmongLayers ), mRequest.timeout(), mRequest.requestMayBeNested() );
+    mConn = QgsOgrConnPool::instance()->acquireConnection( QgsOgrProviderUtils::connectionPoolId( mSource->mDataSource, mSource->mShareSameDatasetAmongLayers ),
+            mRequest.timeout(),
+            mRequest.requestMayBeNested(),
+            mRequest.feedback() );
     if ( !mConn || !mConn->ds )
     {
+      iteratorClosed();
       return;
     }
 

diff --git a/src/core/qgsconnectionpool.h b/src/core/qgsconnectionpool.h
@@ -20,6 +20,8 @@
 
 #include "qgis.h"
 #include "qgsapplication.h"
+#include "qgsfeedback.h"
+
 #include <QCoreApplication>
 #include <QMap>
 #include <QMutex>
@@ -288,9 +290,12 @@ class QgsConnectionPool
      * If \a timeout is a negative value the calling thread will be blocked
      * until a connection becomes available. This is the default behavior.
      *
+     * The optional \a feedback argument can be used to cancel the request
+     * before the connection is acquired.
+     *
      * \returns initialized connection or NULLPTR if unsuccessful
      */
-    T acquireConnection( const QString &connInfo, int timeout = -1, bool requestMayBeNested = false )
+    T acquireConnection( const QString &connInfo, int timeout = -1, bool requestMayBeNested = false, QgsFeedback *feedback = nullptr )
     {
       mMutex.lock();
       typename T_Groups::iterator it = mGroups.find( connInfo );
@@ -301,7 +306,25 @@ class QgsConnectionPool
       T_Group *group = *it;
       mMutex.unlock();
 
-      return group->acquire( timeout, requestMayBeNested );
+      if ( feedback )
+      {
+        QElapsedTimer timer;
+        timer.start();
+
+        while ( !feedback->isCanceled() )
+        {
+          if ( T conn = group->acquire( 300, requestMayBeNested ) )
+            return conn;
+
+          if ( timeout > 0 && timer.elapsed() >= timeout )
+            return nullptr;
+        }
+        return nullptr;
+      }
+      else
+      {
+        return group->acquire( timeout, requestMayBeNested );
+      }
     }
 
     //! Release an existing connection so it will get back into the pool and can be reused

diff --git a/src/core/qgsfeaturerequest.cpp b/src/core/qgsfeaturerequest.cpp
@@ -86,6 +86,7 @@ QgsFeatureRequest &QgsFeatureRequest::operator=( const QgsFeatureRequest &rh )
   mTransformErrorCallback = rh.mTransformErrorCallback;
   mTimeout = rh.mTimeout;
   mRequestMayBeNested = rh.mRequestMayBeNested;
+  mFeedback = rh.mFeedback;
   return *this;
 }
 
@@ -334,6 +335,16 @@ QgsFeatureRequest &QgsFeatureRequest::setRequestMayBeNested( bool requestMayBeNe
   return *this;
 }
 
+void QgsFeatureRequest::setFeedback( QgsFeedback *feedback )
+{
+  mFeedback = feedback;
+}
+
+QgsFeedback *QgsFeatureRequest::feedback()
+{
+  return mFeedback;
+}
+
 
 #include "qgsfeatureiterator.h"
 #include "qgslogger.h"

diff --git a/src/core/qgsfeaturerequest.h b/src/core/qgsfeaturerequest.h
@@ -714,6 +714,29 @@ class CORE_EXPORT QgsFeatureRequest
      */
     QgsFeatureRequest &setRequestMayBeNested( bool requestMayBeNested );
 
+    /**
+     * Attach a \a feedback object that can be queried regularly by the iterator to check
+     * if it should be canceled.
+     *
+     * Ownership of \a feedback is NOT transferred, and the caller must take care that it exists
+     * for the lifetime of the feature request and feature iterators.
+     *
+     * \see feedback()
+     *
+     * \since QGIS 3.20
+     */
+    void setFeedback( QgsFeedback *feedback );
+
+    /**
+     * Returns the feedback object that can be queried regularly by the iterator to check
+     * if it should be canceled, if set.
+     *
+     * \see setFeedback()
+     *
+     * \since QGIS 3.20
+     */
+    QgsFeedback *feedback() const;
+
   protected:
     FilterType mFilter = FilterNone;
     QgsRectangle mFilterRect;
@@ -733,6 +756,7 @@ class CORE_EXPORT QgsFeatureRequest
     QgsCoordinateTransformContext mTransformContext;
     int mTimeout = -1;
     int mRequestMayBeNested = false;
+    QgsFeedback *mFeedback = nullptr;
 };
 
 Q_DECLARE_OPERATORS_FOR_FLAGS( QgsFeatureRequest::Flags )

diff --git a/src/core/vector/qgsvectorlayerrenderer.cpp b/src/core/vector/qgsvectorlayerrenderer.cpp
@@ -392,6 +392,8 @@ bool QgsVectorLayerRenderer::renderInternal( QgsFeatureRenderer *renderer )
     context.setVectorSimplifyMethod( vectorMethod );
   }
 
+  featureRequest.setFeedback( mInterruptionChecker.get() );
+
   QgsFeatureIterator fit = mSource->getFeatures( featureRequest );
   // Attach an interruption checker so that iterators that have potentially
   // slow fetchFeature() implementations, such as in the WFS provider, can